[Django] #32131: ManifestStaticFilesStorage should also check in debug mode whether referenced file actually exists

18 views
Skip to first unread message

Django

unread,
Oct 22, 2020, 4:51:22 AM10/22/20
to django-...@googlegroups.com
#32131: ManifestStaticFilesStorage should also check in debug mode whether
referenced file actually exists
-----------------------------------------------+------------------------
Reporter: Fabian Köster | Owner: nobody
Type: Bug | Status: new
Component: contrib.staticfiles | Version: 3.1
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
-----------------------------------------------+------------------------
We just had a case where we referenced a static file somewhere in the code
like this

{{{#!python
from django.templatetags.static import static
static("/path/to/a/file.png")
}}}

This worked perfectly fine on the development system and also on the
staging systems. After deployment to the production system we noticed that
it does not work when debug mode is disabled (`debug = False`) and the
staticfiles are searched using the staticfiles.json manifest. In the code
above we used an absolute path to the file whereas in the manifest all
files are given with their relative path.

The following code can be used to reproduce the problem (using force=True
is equivalent to debug=False in this case):

{{{#!python
from django.contrib.staticfiles.storage import ManifestStaticFilesStorage
>>> ManifestStaticFilesStorage().url("path/to/a/file.png")
'/static/path/to/a/file.png'
>>> ManifestStaticFilesStorage().url("/path/to/a/file.png")
'/static/path/to/a/file.png'
>>> ManifestStaticFilesStorage().url("path/to/a/file.png", force=True)
'/static/path/to/a/file.f208556b8835.png'
>>> ManifestStaticFilesStorage().url("/path/to/a/file.png", force=True)
Traceback (most recent call last):
File "<input>", line 1, in <module>
ManifestStaticFilesStorage().url('/path/to/a/file.png', force=True)
File "/app/.pyenv/versions/3.7.2/lib/python3.7/site-
packages/django/contrib/staticfiles/storage.py", line 153, in url
return self._url(self.stored_name, name, force)
File "/app/.pyenv/versions/3.7.2/lib/python3.7/site-
packages/django/contrib/staticfiles/storage.py", line 132, in _url
hashed_name = hashed_name_func(*args)
File "/app/.pyenv/versions/3.7.2/lib/python3.7/site-
packages/django/contrib/staticfiles/storage.py", line 420, in stored_name
raise ValueError("Missing staticfiles manifest entry for '%s'" %
clean_name)
ValueError: Missing staticfiles manifest entry for '/path/to/a/file.png'
}}}

I think the behaviour should be the same regardless if debug is `True` or
`False`. I therefore propose to add a check to the `debug = True` code
path that checks for the existance of the file as well so when referencing
a non-existing file or giving an absolute path raises the same exception
as it would on the production system with `debug = False`. I will create a
PR for this in a few minutes.

--
Ticket URL: <https://code.djangoproject.com/ticket/32131>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Oct 22, 2020, 4:54:34 AM10/22/20
to django-...@googlegroups.com
#32131: ManifestStaticFilesStorage should also check in debug mode whether
referenced file actually exists
-------------------------------------+-------------------------------------

Reporter: Fabian Köster | Owner: nobody
Type: Bug | Status: new
Component: contrib.staticfiles | Version: 2.2
Severity: Normal | Resolution:

Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Fabian Köster):

* version: 3.1 => 2.2


--
Ticket URL: <https://code.djangoproject.com/ticket/32131#comment:1>

Django

unread,
Oct 22, 2020, 5:11:37 AM10/22/20
to django-...@googlegroups.com
#32131: ManifestStaticFilesStorage should also check in debug mode whether
referenced file actually exists
-------------------------------------+-------------------------------------

Reporter: Fabian Köster | Owner: nobody
Type: Bug | Status: new
Component: contrib.staticfiles | Version: 3.1
Severity: Normal | Resolution:

Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Fabian Köster):

* version: 2.2 => 3.1


Comment:

Created a PR: https://github.com/django/django/pull/13587

--
Ticket URL: <https://code.djangoproject.com/ticket/32131#comment:2>

Django

unread,
Oct 22, 2020, 5:52:12 AM10/22/20
to django-...@googlegroups.com
#32131: ManifestStaticFilesStorage should also check in debug mode whether
referenced file actually exists
-------------------------------------+-------------------------------------

Reporter: Fabian Köster | Owner: nobody
Type: Bug | Status: new
Component: contrib.staticfiles | Version: 3.1
Severity: Normal | Resolution:

Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Markus Bertheau):

* cc: Markus Bertheau (added)


--
Ticket URL: <https://code.djangoproject.com/ticket/32131#comment:3>

Django

unread,
Oct 22, 2020, 8:10:03 AM10/22/20
to django-...@googlegroups.com
#32131: ManifestStaticFilesStorage should also check in debug mode whether
referenced file actually exists
-------------------------------------+-------------------------------------

Reporter: Fabian Köster | Owner: nobody
Type: Bug | Status: closed
Component: contrib.staticfiles | Version: 3.1
Severity: Normal | Resolution: wontfix

Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Carlton Gibson):

* status: new => closed
* resolution: => wontfix


Comment:

Hi Fabian. I think we have to save wontfix here - this is expected
behaviour.

Passing an absolute path to `static` isn’t correct: all paths should be
relative to `STATIC_ROOT`
([https://docs.djangoproject.com/en/3.1/ref/settings/#static-url which is
quite insistent that it ends in a `/`], so that relative paths can be
appended). **All** the doc examples show using a relative path. Your
examples using relative paths all work as expected so...

I think any change here would need to go via the DevelopersMailingList.
Thanks.

--
Ticket URL: <https://code.djangoproject.com/ticket/32131#comment:4>

Reply all
Reply to author
Forward
0 new messages