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

Ability to skip crc checks for ancillary chunks #1991

Merged
merged 3 commits into from
Jun 30, 2016

Conversation

kkopachev
Copy link
Contributor

I encountered that some PNG files could not be open with Pillow since they have crc mismatch for iTXt chunks.
Browsers and imagemagick can open them just fine (though imagemagick shows a warning)
libpng defines a way to set it's behavior to throw an error or warning for ancillary chunks.

So I am proposing use of LOAD_TRUNCATED_IMAGES to skip crc checks for ancillary chunks. No need to calculate if we're going to ignore an error.

@@ -138,6 +138,12 @@ def call(self, cid, pos, length):
def crc(self, cid, data):
"Read and verify checksum"

# Skip CRC checks for ancillary chunks if allowed to load truncated images
# 5th byte of first char is 1 [specs, section 5.4]
if ImageFile.LOAD_TRUNCATED_IMAGES and (i8(cid[0]) >> 5 & 1):
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer to ignore the error after it occurs. Of course, it is meaningless to do the check, if we are going to accept the result in any case. But in the future, we can add some warnings or other decoder info.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, why bother doing this now, if in both cases code has to be changed to throw a warning?
But anyway, I can keep crc check if you'd like. Would you like to ignore checksum mismatch or incomplete checksum as well?

@homm
Copy link
Member

homm commented Jun 29, 2016

Does libpng and friends open images with errors in data chunks?

@kkopachev
Copy link
Contributor Author

kkopachev commented Jun 29, 2016

From what I can tell, in libpng user of it can ask to throw errors for any crc corruptions, or ignore ancillary, or ignore all. Not sure what happens if it encounters errors in data chunks, though. My thought here was that since Pillow does not expose any methods to change decoder behavior, but only LOAD_TRUNCATED_IMAGES, using that flag would be pretty safe bet to let read files at least with ancillary chunks broken.
Not sure where my original image came from, but looks like there was an issue with libpng where it sometimes could produce invalid CRC for iTXt chunk.


image_data = HEAD + broken_crc_chunk_data + TAIL
self.assertRaises(SyntaxError,
lambda: PngImagePlugin.PngImageFile(BytesIO(image_data)))
Copy link
Member

Choose a reason for hiding this comment

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

self.assertRaises(SyntaxError, PngImagePlugin.PngImageFile, BytesIO(image_data))

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed. FYI there are few more lambda usages in that file in same context.

@homm
Copy link
Member

homm commented Jun 30, 2016

I haven't met such images in a wild. But I'm always for decoding more with LOAD_TRUNCATED_IMAGES.

@homm homm merged commit 639bdd3 into python-pillow:master Jun 30, 2016
@kkopachev kkopachev deleted the png-crc-error-ignore branch June 30, 2016 18:06
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.

None yet

2 participants