Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix issue in PNG identify method to skip "uninteresting" chunks, incl… #2019

Conversation

flew2bits
Copy link
Contributor

…uding sBIT.

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

Add a default clause in the PNG Identify method to skip over "uninteresting" chunks so that the data is properly read.
This fixes issue #2018

@CLAassistant
Copy link

CLAassistant commented Feb 19, 2022

CLA assistant check
All committers have signed the CLA.

@flew2bits flew2bits reopened this Feb 19, 2022
@flew2bits
Copy link
Contributor Author

The call to SkipChunkDataAndCrc was in the wrong location to fix the problem. I've moved it to a location that should fix the problem.

@flew2bits
Copy link
Contributor Author

I moved the call to SkipChunkDataAndCrc back to the default switch clause based on how other chunks are handled in the switch statement. It's not wrapped in a check to see if the identification is for color metadata only, also based on other clauses.

@brianpopow
Copy link
Collaborator

I have found an png with a sBit chunk:
length_sbit

Would that reproduce the described issue in #2018? If so, it would be good to have a unit test covering this issue. See PngDecoderTests for an example howto do that.

@flew2bits
Copy link
Contributor Author

Unfortunately that image doesn't exhibit the problem. I'll see if I can put one together that can be used for testing.

@JimBobSquarePants
Copy link
Member

I'm actually happy to push this in without the offending image. As a performance optimization alone it's worth the addition.

@flew2bits if you could supply an image at some point that would be really great.

@brianpopow
Copy link
Collaborator

I'm actually happy to push this in without the offending image. As a performance optimization alone it's worth the addition.

@flew2bits if you could supply an image at some point that would be really great.

@JimBobSquarePants should we milestone this for 2.0.1, also?

@JimBobSquarePants JimBobSquarePants added this to the 2.*.* milestone Feb 27, 2022
@JimBobSquarePants
Copy link
Member

@brianpopow Yeah... Let's get it merged now.

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.

Get "Invalid PNG data." exception during Identify phase of loading PNG image when image contains sBIT chunk
4 participants