Skip to content

Conversation

@JimBobSquarePants
Copy link
Member

Prerequisites

  • I have written a descriptive pull-request title
  • I have verified that there are no overlapping pull-requests open
  • I have verified that I am following the existing coding patterns and practice as demonstrated in the repository. These follow strict Stylecop rules 👮.
  • I have provided test coverage for my change (where applicable)

Description

Fixes #2924

With the update to the chunk identification code in v3.1.8, now detects an invalid iDAT chunk containing 6 bytes of data immediately following the correct and complete data. validation code throws an exception in cases, as this information is considered critical.

The revised behavior now ensures an early return when an issue with zLib data arises after the complete decoding of pixel data.

Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR prevents decoding of extra iDAT chunks once the image data is fully decoded to avoid exceptions from invalid trailing zlib data.

  • Introduce a hasImageData flag to skip subsequent chunks after decoding completes
  • Update ReadScanlines, DecodePixelData, and DecodeInterlacedPixelData to early return and set the flag
  • Add a new test (Issue 2924) and reference images to validate the fix

Reviewed Changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
tests/Images/Input/Png/issues/Issue_2924.png Add LFS pointer for the malformed PNG used in the new test
tests/Images/External/ReferenceOutput/PngDecoderTests/CanDecode_Issue2924_Rgba32_Issue_2924.png Add the expected decoded output for Issue 2924
tests/ImageSharp.Tests/TestImages.cs Register the new Issue2924 constant
tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs Add CanDecode_Issue2924 test case
src/ImageSharp/Formats/Png/PngDecoderCore.cs Add hasImageData flag and guard logic to skip extra zlib chunks
Comments suppressed due to low confidence (2)

src/ImageSharp/Formats/Png/PngDecoderCore.cs:137

  • [nitpick] The field name 'hasImageData' is a bit vague. Consider renaming it to something like 'isImageDecoded' or 'imageDataRead' to clarify its purpose.
private bool hasImageData;

tests/ImageSharp.Tests/Formats/Png/PngDecoderTests.cs:730

  • [nitpick] This test only checks the Rgba32 pixel type. Consider adding additional pixel type cases (e.g., Rgb24, Argb32) or using a generic theory to ensure the fix works across all supported formats.
[WithFile(TestImages.Png.Issue2924, PixelTypes.Rgba32)]

@JimBobSquarePants JimBobSquarePants merged commit c2a8d6f into main May 28, 2025
22 checks passed
@JimBobSquarePants JimBobSquarePants deleted the js/v4-issue-2924 branch May 28, 2025 23:47
mus65 pushed a commit to mus65/ImageSharp that referenced this pull request Sep 16, 2025
mus65 added a commit to mus65/ImageSharp that referenced this pull request Sep 16, 2025
mus65 added a commit to mus65/ImageSharp that referenced this pull request Sep 16, 2025
mus65 added a commit to mus65/ImageSharp that referenced this pull request Sep 16, 2025
@mus65 mus65 mentioned this pull request Sep 16, 2025
4 tasks
JimBobSquarePants added a commit that referenced this pull request Sep 29, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SixLabors.ImageSharp.ImageFormatException: Bad method for ZLIB header: cmf=3 after updating to 3.1.8

2 participants