From 93657cf88e14b919cb726864814617a6a639e507 Mon Sep 17 00:00:00 2001 From: David Evans Date: Tue, 10 Dec 2019 11:35:25 +0000 Subject: [PATCH] Workaround breaking change in Django 3.0 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=""`. 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. --- tests/test_django_whitenoise.py | 18 ++++++++++++++++-- whitenoise/middleware.py | 14 +++++++++++++- 2 files changed, 29 insertions(+), 3 deletions(-) diff --git a/tests/test_django_whitenoise.py b/tests/test_django_whitenoise.py index 712d31b9..94535aa0 100644 --- a/tests/test_django_whitenoise.py +++ b/tests/test_django_whitenoise.py @@ -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() @@ -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() @@ -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"} diff --git a/whitenoise/middleware.py b/whitenoise/middleware.py index bb82db3e..e8f5a87a 100644 --- a/whitenoise/middleware.py +++ b/whitenoise/middleware.py @@ -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 @@ -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: