Skip to content

Commit

Permalink
Workaround breaking change in Django 3.0
Browse files Browse the repository at this point in the history
Django 3.0 adds a default Content-Disposition header to all FileResponse
instances. Where the response isn't flagged as an attachment this header
has the form `inline; filename="<filename>"`. See:
https://code.djangoproject.com/ticket/30196

This header causes problems with Safari (see #240 and #241) so we want
to make sure we don't send it. Looking at the code for FileResponse I
can see that Django is also setting other default headers. These will
generally be overwritten by WhiteNoise, but they cause unnecessary file
accesses and provide more possibilities for errors.

This patch creates a tiny wrapper around FileResponse which disables the
setting of any default headers.
  • Loading branch information
evansd committed Dec 10, 2019
1 parent 4de06ef commit 93657cf
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 3 deletions.
18 changes: 16 additions & 2 deletions tests/test_django_whitenoise.py
Expand Up @@ -12,12 +12,11 @@
from django.core.management import call_command
from django.utils.functional import empty

from whitenoise.middleware import WhiteNoiseMiddleware
from whitenoise.middleware import WhiteNoiseMiddleware, WhiteNoiseFileResponse

from .utils import AppServer, Files



@pytest.fixture(autouse=True, scope="module")
def setup():
yield django.setup()
Expand Down Expand Up @@ -142,6 +141,12 @@ def finder_static_files(request):
return files


def test_no_content_disposition_header(server, static_files, _collect_static):
url = settings.STATIC_URL + static_files.js_path
response = server.get(url)
assert response.headers.get("content-disposition") is None


@pytest.fixture(scope="module")
def finder_application(finder_static_files):
return get_wsgi_application()
Expand Down Expand Up @@ -195,3 +200,12 @@ def test_directory_path_without_trailing_slash_redirected(
location = get_url_path(response.url, response.headers["Location"])
assert response.status_code == 302
assert location == settings.STATIC_URL + directory_path


def test_whitenoise_file_response_has_only_one_header():
response = WhiteNoiseFileResponse(open(__file__, "rb"))
response.close()
headers = {key.lower() for key, value in response.items()}
# This subclass should have none of the default headers that FileReponse
# sets
assert headers == {"content-type"}
14 changes: 13 additions & 1 deletion whitenoise/middleware.py
Expand Up @@ -14,6 +14,18 @@
__all__ = ["WhiteNoiseMiddleware"]


class WhiteNoiseFileResponse(FileResponse):
"""
Wrap Django's FileResponse to prevent setting any default headers. For the
most part these just duplicate work already done by WhiteNoise but in some
cases (e.g. the content-disposition header introduced in Django 3.0) they
are actively harmful.
"""

def set_headers(self, *args, **kwargs):
pass


class WhiteNoiseMiddleware(WhiteNoise):
"""
Wrap WhiteNoise to allow it to function as Django middleware, rather
Expand Down Expand Up @@ -58,7 +70,7 @@ def process_request(self, request):
def serve(static_file, request):
response = static_file.get_response(request.method, request.META)
status = int(response.status)
http_response = FileResponse(response.file or (), status=status)
http_response = WhiteNoiseFileResponse(response.file or (), status=status)
# Remove default content-type
del http_response["content-type"]
for key, value in response.headers:
Expand Down

0 comments on commit 93657cf

Please sign in to comment.