django
django copied to clipboard
Fixed #13314 -- Make FileField validate the full filepath against max_length on ModelForms
Ticket: https://code.djangoproject.com/ticket/13314
This ticket has been laying about for over a decade and I decided I'd have a stab at closing it.
The problem
Django forms do not validate the full file path against the models.fields.FileField.max_length
attribute, only against the file name of the file uploaded by the user. The file name of the file uploaded by the user and the actual file path provided to the database can be wildly different in length, due to the impact of themodels.fields.FileField.upload_to
attribute, which can be dynamic.
If a file path is too long, it produces an error which will be presented as an Http500 to the user.
My approach
It makes sense that this validation can occur automatically in case of a ModelForm
, where Django is able to easily link the forms.fields.FileField
to the models.fields.FileField
and access the generate_filename()
method on it, to determine what the true length of the file path will be for any given file.
I have added two new attributes to forms.fields.FileField
:
-
forms.fields.FileField._model_instance
-
forms.fields.FileField._model_field_name
The ModelForm
constructor will automatically set these attributes when generating fields for a ModelForm
, and as such proper validation will take place 'automatically'.
Concerns
- It is not possible to provide a helpful error message to the user of the form as it is not possible to determine how long their file name should be due to the dynamic nature of file path generation. For now I have settled with
The filename is too long. Rename the file to something shorter.
, but perhaps something even more generic such asThere was an issue uploading the file.
should be used? - Translation of any new error message will be required.
@anorthall Thanks for this patch :+1: IMO, the only reasonable way to solve this is to introduce a new FileField
kwarg (e.g. max_upload_to_length
) where user could declare a length. Like max_digits
and decimal_places
work for DecimalField
. There are of course backward compatibility concerns. I'm not sure it's worth fixing. Maybe we could just add a warning to docs that you should take the length of upload_to
into account and limit forms.FileField.max_length
:thinking:.
@anorthall Thanks for this patch 👍 IMO, the only reasonable way to solve this is to introduce a new
FileField
kwarg (e.g.max_upload_to_length
) where user could declare a length. Likemax_digits
anddecimal_places
work forDecimalField
. There are of course backward compatibility concerns. I'm not sure it's worth fixing. Maybe we could just add a warning to docs that you should take the length ofupload_to
into account and limitforms.FileField.max_length
🤔.
I agree that it doesn't seem worth the added complexity of doing it 'automatically' (but you don't know until you try!) and that just highlighting the issue in the docs would likely be the best solution here.
I won't be coding for a few days now but I will come back to this and unless anyone has any other thoughts I will close this PR and submit a new one with a docs change.