#6198 closed (wontfix)
MEDIA_URL setting requires trailing slash
Reported by: | Owned by: | nobody | |
---|---|---|---|
Component: | Core (Other) | Version: | dev |
Severity: | Keywords: | ||
Cc: | Triage Stage: | Design decision needed | |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
If I use MEDIA_URL = 'http://localhost/djangomedia', django will give me links prefixed by 'http://localhost/' and drop the 'djangomedia'.
I have to include the trailing slash, with MEDIA_URL = 'http://localhost/djangomedia/' for it to work.
I think django should work without the slash.
Change History (11)
comment:1 by , 17 years ago
Resolution: | → wontfix |
---|---|
Status: | new → closed |
comment:2 by , 17 years ago
I don't think that's a good reason. It would still be better to accept a path without a trailing slash.
follow-up: 5 comment:3 by , 17 years ago
By the way, the message you're talking about is not on MEDIA_URL in my settings.py. It's actually on a different setting called ADMIN_MEDIA_PREFIX.
comment:4 by , 17 years ago
Component: | Uncategorized → Core framework |
---|---|
Resolution: | wontfix |
Status: | closed → reopened |
Triage Stage: | Unreviewed → Design decision needed |
Maybe it needs clarification then.
Alternatively, if you can hunt down why it's not working for you and provide a solution, that could get things moving too.
comment:5 by , 17 years ago
Replying to Michael Toomim <toomim@cs.washington.edu>:
By the way, the message you're talking about is not on MEDIA_URL in my settings.py. It's actually on a different setting called ADMIN_MEDIA_PREFIX.
Current setting.py
template clearly indicates this:
# URL that handles the media served from MEDIA_ROOT. Make sure to use a # trailing slash if there is a path component (optional in other cases). # Examples: "http://media.lawrence.com", "http://example.com/media/" MEDIA_URL = ''
maybe your project has been created on times that template didn't ?
The documentation also indicates a trailing slash is needed: http://www.djangoproject.com/documentation/settings/#media-url (doc/settings.txt
)
So if you plan to submit a patch to change this behavior, make sure it also modify these two files.
comment:7 by , 17 years ago
I have a patch, but want advice on proper code style.
The problem is that urlparse.urljoin() function strips everything after a slash in order to implement "relative" urls according to RFC1808. In this situation, we just want to append the strings MEDIA_URL with a suffix. We can do this with a + and inserted slash:
return (settings.MEDIA_URL + '/' + getattr(self, field.attname)).replace('\\', '/')
But this will give us a double slash if MEDIA_URL already ends in a slash. If this is a problem, we can check for the slash:
return (settings.MEDIA_URL[:-1] + settings.MEDIA_URL[-1].replace('/','') + '/' + getattr(self, field.attname))\ .replace('\\', '/')
But this code is a bit more complicated. We could also use an if statement. Is there a utility library somewhere that we could put a "append two paths and take care of duplicate slash" function, and then use that here?
comment:8 by , 17 years ago
I'm still not sure this is all worth the hassle. If we just require MEDIA_URL to end in a slash, there is no problem.
As a side note, I think it should always be recommended to end in a slash. Common use in templates is "{{ MEDIA_URL }}css/main.css"
-- I don't see the advantage of the current wishy-washy advice of "Make sure to use a trailing slash if there is a path component (optional in other cases)". Seems better to just say "Always use a trailing slash".
comment:9 by , 17 years ago
I'm down with this, I like your reasoning. How about I submit a patch that requires a trailing slash, and does away with the urljoin() wackiness?
comment:10 by , 17 years ago
Resolution: | → wontfix |
---|---|
Status: | reopened → closed |
Sure. What's there to patch apart from documentation to do that though?
I'll mark this ticket as "wontfix", mention it in the new ticket you create (along with some of the reasoning if you like).
comment:11 by , 17 years ago
Ok great.
Just the replacement of "urljoin(base, suffix)" with "base + suffix". urljoin isn't the right thing to use here, if you forget the trailing slash, it deletes everything up to the trailing slash before adding the suffix, which is weird. Using + will make it fail in a more obvious way.
Your settings module clearly says:
"Make sure to use a trailing slash if there is a path component"
I'm not sure I see why it should work without it after such clear instructions ;)