Opened 9 years ago
Last modified 9 months ago
#26329 new Bug
StaticFilesStorage permits leading slash, CachedStaticFilesStorage doesn't
Reported by: | Seán Hayes | Owned by: | |
---|---|---|---|
Component: | contrib.staticfiles | Version: | 1.8 |
Severity: | Normal | Keywords: | |
Cc: | matt@…, Adam Zapletal | Triage Stage: | Accepted |
Has patch: | no | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
I got the following email from our staging server:
The joined path (/images/no-image.jpg) is located outside of the base path component (/full-path/collected-static)
Someone was using the following template tag:
{% static "/images/no-image.jpg" as no_image_url %}
I checked to see why our tests didn't raise the same error, and it turns out it only happens with CachedStaticFilesStorage (and likely the other manifest storages), StaticFilesStorage and FileSystemStorage seem to just ignore this error.
Since CachedStaticFilesStorage shouldn't be used during testing, I think the parent classes should raise the same error.
Change History (7)
comment:1 by , 9 years ago
Component: | Uncategorized → contrib.staticfiles |
---|---|
Triage Stage: | Unreviewed → Accepted |
comment:2 by , 9 years ago
Cc: | added |
---|---|
Owner: | changed from | to
Status: | new → assigned |
comment:3 by , 9 years ago
If no one knows of a legitimate use case for absolute paths being passed to the static templatetag, it seems like this could be fixed fairly easily by raising a ValueError
in the StaticFileStorage url
method if the path has a leading slash.
However, while we're on the topic there are some other inconsistencies such as for CachedFileStorage
leading spaces (but no initial slash) in DEBUG returns the path with a leading space, where as not in DEBUG chomps the leading space. For StaticFileStorage
the leading space is always URL encoded.
Seems like a general 'clean_url' method for StaticFileStorage would be useful, that strips spaces and raises a ValueError
for a leading slash.
comment:4 by , 7 years ago
Owner: | removed |
---|---|
Status: | assigned → new |
comment:5 by , 9 months ago
Cc: | added |
---|
I tested this in Django v5.0.2, and the error message now takes this form: ValueError: Missing staticfiles manifest entry for '/test.css'
.
The error message seems plenty clear to me, so I wonder if a change is even needed here. Also, it makes sense to me that this only happens with ManifestStaticFilesStorage
since StaticFilesStorage
doesn't have a manifest. The inconsistency between the two storage classes is expected.
comment:7 by , 9 months ago
I'm not sure what that would look like since the current behavior seems to be expected. Do you want a test showing that the leading slash typo gives the expected error message in the static
templatetag tests? I'm happy to help, but I need some guidance.
As a note to anyone tackling this, it only happens with
DEBUG = False
.