Pillow silently fails with "Cannot identify image file" long after an image has been identified

[jleclanche]

When creating a custom image plugin, if Plugin._open() fails for whatever reason, Pillow will fail:

Traceback (most recent call last):
  File "./FtexImagePlugin.py", line 106, in <module>
    im = Image.open(sys.argv[1])
  File "/usr/lib/python3.5/site-packages/PIL/Image.py", line 2295, in open
    % (filename if filename else fp))
OSError: cannot identify image file '/home/adys/archive/eoc/images/envmaps/Copper.ftc'

This hints at the error being in the magic check, which of course it isn't. It silently ignores errors and just thinks the image is simply not a valid file of that type and moves on, eventually raising an unrelated error. This is extremely frustrating.

[jleclanche]

It's because of this:

Pillow/PIL/Image.py

Line 2278 in 3d6e137

except (SyntaxError, IndexError, TypeError, struct.error):

[wiredfool]

See #1474 and #1532 and #1423 where it was actually taken out.

This error is useful when hacking on pillow, but not when using pillow. Anything that spams the debug log when opening an image in django is a support pita.

[jleclanche]

It just shouldn't catch everything... Plugins should raise an InvalidFile error or some such, and it should catch only that type of exception. This is related to the ticket I filed recently wrt SyntaxError.

[radarhere]

I'm guessing that the issue that @jleclanche is referring to is #1643

[jleclanche]

Partly, yes.

[aclark4life]

You want to submit a PR for this one, too? Not sure about the logistics of getting it merged but always nice to have specific implementation to reference.

[jleclanche]

@aclark4life I don't feel comfortable PRing something for this until the first PR is landed and has been pretty thoroughly tested. Also need to start using those new exception types everywhere and make sure PIL doesn't raise any of those errors anymore, always PIL-based exceptions.

Once that happens, the except block can be replaced with a single except PILReadError.

[radarhere]

The 'first PR' being referred to is #2339

[radarhere]

I think the spirit is that if there is an error when loading an image, it doesn't necessarily mean that there is a problem with Pillow's code - it may mean that there is a problem with the file. I don't think Pillow's approach to this should be changed without a compelling reason.

---

I had a thought that we could distinguish between not opening an image and not identifying an image. However, I don't think our formats allow for this.

• FpxImagePlugin initially _accepts an image, but may later decide it's not FPX after all. So if an error occurs before that second line, it shouldn't be marked as identified. I don't see how you could communicate that distinction purely by catching errors without further obscuring the original cause.
• ImtImagePlugin does not have a good ability to distinguish between a malformed file and a file of another type.

It could have been a good idea if we were starting Pillow from scratch, but for a library concerned with backwards compatibility, I don't think it can be achieved without rewrites that seem excessive.