Fix UnboundLocalError in ImageFile

[davarisg]

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/.../lib/python2.7/site-packages/PIL/Image.py", line 832, in convert
    self.load()
  File "/.../lib/python2.7/site-packages/PIL/ImageFile.py", line 248, in load
    if not self.map and (not LOAD_TRUNCATED_IMAGES or t == 0) and e < 0:
UnboundLocalError: local variable 't' referenced before assignment

Steps to reproduce:

from PIL import Image, ImageFile
ImageFile.LOAD_TRUNCATED_IMAGES = True
img = Image.open(<attached gif>)
img.convert("RGB")

It seems like d.setimage(self.im, e) (https://github.com/python-pillow/Pillow/blob/master/PIL/ImageFile.py#L203) raises a ValueError so the variable t will never be set.

I am not 100% sure if t = None is the right default value, but this seems to work.

[hugovk]

@davarisg It would be nice to add a unit test for this. Can this image be distributed under a MIT licence? Or do you have one which can be?

[davarisg]

@hugovk I am afraid it can't be distributed. Not sure if I can find another one.
I ll let you know.

[aclark4life]

@davarisg Can you find out if t = None is the right default value and add a test within 8 hours from now? If so, this can go in 2.8.0.

[wiredfool]

I think t=0 is probably correct. T is a length, indicating how much of a buffer has been read. If we get to the point that's erroring, something is really hosed in the image. We're not able to mmap it, and we haven't even been able to do anything reasonable with the decoder. Booting it with an ioerror seems reasonable.

And style wise, I'd rather see it defined outside the for loop, rather than have two code paths that initially define it.

[davarisg]

Sorry guys, but I cant find an image that can be distributed under the MIT licence. So I cant write a unit test right now. I ll keep looking though.
I ve pushed a commit that defines the t variable in a more appropriate place.

[coveralls]

Changes Unknown when pulling 68663ad on mixcloud:gd-unbound-local-variable into * on python-pillow:master*.

[aclark4life]

@davarisg Can we create an image and/or add a stub for the test so we don't forget? Thanks

[hugovk]

I've merged this.

It's just a single line to initialise a variable before use. Of course it'd be great to find a test image matching the problem one and add a test, but there's no need for that to delay this from today's release.

[aclark4life]

Thanks

[davarisg]

Thanks guys.
I ve managed to modify the binary of the image so that the UnboundLocalError is still raised and the content of the image is not viewable anymore. Is this something we can distribute under the MIT license?

Also have you considered adding the mock library in the requirements.txt? It would be really easy to write a test that patches setimage to raise a ValueError, in which case we would not even need an image to reproduce the error.

[aclark4life]

+1 for mock