Opened 16 years ago

Closed 16 years ago

Last modified 16 years ago

#6198 closed (wontfix)

MEDIA_URL setting requires trailing slash

Reported by: Michael Toomim <toomim@…> 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 Chris Beaven, 16 years ago

Resolution: wontfix
Status: newclosed

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 ;)

comment:2 by Michael Toomim <toomim@…>, 16 years ago

I don't think that's a good reason. It would still be better to accept a path without a trailing slash.

comment:3 by Michael Toomim <toomim@…>, 16 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 Chris Beaven, 16 years ago

Component: UncategorizedCore framework
Resolution: wontfix
Status: closedreopened
Triage Stage: UnreviewedDesign 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.

in reply to:  3 comment:5 by Ramiro Morales, 16 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:6 by Michael Toomim <toomim@…>, 16 years ago

Sweet, thanks!

comment:7 by Michael Toomim <toomim@…>, 16 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 Chris Beaven, 16 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 Michael Toomim <toomim@…>, 16 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 Chris Beaven, 16 years ago

Resolution: wontfix
Status: reopenedclosed

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 Michael Toomim <toomim@…>, 16 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.

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