Skip to content

Commit

Permalink
Introduce option to check frame bound consistency
Browse files Browse the repository at this point in the history
This restores the behaviour of previous version to NOT check it. This is
not valid according to strict reading of the specification but it may
appear in practice. In particular, imagemagick handles this in weird
ways that are not errors but silently adjust the width and height.. Not
sure about that style.
  • Loading branch information
HeroicKatora committed Sep 23, 2020
1 parent 4502553 commit 47e4187
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 14 deletions.
20 changes: 13 additions & 7 deletions src/reader/decoder.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,15 @@
use std::cmp;
use std::error;
use std::fmt;
use std::io;
use std::mem;
use std::default::Default;

use std::io;

use std::fmt;
use std::error;
use crate::common::{AnyExtension, Block, DisposalMethod, Extension, Frame};
use crate::reader::DecodeOptions;

use weezl::{BitOrder, decode::Decoder as LzwDecoder, LzwStatus};

use crate::common::{AnyExtension, Block, DisposalMethod, Extension, Frame};

/// GIF palettes are RGB
pub const PLTE_CHANNELS: usize = 3;

Expand Down Expand Up @@ -192,6 +191,7 @@ pub struct StreamingDecoder {
lzw_reader: Option<LzwDecoder>,
decode_buffer: Vec<u8>,
skip_extensions: bool,
check_frame_consistency: bool,
version: &'static str,
width: u16,
height: u16,
Expand All @@ -206,11 +206,17 @@ pub struct StreamingDecoder {
impl StreamingDecoder {
/// Creates a new streaming decoder
pub fn new() -> StreamingDecoder {
let options = DecodeOptions::new();
Self::with_options(&options)
}

pub(crate) fn with_options(options: &DecodeOptions) -> Self {
StreamingDecoder {
state: Some(Magic(0, [0; 6])),
lzw_reader: None,
decode_buffer: vec![],
skip_extensions: true,
check_frame_consistency: options.check_frame_consistency,
version: "",
width: 0,
height: 0,
Expand Down Expand Up @@ -431,7 +437,7 @@ impl StreamingDecoder {

self.current_frame_mut().interlaced = interlaced;

{
if self.check_frame_consistency {
// Consistency checks.
let (width, height) = (self.width, self.height);
let frame = self.current_frame_mut();
Expand Down
19 changes: 12 additions & 7 deletions src/reader/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ impl MemoryLimit {
pub struct DecodeOptions {
memory_limit: MemoryLimit,
color_output: ColorOutput,
check_frame_consistency: bool,
}

impl DecodeOptions {
Expand All @@ -63,6 +64,7 @@ impl DecodeOptions {
DecodeOptions {
memory_limit: MemoryLimit(50_000_000), // 50 MB
color_output: ColorOutput::Indexed,
check_frame_consistency: true,
}
}

Expand All @@ -76,11 +78,16 @@ impl DecodeOptions {
self.memory_limit = limit;
}

/// Configure if frames must be within the
pub fn check_frame_consistency(&mut self, check: bool) {
self.check_frame_consistency = check;
}

/// Reads the logical screen descriptor including the global color palette
///
/// Returns a `Decoder`. All decoder configuration has to be done beforehand.
pub fn read_info<R: Read>(self, r: R) -> Result<Decoder<R>, DecodingError> {
Decoder::with_no_init(r, StreamingDecoder::new(), self.color_output, self.memory_limit).init()
Decoder::with_no_init(r, StreamingDecoder::with_options(&self), self).init()
}
}

Expand Down Expand Up @@ -141,20 +148,18 @@ impl<R> Decoder<R> where R: Read {
DecodeOptions::new()
}

fn with_no_init(reader: R, decoder: StreamingDecoder,
color_output: ColorOutput, memory_limit: MemoryLimit
) -> Decoder<R> {
fn with_no_init(reader: R, decoder: StreamingDecoder, options: DecodeOptions) -> Decoder<R> {
Decoder {
decoder: ReadDecoder {
reader: io::BufReader::new(reader),
decoder: decoder,
decoder,
at_eof: false
},
bg_color: None,
global_palette: None,
buffer: Vec::with_capacity(32),
color_output: color_output,
memory_limit: memory_limit,
color_output: options.color_output,
memory_limit: options.memory_limit,
current_frame: Frame::default(),
}
}
Expand Down
49 changes: 49 additions & 0 deletions tests/decode.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
use gif::{DecodeOptions, DisposalMethod, Encoder, Frame};

#[test]
fn frame_consistency_is_configurable() {
let image = create_image_with_oob_frames();
let mut options = DecodeOptions::new();

{
let mut data = image.as_slice();
let mut decoder = options.clone().read_info(&mut data).unwrap();
assert!(decoder.read_next_frame().is_ok());
assert!(decoder.read_next_frame().is_err());
}

{
options.check_frame_consistency(false);
let mut data = image.as_slice();
let mut decoder = options.clone().read_info(&mut data).unwrap();
assert!(decoder.read_next_frame().is_ok());
assert!(decoder.read_next_frame().is_ok());
}
}

fn create_image_with_oob_frames() -> Vec<u8> {
let mut data = vec![];
let mut encoder = Encoder::new(&mut data, 2, 2, &[0, 0, 0]).unwrap();

let mut frame = Frame {
delay: 1,
dispose: DisposalMethod::Any,
transparent: None,
needs_user_input: false,
top: 0,
left: 0,
width: 2,
height: 2,
interlaced: false,
palette: None,
buffer: vec![0, 0, 0, 0].into(),
};

encoder.write_frame(&frame).unwrap();
frame.top = 1;
frame.left = 1;
encoder.write_frame(&frame).unwrap();

drop(encoder);
data
}

0 comments on commit 47e4187

Please sign in to comment.