Skip to content

Commit 975752e

Browse files
1c3t3aShaddyDC
andcommitted
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 <piorr@google.com>
1 parent e497980 commit 975752e

File tree

2 files changed

+38
-12
lines changed

2 files changed

+38
-12
lines changed

src/reader/decoder.rs

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -773,28 +773,25 @@ impl StreamingDecoder {
773773
}
774774
ApplicationExtension => {
775775
debug_assert_eq!(0, b);
776-
// We can consume the extension data here as the next states won't access it anymore.
777-
let mut data = std::mem::take(&mut self.ext.data);
778776

779777
// the parser removes sub-block lenghts, so app name and data are concatenated
780-
if let Some(&[first, second, ..]) = data.strip_prefix(EXT_NAME_NETSCAPE) {
778+
if let Some(&[first, second, ..]) = self.ext.data.strip_prefix(EXT_NAME_NETSCAPE) {
781779
let repeat = u16::from(first) | u16::from(second) << 8;
782780
goto!(BlockEnd, emit Decoded::Repetitions(if repeat == 0 { Repeat::Infinite } else { Repeat::Finite(repeat) }))
783-
} else if data.starts_with(EXT_NAME_XMP) {
784-
data.drain(..EXT_NAME_XMP.len());
781+
} else if let Some(mut rest) = self.ext.data.strip_prefix(EXT_NAME_XMP) {
785782
// XMP adds a "ramp" of 257 bytes to the end of the metadata to let the "pascal-strings"
786783
// parser converge to the null byte. The ramp looks like "0x01, 0xff, .., 0x01, 0x00".
787784
// For convenience and to allow consumers to not be bothered with this implementation detail,
788785
// we cut the ramp.
789786
const RAMP_SIZE: usize = 257;
790-
if data.len() >= RAMP_SIZE
791-
&& data.ends_with(&[0x03, 0x02, 0x01, 0x00])
792-
&& data[data.len() - RAMP_SIZE..].starts_with(&[0x01, 0x0ff])
787+
if rest.len() >= RAMP_SIZE
788+
&& rest.ends_with(&[0x03, 0x02, 0x01, 0x00])
789+
&& rest[rest.len() - RAMP_SIZE..].starts_with(&[0x01, 0x0ff])
793790
{
794-
data.truncate(data.len() - RAMP_SIZE);
791+
rest = &rest[..(rest.len() - RAMP_SIZE)];
795792
}
796793

797-
self.xmp_metadata = Some(data);
794+
self.xmp_metadata = Some(rest.to_vec());
798795
goto!(BlockEnd)
799796
} else {
800797
goto!(BlockEnd)

tests/decode.rs

Lines changed: 31 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
11
#![cfg(feature = "std")]
22

3-
use gif::{DecodeOptions, Decoder, DisposalMethod, Encoder, Frame};
4-
use std::fs::File;
3+
use gif::{
4+
streaming_decoder::{Decoded, OutputBuffer, StreamingDecoder},
5+
DecodeOptions, Decoder, DisposalMethod, Encoder, Frame,
6+
};
7+
use std::{fs::File, io::BufRead};
58

69
#[test]
710
fn test_simple_indexed() {
@@ -269,3 +272,29 @@ fn xmp_metadata() {
269272

270273
assert_eq!(decoder.xmp_metadata(), Some(EXPECTED_METADATA.as_bytes()))
271274
}
275+
276+
#[test]
277+
fn check_last_extension_returns() {
278+
let mut buf: &[u8] = include_bytes!("samples/beacon_xmp.gif");
279+
280+
let mut out_buf = Vec::new();
281+
let mut output = OutputBuffer::Vec(&mut out_buf);
282+
283+
let mut decoder = StreamingDecoder::new();
284+
285+
loop {
286+
let (consumed, result) = {
287+
if buf.is_empty() {
288+
break;
289+
}
290+
291+
decoder.update(&mut buf, &mut output).unwrap()
292+
};
293+
buf.consume(consumed);
294+
if let Decoded::HeaderEnd = result {
295+
break;
296+
}
297+
}
298+
299+
assert_eq!(decoder.last_ext().1.len(), 3129);
300+
}

0 commit comments

Comments
 (0)