Opened 3 years ago

Closed 2 years ago

Last modified 2 weeks ago

#34028 closed Bug (invalid)

Django 'static' template tag fails to generate URLs with SCRIPT_NAME prefix

Reported by: Stewart Adam Owned by: Sarah Boyce
Component: Template system Version: dev
Severity: Normal Keywords:
Cc: David Sanders Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

According to the docs, when MEDIA_URL and STATIC_URL are relative (now the default), SCRIPT_NAME should automatically be prepended to them when it is set to ensure that paths are generated correctly. This appears to work everywhere *except* when calling {% static %} in HTML templates.

Best I can tell, when this functionality was first added, it appears that SCRIPT_NAME was evaluated every request: https://github.com/django/django/commit/c574bec0929cd2527268c96a492d25223a9fd576

Looking at this same file today, it appears that setting values are now cached in a dict. When the Django application initializes, it reads SCRIPT_NAME as / and so all subsequent uses of {% static %} are output without the current value of SCRIPT_NAME. This happens even if using a hardcoded FORCE_SCRIPT_NAME.

url() appears to be unaffected by the same issue, so the net result is that links and redirects honor SCRIPT_NAME as expected, and static files are served using the SCRIPT_NAME prefix, but all output from HTML templates simply use /static/... for their links which causes all css/js/favicon/related media to fail to load with a 404.

Change History (16)

comment:2 by Stewart Adam, 3 years ago

The code path leading to the issue appears to be:

  1. static() handler for the template tag calls StaticNode.handle_simple() in templatetags/static.py L174
  2. StaticNode.handle_simple() calls staticfiles_storage.url() at templatetags/static.py L127
  3. staticfiles_storage.url() is backed by a FileSystemStorage: core/files/storage.py L392

None of these calls attempt to verify SCRIPT_NAME or inject STATIC_URL. An alternate code path during step 2 *does* appear to inject STATIC_URL. however to due application settings now being cached into a dict/module the value does not reflect the current value of SCRIPT_NAME and uses the value at application initialization (empty string) instead, so STATIC_URL is always just /static/.

comment:3 by Florian Apolloner, 3 years ago

Resolution: worksforme
Status: newclosed

Hi,

I cannot reproduce this. It works as it should:

from django.template import engines
django_engine = engines['django']
from django.conf import settings
print(settings.FORCE_SCRIPT_NAME)
print(settings.STATIC_URL) # Note: in settings.py it is 'static/' but the output here will include FORCE_SCRIPT_NAME
print(django_engine.from_string("{% load static %}{% static 'test_file' %}").render())
/forced_script_name
/forced_script_name/static/
/forced_script_name/static/test_file

None of these calls attempt to verify SCRIPT_NAME or inject STATIC_URL.

This isn't true: {% static %} uses staticfiles_storage which in turn uses StaticFilesStorage which sets the prefix to STATIC_URL which includes FORCE_SCRIPT_NAME. Feel free to reopen with a reproducer if you are still running into issues.

Last edited 3 years ago by Florian Apolloner (previous) (diff)

comment:4 by Florian Apolloner, 3 years ago

Setting a custom script name also works:

from django.urls.base import *
set_script_prefix('/lala')
from django.template import engines
django_engine = engines['django']
from django.conf import settings
print(settings.FORCE_SCRIPT_NAME)
print(settings.STATIC_URL) # Note: in settings.py it is 'static/' but the output here will include FORCE_SCRIPT_NAME
print(django_engine.from_string("{% load static %}{% static 'test_file' %}").render())
None
/lala/static/
/lala/static/test_file

That said it is true that the value of SCRIPT_NAME is assumed to be static and not change over the runtime of Django. If Django is "loaded" outside of a webcontext first, then you will see the behavior you are describing. I'll accept the ticket for now but I am not sure if this is fixable easily or at all.

comment:5 by Florian Apolloner, 3 years ago

Resolution: worksforme
Status: closednew
Triage Stage: UnreviewedAccepted

in reply to:  4 comment:6 by Stewart Adam, 3 years ago

Replying to Florian Apolloner:

None
/lala/static/
/lala/static/test_file

