Django

Code

Ticket #3848 (closed: fixed)

Opened 1 year ago

Last modified 10 months ago

Image validator: fails to spot image truncation or corruption

Reported by: andrewc-djangotrac1@piffle.org Assigned to: nobody
Milestone: Component: Core framework
Version: SVN Keywords: images, image, validation, verification, PIL
Cc: dev@simon.net.nz Triage Stage: Ready for checkin
Has patch: 1 Needs documentation: 0
Needs tests: 0 Patch needs improvement: 0

Description

django.core.validators.isValidImage in trunk does not detect corrupt or truncated images: more PIL calls than just the constructor need to be made in order to effect the full set of checks PIL is capable of, because most PIL.xxxImagePlugin modules read no more data than is necessary at construction time. One of load() or verify() (or both if possible) must be called on the PIL.Image object in order to perform the fullest set of checks.

To reproduce:

  • Prepare truncated/corrupted PNG or JPEG files with vi, dd, based on some known-good images, and leaving the first few bytes of the image header intact.
  • Test that PIL can spot that the images are corrupt:
    >>> from PIL import Image
    >>> from cStringIO import StringIO
    >>> i = Image.open(StringIO(open('hi_truncated.png', 'r').read()))
    >>> i.verify()
    [...]
    IndexError: string index out of range
    >>> i = Image.open(StringIO(open('hi_truncated.jpg', 'r').read()))
    >>> i.load()
    [...]
    IOError: image file is truncated (46 bytes not processed)
    >>>
  • Try to upload one of the bad images you prepared via the django admin interface.

Expected behaviour:

  • Upload will be rejected with the message "Upload a valid image. The file you uploaded was either not an image or a corrupted image."

Observed behaviour as of [4836] (and presumably all the way back to [3]):

  • Image is accepted as if it were non-corrupt.

This is my first django bug report, so I hope I'm doing it right. I could provide a test case if you like, but I noticed that the existing test framework makes no attempt to test this area of code in order to avoid a dependency on PIL at test time.

Attachments

django_core_validators_isValidImage.diff (1.7 kB) - added by Andrew C. <andrewc-djangotrac1@piffle.org> on 03/28/07 09:35:00.
django_core_validators_isValidImage.better-message.diff (1.8 kB) - added by Andrew C. <andrewc-djangotrac1@piffle.org> on 03/28/07 09:42:29.
Improve the phrasing of the Debug-mode error message addendum

Change History

03/28/07 09:35:00 changed by Andrew C. <andrewc-djangotrac1@piffle.org>

  • attachment django_core_validators_isValidImage.diff added.

03/28/07 09:42:29 changed by Andrew C. <andrewc-djangotrac1@piffle.org>

  • attachment django_core_validators_isValidImage.better-message.diff added.

Improve the phrasing of the Debug-mode error message addendum

(follow-up: ↓ 2 ) 03/29/07 01:56:17 changed by Simon G. <dev@simon.net.nz>

  • cc set to dev@simon.net.nz.
  • needs_better_patch changed.
  • stage changed from Unreviewed to Ready for checkin.
  • needs_tests changed.
  • needs_docs changed.

Looks good to me.

(in reply to: ↑ 1 ) 07/06/07 11:18:12 changed by andrewc-djangotrac1@piffle.org

Replying to Simon G. <dev@simon.net.nz>:

Looks good to me.

Does this patch need any further work from me? It's been "Ready for checkin" for 3 months without any flagged need for documentation, improvement, or tests. Do you need anything more from me? I'd be happy to oblige, just let me know.

09/14/07 02:18:27 changed by russellm

  • status changed from new to closed.
  • resolution set to fixed.

(In [6175]) Fixed #3848 -- Added more comprehensive checks to ImageField? validation, checking for image truncation or corruption. Thanks to Andrew C <andrewc-djangotrac1@piffle.org> for the patch.


Add/Change #3848 (Image validator: fails to spot image truncation or corruption)




Change Properties
Action