Raise an EOFError when seeking too far in PNG#4528
Raise an EOFError when seeking too far in PNG#4528hugovk merged 4 commits intopython-pillow:masterfrom
Conversation
|
@pmrowla Please could you have a look at this PR? Thank you! |
|
This has the same effect as what I suggested in #4519, and it's simpler. Testing w/scikit-image also shows the the original downstream issue would be resolved. |
|
@radarhere Thoughts about the doccheck failure? I checked the branch out locally and cannot reproduce. I'd restart it, but the restart button is now missing for me on this repo on Travis CI. |
|
It's not just this branch. It's appeared in others. Why now? The last master run - https://travis-ci.org/github/python-pillow/Pillow/jobs/671262347#L4634 - used Sphinx v2.4.4. The issue would be that then, Sphinx v3 was released - https://github.com/sphinx-doc/sphinx/releases/tag/v3.0.0 And so then this PR uses Sphinx v3.0.0 - https://travis-ci.org/github/python-pillow/Pillow/jobs/671293213#L4559 What is the error? https://travis-ci.org/github/python-pillow/Pillow/jobs/671293213#L4567-L4573 A series of warnings like
Sphinx has realised that we are documenting PngImageFile twice. You can see this at https://pillow.readthedocs.io/en/latest/reference/plugins.html#module-PIL.PngImagePlugin - once at the start of the module, and once if you scroll down under ChunkStream. I've pushed a commit removing the first one. See https://pillow-radarhere.readthedocs.io/en/png_seek/reference/plugins.html#module-PIL.PngImagePlugin for what this now looks like. |
|
And I had 2.4.1 locally. Thanks! |
There is a regression in the latest Pillow releases python-pillow/Pillow#4528 The fix has been merged but not released yet
|
Kinda nudge toward a release since this issue seems to affecting one passionate user for a release |
|
Thanks for the nudge! We'll do a release this weekend, keep an eye on #4354 (comment). |
|
Pillow 7.1.2 has now been released with this fix. |
|
Thanks! |
Resolves #4518
Looking at PngImagePlugin, there's no need for
n_framesoris_animatedto use property decorators - they can just be regular properties.You might think this is just a simplification of the code, and that it would have no effect on behaviour. This is not the case - the common
_seek_checkmethod changes behaviour. If_n_framesis present, andNone, then it doesn't check the upper limit on the frame number, because that would potentially require seeking all the way to the end of the file. Now that there is no_n_frames, it checks the upper limit, and throws anEOFError.I've added a test to ensure that an EOFError is thrown moving forward.
Testing, I find that this resolves the issue. This is also noted in #4518 (comment).