Opened 10 years ago

Closed 10 years ago

#22717 closed Bug (fixed)

FileField.url returns a wrong URL

Reported by: david.fischer.ch@… Owned by: nobody
Component: File uploads/storage Version: dev
Severity: Normal Keywords: storage, filefield, url
Cc: Triage Stage: Ready for checkin
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: yes UI/UX: no

Description

models.py:

...
class Media(models.Model):
    ....
    file = models.FileField(upload_to=storages.upload_to, storage=storages.media_storage)
    ...

storages.py:

...
def upload_to(instance, filename):
    return '%s/%s' % (instance.pk, filename)

media_storage = FileSystemStorage(location='/test/media', base_url='/test/media')

Django shell:

...
>>> m = Media.objects.get(pk=27)
>>> m.file.storage.base_url
'/test/media'
>>> m.file.name
'salut.txt'
>>> m.file.url
'/test/salut.txt'

Expected -> '/test/media/salut.txt'

Investigating:

>>> from django.utils.six.moves.urllib.parse import urljoin
>>> urljoin('/test/media', 'salut.txt')
'/test/salut.txt'

Change History (9)

comment:1 by david.fischer.ch@…, 10 years ago

Hello,

Using '/test/media/' instead of '/test/media' fix the issue.

This is an unexpected behavior, I would like that the default implementation of FileSystemStorage takes care of this.

comment:2 by Claude Paroz, 10 years ago

Easy pickings: set
Triage Stage: UnreviewedAccepted

comment:4 by Tim Graham, 10 years ago

We require MEDIA_URL to end with a trailing slash (see #6198 for reasons why it won't be changed). I am not sure if the same reasoning holds for this, but I thought it was worth mentioning.

comment:5 by mardini, 10 years ago

urljoin() is implemented like this in Python to be consistent with RFC 3986, section 5.2.3. (http://tools.ietf.org/html/rfc3986#section-5.2.3) which states how merging a relative-path with the base URI should work. It *might* not be a good idea to override that standard for the convenience.

Last edited 10 years ago by mardini (previous) (diff)

comment:6 by Claude Paroz, 10 years ago

IMHO, RFC 3986 is aimed at "generic" merging, where you can't assume that /url point to a path, but can also be an arbitrary resource. In our case, base_url can be considered as a path in all cases (AFAIK), that's why I don't see the problem of auto-adding the ending slash. But if anyone can come with a use case where this is not wanted behavior, I'll happily change my mind.

comment:7 by mardini, 10 years ago

That's a solid reasoning I guess.
I can't think of any use case where adding a trailing slash to base_url would cause undesirable behavior either. And since base_url isn't necessarily MEDIA_URL, I see no harm in implementing this feature, even though MEDIA_URL must always end in a slash.

Thanks.

comment:8 by Tim Graham, 10 years ago

Triage Stage: AcceptedReady for checkin

comment:9 by Claude Paroz <claude@…>, 10 years ago

Resolution: fixed
Status: newclosed

In fb9d8f06520f495d0c36236f7534dbe660c7e164:

Fixed #22717 -- Auto-corrected missing ending slash in FileSystemStorage

Thanks David Fischer for the report and Moayad Mardini for the
review.

Note: See TracTickets for help on using tickets.
Back to Top