From 0ed15a8e32ef64fee0a9d7c23e8a4bd7e575af6f Mon Sep 17 00:00:00 2001 From: Jonathan Behrens Date: Mon, 19 Feb 2024 12:45:10 -0800 Subject: [PATCH] Add BufRead + Seek bound on many decoders (#2149) --- fuzz/fuzzers/fuzzer_script_exr.rs | 15 ++++++++------- src/codecs/bmp/decoder.rs | 13 +++++++------ src/codecs/gif.rs | 11 ++++++----- src/codecs/ico/decoder.rs | 12 ++++++------ src/codecs/jpeg/decoder.rs | 6 +++--- src/codecs/openexr.rs | 10 +++++----- src/codecs/png.rs | 26 +++++++++++++------------- src/codecs/tga/decoder.rs | 8 ++++---- src/codecs/tiff.rs | 8 ++++---- src/codecs/webp/decoder.rs | 6 +++--- src/io/reader.rs | 4 ++-- src/lib.rs | 4 ++-- tests/limits_anim.rs | 4 +++- 13 files changed, 66 insertions(+), 61 deletions(-) diff --git a/fuzz/fuzzers/fuzzer_script_exr.rs b/fuzz/fuzzers/fuzzer_script_exr.rs index c07d23ee15..f42ffec4d6 100644 --- a/fuzz/fuzzers/fuzzer_script_exr.rs +++ b/fuzz/fuzzers/fuzzer_script_exr.rs @@ -9,17 +9,13 @@ use image::ExtendedColorType; use image::ImageDecoder; use image::ImageEncoder; use image::ImageResult; -use std::convert::TryFrom; -use std::io::Cursor; -use std::io::Read; -use std::io::Seek; -use std::io::Write; +use std::io::{BufRead, Cursor, Seek, Write}; // "just dont panic" fn roundtrip(bytes: &[u8]) -> ImageResult<()> { /// Read the file from the specified path into an `Rgba32FImage`. // TODO this method should probably already exist in the main image crate - fn read_as_rgba_byte_image(read: impl Read + Seek) -> ImageResult<(u32, u32, Vec)> { + fn read_as_rgba_byte_image(read: impl BufRead + Seek) -> ImageResult<(u32, u32, Vec)> { let mut decoder = OpenExrDecoder::with_alpha_preference(read, Some(true))?; match usize::try_from(decoder.total_bytes()) { Ok(decoded_size) if decoded_size <= 256 * 1024 * 1024 => { @@ -45,7 +41,12 @@ fn roundtrip(bytes: &[u8]) -> ImageResult<()> { write: impl Write + Seek, (width, height, data): &(u32, u32, Vec), ) -> ImageResult<()> { - OpenExrEncoder::new(write).write_image(data.as_slice(), *width, *height, ExtendedColorType::Rgba32F) + OpenExrEncoder::new(write).write_image( + data.as_slice(), + *width, + *height, + ExtendedColorType::Rgba32F, + ) } let decoded_image = read_as_rgba_byte_image(Cursor::new(bytes))?; diff --git a/src/codecs/bmp/decoder.rs b/src/codecs/bmp/decoder.rs index 1154976572..575d99603b 100644 --- a/src/codecs/bmp/decoder.rs +++ b/src/codecs/bmp/decoder.rs @@ -1,5 +1,5 @@ use std::cmp::{self, Ordering}; -use std::io::{self, Read, Seek, SeekFrom}; +use std::io::{self, BufRead, Seek, SeekFrom}; use std::iter::{repeat, Rev}; use std::slice::ChunksMut; use std::{error, fmt}; @@ -502,7 +502,7 @@ enum RLEInsn { PixelRun(u8, u8), } -impl BmpDecoder { +impl BmpDecoder { fn new_decoder(reader: R) -> BmpDecoder { BmpDecoder { reader, @@ -1329,7 +1329,7 @@ impl BmpDecoder { } } -impl ImageDecoder for BmpDecoder { +impl ImageDecoder for BmpDecoder { fn dimensions(&self) -> (u32, u32) { (self.width as u32, self.height as u32) } @@ -1354,7 +1354,7 @@ impl ImageDecoder for BmpDecoder { } } -impl ImageDecoderRect for BmpDecoder { +impl ImageDecoderRect for BmpDecoder { fn read_rect( &mut self, x: u32, @@ -1384,7 +1384,7 @@ impl ImageDecoderRect for BmpDecoder { #[cfg(test)] mod test { - use std::io::Cursor; + use std::io::{BufReader, Cursor}; use super::*; @@ -1405,7 +1405,8 @@ mod test { #[test] fn read_rect() { - let f = std::fs::File::open("tests/images/bmp/images/Core_8_Bit.bmp").unwrap(); + let f = + BufReader::new(std::fs::File::open("tests/images/bmp/images/Core_8_Bit.bmp").unwrap()); let mut decoder = super::BmpDecoder::new(f).unwrap(); let mut buf: Vec = vec![0; 8 * 8 * 3]; diff --git a/src/codecs/gif.rs b/src/codecs/gif.rs index b6946f3cdd..d1c6a98db7 100644 --- a/src/codecs/gif.rs +++ b/src/codecs/gif.rs @@ -10,9 +10,10 @@ //! use image::codecs::gif::{GifDecoder, GifEncoder}; //! use image::{ImageDecoder, AnimationDecoder}; //! use std::fs::File; +//! use std::io::BufReader; //! # fn main() -> std::io::Result<()> { //! // Decode a gif into frames -//! let file_in = File::open("foo.gif")?; +//! let file_in = BufReader::new(File::open("foo.gif")?); //! let mut decoder = GifDecoder::new(file_in).unwrap(); //! let frames = decoder.into_frames(); //! let frames = frames.collect_frames().expect("error decoding gif"); @@ -26,7 +27,7 @@ //! ``` #![allow(clippy::while_let_loop)] -use std::io::{self, Cursor, Read, Write}; +use std::io::{self, BufRead, Cursor, Read, Seek, Write}; use std::marker::PhantomData; use std::mem; @@ -82,7 +83,7 @@ impl Read for GifReader { } } -impl ImageDecoder for GifDecoder { +impl ImageDecoder for GifDecoder { fn dimensions(&self) -> (u32, u32) { ( u32::from(self.reader.width()), @@ -226,7 +227,7 @@ struct GifFrameIterator { limits: Limits, } -impl GifFrameIterator { +impl GifFrameIterator { fn new(decoder: GifDecoder) -> GifFrameIterator { let (width, height) = decoder.dimensions(); let limits = decoder.limits.clone(); @@ -392,7 +393,7 @@ impl Iterator for GifFrameIterator { } } -impl<'a, R: Read + 'a> AnimationDecoder<'a> for GifDecoder { +impl<'a, R: BufRead + Seek + 'a> AnimationDecoder<'a> for GifDecoder { fn into_frames(self) -> animation::Frames<'a> { animation::Frames::new(Box::new(GifFrameIterator::new(self))) } diff --git a/src/codecs/ico/decoder.rs b/src/codecs/ico/decoder.rs index 760ac0bec0..0ad4cf613d 100644 --- a/src/codecs/ico/decoder.rs +++ b/src/codecs/ico/decoder.rs @@ -1,5 +1,5 @@ use byteorder::{LittleEndian, ReadBytesExt}; -use std::io::{Read, Seek, SeekFrom}; +use std::io::{BufRead, Read, Seek, SeekFrom}; use std::{error, fmt}; use crate::color::ColorType; @@ -106,12 +106,12 @@ impl From for ImageFormat { } /// An ico decoder -pub struct IcoDecoder { +pub struct IcoDecoder { selected_entry: DirEntry, inner_decoder: InnerDecoder, } -enum InnerDecoder { +enum InnerDecoder { Bmp(BmpDecoder), Png(Box>), } @@ -141,7 +141,7 @@ struct DirEntry { image_offset: u32, } -impl IcoDecoder { +impl IcoDecoder { /// Create a new decoder that decodes from the stream ```r``` pub fn new(mut r: R) -> ImageResult> { let entries = read_entries(&mut r)?; @@ -248,7 +248,7 @@ impl DirEntry { Ok(signature == PNG_SIGNATURE) } - fn decoder(&self, mut r: R) -> ImageResult> { + fn decoder(&self, mut r: R) -> ImageResult> { let is_png = self.is_png(&mut r)?; self.seek_to_start(&mut r)?; @@ -260,7 +260,7 @@ impl DirEntry { } } -impl ImageDecoder for IcoDecoder { +impl ImageDecoder for IcoDecoder { fn dimensions(&self) -> (u32, u32) { match self.inner_decoder { Bmp(ref decoder) => decoder.dimensions(), diff --git a/src/codecs/jpeg/decoder.rs b/src/codecs/jpeg/decoder.rs index 01da58d14e..2fc51c1780 100644 --- a/src/codecs/jpeg/decoder.rs +++ b/src/codecs/jpeg/decoder.rs @@ -1,4 +1,4 @@ -use std::io::Read; +use std::io::{BufRead, Seek}; use std::marker::PhantomData; use crate::color::ColorType; @@ -22,7 +22,7 @@ pub struct JpegDecoder { phantom: PhantomData, } -impl JpegDecoder { +impl JpegDecoder { /// Create a new decoder that decodes from the stream ```r``` pub fn new(r: R) -> ImageResult> { let mut input = Vec::new(); @@ -50,7 +50,7 @@ impl JpegDecoder { } } -impl ImageDecoder for JpegDecoder { +impl ImageDecoder for JpegDecoder { fn dimensions(&self) -> (u32, u32) { (u32::from(self.width), u32::from(self.height)) } diff --git a/src/codecs/openexr.rs b/src/codecs/openexr.rs index 86ca2a9298..52ffbbf471 100644 --- a/src/codecs/openexr.rs +++ b/src/codecs/openexr.rs @@ -27,7 +27,7 @@ use crate::{ ColorType, ExtendedColorType, ImageDecoder, ImageEncoder, ImageError, ImageFormat, ImageResult, }; -use std::io::{Read, Seek, Write}; +use std::io::{BufRead, Seek, Write}; /// An OpenEXR decoder. Immediately reads the meta data from the file. #[derive(Debug)] @@ -45,7 +45,7 @@ pub struct OpenExrDecoder { alpha_present_in_file: bool, } -impl OpenExrDecoder { +impl OpenExrDecoder { /// Create a decoder. Consumes the first few bytes of the source to extract image dimensions. /// Assumes the reader is buffered. In most cases, /// you should wrap your reader in a `BufReader` for best performance. @@ -104,7 +104,7 @@ impl OpenExrDecoder { } } -impl ImageDecoder for OpenExrDecoder { +impl ImageDecoder for OpenExrDecoder { fn dimensions(&self) -> (u32, u32) { let size = self .selected_exr_header() @@ -382,7 +382,7 @@ mod test { } /// Read the file from the specified path into an `Rgb32FImage`. - fn read_as_rgb_image(read: impl Read + Seek) -> ImageResult { + fn read_as_rgb_image(read: impl BufRead + Seek) -> ImageResult { let decoder = OpenExrDecoder::with_alpha_preference(read, Some(false))?; let (width, height) = decoder.dimensions(); let buffer: Vec = crate::image::decoder_to_vec(decoder)?; @@ -396,7 +396,7 @@ mod test { } /// Read the file from the specified path into an `Rgba32FImage`. - fn read_as_rgba_image(read: impl Read + Seek) -> ImageResult { + fn read_as_rgba_image(read: impl BufRead + Seek) -> ImageResult { let decoder = OpenExrDecoder::with_alpha_preference(read, Some(true))?; let (width, height) = decoder.dimensions(); let buffer: Vec = crate::image::decoder_to_vec(decoder)?; diff --git a/src/codecs/png.rs b/src/codecs/png.rs index 8458b8d060..bdaa6b3aac 100644 --- a/src/codecs/png.rs +++ b/src/codecs/png.rs @@ -7,7 +7,7 @@ //! use std::fmt; -use std::io::{Read, Write}; +use std::io::{BufRead, Seek, Write}; use png::{BlendOp, DisposeOp}; @@ -26,13 +26,13 @@ use crate::{DynamicImage, GenericImage, ImageBuffer, Luma, LumaA, Rgb, Rgba, Rgb pub(crate) const PNG_SIGNATURE: [u8; 8] = [137, 80, 78, 71, 13, 10, 26, 10]; /// PNG decoder -pub struct PngDecoder { +pub struct PngDecoder { color_type: ColorType, reader: png::Reader, limits: Limits, } -impl PngDecoder { +impl PngDecoder { /// Creates a new decoder that decodes from the stream ```r``` pub fn new(r: R) -> ImageResult> { Self::with_limits(r, Limits::no_limits()) @@ -164,7 +164,7 @@ fn unsupported_color(ect: ExtendedColorType) -> ImageError { )) } -impl ImageDecoder for PngDecoder { +impl ImageDecoder for PngDecoder { fn dimensions(&self) -> (u32, u32) { self.reader.info().size() } @@ -221,7 +221,7 @@ impl ImageDecoder for PngDecoder { /// [`AnimationDecoder`]: ../trait.AnimationDecoder.html /// [`PngDecoder`]: struct.PngDecoder.html /// [`PngDecoder::apng`]: struct.PngDecoder.html#method.apng -pub struct ApngDecoder { +pub struct ApngDecoder { inner: PngDecoder, /// The current output buffer. current: Option, @@ -235,7 +235,7 @@ pub struct ApngDecoder { has_thumbnail: bool, } -impl ApngDecoder { +impl ApngDecoder { fn new(inner: PngDecoder) -> Self { let info = inner.reader.info(); let remaining = match info.animation_control() { @@ -419,11 +419,11 @@ impl ApngDecoder { } } -impl<'a, R: Read + 'a> AnimationDecoder<'a> for ApngDecoder { +impl<'a, R: BufRead + Seek + 'a> AnimationDecoder<'a> for ApngDecoder { fn into_frames(self) -> Frames<'a> { - struct FrameIterator(ApngDecoder); + struct FrameIterator(ApngDecoder); - impl Iterator for FrameIterator { + impl Iterator for FrameIterator { type Item = ImageResult; fn next(&mut self) -> Option { @@ -685,14 +685,14 @@ impl std::error::Error for BadPngRepresentation {} #[cfg(test)] mod tests { use super::*; - use std::io::Cursor; + use std::io::{BufReader, Cursor, Read}; #[test] fn ensure_no_decoder_off_by_one() { - let dec = PngDecoder::new( + let dec = PngDecoder::new(BufReader::new( std::fs::File::open("tests/images/png/bugfixes/debug_triangle_corners_widescreen.png") .unwrap(), - ) + )) .expect("Unable to read PNG file (does it exist?)"); assert_eq![(2000, 1000), dec.dimensions()]; @@ -721,7 +721,7 @@ mod tests { .unwrap(); not_png[0] = 0; - let error = PngDecoder::new(¬_png[..]).err().unwrap(); + let error = PngDecoder::new(Cursor::new(¬_png)).err().unwrap(); let _ = error .source() .unwrap() diff --git a/src/codecs/tga/decoder.rs b/src/codecs/tga/decoder.rs index 9da2392635..f3046edd7f 100644 --- a/src/codecs/tga/decoder.rs +++ b/src/codecs/tga/decoder.rs @@ -7,7 +7,7 @@ use crate::{ image::{ImageDecoder, ImageFormat}, }; use byteorder::ReadBytesExt; -use std::io::{self, Read, Seek}; +use std::io::{self, Read}; struct ColorMap { /// sizes in bytes @@ -59,7 +59,7 @@ pub struct TgaDecoder { color_map: Option, } -impl TgaDecoder { +impl TgaDecoder { /// Create a new decoder that decodes from the stream `r` pub fn new(r: R) -> ImageResult> { let mut decoder = TgaDecoder { @@ -172,7 +172,7 @@ impl TgaDecoder { /// is present fn read_image_id(&mut self) -> ImageResult<()> { self.r - .seek(io::SeekFrom::Current(i64::from(self.header.id_length)))?; + .read_exact(&mut vec![0; self.header.id_length as usize])?; Ok(()) } @@ -336,7 +336,7 @@ impl TgaDecoder { } } -impl ImageDecoder for TgaDecoder { +impl ImageDecoder for TgaDecoder { fn dimensions(&self) -> (u32, u32) { (self.width as u32, self.height as u32) } diff --git a/src/codecs/tiff.rs b/src/codecs/tiff.rs index d6426fd53f..9f4dd7358e 100644 --- a/src/codecs/tiff.rs +++ b/src/codecs/tiff.rs @@ -8,7 +8,7 @@ extern crate tiff; -use std::io::{self, Cursor, Read, Seek, Write}; +use std::io::{self, BufRead, Cursor, Read, Seek, Write}; use std::marker::PhantomData; use std::mem; @@ -22,7 +22,7 @@ use crate::image::{ImageDecoder, ImageEncoder, ImageFormat}; /// Decoder for TIFF images. pub struct TiffDecoder where - R: Read + Seek, + R: BufRead + Seek, { dimensions: (u32, u32), color_type: ColorType, @@ -34,7 +34,7 @@ where impl TiffDecoder where - R: Read + Seek, + R: BufRead + Seek, { /// Create a new TiffDecoder. pub fn new(r: R) -> Result, ImageError> { @@ -183,7 +183,7 @@ impl Read for TiffReader { } } -impl ImageDecoder for TiffDecoder { +impl ImageDecoder for TiffDecoder { fn dimensions(&self) -> (u32, u32) { self.dimensions } diff --git a/src/codecs/webp/decoder.rs b/src/codecs/webp/decoder.rs index df1985b84b..7dd174346a 100644 --- a/src/codecs/webp/decoder.rs +++ b/src/codecs/webp/decoder.rs @@ -1,4 +1,4 @@ -use std::io::{Read, Seek}; +use std::io::{BufRead, Read, Seek}; use crate::buffer::ConvertBuffer; use crate::error::{DecodingError, ImageError, ImageResult}; @@ -10,7 +10,7 @@ pub struct WebPDecoder { inner: image_webp::WebPDecoder, } -impl WebPDecoder { +impl WebPDecoder { /// Create a new WebPDecoder from the Reader ```r```. /// This function takes ownership of the Reader. pub fn new(r: R) -> ImageResult { @@ -32,7 +32,7 @@ impl WebPDecoder { } } -impl ImageDecoder for WebPDecoder { +impl ImageDecoder for WebPDecoder { fn dimensions(&self) -> (u32, u32) { self.inner.dimensions() } diff --git a/src/io/reader.rs b/src/io/reader.rs index 75565de73f..c76a27b1e6 100644 --- a/src/io/reader.rs +++ b/src/io/reader.rs @@ -1,5 +1,5 @@ use std::fs::File; -use std::io::{self, BufReader, Cursor, Read, Seek, SeekFrom}; +use std::io::{self, BufRead, BufReader, Cursor, Read, Seek, SeekFrom}; use std::path::Path; use crate::dynimage::DynamicImage; @@ -67,7 +67,7 @@ pub struct Reader { limits: super::Limits, } -impl<'a, R: 'a + Read + Seek> Reader { +impl<'a, R: 'a + BufRead + Seek> Reader { /// Create a new image reader without a preset format. /// /// Assumes the reader is already buffered. For optimal performance, diff --git a/src/lib.rs b/src/lib.rs index cae92169bd..0340feb053 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -91,14 +91,14 @@ //! While [`ImageDecoder`] and [`ImageDecoderRect`] give access to more advanced decoding options: //! //! ```rust,no_run -//! # use std::io::Read; +//! # use std::io::{BufReader, Cursor}; //! # use image::DynamicImage; //! # use image::ImageDecoder; //! # #[cfg(feature = "png")] //! # fn main() -> Result<(), image::ImageError> { //! # use image::codecs::png::PngDecoder; //! # let img: DynamicImage = unimplemented!(); -//! # let reader: Box = unimplemented!(); +//! # let reader: BufReader> = unimplemented!(); //! let decoder = PngDecoder::new(&mut reader)?; //! let icc = decoder.icc_profile(); //! let img = DynamicImage::from_decoder(decoder)?; diff --git a/tests/limits_anim.rs b/tests/limits_anim.rs index ed191800d8..d332f81b5c 100644 --- a/tests/limits_anim.rs +++ b/tests/limits_anim.rs @@ -7,7 +7,9 @@ use image::codecs::gif::GifDecoder; #[cfg(feature = "gif")] fn gif_decode(data: &[u8], limits: Limits) -> ImageResult<()> { - let mut decoder = GifDecoder::new(data).unwrap(); + use std::io::Cursor; + + let mut decoder = GifDecoder::new(Cursor::new(data)).unwrap(); decoder.set_limits(limits)?; { let frames = decoder.into_frames();