Admittedly I'm not a Django export, I'm just trying to troubleshoot another OSS app I use that's based on Django. Can you share how you're running this?

That said, when I put this code into view on a sample Django app (django-admin startproject repro), I can confirm the behavior that the first request made to the server sets the prefix for the lifetime of the app. Code at https://github.com/stewartadam/django-static-repro

./run.sh &

# try using SCRIPT_NAME to set the prefix on first request
curl -H 'SCRIPT_NAME: /prefix' http://localhost:8000/prefix/foo/
   FORCE_SCRIPT_NAME: None
   STATIC_URL: /prefix/static/
   call static for test_file: /prefix/static/test_file

# second request paths correctly with /another, but uses first request's /prefix for all variable values/output
curl -H 'SCRIPT_NAME: /another' localhost:8000/another/foo/
   FORCE_SCRIPT_NAME: None
   STATIC_URL: /prefix/static/
   call static for test_file: /prefix/static/test_file

FORCE_SCRIPT_NAME also doesn't behave how I'd expect (overriding SCRIPT_NAME header):

FORCE_SCRIPT_NAME=prefix ./run.sh &

# FORCE_SCRIPT_NAME is supposed to override SCRIPT_NAME header, but isn't here in the pathing
curl -H 'SCRIPT_NAME: /another' http://localhost:8000/another/foo/
   FORCE_SCRIPT_NAME: prefix
   STATIC_URL: /prefix/static/
   call static for test_file: /prefix/static/test_file

# if supplied SCRIPT_NAME doesn't match FORCE_SCRIPT_NAME, serve fails to serve content
curl -I -H 'SCRIPT_NAME: /another' http://localhost:8000/prefix/foo/
  HTTP/1.1 500 Internal Server Error
  Connection: close
  Content-Type: text/html
  Content-Length: 141

That said it is true that the value of SCRIPT_NAME is assumed to be static and not change over the runtime of Django. If Django is "loaded" outside of a webcontext first, then you will see the behavior you are describing. I'll accept the ticket for now but I am not sure if this is fixable easily or at all.

I agree at face value this might be an unusual thing to support, however I can see a few cases where this might happen:

  1. middleware initializes and accesses settings before a request arrives (e.g. whitenoise appears to do this) - SCRIPT_NAME won't be set from the HTTP request at that point in time
  2. on many servers healthchecks or other internal requests might result in a request coming through without SCIRPT_NAME set as it would be for end-users via reverse proxy.

It seems like for configuring the Django prefix when hosted in a sub-paths, it should either (a) move to a hardcoded config-based value so the prefix is the same in all contexts or (b) reset the thread locals based on the request headers - but right now behavior appears to be a mix of both which is what causes issues.

Last edited 3 years ago by Stewart Adam (previous) (diff)

comment:7 by Stewart Adam, 3 years ago

Had a chance to debug a bit more and caching of SCRIPT_NAME aside, even if users use FORCE_SCRIPT_NAME to ensure the prefix is hardcoded and correctly set at all times it looks like there's still something awry:

Whitenoise middleware sees the prefix as '/' during initialization - set_script_prefix() only appears to be called during the first HTTP request, but whitenoise needs to verify the user-provided values for STATIC_URL during middleware initialization. During that middleware initialization, the prefix remains '/' despite FORCE_SCRIPT_NAME being set.

I've posted more details here: https://github.com/evansd/whitenoise/issues/271#issuecomment-1257389536

comment:8 by David Sanders, 3 years ago

Cc: David Sanders added

I've been investigating SCRIPT_NAME / FORCE_SCRIPT_NAME and found some interesting quirks when used with runserver which may be affecting testing these issues (at least it was affecting me when investigating #34185

I'm not entirely sure why yet but I'll post the following reproducible issue (at least for me) to show that prefixing STATIC_URL works intermittently due to a race condition with runserver and the multiple processes setup with autoreload:

In django/conf/__init__.py I added a line to print the result of prefixing STATIC_URL:

    @staticmethod
    def _add_script_prefix(value):
        """
        Add SCRIPT_NAME prefix to relative paths.

        Useful when the app is being served at a subpath and manually prefixing
        subpath to STATIC_URL and MEDIA_URL in settings is inconvenient.
        """
        # Don't apply prefix to absolute paths and URLs.
        if value.startswith(("http://", "https://", "/")):
            return value
        from django.urls import get_script_prefix

        val = "%s%s" % (get_script_prefix(), value)
        print("*********** PREFIXING ***********: " + val)
        return val

