Opened 7 years ago

Closed 7 years ago

Last modified 7 years ago

#6198 closed (wontfix)

MEDIA_URL setting requires trailing slash

Reported by: Michael Toomim <toomim@…> Owned by: nobody
Component: Core (Other) Version: master
Severity: Keywords:
Cc: Triage Stage: Design decision needed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

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 Changed 7 years ago by SmileyChris

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Resolution set to wontfix
  • Status changed from new to closed

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 Changed 7 years ago by Michael Toomim <toomim@…>

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

comment:3 follow-up: Changed 7 years ago by Michael Toomim <toomim@…>

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 Changed 7 years ago by SmileyChris

  • Component changed from Uncategorized to Core framework
  • Resolution wontfix deleted
  • Status changed from closed to reopened
  • Triage Stage changed from Unreviewed to 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 in reply to: ↑ 3 Changed 7 years ago by ramiro

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 Changed 7 years ago by Michael Toomim <toomim@…>

Sweet, thanks!

comment:7 Changed 7 years ago by Michael Toomim <toomim@…>

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 Changed 7 years ago by SmileyChris

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 Changed 7 years ago by Michael Toomim <toomim@…>

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 Changed 7 years ago by SmileyChris

  • Resolution set to wontfix
  • Status changed from reopened to 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 Changed 7 years ago by Michael Toomim <toomim@…>

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