Skip to content

fix: decode TIFFs with RowsPerStrip greater than image height#389

Open
jmole-turtlebot wants to merge 3 commits intoimage-rs:mainfrom
jmole:fix/rows-per-strip-clamp
Open

fix: decode TIFFs with RowsPerStrip greater than image height#389
jmole-turtlebot wants to merge 3 commits intoimage-rs:mainfrom
jmole:fix/rows-per-strip-clamp

Conversation

@jmole-turtlebot
Copy link
Copy Markdown

Summary

Two real-world fax4 TIFFs fail to decode on image-tiff 0.11.x and decode successfully after this PR:

  • A fax4 sample with FillOrder = 2 (LSB-first) returns failed to fill whole buffer. The fax decoders already reverse bits when FillOrder = 2; this PR pins that behavior with a regression test and updates the tags.rs comment, which still claimed the tag was unhandled.
  • A fax4 sample with ImageLength = 38 and RowsPerStrip = 95318 returns Format error decoding Tiff: platform or format size limits exceeded. Tracing it: strip_count() returns Ok(1) cleanly, but the unclamped chunk_dimensions().1 = 95318 propagates into a try_into::<usize>() during readout buffer sizing and surfaces as IntSizeError.

Fix

RowsPerStrip is clamped to image.height once at StripDecodeState construction in src/decoder/image.rs, so every internal read site sees a consistent value and downstream stride/buffer math never observes a strip taller than the image. The raw on-disk RowsPerStrip remains accessible through the IFD/tag APIs (Directory::find_tag(Tag::RowsPerStrip)); only the decoder's internal strip-geometry state is normalized.

As a related cleanup, strip_count() now uses div_ceil instead of checked_add(rows_per_strip - 1) plus manual division. This removes a defensive IntSizeError path that the new clamping makes unreachable; it is not the user-visible failure mode in any known input.

Commits

  1. fix: decode TIFFs with RowsPerStrip greater than image height — the structural fix, the strip_count cleanup, the two tests/images/*.tiff regression fixtures, and their tests.
  2. docs: note that fax decoders honor FillOrder tagsrc/tags.rs comment only.

Fixtures

Both fixtures are minimal (~450 bytes each) real producer files:

  • tests/images/fax4-fillorder-lsb.tiff — fax4 1-bit grayscale, FillOrder = 2, 315x101.
  • tests/images/fax4-rowsperstrip-oversize.tiff — fax4 1-bit grayscale, 83x38, RowsPerStrip = 95318.

Tests assert colortype(), dimensions(), strip_count(), chunk_dimensions(), chunk_data_dimensions(0) (for the second fixture, which is the precise regression check for the structural fix), and the full decoded-output CRC32 via the existing compute_tiff_hash helper.

Test plan

  • `cargo test --test decode_images` passes locally (72 tests, including the two new ones).

jmole-turtlebot added 3 commits April 24, 2026 10:39
Two real-world fax4 TIFFs failed to decode on image-tiff 0.11.x:

- A FillOrder=2 (LSB-first) sample failed with "failed to fill whole
  buffer" when the bit-stream was read MSB-first despite the tag.
  The fax decoder already honors FillOrder; this change pins that
  behavior with a regression test.

- A sample with ImageLength=38 and RowsPerStrip=95318 failed with
  "platform or format size limits exceeded" because the unclamped
  strip dimensions propagated into a try_into::<usize>() during
  readout buffer sizing.

Clamp RowsPerStrip to image height once at StripDecodeState
construction so every internal read site sees a consistent value.
The raw on-disk RowsPerStrip remains available through the IFD/tag
APIs; only the decoder's internal strip-geometry state is normalized.

As a related cleanup, strip_count() now uses div_ceil instead of
checked_add(rows_per_strip - 1) plus manual division, removing a
defensive overflow path that the new clamping makes unreachable.

Two regression fixtures land alongside the fix:
- tests/images/fax4-fillorder-lsb.tiff
- tests/images/fax4-rowsperstrip-oversize.tiff
The FillOrder=266 entry in the tag table was marked "TODO add
support", but the fax3/fax4 decoders already reverse the bit order
when FillOrder=2 is set. Update the comment to reflect actual
behavior so future readers don't assume the tag is unhandled.
@jmole
Copy link
Copy Markdown

jmole commented Apr 25, 2026

We unintentionally committed a random image from our corpus that had the RowsPerStrip overflow, without actually looking at it. Replaced it in 30f6e97 with a more reasonable image. I apologize for the oversight - no harm was meant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants