Skip to content

Commit

Permalink
Consume exact amount of bytes
Browse files Browse the repository at this point in the history
Fixes consuming of the last byte in the stream
BlockEnd doesn't need to check the terminator, because it's only set when it can't fail
  • Loading branch information
kornelski committed Jan 10, 2024
1 parent 870a3d5 commit a482997
Show file tree
Hide file tree
Showing 3 changed files with 21 additions and 24 deletions.
40 changes: 18 additions & 22 deletions src/reader/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,9 +124,8 @@ pub enum Decoded<'a> {
/// Palette and optional `Application` extension have been parsed,
/// reached frame data.
HeaderEnd,
/// Decoded the image trailer.
Trailer,
/// The start of a block.
/// `BlockStart(Block::Trailer)` is the very last decode event
BlockStart(Block),
/// Decoded a sub-block. More sub-block are available.
///
Expand Down Expand Up @@ -159,8 +158,7 @@ enum State {
Byte(ByteValue),
GlobalPalette(usize),
BlockStart(u8),
/// Block end, with remaining expected data. NonZero for invalid EOF.
BlockEnd(u8),
BlockEnd,
ExtensionBlock(AnyExtension),
ApplicationExtension,
SkipBlock(usize),
Expand Down Expand Up @@ -620,7 +618,8 @@ impl StreamingDecoder {
goto!(ExtensionBlock(AnyExtension(b)), emit Decoded::BlockStart(Block::Extension))
}
Some(Block::Trailer) => {
goto!(0, State::Trailer, emit Decoded::BlockStart(Block::Trailer))
// The `Trailer` is the final state, and isn't reachable without extraneous data after the end of file
goto!(Trailer, emit Decoded::BlockStart(Block::Trailer))
}
None => {
if self.allow_unknown_blocks {
Expand All @@ -631,18 +630,13 @@ impl StreamingDecoder {
}
}
},
BlockEnd(terminator) => {
if terminator == 0 {
if b == Block::Trailer as u8 {
// can't consume, because the trailer is not a real block, and won't have futher data
goto!(0, BlockStart(b))
} else {
goto!(BlockStart(b))
}
BlockEnd => {
if b == Block::Trailer as u8 {
// can't consume yet, because the trailer is not a real block,
// and won't have futher data for BlockStart
goto!(0, BlockStart(b))
} else {
Err(DecodingError::format(
"expected block terminator not found"
))
goto!(BlockStart(b))
}
}
ExtensionBlock(id) => {
Expand Down Expand Up @@ -671,23 +665,24 @@ impl StreamingDecoder {
} else if b == 0 {
self.ext.is_block_end = true;
if self.ext.id.into_known() == Some(Extension::Application) {
goto!(ApplicationExtension, emit Decoded::BlockFinished(self.ext.id, &self.ext.data))
goto!(0, ApplicationExtension, emit Decoded::BlockFinished(self.ext.id, &self.ext.data))
} else {
goto!(BlockEnd(b), emit Decoded::BlockFinished(self.ext.id, &self.ext.data))
goto!(BlockEnd, emit Decoded::BlockFinished(self.ext.id, &self.ext.data))
}
} else {
self.ext.is_block_end = false;
goto!(SkipBlock(b as usize), emit Decoded::SubBlockFinished(self.ext.id, &self.ext.data))
}
}
ApplicationExtension => {
debug_assert_eq!(0, b);
// the parser removes sub-block lenghts, so app name and data are concatenated
if self.ext.data.len() >= 15 && &self.ext.data[1..13] == b"NETSCAPE2.0\x01" {
let repeat = &self.ext.data[13..15];
let repeat = u16::from(repeat[0]) | u16::from(repeat[1]) << 8;
goto!(0, BlockEnd(0), emit Decoded::Repetitions(if repeat == 0 { Repeat::Infinite } else { Repeat::Finite(repeat) }))
goto!(BlockEnd, emit Decoded::Repetitions(if repeat == 0 { Repeat::Infinite } else { Repeat::Finite(repeat) }))
} else {
goto!(0, BlockEnd(0))
goto!(BlockEnd)
}
}
LocalPalette(left) => {
Expand Down Expand Up @@ -771,9 +766,10 @@ impl StreamingDecoder {
FrameDecoded => {
// end of image data reached
self.current = None;
goto!(BlockEnd(b), emit Decoded::DataEnd)
debug_assert_eq!(0, b);
goto!(BlockEnd, emit Decoded::DataEnd)
}
Trailer => Ok((0, Decoded::Trailer)),
Trailer => goto!(0, Trailer, emit Decoded::Nothing),
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/reader/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -286,7 +286,7 @@ impl<R> Decoder<R> where R: Read {
Some(Decoded::Repetitions(repeat)) => {
self.repeat = repeat;
},
Some(Decoded::HeaderEnd | Decoded::Trailer) => {
Some(Decoded::HeaderEnd) => {
break
},
Some(_) => {
Expand Down
3 changes: 2 additions & 1 deletion tests/roundtrip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ fn round_trip_from_image(original: &[u8]) {
assert_eq!(decoder.height(), height);
assert_eq!(decoder.repeat(), repeat);
assert_eq!(global_palette, decoder.global_palette().unwrap_or_default());
let new_frames: Vec<_> = core::iter::from_fn(move || {
let new_frames: Vec<_> = core::iter::from_fn(|| {
decoder.read_next_frame().unwrap().cloned()
}).collect();
assert_eq!(new_frames.len(), frames.len(), "Diverging number of frames");
Expand All @@ -52,6 +52,7 @@ fn round_trip_from_image(original: &[u8]) {
assert_eq!(new.palette, reference.palette);
assert_eq!(new.buffer, reference.buffer);
}
assert_eq!(0, decoder.into_inner().buffer().len());
}
}

Expand Down

0 comments on commit a482997

Please sign in to comment.