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

Fixed Tiff handling of bad EXIF data #1256

Merged
merged 2 commits into from Jun 6, 2015

Conversation

radarhere
Copy link
Member

Fixes the most recent problem in #1163 - that bad EXIF is causing a _binary error to occur, rather than generating a simple warning that the EXIF data is flawed.

This is a patch for the 2.8.x branch as requested there, rather than a fix for master.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 566cb22 on radarhere:2.8.x into * on python-pillow:master*.

@hugovk
Copy link
Member

hugovk commented Jun 5, 2015

It would be nice to include a test for this, but it would need a smallish test image that can be redistributed under our licence.

Perhaps we already have a suitable image in the test suite. If not, please can you create one or ask for one from a bug reporter? Thanks.

@radarhere
Copy link
Member Author

I've had no luck looking through the existing test suite images, or creating one myself. So yes, for the moment, requested one from either of the bug reporters.

@radarhere
Copy link
Member Author

Okay, I've managed to modify the EXIF data by hand from the hopper image to trigger the two new warnings.

@hugovk
Copy link
Member

hugovk commented Jun 6, 2015

Thanks!


Wow, GitHub has trouble showing that image! Mac Preview also just shows a blank, black box. I wouldn't have expected bad EXIF data to cause problems with the image data itself, but I guess it's not relevant for this test.


@aclark4life This looks good to merge.

This is a patch for the 2.8.x branch as requested there [#1163], rather than a fix for master.

Could you handle the merge/release? This'll also need to go into master too.

https://github.com/python-pillow/Pillow/blob/master/RELEASING.md#point-release

aclark4life added a commit that referenced this pull request Jun 6, 2015
Fixed Tiff handling of bad EXIF data
@aclark4life aclark4life merged commit 438104b into python-pillow:master Jun 6, 2015
@aclark4life
Copy link
Member

Thanks!

@radarhere radarhere deleted the 2.8.x branch June 7, 2015 02:08
@aclark4life
Copy link
Member

Uploaded! New policy proposal: after each release, including this one; we'll wait 24 hours before asking @cgohlke for binaries. If no issues appear in that time then we'll upload binaries. If an issue does occur, we release a new point release and repeat the process. I'd like to avoid creating and uploading binaries for "brown bag" releases, if possible. CC: @wiredfool @homm

@aclark4life
Copy link
Member

Great thanks @cgohlke

@hugovk
Copy link
Member

hugovk commented Jun 8, 2015

@aclark4life Sounds sensible, except for really-really-super-urgent stuff.

I've never heard of "brown bag releases" before, what does it mean? Where does the name come from?

@aclark4life
Copy link
Member

@hugovk Good question. I've been hearing the term "brown bag release" in Python circles ever since I've been publishing software to the Python Package Index. I'm not sure of the origin, in the Python community or otherwise. In general, it means "bad release" or "throw away release" and is typically used to describe a release in which some fatal flaw is discovered immediately after publication. Such releases are effectively useless and are typically followed by another "fixed" release shortly after. I suspect the origin has something to do with the "throw away nature" of a brown paper bag.

@wiredfool
Copy link
Member

@aclark4life Have there been any releases recently where that would have reduced the load on @cgohlke? This last set has been much more of a slow burn from changing the behavior of the binary functions. OTOH I can remember a release or two where the immediate +1 has been due to breaking the windows build. (which would indicate that it would be really good to get CI working for windows)

@aclark4life
Copy link
Member

@wiredfool No idea but the point is to try to be as considerate as possible. If we don't ask for 24 hours, then @cgohlke can decide whether or not he wants to build within that time frame. If we don't hear any issues, then we definitely ask at 24 hours. And yes I'd like to build a Pillow package factory at some point. Maybe Gratipay 2.0 will help incentivize and grow our team.

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

5 participants