Changes between Initial Version and Version 2 of Ticket #36206


Ignore:
Timestamp:
Feb 21, 2025, 6:16:42 PM (23 hours ago)
Author:
Tim Graham
Comment:
  1. HttpResponse.set_default() ​exists.
  1. 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."
  1. 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.

Legend:

Unmodified
Added
Removed
Modified

  • Ticket #36206

    • Property Component Uncategorized → HTTP handling
    • Property Keywords security.py removed
    • Property Resolution → invalid
    • Property Status new → closed
    • Property Type Bug → Cleanup/optimization

  • Ticket #36206 – Description

    initial v2  
    1 1. Incorrect use of response.setdefault() instead of response.headers.setdefault()
     11. Incorrect use of `response.setdefault()` instead of `response.headers.setdefault()`
     2
    23Issue:
    34In the original code, the Cross-Origin-Opener-Policy (COOP) header is set using:
    4 
     5{{{#!python
    56response.setdefault("Cross-Origin-Opener-Policy", self.cross_origin_opener_policy)
    6 
     7}}}
    78This is incorrect because:
    89
    9 a. response.setdefault() does not exist in Django’s HttpResponse class.
    10 b. Headers should be set using response.headers.setdefault() to ensure they are only added if they don’t already exist.
     10a. `response.setdefault()` does not exist in Django’s `HttpResponse` class.
     11b. Headers should be set using `response.headers.setdefault()` to ensure they are only added if they don’t already exist.
    1112
    1213Suggested Modification:
    1314Replace:
    14 
     15{{{#!python
    1516response.setdefault("Cross-Origin-Opener-Policy", self.cross_origin_opener_policy)
    16 
     17}}}
    1718With:
    18 
     19{{{#!python
    1920response.headers.setdefault("Cross-Origin-Opener-Policy", self.cross_origin_opener_policy)
     21}}}
    2022
    21232. Improving String Formatting for Readability & Performance
    … …  
    2325Issue:
    2426In the process_request() method, HTTPS redirection is done using:
    25 
     27{{{#!python
    2628return HttpResponsePermanentRedirect(
    2729    "https://%s%s" % (host, request.get_full_path())
    2830)
    29 
     31}}}
    3032While this works, %-formatting is less readable and slightly less performant than modern alternatives like f-strings.
    3133
    3234Suggested Modification:
    3335Change:
    34 
     36{{{#!python
    3537return HttpResponsePermanentRedirect(
    3638    "https://%s%s" % (host, request.get_full_path())
    3739)
    38 
     40}}}
    3941To:
     42{{{#!python
    4043return HttpResponsePermanentRedirect(f"https://{host}{request.get_full_path()}")
    41 
     44}}}
    4245
    43463. Preventing Overwriting of Existing Headers
    … …  
    4649
    4750The original code unconditionally sets security headers like:
    48 
     51{{{#!python
    4952response.headers["Strict-Transport-Security"] = sts_header
    5053response.headers["X-Content-Type-Options"] = "nosniff"
    51 
    52 
    53 This could Override existing security policies set by other middleware or custom responses &
     54}}}
     55is could Override existing security policies set by other middleware or custom responses &
    5456Prevent flexibility in modifying security headers dynamically.
    5557
    5658Suggested Modification:
    5759
    58 Use setdefault() instead of direct assignment:
    59 
     60Use `setdefault()` instead of direct assignment:
     61{{{#!python
    6062response.headers.setdefault("Strict-Transport-Security", sts_header)
    6163response.headers.setdefault("X-Content-Type-Options", "nosniff")
    62 
     64}}}
    6365
    6466Suggested Code:
    65 
     67{{{#!python
    6668import re
    6769
    … …  
    128130
    129131        return response
     132}}}
Back to Top