From 1b38ff9410026f12e0b3559f349deea153830cb9 Mon Sep 17 00:00:00 2001 From: Robert Wassens Date: Fri, 4 Dec 2020 12:45:46 +0100 Subject: [PATCH 1/6] strip size = image height --- src/encoder/mod.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/encoder/mod.rs b/src/encoder/mod.rs index e1845e8..e055fad 100644 --- a/src/encoder/mod.rs +++ b/src/encoder/mod.rs @@ -623,7 +623,8 @@ impl<'a, W: 'a + Write + Seek, T: ColorType> ImageEncoder<'a, W, T> { let row_bytes = row_samples * u64::from(::BYTE_LEN); // As per tiff spec each strip should be about 8k long - let rows_per_strip = (8000 + row_bytes - 1) / row_bytes; + // let rows_per_strip = (8000 + row_bytes - 1) / row_bytes; + let rows_per_strip = height as u64; let strip_count = (u64::from(height) + rows_per_strip - 1) / rows_per_strip; From 7da8f01f7523ea3a65f883d4ea128a368096345a Mon Sep 17 00:00:00 2001 From: Robert Wassens Date: Sat, 5 Dec 2020 01:39:37 +0100 Subject: [PATCH 2/6] make strip size configurable --- src/encoder/mod.rs | 29 ++++++++++++++++++++++++++--- tests/encode_images.rs | 16 ++++++++++++++++ 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/src/encoder/mod.rs b/src/encoder/mod.rs index e055fad..952c3e0 100644 --- a/src/encoder/mod.rs +++ b/src/encoder/mod.rs @@ -589,6 +589,9 @@ impl<'a, W: Write + Seek> Drop for DirectoryEncoder<'a, W> { /// // You can encode tags here /// image.encoder().write_tag(Tag::Artist, "Image-tiff").unwrap(); /// +/// // Smaller strip size can be configured before writing data +/// image.rows_per_strip(2).unwrap(); +/// /// let mut idx = 0; /// while image.next_strip_sample_count() > 0 { /// let sample_count = image.next_strip_sample_count() as usize; @@ -620,10 +623,9 @@ impl<'a, W: 'a + Write + Seek, T: ColorType> ImageEncoder<'a, W, T> { height: u32, ) -> TiffResult> { let row_samples = u64::from(width) * u64::try_from(::BITS_PER_SAMPLE.len())?; - let row_bytes = row_samples * u64::from(::BYTE_LEN); - // As per tiff spec each strip should be about 8k long - // let rows_per_strip = (8000 + row_bytes - 1) / row_bytes; + // By default write the image as single strip. This can be changed by calling + // rows_per_strip() before the first data is written. let rows_per_strip = height as u64; let strip_count = (u64::from(height) + rows_per_strip - 1) / rows_per_strip; @@ -756,6 +758,27 @@ impl<'a, W: 'a + Write + Seek, T: ColorType> ImageEncoder<'a, W, T> { self.encoder.write_tag(Tag::YResolution, value).unwrap(); } + /// Set image number of lines per strip + /// + /// This function must be called before calling write_data() or write_strip(). + pub fn rows_per_strip(&mut self, value: u32) -> TiffResult<()> { + if self.strip_idx != 0 { + return Err(io::Error::new( + io::ErrorKind::InvalidInput, + "Cannot change strip size after data was written", + ) + .into()); + } + // Write tag as 32 bits + self.encoder.write_tag(Tag::RowsPerStrip, value)?; + + let value: u64 = value as u64; + self.strip_count = (self.height as u64 + value - 1) / value; + self.rows_per_strip = value; + + Ok(()) + } + fn finish_internal(&mut self) -> TiffResult<()> { self.encoder .write_tag(Tag::StripOffsets, &*self.strip_offsets)?; diff --git a/tests/encode_images.rs b/tests/encode_images.rs index e5c6a8c..d389122 100644 --- a/tests/encode_images.rs +++ b/tests/encode_images.rs @@ -360,3 +360,19 @@ fn test_multipage_image() { assert_eq!(img_decoder.dimensions().unwrap(), (3, 3)); } } + +#[test] +/// verify rows per strip setting +fn test_rows_per_strip() { + let mut file = Cursor::new(Vec::new()); + let mut img_encoder = TiffEncoder::new(&mut file).unwrap(); + + let mut image = img_encoder.new_image::(100, 100).unwrap(); + assert_eq!(image.next_strip_sample_count(), 100 * 100); + image.rows_per_strip(2).unwrap(); + assert_eq!(image.next_strip_sample_count(), 2 * 100); + + let img2: Vec = vec![0; 2 * 100]; + image.write_strip(&img2[..]).unwrap(); + assert!(image.rows_per_strip(5).is_err()); +} From 76173069a36cdff1b36c17573ddfb243878ecaad Mon Sep 17 00:00:00 2001 From: Robert Wassens Date: Sun, 6 Dec 2020 23:34:24 +0100 Subject: [PATCH 3/6] round trip test --- tests/encode_images.rs | 39 +++++++++++++++++++++++++++++++-------- 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/tests/encode_images.rs b/tests/encode_images.rs index d389122..b2cce41 100644 --- a/tests/encode_images.rs +++ b/tests/encode_images.rs @@ -365,14 +365,37 @@ fn test_multipage_image() { /// verify rows per strip setting fn test_rows_per_strip() { let mut file = Cursor::new(Vec::new()); - let mut img_encoder = TiffEncoder::new(&mut file).unwrap(); + { + let mut img_encoder = TiffEncoder::new(&mut file).unwrap(); - let mut image = img_encoder.new_image::(100, 100).unwrap(); - assert_eq!(image.next_strip_sample_count(), 100 * 100); - image.rows_per_strip(2).unwrap(); - assert_eq!(image.next_strip_sample_count(), 2 * 100); + let mut image = img_encoder.new_image::(100, 100).unwrap(); + assert_eq!(image.next_strip_sample_count(), 100 * 100); + image.rows_per_strip(2).unwrap(); + assert_eq!(image.next_strip_sample_count(), 2 * 100); - let img2: Vec = vec![0; 2 * 100]; - image.write_strip(&img2[..]).unwrap(); - assert!(image.rows_per_strip(5).is_err()); + let img2: Vec = vec![0; 2 * 100]; + image.write_strip(&img2[..]).unwrap(); + assert!(image.rows_per_strip(5).is_err()); + for i in 1..50 { + let img2: Vec = vec![i; 2 * 100]; + image.write_strip(&img2[..]).unwrap(); + } + assert!(image.write_strip(&img2[..]).is_err()); + } + + file.seek(SeekFrom::Start(0)).unwrap(); + { + let mut decoder = Decoder::new(&mut file).unwrap(); + assert_eq!(decoder.get_tag_u64(Tag::RowsPerStrip).unwrap(), 2); + assert_eq!(decoder.strip_count().unwrap(), 50); + + for i in 0..50 { + let img2 = [i; 2 * 100]; + if let DecodingResult::U8(data) = decoder.read_strip().unwrap() { + assert_eq!(&img2[..], &data[..]); + } else { + assert!(false); + } + } + } } From 5346a441aeb347132469519e906950e06e27f58e Mon Sep 17 00:00:00 2001 From: Robert Wassens Date: Mon, 7 Dec 2020 11:43:09 +0100 Subject: [PATCH 4/6] Processed review comment --- src/encoder/mod.rs | 2 +- tests/encode_images.rs | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/encoder/mod.rs b/src/encoder/mod.rs index 952c3e0..7430799 100644 --- a/src/encoder/mod.rs +++ b/src/encoder/mod.rs @@ -760,7 +760,7 @@ impl<'a, W: 'a + Write + Seek, T: ColorType> ImageEncoder<'a, W, T> { /// Set image number of lines per strip /// - /// This function must be called before calling write_data() or write_strip(). + /// `write_strip` and will return an error otherwise. pub fn rows_per_strip(&mut self, value: u32) -> TiffResult<()> { if self.strip_idx != 0 { return Err(io::Error::new( diff --git a/tests/encode_images.rs b/tests/encode_images.rs index b2cce41..7ba8eb3 100644 --- a/tests/encode_images.rs +++ b/tests/encode_images.rs @@ -381,6 +381,7 @@ fn test_rows_per_strip() { image.write_strip(&img2[..]).unwrap(); } assert!(image.write_strip(&img2[..]).is_err()); + image.finish().unwrap(); } file.seek(SeekFrom::Start(0)).unwrap(); @@ -391,10 +392,9 @@ fn test_rows_per_strip() { for i in 0..50 { let img2 = [i; 2 * 100]; - if let DecodingResult::U8(data) = decoder.read_strip().unwrap() { - assert_eq!(&img2[..], &data[..]); - } else { - assert!(false); + match decoder.read_strip().unwrap() { + DecodingResult::U8(data) => assert_eq!(&img2[..], &data[..]), + other => panic!("Incorrect strip type {:?}", other), } } } From 3e042df4382bcf241c9a111b8627a605f16a3891 Mon Sep 17 00:00:00 2001 From: Robert Wassens Date: Mon, 7 Dec 2020 11:44:21 +0100 Subject: [PATCH 5/6] Set default strip size to 1MB --- src/encoder/mod.rs | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/encoder/mod.rs b/src/encoder/mod.rs index 7430799..58663c2 100644 --- a/src/encoder/mod.rs +++ b/src/encoder/mod.rs @@ -623,10 +623,11 @@ impl<'a, W: 'a + Write + Seek, T: ColorType> ImageEncoder<'a, W, T> { height: u32, ) -> TiffResult> { let row_samples = u64::from(width) * u64::try_from(::BITS_PER_SAMPLE.len())?; + let row_bytes = row_samples * u64::from(::BYTE_LEN); - // By default write the image as single strip. This can be changed by calling - // rows_per_strip() before the first data is written. - let rows_per_strip = height as u64; + // Limit the strip size to prevent potential memory and security issues. + // Also keep the multiple strip handling 'oiled' + let rows_per_strip = (1_000_000 + row_bytes - 1) / row_bytes; let strip_count = (u64::from(height) + rows_per_strip - 1) / rows_per_strip; @@ -760,6 +761,7 @@ impl<'a, W: 'a + Write + Seek, T: ColorType> ImageEncoder<'a, W, T> { /// Set image number of lines per strip /// + /// This function needs to be called before any calls to `write_data` or /// `write_strip` and will return an error otherwise. pub fn rows_per_strip(&mut self, value: u32) -> TiffResult<()> { if self.strip_idx != 0 { From 95f0a34bcc112b9675e707b3d7fa8d5f2abb0c67 Mon Sep 17 00:00:00 2001 From: Robert Wassens Date: Mon, 7 Dec 2020 12:14:36 +0100 Subject: [PATCH 6/6] Update comment --- src/encoder/mod.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/encoder/mod.rs b/src/encoder/mod.rs index 58663c2..301732e 100644 --- a/src/encoder/mod.rs +++ b/src/encoder/mod.rs @@ -589,7 +589,7 @@ impl<'a, W: Write + Seek> Drop for DirectoryEncoder<'a, W> { /// // You can encode tags here /// image.encoder().write_tag(Tag::Artist, "Image-tiff").unwrap(); /// -/// // Smaller strip size can be configured before writing data +/// // Strip size can be configured before writing data /// image.rows_per_strip(2).unwrap(); /// /// let mut idx = 0;