[Image-SIG] PIL 1.1.6 ImageFile.Parser destroys data for PngImagePlugin

[Fredrik Lundh]

Thanks for the detailed analysis.  The fix in 1.1.7 is slightly
different from the one you propose:

    http://hg.effbot.org/pil-2009-raclette/changeset/fe4688f15fed/

Not sure why the code considers it important to close the file at that
point; I'll take another look at a look at the code and the history of
that file when I find the time.

</F>

On Wed, Nov 4, 2009 at 2:16 PM, Nils Olaf de Reus
<nils.de.reus at ivm.vu.nl> wrote:
> I am experiencing inconvenience from the behaviour of ImageFile.py in
> PIL 1.1.6 when attempting to read in a PNG image.
>
> The method I see documented to retreive an image through ImageFile is by
> invoking close() on the ImageFile.Parser, like this.
>
>>>> import ImageFile
>>>> import urllib
>>>>
>>>> f = urllib.urlopen('<url-path-to-png-file>')
>>>>
>>>> p = ImageFile.Parser()
>>>>
>>>> while True:
> ...     s = f.read(1024)
> ...     if not s:
> ...         break
> ...     p.feed(s)
> ...
>>>> im = p.close()
>
> The way ImageFile.Parser works, it creates an ImageFile._ParserFile to
> act as a file, and calls Image.open() on that. So far so good, and this
> results in a valid png image object.. for a very brief moment, because
> in the 'finally' section immediately after, it calls close() on that
> ImageFile._ParserFile.
>
> The definition of close() in ImageFile._ParserFile is as follows:
>
>    def close(self):
>        self.data = self.offset = None
>
> But PngImagePlugin does not copy data on opening - it just keeps a
> reference to the source. So this just now destroyed the data on the
> object that PngImagePlugin is still referring to as the location of its
> IDAT chunks.. not a bright move, because we have now created a condition
> where from the point of view of the PngImagePlugin, self.fp is an
> ImageFile._ParserFile instance, and self.fp.data is None.
>
> Which means that as soon as we try to do anything with that image,
> PngImagePlugin is going to be firing read() instructions at its self.fp,
> which in turn blindly trusts that its self.data supports __getitem__()..
> and predictably, there is our Traceback:
>
>>>> im.convert('RGB')
> Traceback (most recent call last):
>  File "<stdin>", line 1, in <module>
>  File "/usr/lib/python2.5/site-packages/PIL/Image.py", line 653, in
> convert
>    self.load()
>  File "/usr/lib/python2.5/site-packages/PIL/ImageFile.py", line 189, in
> load
>    s = read(self.decodermaxblock)
>  File "/usr/lib/python2.5/site-packages/PIL/PngImagePlugin.py", line
> 365, in load_read
>    return self.fp.read(bytes)
>  File "/usr/lib/python2.5/site-packages/PIL/ImageFile.py", line 300, in
> read
>    data = self.data[pos:pos+bytes]
> TypeError: 'NoneType' object is unsubscriptable
>
>
> I am currently working around the issue by creating my own
> ImageFile._ParserFile object that avoids having its data None'd.
>
>>>> import ImageFile
>>>> import urllib
>>>>
>>>> f = urllib.urlopen('<url-path-to-png-file>')
>>>>
>>>> p = ImageFile.Parser()
>>>>
>>>> while True:
> ...     s = f.read(1024)
> ...     if not s:
> ...         break
> ...     p.feed(s)
> ...
>>>> virtualfile = ImageFile._ParserFile(p.data)
>>>> im = Image.open(virtualfile)
>>>> p.close()
>>>> im.convert('RGB')
> <Image.Image instance at 0x2b890f5cff80>
>
>
> This works great -just need to keep the virtualfile around until I'm
> really done with it- but obviously I am not happy having to use what is
> supposed to be an internal from ImageFile.
>
> >From where I stand, the sensible thing would be to eliminate these two
> lines from ImageFile.Parser.close():
>
>            finally:
>                fp.close() # explicitly close the virtual file
>
> ..except that because of the comment, I must assume that there is a
> reason to be doing this. Could someone please shed light on why it is
> necessary to explicitly close fp there?
>
> Kind regards,
>  Nils Olaf de Reus
>
>
> _______________________________________________
> Image-SIG maillist  -  Image-SIG at python.org
> http://mail.python.org/mailman/listinfo/image-sig
>