Storage backends should know about the max_length attribute of the FileFields

[Florian Apolloner]

Currently the Storage Backends may return a filename which is longer than the FileField actually supports. This of course breaks it. If the Storage knew about the maxmimum length it is allowed to returned, it could make sure, that the filenames aren't longer.

[James Bennett]

1.2 is feature-frozen, moving this feature request off the milestone.

[steph]

When fixing this ticket, we should keep in mind, that the storage location (maybe a larger directory structure) needs to be considered too. If my file field has a max_length of 45, and my upload_to/storage location is "uploads/users/images/gallery/photos/", the filename has a max length of 8 (the directory name has 37 chars).

[steph]

After a short discussion with jezdez, this should be considered as DDN.

[Ramiro Morales]

#15247 has some more discussion about this topic worth reviewing. It was closed as a duplicate.

[Carl Meyer]

Marking this as a bug, since #15247 makes it clear that's what it is. Discussed briefly with jezdez on IRC and concluded that the fix proposed here probably is the right one, adding a max_length argument to the get_valid_name method of storage classes. Unfortunately, this is backwards incompatible, and it won't just break code that was relying on buggy behavior, it will break all custom storage backends. So it requires a deprecation path, and during the deprecation period that will require introspecting the get_valid_name method to see whether it accepts the valid_name argument (or trying with, catching the error, and then trying without).

Either one of those is kind of ugly, but the alternative is to introduce a new method, like "get_valid_name_with_max_length". That makes the deprecation-period code a bit nicer, but then we're stuck with the longer method name for good. Probably better to take the short-term hit.

[ishalyapin]

Hey, it is not very difficult problem. I wrote a solution and use it in my projects.
It is not perfect solution, but it is much better then unhandled exception.
I hope you take it in django.

def filefield_maxlength_validator(value):
    """"
        Check if absolute file path can fit in database table 
        and filename length can fit in filesystem.
    """
     
    FILESYSTEM_FILENAME_MAXLENGTH = 255 # ext4, for example
    RESERVE = 50 # We need reserve for generating thumbnails and for suffix if filename already exists, example - filename_85x85_1.jpg

    # For the initial file saving value.path contains not the same path, which will be used to save file
    filename = value.name
    filepath = os.path.join(
        settings.MEDIA_ROOT, 
        value.field.generate_filename(value.instance, filename)
    )
    bytes_filename = len(filename.encode('utf-8')) # filename length in bytes
    bytes_filepath = len(filepath.encode('utf-8')) # path length in bytes

    # File path length should fit in table cell and filename lenth should fit in in filesystem
    if bytes_filepath > value.field.max_length or bytes_filename > FILESYSTEM_FILENAME_MAXLENGTH - RESERVE:
        if os.path.isfile(value.path):
            os.remove(value.path)
        raise exceptions.ValidationError(_(u"File name too long."))
    return value

FileField.default_validators = FileField.default_validators[:] + [filefield_maxlength_validator]

[Jacob]

Milestone 1.3 deleted

[Stephen Burrows]

Overriding get_valid_name is probably not a workable solution. Currently, it claims only to return a filename "that's suitable for use in the target storage system." This just means a short cleaning of whitespace characters, hyphens, etc. So making it actually return a valid filename (in terms of length) would be a change in mission. Additionally, we actually need to validate the length of the entire storage path (which get_valid_name doesn't have access to). And finally, even if we got a valid name, get_available_name can increase the length of that name during the saving process, leaving us exactly where we started.

get_available_name needs to take a max_length argument, or it needs to be able to guarantee that it won't increase the length of the filename. Since the latter isn't possible as far as I can see, I expect to do the former. If get_available_name *can't* make a name within the given max length, it will raise a ValueError. FileField validation would then need to call get_available_name *after* generating a filename and instead of (or after) a simple check of the length of the filename. Any ValueError raised can be re-raised as a ValidationError.

Since get_available_name is part of the public API, this would mean either doing introspection, wrapping every call to get_available_name in try/except, or adding a new method called (say) get_valid_storage_path, which would handle max_length and the available_name checks, then deprecate get_available_name. Personally, I would lean toward the latter option, since I find the whole name/filename/path/storage path naming mess confusing.

[Anssi Kääriäinen]

I kind of like the idea that get_valid_name() just converts the name to be something valid for the storage backend. Then, get_available_name has the final control of how to truncate the name, and to make sure the name is within the name length limit. The contract is altered from "gimme an available file name as close to the requested name" to "gimme an available file name as close to the requested name, max length N".

It seems the final name is determined by .save() - if the file name clashes due to race conditions, then .save() can alter the file name. This brings another point to the discussion: if final file name is determined by .save(), then shouldn't .save() also know the file name length restriction?

As for implementation, I like the try-except approach most.

[Stephen Burrows]

Yes, .save() (and ._save()) would also need to know about the max_length restriction.

[Collin Anderson]

I think this might be a 5 year long duplicate of #9893 which is now fixed :) Feel free to reopen if it's different.