fix: decode TIFFs with RowsPerStrip greater than image height#389
Open
jmole-turtlebot wants to merge 3 commits intoimage-rs:mainfrom
Open
fix: decode TIFFs with RowsPerStrip greater than image height#389jmole-turtlebot wants to merge 3 commits intoimage-rs:mainfrom
jmole-turtlebot wants to merge 3 commits intoimage-rs:mainfrom
Conversation
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.
|
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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Two real-world fax4 TIFFs fail to decode on
image-tiff0.11.x and decode successfully after this PR:FillOrder = 2(LSB-first) returnsfailed to fill whole buffer. The fax decoders already reverse bits whenFillOrder = 2; this PR pins that behavior with a regression test and updates thetags.rscomment, which still claimed the tag was unhandled.ImageLength = 38andRowsPerStrip = 95318returnsFormat error decoding Tiff: platform or format size limits exceeded. Tracing it:strip_count()returnsOk(1)cleanly, but the unclampedchunk_dimensions().1 = 95318propagates into atry_into::<usize>()during readout buffer sizing and surfaces asIntSizeError.Fix
RowsPerStripis clamped toimage.heightonce atStripDecodeStateconstruction insrc/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-diskRowsPerStripremains 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 usesdiv_ceilinstead ofchecked_add(rows_per_strip - 1)plus manual division. This removes a defensiveIntSizeErrorpath that the new clamping makes unreachable; it is not the user-visible failure mode in any known input.Commits
fix: decode TIFFs with RowsPerStrip greater than image height— the structural fix, thestrip_countcleanup, the twotests/images/*.tiffregression fixtures, and their tests.docs: note that fax decoders honor FillOrder tag—src/tags.rscomment 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 existingcompute_tiff_hashhelper.Test plan