FileSystemStorage().get_valid_name() may return empty string

[Victor Porton]

FileSystemStorage().get_valid_name('') returns empty string. Empty string is not a valid file name.

One possible effect of this (I have not checked), that if the media/ dir is not yet created, it instead uploads a file with the name media and blocks further updates.

[Tim Graham]

What are the steps to reproduce get_valid_name() called with an empty string? How can you upload a file without a name? At least browsers don't seem to allow that. Maybe you can provide a test that uses the test client? It seems like the expected behavior would be for Django's form validation to prevent that call in the first place, or did you have some other resolution in mind?

[Victor Porton]

I am not about forms.

The test is:

def test_view(request):
    return HttpResponse('[' + FileSystemStorage().get_valid_name('') + ']')

Even if forms are validated, the above test outputs a wrong value, because FileSystemStorage().get_valid_name('') MUST return a valid file name.

[Tim Graham]

Perhaps you can explain your use case a bit more, but I don't think the documentation implies that's a valid usage. ​It says:

Returns a filename suitable for use with the underlying storage system. The name argument passed to this method is either the original filename sent to the server or, if upload_to is a callable, the filename returned by that method after any path information is removed.

Anyway, if the issue is valid, what's the expected behavior -- to raise an exception?

[Victor Porton]

For me the expected behavior is to make some valid file name, such as file.bin when empty string is used.

It is a good behavior in any case. This will we will make it work as expected, as documented, and not to produce possible security holes in user code.

[Tim Graham]

I disagree -- the documentation does not imply that behavior. While we could change how FileSystemStorage works, we couldn't also force all the existing third-party storages to obey the proposed contract. Perhaps the get_valid_name() name is misleading and might be better named something like sanitize_name(), however, I don't see value in putting a change like that through deprecation process at this point. For what it's worth, it seems like ​get_available_name() might be a better match for your use case.

I'll leave the ticket open for other opinions.

[Tim Graham]

Feel free to write to the DevelopersMailingList to get other opinions.