heroku / django-heroku

[DEPRECATED] Do not use! See https://github.com/heroku/django-heroku/issues/56
BSD 3-Clause "New" or "Revised" License
459 stars 142 forks source link

WhiteNoise middleware added in the wrong place #32

Open pupeno opened 5 years ago

pupeno commented 5 years ago

From WhiteNoise's documentation, the middleware should be added after the security one:

Edit your settings.py file and add WhiteNoise to the MIDDLEWARE_CLASSES list, above all other middleware apart from Django’s SecurityMiddleware

http://whitenoise.evans.io/en/stable/#quickstart-for-django-apps

Yet django-heroku adds it before the security middleware.

ntravis commented 5 years ago

Looks pretty straightforward as to why: https://github.com/heroku/django-heroku/blob/8396b5544c36274942ee6d16ab46d4cddcdd3b4b/django_heroku/core.py#L101-L105

For a fix, I've submitted a PR. Not as clean-looking, but does operate within the expected parameters of whitenoise. You could make an assumption about SecurityMiddleware always being first in the list, but that seems fraught. You have to go out to a new variable because list.index modifies the original list, rather than returning the newly modified list, so the original list(config...) won't work since that created an on-the-fly reference.

superandrew commented 5 years ago

+1

udit-001 commented 4 years ago

Any updates on this so far? It doesn't seem to work with Django 2.2 :disappointed: