Opened 3 years ago

Closed 3 years ago

#18374 closed Cleanup/optimization (fixed)

ImageField not validating due to missing libraries

Reported by: fabian@… Owned by: nobody
Component: Documentation Version: 1.4
Severity: Normal Keywords:
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

IMHO, it is worth to be mentioned in the documentation that libjpeg must be available on the system PRIOR to PIL being installed.
Else, validating a form with an ImageField will fail with corrupt image error, due to django/forms/fields.py plays down the Exception in line 592, which most probably is IOError: "decoder jpeg not available"

On a Mac, it can be easily downloaded on
http://ethan.tira-thompson.com/Mac_OS_X_Ports.html

On most Linux distributions, libjpeg-dev is available in the repositories.

Afterwards, PIL must be re-compiled and -installed in order to let it know about it.

Attachments (1)

18374.diff (924 bytes) - added by aaugustin 3 years ago.

Download all attachments as: .zip

Change History (12)

comment:1 Changed 3 years ago by charettes

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

When installing PIL using setup.py install or via pip you get clear warnings that support for a specific format is not available.

    --------------------------------------------------------------------
    *** TKINTER support not available (Tcl/Tk 8.5 libraries needed)
    *** JPEG support not available
    *** ZLIB (PNG/ZIP) support not available
    *** FREETYPE2 support not available
    *** LITTLECMS support not available
    --------------------------------------------------------------------
    To add a missing option, make sure you have the required
    library, and set the corresponding ROOT variable in the
    setup.py script.

IMHO the warning raised by PIL's setup nails it but I'd like to hear someone else about that.

comment:2 Changed 3 years ago by aaugustin

I also don't believe that our installation documentation has to go into this level of detail, especially since PIL's installer does the right thing.

The ticket also says that Django is masking an exception. Maybe we could improve that part?

comment:3 Changed 3 years ago by charettes

I can't see how we can be more explicit without confusing the user. The actual exception message is "Upload a valid image. The file you uploaded was either not an image or a corrupted image.".

The only think I can think of is raising a different error message if DEBUG=True by telling the developer this might be due to an unavailable decoder?

comment:4 follow-up: Changed 3 years ago by fabian@…

django/forms/fields.py, lines 592 - 596:

        except Exception: # Python Imaging Library doesn't recognize it as an image
            raise ValidationError(self.error_messages['invalid_image'])
        if hasattr(f, 'seek') and callable(f.seek):
            f.seek(0)
        return f

I suggest to put the Exception message into the error if DEBUG=True.
Otherwise, the developer always has to check the django code to get those details.

So, for instance:

        except Exception: # Python Imaging Library doesn't recognize it as an image
            if settings.DEBUG: raise ValidationError(self.error_messages['invalid_image'], e)
            raise ValidationError(self.error_messages['invalid_image'])
        if hasattr(f, 'seek') and callable(f.seek):
            f.seek(0)
        return f

comment:5 in reply to: ↑ 4 Changed 3 years ago by anonymous

Of course it'll be

        except Exception, e: # Python Imaging Library doesn't recognize it as an image
            if settings.DEBUG: raise ValidationError(self.error_messages['invalid_image'], e)
            raise ValidationError(self.error_messages['invalid_image'])
        if hasattr(f, 'seek') and callable(f.seek):
            f.seek(0)
        return f

...

comment:6 Changed 3 years ago by aaugustin

  • Triage Stage changed from Unreviewed to Accepted

Well, this won't help debug the issue when it happens in production. I'm going to improve the docs a bit.

Changed 3 years ago by aaugustin

comment:7 Changed 3 years ago by aaugustin

I just attached a proposal. What do you think?

comment:8 Changed 3 years ago by charettes

  • Triage Stage changed from Accepted to Ready for checkin

Looks good to me.

comment:9 Changed 3 years ago by charettes

  • Has patch set

comment:10 Changed 3 years ago by anonymous

Agreed, I think this should do it. Thanks.

comment:11 Changed 3 years ago by Aymeric Augustin <aymeric.augustin@…>

  • Resolution set to fixed
  • Status changed from new to closed

In [5e94ef293cb1cfe4dc43cbd5509653c0e0bf66cf]:

Fixed #18374 -- Explained "corrupt image" error

Thanks fabian and charettes.

Note: See TracTickets for help on using tickets.
Back to Top