[Django] #27689: `FileSystemStorage().get_valid_name may return empty string
Django
------------------------------------------------+------------------------
Reporter: Victor Porton | Owner: nobody
Type: Bug | Status: new
Component: File uploads/storage | Version: 1.10
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+------------------------
`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.
--
Ticket URL: <https://code.djangoproject.com/ticket/27689>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
-------------------------------------+-------------------------------------
Reporter: Victor Porton | Owner: nobody
Type: Bug | Status: new
uploads/storage |
Severity: Normal | Resolution:
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by 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?
--
Ticket URL: <https://code.djangoproject.com/ticket/27689#comment:1>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
uploads/storage |
Severity: Normal | Resolution:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/27689#comment:2>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
uploads/storage |
Severity: Normal | Resolution:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by 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.
[https://docs.djangoproject.com/en/dev/howto/custom-file-
storage/#django.core.files.storage.get_valid_name 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?
--
Ticket URL: <https://code.djangoproject.com/ticket/27689#comment:3>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
uploads/storage |
Severity: Normal | Resolution:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/27689#comment:4>
Django
-------------------------------------+-------------------------------------
Type: Bug | Status: new
uploads/storage |
Severity: Normal | Resolution:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Comment (by 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 [https://docs.djangoproject.com/en/dev/howto/custom-file-
storage/#django.core.files.storage.get_available_name
get_available_name()] might be a better match for your use case.
I'll leave the ticket open for other opinions.
--
Ticket URL: <https://code.djangoproject.com/ticket/27689#comment:5>
Django
-------------------------------------+-------------------------------------
Component: File | Version: 1.10
uploads/storage |
Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham):
* status: new => closed
* resolution: => wontfix
Comment:
Feel free to write to the DevelopersMailingList to get other opinions.
--
Ticket URL: <https://code.djangoproject.com/ticket/27689#comment:6>