Opened 4 years ago

Closed 4 years ago

Last modified 4 years ago

#15247 closed Bug (duplicate)

FileField has max_length=100 but filestorage may return something way longer

Reported by: hvdklauw Owned by: nobody
Component: File uploads/storage Version: master
Severity: Normal Keywords:
Cc: hvdklauw Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

So to keep al the uploaded files a bit sorted they all get added to specific folders. MEDIA_ROOT + 'user/10/profile/files/'

Then the user comes along and uploads a file with a very long name then the FileStorage stores the file just fine, the path get's returned and through the ORM passed to the database backend which might throw an error about the filefield exceeting 100 characters.

Mysql for instance just trims the value without complaining, thus storing an incomplete path.
Postgres will throw an exception which the user gets as a 500 error and we get an e-mail that we have to fix it.

Solutions:

  • Don't store the path in the database, keep that in the upload_to for the model, downside is that if it changes all the already uploaded files will not be found.
  • FileStorage should make sure the path doesn't exceed max_length, downside is that if the path part is 99 characters long the filename gets 1 character. (Or 120 and -20 :P)
  • FileField should validate the length of the path returned and give the user the error that they should shorted their filename to max X characters (X being dependand on the path prefix)

Which is best, I don't know, maybe a combination of those things.

Change History (9)

comment:1 Changed 4 years ago by jgelens

  • Cc jeffrey@… added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

comment:2 Changed 4 years ago by PaulM

  • Triage Stage changed from Unreviewed to Accepted

This is definitely a bug as-is. Something in this chain needs to validate the length before it hits the DB.

My preference would be to pass the max_length parameter along to the file backend and let it produce (however it can) a filename that will fit that. It already manipulates the name in case of duplicate files, so trimming if necessary shouldn't be much more work.

I think we would prefer to mangle filenames over throwing an error back at the user after they've finished uploading the file.

comment:3 Changed 4 years ago by ramiro

Duplicate of #11027.

comment:4 Changed 4 years ago by kmtracey

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

comment:5 Changed 4 years ago by russellm

  • Resolution changed from duplicate to fixed

In [15540]:

Fixed #15247 -- Ensured that if a SingleObject view defines get_object but not get_queryset, the ModelFormMixin doesn't fail. Thanks to Sergey N. Belinsky for the report and test case.

comment:6 Changed 4 years ago by russellm

Oops - fat fingers. That commit message should be for #15274.

comment:7 Changed 4 years ago by carljm

  • Easy pickings unset
  • Resolution fixed deleted
  • Severity set to Normal
  • Status changed from closed to reopened
  • Type set to Uncategorized

Reopening just to re-close as duplicate of #11027. Sorry for the noise, but it's confusing for this to be labeled "fixed" in big letters at the top of the Trac page, when it is not fixed.

comment:8 Changed 4 years ago by carljm

  • Resolution set to duplicate
  • Status changed from reopened to closed
  • Type changed from Uncategorized to Bug

comment:9 Changed 4 years ago by jgelens

  • Cc jeffrey@… removed
  • UI/UX unset
Note: See TracTickets for help on using tickets.
Back to Top