From 975752ebd55e680382a860fcc8be7106fd0592b3 Mon Sep 17 00:00:00 2001 From: Bastian Kersting Date: Thu, 4 Sep 2025 09:12:30 +0000 Subject: [PATCH 1/2] Fix `last_ext` always returning empty data The XMP patch that landed recently did a `std::mem::take` on the extension data - this broke the `last_ext` API. Co-authored-by: Rasmus Piorr --- src/reader/decoder.rs | 17 +++++++---------- tests/decode.rs | 33 +++++++++++++++++++++++++++++++-- 2 files changed, 38 insertions(+), 12 deletions(-) diff --git a/src/reader/decoder.rs b/src/reader/decoder.rs index 4c97c8d..80a555e 100644 --- a/src/reader/decoder.rs +++ b/src/reader/decoder.rs @@ -773,28 +773,25 @@ impl StreamingDecoder { } ApplicationExtension => { debug_assert_eq!(0, b); - // We can consume the extension data here as the next states won't access it anymore. - let mut data = std::mem::take(&mut self.ext.data); // the parser removes sub-block lenghts, so app name and data are concatenated - if let Some(&[first, second, ..]) = data.strip_prefix(EXT_NAME_NETSCAPE) { + if let Some(&[first, second, ..]) = self.ext.data.strip_prefix(EXT_NAME_NETSCAPE) { let repeat = u16::from(first) | u16::from(second) << 8; goto!(BlockEnd, emit Decoded::Repetitions(if repeat == 0 { Repeat::Infinite } else { Repeat::Finite(repeat) })) - } else if data.starts_with(EXT_NAME_XMP) { - data.drain(..EXT_NAME_XMP.len()); + } else if let Some(mut rest) = self.ext.data.strip_prefix(EXT_NAME_XMP) { // XMP adds a "ramp" of 257 bytes to the end of the metadata to let the "pascal-strings" // parser converge to the null byte. The ramp looks like "0x01, 0xff, .., 0x01, 0x00". // For convenience and to allow consumers to not be bothered with this implementation detail, // we cut the ramp. const RAMP_SIZE: usize = 257; - if data.len() >= RAMP_SIZE - && data.ends_with(&[0x03, 0x02, 0x01, 0x00]) - && data[data.len() - RAMP_SIZE..].starts_with(&[0x01, 0x0ff]) + if rest.len() >= RAMP_SIZE + && rest.ends_with(&[0x03, 0x02, 0x01, 0x00]) + && rest[rest.len() - RAMP_SIZE..].starts_with(&[0x01, 0x0ff]) { - data.truncate(data.len() - RAMP_SIZE); + rest = &rest[..(rest.len() - RAMP_SIZE)]; } - self.xmp_metadata = Some(data); + self.xmp_metadata = Some(rest.to_vec()); goto!(BlockEnd) } else { goto!(BlockEnd) diff --git a/tests/decode.rs b/tests/decode.rs index 0ea3f74..06a136c 100644 --- a/tests/decode.rs +++ b/tests/decode.rs @@ -1,7 +1,10 @@ #![cfg(feature = "std")] -use gif::{DecodeOptions, Decoder, DisposalMethod, Encoder, Frame}; -use std::fs::File; +use gif::{ + streaming_decoder::{Decoded, OutputBuffer, StreamingDecoder}, + DecodeOptions, Decoder, DisposalMethod, Encoder, Frame, +}; +use std::{fs::File, io::BufRead}; #[test] fn test_simple_indexed() { @@ -269,3 +272,29 @@ fn xmp_metadata() { assert_eq!(decoder.xmp_metadata(), Some(EXPECTED_METADATA.as_bytes())) } + +#[test] +fn check_last_extension_returns() { + let mut buf: &[u8] = include_bytes!("samples/beacon_xmp.gif"); + + let mut out_buf = Vec::new(); + let mut output = OutputBuffer::Vec(&mut out_buf); + + let mut decoder = StreamingDecoder::new(); + + loop { + let (consumed, result) = { + if buf.is_empty() { + break; + } + + decoder.update(&mut buf, &mut output).unwrap() + }; + buf.consume(consumed); + if let Decoded::HeaderEnd = result { + break; + } + } + + assert_eq!(decoder.last_ext().1.len(), 3129); +} From 85f42740263299c0da24b9aeb31f092e1f4b388e Mon Sep 17 00:00:00 2001 From: Bastian Kersting Date: Thu, 4 Sep 2025 09:57:59 +0000 Subject: [PATCH 2/2] Add support for reading the ICC profile Co-authored-by: Rasmus Piorr --- src/reader/decoder.rs | 77 +++++++++++++++++++++++++---------- src/reader/mod.rs | 6 +++ tests/decode.rs | 11 ++++- tests/results.txt | 1 + tests/samples/beacon_icc.gif | Bin 0 -> 589 bytes tests/samples/profile.icc | Bin 0 -> 474 bytes 6 files changed, 72 insertions(+), 23 deletions(-) create mode 100644 tests/samples/beacon_icc.gif create mode 100644 tests/samples/profile.icc diff --git a/src/reader/decoder.rs b/src/reader/decoder.rs index 80a555e..177a73f 100644 --- a/src/reader/decoder.rs +++ b/src/reader/decoder.rs @@ -19,8 +19,9 @@ use weezl::{decode::Decoder as LzwDecoder, BitOrder, LzwError, LzwStatus}; /// GIF palettes are RGB pub const PLTE_CHANNELS: usize = 3; /// Headers for supported extensions. -const EXT_NAME_NETSCAPE: &[u8] = b"\x0bNETSCAPE2.0\x03"; +const EXT_NAME_NETSCAPE: &[u8] = b"\x0bNETSCAPE2.0\x01"; const EXT_NAME_XMP: &[u8] = b"\x0bXMP DataXMP"; +const EXT_NAME_ICC: &[u8] = b"\x0bICCRGBG1012"; /// An error returned in the case of the image not being formatted properly. #[derive(Debug)] @@ -382,6 +383,8 @@ pub struct StreamingDecoder { header_end_reached: bool, /// XMP metadata bytes. xmp_metadata: Option>, + /// ICC profile bytes. + icc_profile: Option>, } /// One version number of the GIF standard. @@ -396,6 +399,7 @@ pub enum Version { struct ExtensionData { id: AnyExtension, data: Vec, + sub_block_lens: Vec, is_block_end: bool, } @@ -465,11 +469,13 @@ impl StreamingDecoder { ext: ExtensionData { id: AnyExtension(0), data: Vec::with_capacity(256), // 0xFF + 1 byte length + sub_block_lens: Vec::new(), is_block_end: true, }, current: None, header_end_reached: false, xmp_metadata: None, + icc_profile: None, } } @@ -542,6 +548,12 @@ impl StreamingDecoder { self.xmp_metadata.as_deref() } + /// ICC profile stored in the image. + #[must_use] + pub fn icc_profile(&self) -> Option<&[u8]> { + self.icc_profile.as_deref() + } + /// The version number of the GIF standard used in this image. /// /// We suppose a minimum of `V87a` compatibility. This value will be reported until we have @@ -705,6 +717,7 @@ impl StreamingDecoder { } Some(Block::Extension) => { self.ext.data.clear(); + self.ext.sub_block_lens.clear(); self.ext.id = AnyExtension(b); if self.ext.id.into_known().is_none() { if !self.allow_unknown_blocks { @@ -766,33 +779,15 @@ impl StreamingDecoder { } } } else { - self.ext.data.push(b); + self.ext.sub_block_lens.push(b); self.ext.is_block_end = false; goto!(ExtensionDataBlock(b as usize), emit Decoded::SubBlockFinished(self.ext.id)) } } ApplicationExtension => { debug_assert_eq!(0, b); - - // the parser removes sub-block lenghts, so app name and data are concatenated - if let Some(&[first, second, ..]) = self.ext.data.strip_prefix(EXT_NAME_NETSCAPE) { - let repeat = u16::from(first) | u16::from(second) << 8; - goto!(BlockEnd, emit Decoded::Repetitions(if repeat == 0 { Repeat::Infinite } else { Repeat::Finite(repeat) })) - } else if let Some(mut rest) = self.ext.data.strip_prefix(EXT_NAME_XMP) { - // XMP adds a "ramp" of 257 bytes to the end of the metadata to let the "pascal-strings" - // parser converge to the null byte. The ramp looks like "0x01, 0xff, .., 0x01, 0x00". - // For convenience and to allow consumers to not be bothered with this implementation detail, - // we cut the ramp. - const RAMP_SIZE: usize = 257; - if rest.len() >= RAMP_SIZE - && rest.ends_with(&[0x03, 0x02, 0x01, 0x00]) - && rest[rest.len() - RAMP_SIZE..].starts_with(&[0x01, 0x0ff]) - { - rest = &rest[..(rest.len() - RAMP_SIZE)]; - } - - self.xmp_metadata = Some(rest.to_vec()); - goto!(BlockEnd) + if let Some(decoded) = self.read_application_extension() { + goto!(BlockEnd, emit decoded) } else { goto!(BlockEnd) } @@ -901,6 +896,44 @@ impl StreamingDecoder { Ok(()) } + fn read_application_extension(&mut self) -> Option { + if let Some(&[first, second]) = self.ext.data.strip_prefix(EXT_NAME_NETSCAPE) { + let repeat = u16::from(first) | u16::from(second) << 8; + return Some(Decoded::Repetitions(if repeat == 0 { + Repeat::Infinite + } else { + Repeat::Finite(repeat) + })); + } else if let Some(mut rest) = self.ext.data.strip_prefix(EXT_NAME_XMP) { + // XMP is not written as a valid "pascal-string", so we need to stitch together + // the text from our collected sublock-lengths. + let mut xmp_metadata = Vec::new(); + for len in self.ext.sub_block_lens.iter() { + xmp_metadata.push(*len); + let (sub_block, tail) = rest.split_at(*len as usize); + xmp_metadata.extend_from_slice(sub_block); + rest = tail; + } + + // XMP adds a "ramp" of 257 bytes to the end of the metadata to let the "pascal-strings" + // parser converge to the null byte. The ramp looks like "0x01, 0xff, .., 0x01, 0x00". + // For convenience and to allow consumers to not be bothered with this implementation detail, + // we cut the ramp. + const RAMP_SIZE: usize = 257; + if xmp_metadata.len() >= RAMP_SIZE + && xmp_metadata.ends_with(&[0x03, 0x02, 0x01, 0x00]) + && xmp_metadata[xmp_metadata.len() - RAMP_SIZE..].starts_with(&[0x01, 0x0ff]) + { + xmp_metadata.truncate(xmp_metadata.len() - RAMP_SIZE); + } + + self.xmp_metadata = Some(xmp_metadata); + } else if let Some(rest) = self.ext.data.strip_prefix(EXT_NAME_ICC) { + self.icc_profile = Some(rest.to_vec()); + } + None + } + fn add_frame(&mut self) { if self.current.is_none() { self.current = Some(Frame::default()); diff --git a/src/reader/mod.rs b/src/reader/mod.rs index 87ef363..a9dc262 100644 --- a/src/reader/mod.rs +++ b/src/reader/mod.rs @@ -479,6 +479,12 @@ where self.decoder.decoder.xmp_metadata() } + /// ICC profile stored in the image. + #[inline] + pub fn icc_profile(&self) -> Option<&[u8]> { + self.decoder.decoder.icc_profile() + } + /// Abort decoding and recover the `io::Read` instance pub fn into_inner(self) -> io::BufReader { self.decoder.into_inner() diff --git a/tests/decode.rs b/tests/decode.rs index 06a136c..392b98b 100644 --- a/tests/decode.rs +++ b/tests/decode.rs @@ -273,6 +273,15 @@ fn xmp_metadata() { assert_eq!(decoder.xmp_metadata(), Some(EXPECTED_METADATA.as_bytes())) } +#[test] +fn icc_profile() { + let image: &[u8] = include_bytes!("samples/beacon_icc.gif"); + let icc_profile: &[u8] = include_bytes!("samples/profile.icc"); + let decoder = DecodeOptions::new().read_info(image).unwrap(); + + assert_eq!(decoder.icc_profile(), Some(icc_profile)) +} + #[test] fn check_last_extension_returns() { let mut buf: &[u8] = include_bytes!("samples/beacon_xmp.gif"); @@ -296,5 +305,5 @@ fn check_last_extension_returns() { } } - assert_eq!(decoder.last_ext().1.len(), 3129); + assert_eq!(decoder.last_ext().1.len(), 3048); } diff --git a/tests/results.txt b/tests/results.txt index b9e50a1..118402d 100644 --- a/tests/results.txt +++ b/tests/results.txt @@ -1,5 +1,6 @@ tests/samples/alpha_gif_a.gif: 3871893825 tests/samples/anim-gr.gif: 291646878 +tests/samples/beacon_icc.gif: 2462153529 tests/samples/beacon_xmp.gif: 2462153529 tests/samples/beacon.gif: 2462153529 tests/samples/interlaced.gif: 2115495372 diff --git a/tests/samples/beacon_icc.gif b/tests/samples/beacon_icc.gif new file mode 100644 index 0000000000000000000000000000000000000000..a63eed6639bfea8318857f7b133480cba41a5777 GIT binary patch literal 589 zcmZ?wbhEHbWMg1sXkcLY|NlP&1B2p!Za>$MU}whwS0gu3)QUsJxh=`0*V86=1#UR8Wz`(@7l9*gv;203#1Ja(C zpO*@vkpR&2Z`T+=b}(Gl0hxs)M+}oQxuB>7sPF?2tEHqCCj;Fi0c59?mlOcS8Eb%S zks_d@fb0n%b~=Q;1jJ5)uy=sfgakPQ)tmvc`O=ZtNl5IH)C!PY49@ull|`B9w=+r< zj0_A+6+BY&iZYW+ixqqm)ALG;fwqGk0#eGr<(!|BUld%BnFr=lj{pTU#I(;r-~eP4 zJA)8Qn$Ezmy`6zU;wC~&WgY_qe+vV{w!;Xq{Dlk*7GD_{m?tEc7L`F=$H;h=K?lSJ hr3(fojuw8w6&bY)wBhLhNs<#NxuYU?iZLsLH2^^RU(o;n literal 0 HcmV?d00001 diff --git a/tests/samples/profile.icc b/tests/samples/profile.icc new file mode 100644 index 0000000000000000000000000000000000000000..e17f5040b0d2186969b7d5c66ad012c52fa4df44 GIT binary patch literal 474 zcmZQzV7%oU>=wc#z`&53S5g$@?xYYA8KuB}m4S;vh(UmXiGd|CxwybFAi#$K2=emt zQb9BlU}RwUc8vi@GB8}$0hxs)M+}oQxuB>7sPF?2tEHqCCo?cGN&wj@^T1r{5uk8}nD#jc9Da;q zXAojZ(-|1Hw=*zE+(d|}%wu5SZ((5Ab{HX+zmS2!;wu9K^MvHmqB5xK7#Yt306?op A#{d8T literal 0 HcmV?d00001