Opened 14 years ago

Last modified 5 months ago

#13314 assigned Bug

"FileField" validation does not account for "upload_to" when counting characters

Reported by: Denilson Figueiredo de Sá Owned by: Andrew Northall
Component: Forms Version: 1.1
Severity: Normal Keywords:
Cc: Łukasz Rekucki, walter+django@…, Andrew Northall Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: yes
Easy pickings: no UI/UX: no

Description

I have a model with a FileField object:

def set_file (instance, filename):
    return os.path.join("uploaded_files/my_obj_%d" % instance.my_obj_id, os.path.basename(filename))

class MyModel(models.Model):
    my_file = models.FileField(upload_to=set_file, null=True)

And I have a view that receives the POST and handles it to a trivial forms.ModelForm object. In that view I check for .is_valid().

Since the FileField creates a 100-char column at the database, any filename greater than 100 chars will be rejected, and the form instance will have a nice error message talking about this.

However, this comparison is broken, because the actual data stored at the database won't be the filename, but instead the return value of the "upload_to" callable. This returned value, in my case, has more characters than the actual filename.

Thus, in this case, filenames between 75 and 100 characters will be accepted by the form validation, but will be rejected by the database when the actual .save() occurs.

I'm not very sure about what is the best solution, but the forms.fields.FileField shouldn't rely just on the max_length parameter when validating the input.

Change History (12)

comment:1 by Russell Keith-Magee, 14 years ago

Triage Stage: UnreviewedAccepted

comment:2 by Dominic Rodger, 14 years ago

Summary: "FIleField" validation does not account for "upload_to" when counting characters"FileField" validation does not account for "upload_to" when counting characters

comment:3 by Łukasz Rekucki, 14 years ago

Cc: Łukasz Rekucki added

I just go bitten by this. I'm not sure about how to fix this properly. Checking it at forms.FileField works as long as you are using ModelForms. IMHO, the FileField itself should reject names that exceed max_length after being processed by upload_to.

Maybe we should also pass max_length as an argument to upload_to, so the user could handle this (with hashing for example). Either way, a custom exception would be much better than leaving it to the DB layer which yields a DatabaseError (and it would show up on sqlite, so people can catch this earlier).

comment:4 by Julien Phalip, 13 years ago

Severity: Normal
Type: Bug

comment:5 by Thejaswi Puthraya, 13 years ago

Component: UncategorizedForms
Easy pickings: unset
UI/UX: unset

comment:6 by Claude Paroz, 12 years ago

Resolution: duplicate
Status: newclosed

Duplicate of #9893.

comment:7 by Walter Doekes, 7 years ago

Resolution: duplicate
Status: closednew

I've taken the liberty to re-open this, as #9893 never did address this. See:
https://code.djangoproject.com/ticket/9893#comment:46
https://code.djangoproject.com/ticket/9893#comment:36

comment:8 by Walter Doekes, 7 years ago

Cc: walter+django@… added

comment:9 by Denilson Figueiredo de Sá, 7 years ago

Just an idea… Are we ever going to query the database for the filename? If not, we could store it as TEXT instead of VARCHAR.

comment:10 by Tim Graham, 7 years ago

Changing the datatype to TEXT is likely unacceptable due to backwards compatibility.

comment:11 by Andrew Northall, 5 months ago

Cc: Andrew Northall added
Has patch: set
Owner: changed from nobody to Andrew Northall
Status: newassigned

PR

Implementation details and further comments on GitHub.

comment:12 by Mariusz Felisiak, 5 months ago

Patch needs improvement: set

Marking as "needs improvement" per comment.

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