#18515 closed Bug (fixed)
Problematic validation code in FileField
| Reported by: | Michael Angeletti | Owned by: | nobody |
|---|---|---|---|
| Component: | File uploads/storage | Version: | 1.4 |
| Severity: | Normal | Keywords: | max_length, FileField, fields |
| Cc: | Michael Angeletti | Triage Stage: | Accepted |
| Has patch: | yes | Needs documentation: | no |
| Needs tests: | no | Patch needs improvement: | no |
| Easy pickings: | no | UI/UX: | no |
Description
The following code in django.db.models.fields.files (line # 235 in master @ HEAD) is problematic.
def validate(self, value, model_instance):
"""
Validates that the generated file name still fits within max_length.
"""
# The generated file name stored in the database is generally longer
# than the uploaded file name. Using the length of generated name in
# the error message would be confusing. However, in the common case
# (ie. upload_to='path/to/upload/dir'), the length of the generated
# name equals the length of the uploaded name plus a constant. Thus
# we can tell the user how much shorter the name should be (roughly).
length = len(self.generate_filename(model_instance, value.name))
if self.max_length and length > self.max_length:
error_values = {'extra': length - self.max_length}
raise ValidationError(self.error_messages['max_length'] % error_values)
Given the following:
def oops_asset_path(instance, filename):
"""
Returns the upload path for a ``Oops.asset``.
"""
return 'oops/user_{upk}/{dt}/{fn}'.format(
upk=instance.user_id,
dt=timezone.now().strftime('%Y_%j_%H_%M_%S_%f'),
fn=re.sub(r'[^\w\.]', '_', filename).lower())
class Oops(models.Model):
asset = models.FileField(_(u'asset'), upload_to=oops_asset_path)
When an Oops instance is saved with the asset file name the-best-file-name-ever.jpg, it's converted to something like:
oops/user_3145/2012_178_00_16_02_738638/the_best_file_name_ever.jpg
which is expected. Then, when the Oops is saved again in a form (e.g., an inline model formset I use for multiple uploads) the above validation code runs the existing name through my oops_asset_path, which returns:
oops/user_3145/2012_178_00_16_03_457362/oops_user_3145_2012_178_00_16_02_738638_the_best_file_name_ever.jpg
This causes the max_length validation error, since the new name is clearly too long:
Filename is 7 characters too long.
The file name isn't changed permanently, but these erroneous validation errors are only avoided by increasing the field's max_length.
I can't think of any solution to this that doesn't include either a database query or else some sort of annotation on the FileField by forms... both of which scare me.
Attachments (2)
Change History (10)
comment:1 by , 13 years ago
| Cc: | added |
|---|---|
| Type: | Uncategorized → Bug |
comment:2 by , 13 years ago
| Type: | Bug → Uncategorized |
|---|
comment:3 by , 13 years ago
| Component: | Uncategorized → File uploads/storage |
|---|---|
| Has patch: | set |
| Triage Stage: | Unreviewed → Accepted |
| Type: | Uncategorized → Bug |
by , 13 years ago
| Attachment: | 18515-1.diff added |
|---|
comment:4 by , 13 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
Do not always regenerate filename in FileField validation