Update parse_header_parameters to leverage the parsing logic from (stdlib) email Message.

[Natalia Bidart]

Following a security report, which was concluded not to be such, the Security Team agreed to apply a few improvements to the parse_header_parameters function from django/utils/http.py.

This function was historically using Python's (now deprecated) cgi module, but in 34e2148fc725e7200050f74130d7523e3cd8507a the code was changed by porting the cgi implementations into the file to avoid the deprecation (this was in the context of #33173). Later on the header parsing logic was cleaned up in d4d5427571b4bf3a21c902276c2a00215c2a37cc to unify the logic with the one used in the multipartparser.py module (see #33697).

At the time of the cgi deprecation, the ​PEP including the deprecation mentions:

Replacements for the various parts of cgi which are not directly related to executing code are: [ ... ] parse_header with email.message.Message (see example below)

Providing an explicit example of how close parse_header and email.message.Message are:

>>> from cgi import parse_header
>>> from email.message import Message
>>> parse_header(h)
('application/json', {'charset': 'utf8'})
>>> m = Message()
>>> m['content-type'] = h
>>> m.get_params()
[('application/json', ''), ('charset', 'utf8')]
>>> m.get_param('charset')

The goal of this ticket is to track the improvement of the current parse_header_parameters implementation by leveraging the logic from email.message.Message.

The Security Team also agreed that it's worth adding some early checks in the parse_header_parameters function to limit the amount of provided semicolons. This would require some investigation as to what would be a good threshold, considering that it's likely that more than one semicolon may not be necessary in valid HTTP headers.

[Khudyakov Artem]

​PR

[Natalia Bidart]

Setting as patch needs improvements following my comments in the PR (the goal of this ticket is, ideally, to remove/replace our custom parsing logic with Python stdlib's Message methods).

[Khudyakov Artem]

I've proposed a fix in the PR, but have clarifying questions. In case this task is not on the review radar, I change the status, but am ready to make further modifications.

[nessita <124304+nessita@…>]

In 9aabe7e:

Fixed #35440 -- Simplified parse_header_parameters by leveraging stdlid's Message.

The parse_header_parameters function historically used Python's cgi
module (now deprecated). In 34e2148fc725e7200050f74130d7523e3cd8507a,
the logic was inlined to work around this deprecation ( #33173). Later,
in d4d5427571b4bf3a21c902276c2a00215c2a37cc, the header parsing logic
was further cleaned up to align with multipartparser.py (#33697).

This change takes it a step further by replacing the copied cgi logic with
Python's email.message.Message API for a more robust and maintainable header
parsing implementation.

Thanks to Raphael Gaschignard for testing, and to Adam Johnson and Shai
Berger for reviews.

Co-authored-by: Ben Cail <bcail@…>
Co-authored-by: Natalia <124304+nessita@…>

[nessita <124304+nessita@…>]

In 424e0d86:

Fixed #36520 -- Reverted "Fixed #35440 -- Simplified parse_header_parameters by leveraging stdlid's Message."

This partially reverts commit 9aabe7eae3eeb3e64c5a0f3687118cd806158550.

The simplification of parse_header_parameters using stdlib's Message
is reverted due to a performance regression. The check for the header
maximum length remains in place, per Security Team guidance.

Thanks to David Smith for reporting the regression, and Jacob Walls for
the review.

[Carlton Gibson]

Just to say, I think this is the right decision here. 👏 Even if we remerge the same thing later, having time to measure twice seems right to me. Thanks for all the consideration that's gone into it.

[Naveed Qadir]

I ran into this ticket while working on Accept header parsing. I agree that moving toward a stdlib-based solution is the right long-term direction, particularly given the previous performance concerns.
While complex quoted parameters in Accept headers appear to be very rare in real-world traffic, incorrect handling can still lead to broken parsing for otherwise valid headers, so it seems important that the eventual approach handles these cases correctly.

[Pravin]

I tried using a simple string split for normal headers, and only uses email.message when it sees complex symbols like " or *.

def _parse_header_params(line):
    """The 'Slower' logic."""
    m = Message()
    m["content-type"] = line
    params = m.get_params()
    if not params:
        return "", {}
    
    pdict = {}

    key = params.pop(0)[0].lower()
    
    for name, value in params:
        if not name:
            continue
        if isinstance(value, tuple):
            value = collapse_rfc2231_value(value)
        pdict[name] = value
    return key, pdict

def parse_header_parameters(line, max_length=MAX_HEADER_LENGTH):
    """
    Parse a Content-type like header.
    Return the main content-type and a dictionary of options.

    If `line` is longer than `max_length`, `ValueError` is raised.
    """
    if not line:
        return "", {}

    if max_length is not None and len(line) > max_length:
        raise ValueError("Unable to parse header parameters (value too long).")

    if ";" not in line:
        return line.lower().strip(), {}

    if '"' not in line and "*" not in line:
        parts = line.split(";")
        key = parts[0].lower().strip()
        pdict = {}
        for p in parts[1:]:
            if "=" in p:
                name, value = p.split("=", 1)
                pdict[name.lower().strip()] = value.strip()
        return key, pdict

    return _parse_header_params(line)

I ran a benchmark with 50,000 iterations. Here are my results:

Test Case		Existing	New implementation
Simple (no params)	0.050s	 	0.016s
Standard (charset)	0.109s		0.044s
Complex (quotes)	0.214s		0.681s

for me this make sense , It will rarely happened that there would be a complex case for parsing.

[Pravin]

This small adjustment i made.

[Pravin]

my previous suggestion was failing for one test case.

So I wrote new simple patch as below modifying the existing one as below in the diff
attachment:diff.txt​

and i ran 9 to 10 times attachment:bench_mark.py​
50,000 iterations for each. and following result i got on average. I used linux i3 8gb 5 year old laptop may be that's why i'm getting fluctuations in the results.

Complex test case (which triggers the fallback), results varied between -73% and +42% across runs.

Test Case	               Original Avg Time	  Optimized Avg Time	Improvement (Avg)
Simple (No Params)	        ~0.047s	                     ~0.012s	          ~73%
Standard (Charset)	        ~0.110s	                     ~0.045s	          ~59%
Multi-Param	                ~0.190s                      ~0.065s	          ~65%
Complex (RFC 2231)	        ~0.650s	                     ~0.530s	          Variable*

Below are the sampled results when i ran attachment:bench_mark.py​ for 15 consecutive times:

Run | Simple  | Standard | Multi-P | Complex |
----------------------------------------------
1   | 70.26%  | 63.36%   | 65.50%  | -2.99%  |
2   | 66.09%  | 57.83%   | 63.27%  | -34.87% |
3   | 73.31%  | 57.90%   | 59.09%  | -17.48% |
4   | 78.64%  | 58.98%   | 65.31%  | 36.69%  |
5   | 74.02%  | 59.89%   | 60.66%  | 39.80%  |
6   | 74.90%  | 58.15%   | 53.38%  | 23.74%  |
7   | 74.85%  | 60.53%   | 65.36%  | 33.77%  |
8   | 71.63%  | 53.78%   | 54.52%  | 28.88%  |
9   | 72.75%  | 53.04%   | 65.91%  | 33.18%  |
10  | 71.69%  | 60.46%   | 64.59%  | -14.10% |
11  | 75.16%  | 61.84%   | 67.22%  | 36.87%  |
12  | 74.15%  | 58.61%   | 64.92%  | -73.24% |
13  | 72.17%  | 56.63%   | 63.03%  | 33.01%  |
14  | 73.14%  | 47.98%   | 64.39%  | 24.96%  |
15  | 73.54%  | 57.26%   | 64.47%  | 25.37%  |