Skip to content

Commit

Permalink
Merge pull request #172 from kornelski/forbid-unsafe
Browse files Browse the repository at this point in the history
Change Decode event to make unsafe casts unnecessary
  • Loading branch information
kornelski authored Jan 11, 2024
2 parents b5a5b29 + c1f0661 commit b42af4a
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 50 deletions.
1 change: 1 addition & 0 deletions src/lib.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
#![forbid(unsafe_code)]
//! # GIF en- and decoding library [![Build Status](https://github.com/image-rs/image-gif/workflows/Rust%20CI/badge.svg)](https://github.com/image-rs/image-gif/actions)
//!
//! GIF en- and decoder written in Rust ([API Documentation](https://docs.rs/gif)).
Expand Down
68 changes: 25 additions & 43 deletions src/reader/decoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ pub enum FrameDataType {
/// Indicates whether a certain object has been decoded
#[derive(Debug)]
#[non_exhaustive]
pub enum Decoded<'a> {
pub enum Decoded {
/// Decoded nothing.
Nothing,
/// Global palette.
Expand All @@ -133,16 +133,22 @@ pub enum Decoded<'a> {
///
/// Indicates the label of the extension which might be unknown. A label of `0` is used when
/// the sub block does not belong to an extension.
SubBlockFinished(AnyExtension, &'a [u8]),
///
/// Call `last_ext()` to get the data
SubBlockFinished(AnyExtension),
/// Decoded the last (or only) sub-block of a block.
///
/// Indicates the label of the extension which might be unknown. A label of `0` is used when
/// the sub block does not belong to an extension.
BlockFinished(AnyExtension, &'a [u8]),
///
/// Call `last_ext()` to get the data
BlockFinished(AnyExtension),
/// Decoded all information of the next frame, except the image data.
///
/// The returned frame does **not** contain any owned image data.
FrameMetadata(&'a mut Frame<'static>, FrameDataType),
///
/// Call `current_frame_mut()` to access the frame info.
FrameMetadata(FrameDataType),
/// Decoded some data of the current frame. Size is in bytes, always > 0
BytesDecoded(NonZeroUsize),
/// Copied (or consumed and discarded) compressed data of the current frame. In bytes.
Expand Down Expand Up @@ -353,43 +359,19 @@ impl StreamingDecoder {
&'a mut self,
mut buf: &[u8],
write_into: &mut OutputBuffer<'_>,
) -> Result<(usize, Decoded<'a>), DecodingError> {
) -> Result<(usize, Decoded), DecodingError> {
// NOTE: Do not change the function signature without double-checking the
// unsafe block!
let len = buf.len();
while !buf.is_empty() {
// This dead code is a compile-check for lifetimes that otherwise aren't checked
// due to the `mem::transmute` used later.
// Keep it in sync with the other call to `next_state`.
#[cfg(test)]
if false {
return self.next_state(buf, write_into);
}

match self.next_state(buf, write_into) {
Ok((bytes, Decoded::Nothing)) => {
buf = &buf[bytes..];
}
Ok((bytes, result)) => {
buf = &buf[bytes..];
return Ok(
(len-buf.len(),
// This transmute just casts the lifetime away. Since Rust only
// has SESE regions, this early return cannot be worked out and
// such that the borrow region of self includes the whole block.
// The explixit lifetimes in the function signature ensure that
// this is safe.
// ### NOTE
// To check that everything is sound, return the result without
// the match (e.g. `return Ok(self.next_state(buf)?)`). If
// it compiles the returned lifetime is correct.
unsafe {
mem::transmute::<Decoded<'_>, Decoded<'a>>(result)
}
))
}
Err(err) => return Err(err),
}
let (bytes, decoded) = self.next_state(buf, write_into)?;
buf = &buf[bytes..];
match decoded {
Decoded::Nothing => {},
result => {
return Ok((len-buf.len(), result));
},
};
}
Ok((len - buf.len(), Decoded::Nothing))
}
Expand Down Expand Up @@ -435,7 +417,7 @@ impl StreamingDecoder {
self.version
}

fn next_state(&mut self, buf: &[u8], write_into: &mut OutputBuffer<'_>) -> Result<(usize, Decoded<'_>), DecodingError> {
fn next_state(&mut self, buf: &[u8], write_into: &mut OutputBuffer<'_>) -> Result<(usize, Decoded), DecodingError> {
macro_rules! goto (
($n:expr, $state:expr) => ({
self.state = $state;
Expand Down Expand Up @@ -669,13 +651,13 @@ impl StreamingDecoder {
} else if b == 0 {
self.ext.is_block_end = true;
if self.ext.id.into_known() == Some(Extension::Application) {
goto!(0, ApplicationExtension, emit Decoded::BlockFinished(self.ext.id, &self.ext.data))
goto!(0, ApplicationExtension, emit Decoded::BlockFinished(self.ext.id))
} else {
goto!(BlockEnd, emit Decoded::BlockFinished(self.ext.id, &self.ext.data))
goto!(BlockEnd, emit Decoded::BlockFinished(self.ext.id))
}
} else {
self.ext.is_block_end = false;
goto!(ExtensionDataBlock(b as usize), emit Decoded::SubBlockFinished(self.ext.id, &self.ext.data))
goto!(ExtensionDataBlock(b as usize), emit Decoded::SubBlockFinished(self.ext.id))
}
}
ApplicationExtension => {
Expand Down Expand Up @@ -708,9 +690,9 @@ impl StreamingDecoder {
if !self.skip_frame_decoding {
// Reset validates the min code size
self.lzw_reader.reset(min_code_size)?;
goto!(DecodeSubBlock(b as usize), emit Decoded::FrameMetadata(self.current_frame_mut(), FrameDataType::Pixels))
goto!(DecodeSubBlock(b as usize), emit Decoded::FrameMetadata(FrameDataType::Pixels))
} else {
goto!(CopySubBlock(b as usize), emit Decoded::FrameMetadata(self.current_frame_mut(), FrameDataType::Lzw { min_code_size }))
goto!(CopySubBlock(b as usize), emit Decoded::FrameMetadata(FrameDataType::Lzw { min_code_size }))
}
}
CopySubBlock(left) => {
Expand Down
11 changes: 4 additions & 7 deletions src/reader/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -209,7 +209,7 @@ struct ReadDecoder<R: Read> {
}

impl<R: Read> ReadDecoder<R> {
fn decode_next(&mut self, write_into: &mut OutputBuffer<'_>) -> Result<Option<Decoded<'_>>, DecodingError> {
fn decode_next(&mut self, write_into: &mut OutputBuffer<'_>) -> Result<Option<Decoded>, DecodingError> {
while !self.at_eof {
let (consumed, result) = {
let buf = self.reader.fill_buf()?;
Expand All @@ -231,10 +231,7 @@ impl<R: Read> ReadDecoder<R> {
Decoded::BlockStart(Block::Trailer) => {
self.at_eof = true;
},
result => return Ok(unsafe{
// FIXME: #6393
Some(mem::transmute::<Decoded<'_>, Decoded<'_>>(result))
}),
result => return Ok(Some(result))
}
}
Ok(None)
Expand Down Expand Up @@ -330,8 +327,8 @@ impl<R> Decoder<R> where R: Read {
pub fn next_frame_info(&mut self) -> Result<Option<&Frame<'static>>, DecodingError> {
loop {
match self.decoder.decode_next(&mut OutputBuffer::None)? {
Some(Decoded::FrameMetadata(frame, frame_data_type)) => {
self.current_frame = frame.take();
Some(Decoded::FrameMetadata(frame_data_type)) => {
self.current_frame = self.decoder.decoder.current_frame_mut().take();
self.current_frame_data_type = frame_data_type;
if self.current_frame.palette.is_none() && self.global_palette.is_none() {
return Err(DecodingError::format(
Expand Down

0 comments on commit b42af4a

Please sign in to comment.