Issues in the Existing SecurityMiddleware Code 1. Incorrect use of response.setdefault() instead of response.headers.setdefault() 2. In the process_request() method, HTTPS redirection is done While this works, %-formatting is less readable and slightly less performant than modern alternatives like f-strings 3. Preventing Overwriting of Existing Headers

[Abhijeet Kumar]

1. Incorrect use of response.setdefault() instead of response.headers.setdefault()

Issue:
In the original code, the Cross-Origin-Opener-Policy (COOP) header is set using:

response.setdefault("Cross-Origin-Opener-Policy", self.cross_origin_opener_policy)

This is incorrect because:

1. response.setdefault() does not exist in Django’s HttpResponse class.
2. Headers should be set using response.headers.setdefault() to ensure they are only added if they don’t already exist.

Suggested Modification:
Replace:

response.setdefault("Cross-Origin-Opener-Policy", self.cross_origin_opener_policy)

With:

response.headers.setdefault("Cross-Origin-Opener-Policy", self.cross_origin_opener_policy)

2. Improving String Formatting for Readability & Performance

Issue:
In the process_request() method, HTTPS redirection is done using:

return HttpResponsePermanentRedirect(
    "https://%s%s" % (host, request.get_full_path())
)

While this works, %-formatting is less readable and slightly less performant than modern alternatives like f-strings.

Suggested Modification:
Change:

return HttpResponsePermanentRedirect(
    "https://%s%s" % (host, request.get_full_path())
)

To:

return HttpResponsePermanentRedirect(f"https://{host}{request.get_full_path()}")

3. Preventing Overwriting of Existing Headers

Issue:

The original code unconditionally sets security headers like:

response.headers["Strict-Transport-Security"] = sts_header
response.headers["X-Content-Type-Options"] = "nosniff"

is could Override existing security policies set by other middleware or custom responses &
Prevent flexibility in modifying security headers dynamically.

Suggested Modification:

Use setdefault() instead of direct assignment:

response.headers.setdefault("Strict-Transport-Security", sts_header)
response.headers.setdefault("X-Content-Type-Options", "nosniff")

Suggested Code:

import re

from django.conf import settings
from django.http import HttpResponsePermanentRedirect
from django.utils.deprecation import MiddlewareMixin


class SecurityMiddleware(MiddlewareMixin):
    def __init__(self, get_response):
        super().__init__(get_response)
        self.sts_seconds = settings.SECURE_HSTS_SECONDS
        self.sts_include_subdomains = settings.SECURE_HSTS_INCLUDE_SUBDOMAINS
        self.sts_preload = settings.SECURE_HSTS_PRELOAD
        self.content_type_nosniff = settings.SECURE_CONTENT_TYPE_NOSNIFF
        self.redirect = settings.SECURE_SSL_REDIRECT
        self.redirect_host = settings.SECURE_SSL_HOST
        self.redirect_exempt = [re.compile(r) for r in settings.SECURE_REDIRECT_EXEMPT]
        self.referrer_policy = settings.SECURE_REFERRER_POLICY
        self.cross_origin_opener_policy = settings.SECURE_CROSS_ORIGIN_OPENER_POLICY

    def process_request(self, request):
        path = request.path.lstrip("/")
        if (
            self.redirect
            and not request.is_secure()
            and not any(pattern.search(path) for pattern in self.redirect_exempt)
        ):
            host = self.redirect_host or request.get_host()
            return HttpResponsePermanentRedirect(f"https://{host}{request.get_full_path()}")

    def process_response(self, request, response):
        if (
            self.sts_seconds
            and request.is_secure()
            and "Strict-Transport-Security" not in response.headers
        ):
            sts_header = f"max-age={self.sts_seconds}"
            if self.sts_include_subdomains:
                sts_header += "; includeSubDomains"
            if self.sts_preload:
                sts_header += "; preload"
            response.headers.setdefault("Strict-Transport-Security", sts_header)

        if self.content_type_nosniff:
            response.headers.setdefault("X-Content-Type-Options", "nosniff")

        if self.referrer_policy:
            # Support a comma-separated string or iterable of values to allow fallback.
            response.headers.setdefault(
                "Referrer-Policy",
                ",".join(
                    [v.strip() for v in self.referrer_policy.split(",")]
                    if isinstance(self.referrer_policy, str)
                    else self.referrer_policy
                ),
            )

        if self.cross_origin_opener_policy:
            response.headers.setdefault(
                "Cross-Origin-Opener-Policy",
                self.cross_origin_opener_policy,
            )

        return response

[Jake Howard]

The formatting for the description is a little broken - could you fix it?

You're also describing 3 separate issues in one ticket, which makes reviewing difficult. Could you separate them?

They also seem to be in the realm of nitpicks and small changes (eg converting an f-string), which doesn't have a noticable impact on, well anything.

[Tim Graham]

1. HttpResponse.set_default() ​exists.

2. Per ​Python style guide, we won't accept PRs that merely change formatting style: "Don’t waste time doing unrelated refactoring of existing code to adjust the formatting method."

3. It could be useful, but at this point it would be backward-incompatible, so I think we should leave it alone unless we have a very compelling use case. Changing anything security related can't be done lightly.

Thanks for the ideas. As Jake said, in the future, please don't combine separate issues in one ticket.