Repository: image-rs/image-tiff
Pull request #145 (2021-06-16 18:15:03)

Fix the mandrill.tiff regression
This fixes https://github.com/image-rs/image-tiff/issues/137.
I'd consider this to be more of a temporary workaround, hence the TODO.
ChunkType is related to incremental decoding, so fixing this in a better way would mean changing the way strip incremental decoding works. So that should probably be done in a seperate PR.

The `test_div_zero()` was removed because it was now (properly) returning an error in this function.

HeroicKatora: > The test_div_zero() was removed because it was now (properly) returning an error in this function.

I'm not sure if I understand this. This was introduced as a regression test for a certain type of fuzzer failure (https://github.com/image-rs/image-tiff/pull/138) so rather I would update it to ignore the error. The main concern of the test is with panicking.
TomasKralCZ: Oh, ok, I'll update the error.

Patch:
diff --git a/src/decoder/mod.rs b/src/decoder/mod.rs
index 1f5c321..6175149 100644
--- a/src/decoder/mod.rs
+++ b/src/decoder/mod.rs
@@ -662,6 +662,8 @@ impl<R: Read + Seek> Decoder<R> {
         self.width = self.get_tag_u32(Tag::ImageWidth)?;
         self.height = self.get_tag_u32(Tag::ImageLength)?;
         self.strip_decoder = None;
+        self.tile_decoder = None;
+        self.tile_attributes = None;
 
         self.photometric_interpretation = self
             .find_tag_unsigned(Tag::PhotometricInterpretation)?
@@ -698,22 +700,21 @@ impl<R: Read + Seek> Decoder<R> {
             _ => return Err(TiffUnsupportedError::UnsupportedSampleDepth(self.samples).into()),
         }
 
-        self.chunk_type =
-            match (
-                self.get_tag_u32(Tag::RowsPerStrip),
-                self.get_tag_u32(Tag::TileWidth),
-                self.get_tag_u32(Tag::TileLength),
-            ) {
-                (Ok(_), Err(_), Err(_)) => ChunkType::Strip,
-                (Err(_), Ok(_), Ok(_)) => ChunkType::Tile,
-                // TODO: The spec says not to use both strip-oriented fields and tile-oriented fields.
-                // We can relax this later if it becomes a problem
-                _ => return Err(TiffError::FormatError(TiffFormatError::Format(
-                    String::from(
-                        "Neither strips nor tiles were found or both were used in the same file",
-                    ),
-                ))),
-            };
+        let ifd = self.ifd.as_ref().unwrap();
+        self.chunk_type = match (
+            ifd.contains_key(&Tag::StripByteCounts),
+            ifd.contains_key(&Tag::StripOffsets),
+            ifd.contains_key(&Tag::TileByteCounts),
+            ifd.contains_key(&Tag::TileOffsets),
+        ) {
+            (true, true, false, false) => ChunkType::Strip,
+            (false, false, true, true) => ChunkType::Tile,
+            (_, _, _, _) => {
+                return Err(TiffError::FormatError(
+                    TiffFormatError::StripTileTagConflict,
+                ))
+            }
+        };
 
         Ok(())
     }
diff --git a/src/error.rs b/src/error.rs
index cd7ad30..67e5f08 100644
--- a/src/error.rs
+++ b/src/error.rs
@@ -63,6 +63,7 @@ pub enum TiffFormatError {
     SignedIntegerExpected(Value),
     Format(String),
     RequiredTagEmpty(Tag),
+    StripTileTagConflict,
     #[doc(hidden)]
     /// Do not match against this variant. It may get removed.
     __NonExhaustive,
@@ -113,6 +114,7 @@ impl fmt::Display for TiffFormatError {
             }
             Format(ref val) => write!(fmt, "Invalid format: {:?}.", val),
             RequiredTagEmpty(ref val) => write!(fmt, "Required tag {:?} was empty.", val),
+            StripTileTagConflict => write!(fmt, "File should contain either (StripByteCounts and StripOffsets) or (TileByteCounts and TileOffsets), other combination was found."),
             __NonExhaustive => unreachable!(),
         }
     }


Test patch:
diff --git a/tests/decode_images.rs b/tests/decode_images.rs
index 59bcca0..cdb0461 100644
--- a/tests/decode_images.rs
+++ b/tests/decode_images.rs
@@ -287,7 +287,6 @@ fn test_tiled_incremental() {
 
 #[test]
 fn test_div_zero() {
-    use tiff::tags;
     use tiff::{TiffError, TiffFormatError};
 
     let image = [
@@ -298,12 +297,10 @@ fn test_div_zero() {
         178, 178, 178,
     ];
 
-    let mut decoder = tiff::decoder::Decoder::new(std::io::Cursor::new(&image)).unwrap();
-
-    let err = decoder.read_image().unwrap_err();
+    let err = tiff::decoder::Decoder::new(std::io::Cursor::new(&image)).unwrap_err();
 
     match err {
-        TiffError::FormatError(TiffFormatError::InvalidTagValueType(tags::Tag::TileWidth)) => {}
+        TiffError::FormatError(TiffFormatError::StripTileTagConflict) => {}
         unexpected => panic!("Unexpected error {}", unexpected),
     }
 }
@@ -385,3 +382,8 @@ fn test_stripped_image_overflow() {
         unexpected => panic!("Unexpected error {}", unexpected),
     }
 }
+
+#[test]
+fn test_no_rows_per_strip() {
+    test_image_sum_u8("no_rows_per_strip.tiff", ColorType::RGB(8), 99448840);
+}