Opened 6 months ago

Closed 6 months ago

Last modified 6 months ago

#28242 closed Cleanup/optimization (fixed)

Move ImageField file extension validation from models to forms for better backwards compatibility

Reported by: Manatsawin Hanmongkolchai Owned by: nobody
Component: Forms Version: 1.11
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

FileExtensionValidator was added in #21548 and is breaking existing Django application.

To reproduce:

  • Create a model image1 = models.ImageField(upload_to=lambda instance, filename: 'image1/{}'.format(instance.pk), blank=True). Make sure that upload_to does not contains any extension
  • Create an instance of that model in Django Admin with a file uploaded.
  • Try to update the just created model with no changes. It will fail with the error "File extension is not allowed. Allowed extensions are: 'bmp, bufr, cur, pcx, dcx, dds, ps, eps, fit, fits, fli, flc, fpx, ftc, ftu, gbr, gif, grib, h5, hdf, png, jp2, j2k, jpc, jpf, jpx, j2c, icns, ico, im, iim, tif, tiff, jfif, jpe, jpg, jpeg, mic, mpg, mpeg, mpo, msp, palm, pcd, pdf, pxr, pbm, pgm, ppm, psd, bw, rgb, rgba, sgi, ras, tga, webp, wmf, emf, xbm, xpm'." on the field.

This issue is not present in 1.10.7 (as #21548 is not merged yet).

Change History (9)

comment:1 Changed 6 months ago by Manatsawin Hanmongkolchai

I've ran bisect already and confirmed that 12b4280444b58c94197255655e284e4103fe00a9 is the first bad commit. (I haven't wrote an automated test, instead wrote a test Django project and manually tested)

comment:2 Changed 6 months ago by Tim Graham

Can you elaborate on your use case for saving images without extensions? I'm not sure if this is a common enough use case to justify some change in Django. You could subclass the ImageField model field and set default_validators = [] if you don't want the extension validator.

comment:3 Changed 6 months ago by Manatsawin Hanmongkolchai

In my use case, user would upload images that are saved to Amazon S3 (using django-storages). As the image is one per user (eg. avatar), naming the image by their user ID seems to make sense that it make other applications in my system able to find the image easily.

As we're already on S3, I felt it is not neccessary to add an extension to the file name (as S3 would already have the Content-Type metadata, which django-storages would set automatically from the original extension). It also make the upload_to function easier to implement.

(edit) I'd like to add that I still think that having extension validator is useful. (For example, user might upload PDF and the error should tell the user to upload in some other formats) That's why I couldn't just disable the default validator.

Last edited 6 months ago by Manatsawin Hanmongkolchai (previous) (diff)

comment:4 Changed 6 months ago by Tim Graham

Could you offer a patch that would reallow your use case?

comment:5 Changed 6 months ago by Manatsawin Hanmongkolchai

I'd be willing to work on getting this resolved.

I poked a bit around the codebase earlier, and I think that the my solution to this would be to change the validator to skip empty files. However, empty files seems to be a valid upload as well (#13584) so I might need another way to detect that the browser submitted an empty <input type="file">.

I haven't get the full grasp of this issue yet as it involving several related components. If you have any suggestion I'd be appreciated.

comment:6 Changed 6 months ago by Manatsawin Hanmongkolchai

Has patch: set

I've submitted a pull request: https://github.com/django/django/pull/8565

comment:7 Changed 6 months ago by Tim Graham

Component: File uploads/storageForms
Summary: FileExtensionValidator tries to validates existing fileMove ImageField file extension validation from models to forms for better backwards compatibility
Triage Stage: UnreviewedAccepted
Type: BugCleanup/optimization

comment:8 Changed 6 months ago by Tim Graham <timograham@…>

Resolution: fixed
Status: newclosed

In a0c07d77:

Fixed #28242 -- Moved ImageField file extension validation to the form field.

comment:9 Changed 6 months ago by Tim Graham <timograham@…>

In 110bd82:

[1.11.x] Fixed #28242 -- Moved ImageField file extension validation to the form field.

Backport of a0c07d77fc313388c72a17cd59411265069f037f from master

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