Code

#20245 closed Cleanup/optimization (fixed)

STATIC_URL without trailing slash leads to confusingly broken URLs

Reported by: hjkelly 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.

Attachments (0)

Change History (3)

comment:1 Changed 15 months ago by bmispelon

  • Cc bmispelon@… added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

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 Changed 14 months ago by djangsters

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 Changed 14 months ago by aaugustin

  • Resolution set to fixed
  • Status changed from new to closed

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

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed
as The resolution will be set. Next status will be 'closed'
The resolution will be deleted. Next status will be 'new'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.