The result when runserver is run with autoreload on then off:

sample-project % ./manage.py runserver
Watching for file changes with StatReloader
Performing system checks...

*********** PREFIXING ***********: /static/
System check identified no issues (0 silenced).
November 29, 2022 - 13:22:27
Django version 4.2.dev20221129074011, using settings 'django_sample.settings'
Starting development server at http://127.0.0.1:8000/
Quit the server with CONTROL-C.
^C
sample-project % ./manage.py runserver --noreload
Performing system checks...

*********** PREFIXING ***********: /django/static/
System check identified no issues (0 silenced).
November 29, 2022 - 13:22:40
Django version 4.2.dev20221129074011, using settings 'django_sample.settings'
Starting development server at http://127.0.0.1:8000/
Quit the server with CONTROL-C.
^C
sample-project %

Now the interesting thing is that I can get the same results without specifying --noreload by just adding a line to print() in setup() ! :)

Last edited 3 years ago by David Sanders (previous) (diff)

comment:9 by Sarah Boyce, 2 years ago

Has patch: set
Owner: changed from nobody to Sarah Boyce
Status: newassigned
Version: 4.0dev

I was able to replicate the issue that if someone tries to access the script prefix in a middleware init (as they do in whitenoise), the result is unexpected.
I have raised a PR which has a regression test and a potential fix 👍

I think this relates to this ticket a bit: #16734

comment:10 by Mariusz Felisiak, 2 years ago

Has patch: unset

comment:11 by Mariusz Felisiak, 2 years ago

This can be fixed by #34461.

comment:12 by Mariusz Felisiak, 2 years ago

Resolution: invalid
Status: assignedclosed
Triage Stage: AcceptedUnreviewed

Let's document this caveat (see PR) and close this as "invalid". Discussion about constructing URLs outside of the request-response cycle (new feature) was moved to the #34461.

comment:13 by GitHub <noreply@…>, 2 years ago

In bdf59bf:

Refs #34028 -- Doc'd that get_script_prefix() cannot be used outside of the request-response cycle.

comment:14 by Mariusz Felisiak <felisiak.mariusz@…>, 2 years ago

In e34a54a3:

[4.2.x] Refs #34028 -- Doc'd that get_script_prefix() cannot be used outside of the request-response cycle.

Backport of bdf59bff657975e577b86b194b39ec2f77983d2b from main

comment:15 by Klaas van Schelven, 2 weeks ago

IMHO this (or something awefully close to it) is _still_ a bug.

  1. From the 3.1 release notes: https://docs.djangoproject.com/en/3.1/releases/3.1/#backwards-incompatible-3-1

The STATIC_URL and MEDIA_URL settings set to relative paths are now prefixed by the server-provided value of SCRIPT_NAME (or / if not set). This change should not affect settings set to valid URLs or absolute paths.

As per the original commit message https://github.com/django/django/commit/c574bec0929cd2527268c96a492d25223a9fd576

and in my personal opinion this also makes perfect sense... this brings the behavior of "static" inline with that of e.g. "reverse".

  1. when running the django runserver with autoreload, STATIC_URL appears to be evaluated outside the request/response cycle (and then cached)... this is behavior that is dicussed in this issue... I'm not sure I personally fully understand it (haven't dived into it deeply enough). But what I do understand is: I'm not doing the thing that's been described as "don't do this". Nor am I using whitenoise. But I am suffering from exactly this problem (in Django 5.2).

here's a repo that shows the problem:
https://github.com/vanschelven/poc34028

when visiting in the browser I get:
static: /static/hallo
reverse: /scriptname/

but I would expect the line with "static" to also say "/scriptname/static/hallo"

comment:16 by Klaas van Schelven, 2 weeks ago

On the attached poc, I cannot reproduce the problem with "--noreload" nor when using gunicorn. Only the plain runserver (which does autoreload) exhibits the problem

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