Repository navigation
Conversation
…big-endian 16-bit tRNS Zero-length IDAT and fdAT chunks are valid, but the data chunk readers returned their length of 0, which the inflater treats as the end of the compressed stream. An empty chunk inside the zlib header, or two in a row, broke decoding. The readers now skip empty chunks. A PLTE chunk shorter than one entry produced an empty color table, which the palette scanline processor indexed outside of. It is now treated as a missing palette, so decoding throws InvalidImageContentException. 16-bit grayscale and truecolor tRNS samples are big-endian, like all PNG multi-byte values, but were read and written little-endian.
4 tasks done
This branch has not been deployed
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.
Prerequisites
Description
This PR fixes three PNG defects: two make the decoder more robust and one corrects transparency metadata.
Zero-length IDAT / fdAT chunks end the compressed stream early
The PNG compressed stream is all the IDAT (or fdAT) payloads joined together, and zero-length data chunks are allowed.
ReadNextDataChunkandReadNextFrameDataChunkreturned the empty chunk's length of 0.ChunkedReadStreamreads a 0 as "no more data". So when an empty chunk fell inside the zlib header (for example[1 byte][empty][rest]), or when two empty chunks came in a row, the header read failed. Decoding then went wrong:ImageFormatException: Bad method for ZLIB header, or a frame with no pixels. The same happened with fdAT chunks that hold only a sequence number. Both readers now skip empty chunks and return 0 only when the data really ends.A PLTE chunk shorter than one entry reads outside the color table
A PLTE chunk of 1 or 2 bytes gave an empty color table. The palette scanline processor clamps indices to
Length - 1, which is -1 for an empty table, so the decoder read memory before the table. In testing this crashed with anEntryPointNotFoundException, in both non-interlaced and Adam7 images.AssignColorPalettenow treats a palette that is too short as missing, so indexed decoding throws the usualInvalidImageContentException.16-bit tRNS values use the wrong byte order
PNG stores 16-bit grayscale and truecolor tRNS samples big-endian, like all other multi-byte values. The decoder read them little-endian and the encoder wrote them little-endian. A file that marks 0x0102 as transparent therefore made 0x0201 transparent, and files written by ImageSharp marked the wrong color for other readers. Both directions now use big-endian. The 8-bit path was already correct. The existing 16-bit tRNS test images use
FFFF, which reads the same in both byte orders, so they did not catch this.Testing
All new tests build their PNG data in memory. No test images or reference images were added or changed.
Decode_CompressedDataSplitAcrossDataChunks_DecodesPixelExact: encodes a noise image, then rewrites its IDAT data in eight layouts (leading empty, two leading empty, 1-byte first chunk, empty chunk inside the zlib header, one byte per chunk, one byte per chunk with empty chunks between them, empty chunk in the middle, trailing empty). Each layout must decode pixel-exact.Decode_EmptyFrameDataChunks_DecodesPixelExact: an APNG in which every fdAT is split inside its zlib header around an fdAT that holds only a sequence number. Sequence numbers are renumbered.Decode_PaletteChunkShorterThanOneEntry_ThrowsInvalidImageContentException(non-interlaced and Adam7).Decode_16BitGrayscaleTransparency_IsBigEndian,Decode_16BitRgbTransparency_IsBigEndian: check both the Identify metadata and the decoded alpha.TransparencyChunk_16Bit_IsWrittenBigEndian(grayscale and RGB): checks the tRNS bytes on disk and the round trip.The new tests fail on
main(10 cases) and pass with this change. In Release, allFormatstests pass: 42,602 passed, 2 skipped.Fixes #3240