-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Conversation
7811bbf
to
885297d
Compare
@@ -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): |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
Does |
From what I can tell, in |
|
||
image_data = HEAD + broken_crc_chunk_data + TAIL | ||
self.assertRaises(SyntaxError, | ||
lambda: PngImagePlugin.PngImageFile(BytesIO(image_data))) |
There was a problem hiding this comment.
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))
There was a problem hiding this comment.
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.
I haven't met such images in a wild. But I'm always for decoding more with |
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.