diff --git a/desktop/src-tauri/src/commands/media_gif.rs b/desktop/src-tauri/src/commands/media_gif.rs index be2ea1a25..7d0c129f9 100644 --- a/desktop/src-tauri/src/commands/media_gif.rs +++ b/desktop/src-tauri/src/commands/media_gif.rs @@ -28,13 +28,13 @@ fn gif_sub_blocks_end(body: &[u8], mut i: usize) -> Option { /// `validate_gif_metadata_free`), and encoders like Photoshop and Giphy emit /// them routinely, so uploads fail unless the client drops them first. /// -/// Everything the relay accepts — header, colour tables, graphic-control -/// extensions, image descriptors and frame data, standard looping extensions — -/// is copied verbatim, so animation timing, disposal, and pixel data stay -/// byte-identical. Trailing bytes after the trailer (another relay reject) are -/// truncated. Returns `None` when the payload isn't structurally parseable as -/// GIF; the caller then uploads the original bytes and the relay's validator -/// remains the authority. +/// Everything the relay accepts — header, colour tables, image descriptors and +/// frame data, standard looping extensions, and graphic-control extensions for +/// retained images — is copied verbatim, so animation timing, disposal, and +/// pixel data stay byte-identical. Trailing bytes after the trailer (another +/// relay reject) are truncated. Returns `None` when the payload isn't +/// structurally parseable as GIF; the caller then uploads the original bytes +/// and the relay's validator remains the authority. pub(crate) fn strip_gif_metadata(body: &[u8]) -> Option> { if !(body.starts_with(b"GIF87a") || body.starts_with(b"GIF89a")) || body.len() < 13 { return None; @@ -49,6 +49,10 @@ pub(crate) fn strip_gif_metadata(body: &[u8]) -> Option> { let mut out = Vec::with_capacity(body.len()); out.extend_from_slice(&body[..i]); + // A GCE applies to the next graphic-rendering block, which can be either an + // image descriptor or a plain-text extension. Remember emitted GCE ranges + // so stripping plain text can also strip the rendering state it consumed. + let mut pending_gces = Vec::new(); loop { match *body.get(i)? { @@ -67,6 +71,7 @@ pub(crate) fn strip_gif_metadata(body: &[u8]) -> Option> { end = end.checked_add(1).filter(|&e| e <= body.len())?; end = gif_sub_blocks_end(body, end)?; out.extend_from_slice(&body[i..end]); + pending_gces.clear(); i = end; } 0x21 => { @@ -81,7 +86,9 @@ pub(crate) fn strip_gif_metadata(body: &[u8]) -> Option> { return None; } i += 6; + let out_start = out.len(); out.extend_from_slice(&body[start..i]); + pending_gces.push((out_start, out.len())); } // Application extension: keep only the standard looping // extensions; anything else (XMP, Photoshop, Giphy…) is a @@ -97,10 +104,20 @@ pub(crate) fn strip_gif_metadata(body: &[u8]) -> Option> { out.extend_from_slice(&body[start..i]); } } - // Comment (0xFE), plain-text (0x01), and unknown - // extensions: pure metadata channels, dropped. Their - // bodies are all length-prefixed sub-block sequences - // (plain-text's 12-byte header is itself a sub-block). + // Plain text is a graphic-rendering block, so it consumes + // any preceding GCE. Remove that rendering state along with + // the forbidden metadata block rather than letting it alter + // the next retained image. + 0x01 => { + i = gif_sub_blocks_end(body, i)?; + for &(gce_start, gce_end) in pending_gces.iter().rev() { + out.drain(gce_start..gce_end); + } + pending_gces.clear(); + } + // Comments and unknown extensions are metadata channels but + // not graphic-rendering blocks, so they do not consume a + // pending GCE. Their bodies are length-prefixed sub-blocks. _ => { i = gif_sub_blocks_end(body, i)?; } @@ -163,6 +180,21 @@ mod tests { ext } + /// Plain Text Extension with the required 12-byte header and one text block. + fn gif_plain_text_ext() -> Vec { + let mut ext = vec![0x21, 0x01, 12]; + ext.extend_from_slice(&[ + 0, 0, // text grid left + 0, 0, // text grid top + 2, 0, // text grid width + 2, 0, // text grid height + 1, 1, // cell width and height + 1, 0, // foreground and background colour indexes + ]); + ext.extend_from_slice(&[1, b'x', 0]); + ext + } + #[test] fn test_strip_gif_metadata_removes_comment_and_foreign_app_extensions() { let clean = minimal_gif(); @@ -176,6 +208,19 @@ mod tests { assert_eq!(stripped, clean); } + #[test] + fn test_strip_gif_metadata_removes_gce_consumed_by_plain_text() { + let clean = minimal_gif(); + let mut dirty = clean[..19].to_vec(); + // This GCE belongs to the following Plain Text Extension, not to the + // image later in the stream. Both must disappear during sanitization. + dirty.extend_from_slice(&[0x21, 0xf9, 4, 0x09, 0x1e, 0x00, 0x01, 0x00]); + dirty.extend_from_slice(&gif_plain_text_ext()); + dirty.extend_from_slice(&clean[19..]); + + assert_eq!(strip_gif_metadata(&dirty).unwrap(), clean); + } + #[test] fn test_strip_gif_metadata_preserves_clean_gif_byte_identical() { let clean = minimal_gif();