Opened 11 years ago

Closed 11 years ago

#20245 closed Cleanup/optimization (fixed)

STATIC_URL without trailing slash leads to confusingly broken URLs

Reported by: Harrison Kelly Owned by: nobody
Component: contrib.staticfiles Version: 1.5
Severity: Normal Keywords: staticfiles, static files, storage, url
Cc: bmispelon@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I've been working on a project that had its STATIC_URL misconfigured: no trailing slash. Normally, that'd be easy to diagnose, but the way it manifested itself was actually pretty confusing.

Settings example (snippet):

STATIC_URL = '/staticfiles-cache'

Template example:

{% load staticfiles %}
<script src="{% static 'js/script.min.js' %}"></script>

Output:

<script src="/js/script.min.js"></script>

As you can see, the STATIC_URL is totally lost. It'd be a lot easier to debug if the URL came out as /staticfiles-cachejs/script.min.js, but when I first saw this, it left me dumbfounded.

What's more, the correct URL (/staticfiles-cache/js/script.min.js) resolves correctly. It appears the URL resolution or the staticfiles view I'm using (django.contrib.staticfiles.urls.staticfiles_urlpatterns) doesn't need the trailing slash.

The reason is because the template tag indirectly uses urlparse.urljoin or urllib.parse.urljoin. From what I can tell, that method is designed to take one URL as an example (in this case, STATIC_URL) and use it to make an absolute URL out of a second argument (in this case, 'js/script.min.js'). Because it interprets the first argument's staticfiles-cache as a file name rather than a directory, it removes that slug and appends the second argument at the root.

I think that behavior makes sense, but I want to do something to make this less confusing. My initial thought is to check the output (either in django.core.files.storage.FileSystemStorage.url or in the template tag itself) to make sure it starts with the base URL (or in the template tag, STATICFILES_URL directly).

Can I get everyone's input on this approach? Am I barking up the wrong tree entirely? If not, should I do it in FileSystemStorage, or only in the static template tag? If to the template tag specifically, should I set up similar things in other areas (i.e. media URLs)?

I'm more than willing to implement this, but it'd be my first contribution, so I don't want to be presumptuous about whether or not this is a valuable fix.

Change History (3)

comment:1 by Baptiste Mispelon, 11 years ago

Cc: bmispelon@… added
Triage Stage: UnreviewedAccepted

I agree that this behavior is confusing, but it's sort of documented: https://docs.djangoproject.com/en/dev/ref/settings/#static-url mentions that this settings must end with a slash.

However, I don't think the {% static %} tag or FileSystemStorage are good places to put code that basically validates settings.

There used to be a comment for MEDIA_URL that warned you to use a trailing slash [1] and personally, I'd be favorable to put something like this back for STATIC_URL.

The fact that that staticfiles view seems to serve the file wihtout a problem might itself be a bug I think. Note that there's a django.contrib.staticfiles.utils.check_config function where it might makes sense to check that STATIC_URL ends with the required trailing slash.

[1] https://github.com/django/django/blob/21ea58b8ccf95798271157876d59d46dcc745b0d/django/conf/project_template/project_name/settings.py#L51-L52

comment:2 by djangsters, 11 years ago

this seems to be fixed in master by raising ImproperlyConfigured if necessary:
https://github.com/django/django/blob/master/django/conf/__init__.py#L108-L114

comment:3 by Aymeric Augustin, 11 years ago

Resolution: fixed
Status: newclosed

Yes, and that's one of the reasons I removed the comment.

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