Opened 12 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 , 12 years ago
Cc: | added |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 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 , 11 years ago
Resolution: | → fixed |
---|---|
Status: | new → closed |
Yes, and that's one of the reasons I removed the comment.
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 orFileSystemStorage
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 forSTATIC_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 thatSTATIC_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