[Django] #35440: Update parse_header_parameters to leverage the parsing logic from (stdlib) email Message.

[Django]

#35440: Update parse_header_parameters to leverage the parsing logic from (stdlib)
email Message.
------------------------------------------------+------------------------
Reporter: Natalia Bidart | Owner: nobody
Type: Cleanup/optimization | Status: new
Component: HTTP handling | Version: dev
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Needs documentation: 0 | Needs tests: 0
Patch needs improvement: 0 | Easy pickings: 0
UI/UX: 0 |
------------------------------------------------+------------------------
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
[https://peps.python.org/pep-0594/#cgi 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:

{{{#!python
>>> 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`
--
Ticket URL: <https://code.djangoproject.com/ticket/35440>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

[Django]

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

-------------------------------------+-------------------------------------

Reporter: Natalia Bidart | Owner: nobody

Type: | Status: new
Cleanup/optimization |

Component: HTTP handling | Version: dev

Severity: Normal | Resolution:

Keywords: | Triage Stage:
| Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Description changed by Natalia Bidart:

Old description:

New description:

`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.

--
--
Ticket URL: <https://code.djangoproject.com/ticket/35440#comment:1>

[Django]

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

--------------------------------------+------------------------------------

Reporter: Natalia Bidart | Owner: nobody

Type: Cleanup/optimization | Status: new

Component: HTTP handling | Version: dev
Severity: Normal | Resolution:

Keywords: | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

--------------------------------------+------------------------------------
Changes (by David Smith):

* stage: Unreviewed => Accepted

--
Ticket URL: <https://code.djangoproject.com/ticket/35440#comment:2>

[Django]

#35440: Update parse_header_parameters to leverage the parsing logic from (stdlib)
email Message.
--------------------------------------+------------------------------------

Reporter: Natalia Bidart | Owner: chudilo
Type: Cleanup/optimization | Status: assigned

Component: HTTP handling | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------

Changes (by chudilo):

* owner: nobody => chudilo
* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/35440#comment:3>

[Django]

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

-------------------------------------+-------------------------------------
Reporter: Natalia Bidart | Owner: Khudyakov
Type: | Artem

Cleanup/optimization | Status: assigned
Component: HTTP handling | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

-------------------------------------+-------------------------------------
Changes (by Khudyakov Artem):

* has_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/35440#comment:4>

[Django]

#35440: Update parse_header_parameters to leverage the parsing logic from (stdlib)
email Message.
-------------------------------------+-------------------------------------
Reporter: Natalia Bidart | Owner: Khudyakov
Type: | Artem
Cleanup/optimization | Status: assigned
Component: HTTP handling | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Khudyakov Artem):

* has_patch: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/35440#comment:5>

[Django]

#35440: Update parse_header_parameters to leverage the parsing logic from (stdlib)
email Message.
-------------------------------------+-------------------------------------
Reporter: Natalia Bidart | Owner: Khudyakov
Type: | Artem
Cleanup/optimization | Status: assigned
Component: HTTP handling | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by Khudyakov Artem):

* has_patch: 0 => 1

Comment:

[https://github.com/django/django/pull/18424 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/35440#comment:6>

[Django]

#35440: Update parse_header_parameters to leverage the parsing logic from (stdlib)
email Message.
-------------------------------------+-------------------------------------
Reporter: Natalia Bidart | Owner: Khudyakov
Type: | Artem
Cleanup/optimization | Status: assigned
Component: HTTP handling | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Natalia Bidart):

* needs_better_patch: 0 => 1

Comment:

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).
--
Ticket URL: <https://code.djangoproject.com/ticket/35440#comment:7>

[Django]

#35440: Update parse_header_parameters to leverage the parsing logic from (stdlib)
email Message.
-------------------------------------+-------------------------------------
Reporter: Natalia Bidart | Owner: Khudyakov
Type: | Artem
Cleanup/optimization | Status: assigned
Component: HTTP handling | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Khudyakov Artem):

* needs_better_patch: 1 => 0

Comment:

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.
--
Ticket URL: <https://code.djangoproject.com/ticket/35440#comment:8>

[Django]

#35440: Update parse_header_parameters to leverage the parsing logic from (stdlib)
email Message.
-------------------------------------+-------------------------------------
Reporter: Natalia Bidart | Owner: Khudyakov
Type: | Artem
Cleanup/optimization | Status: assigned
Component: HTTP handling | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Natalia Bidart):

* needs_better_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/35440#comment:9>

[Django]

#35440: Update parse_header_parameters to leverage the parsing logic from (stdlib)
email Message.
-------------------------------------+-------------------------------------
Reporter: Natalia Bidart | Owner: Khudyakov
Type: | Artem
Cleanup/optimization | Status: assigned
Component: HTTP handling | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Khudyakov Artem):

* needs_better_patch: 1 => 0

--
Ticket URL: <https://code.djangoproject.com/ticket/35440#comment:10>

[Django]

#35440: Update parse_header_parameters to leverage the parsing logic from (stdlib)
email Message.
-------------------------------------+-------------------------------------
Reporter: Natalia Bidart | Owner: Khudyakov
Type: | Artem
Cleanup/optimization | Status: assigned
Component: HTTP handling | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 1

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Shai Berger):

* cc: Shai Berger (added)

* needs_better_patch: 0 => 1

--
Ticket URL: <https://code.djangoproject.com/ticket/35440#comment:11>

[Django]

#35440: Update parse_header_parameters to leverage the parsing logic from (stdlib)
email Message.
-------------------------------------+-------------------------------------
Reporter: Natalia Bidart | Owner: Khudyakov
Type: | Artem
Cleanup/optimization | Status: assigned
Component: HTTP handling | Version: dev
Severity: Normal | Resolution:

Keywords: | Triage Stage: Ready for
| checkin

Has patch: 1 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0

Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Natalia Bidart):

* needs_better_patch: 1 => 0

* stage: Accepted => Ready for checkin

--
Ticket URL: <https://code.djangoproject.com/ticket/35440#comment:12>

[Django]

#35440: Update parse_header_parameters to leverage the parsing logic from (stdlib)
email Message.
-------------------------------------+-------------------------------------
Reporter: Natalia Bidart | Owner: Khudyakov
Type: | Artem

Cleanup/optimization | Status: closed

Component: HTTP handling | Version: dev

Severity: Normal | Resolution: fixed

Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by nessita <124304+nessita@…>):

* resolution: => fixed
* status: assigned => closed

Comment:

In [changeset:"9aabe7eae3eeb3e64c5a0f3687118cd806158550" 9aabe7e]:
{{{#!CommitTicketReference repository=""
revision="9aabe7eae3eeb3e64c5a0f3687118cd806158550"
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 <bc...@crossway.org>
Co-authored-by: Natalia <124304+...@users.noreply.github.com>
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35440#comment:13>

[Django]

#35440: Update parse_header_parameters to leverage the parsing logic from (stdlib)
email Message.
-------------------------------------+-------------------------------------
Reporter: Natalia Bidart | Owner: Khudyakov
Type: | Artem
Cleanup/optimization | Status: closed
Component: HTTP handling | Version: dev
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Comment (by nessita <124304+nessita@…>):

In [changeset:"424e0d86973d88b402b55f20884938715aad740b" 424e0d86]:
{{{#!CommitTicketReference repository=""
revision="424e0d86973d88b402b55f20884938715aad740b"
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.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/35440#comment:14>

[Django]

#35440: Update parse_header_parameters to leverage the parsing logic from (stdlib)
email Message.
-------------------------------------+-------------------------------------
Reporter: Natalia Bidart | Owner: Khudyakov
Type: | Artem

Cleanup/optimization | Status: new

Component: HTTP handling | Version: dev
Severity: Normal | Resolution:

Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0

Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------------+-------------------------------------

Changes (by Natalia Bidart):

* has_patch: 1 => 0
* resolution: fixed =>
* stage: Ready for checkin => Accepted
* status: closed => new

--
Ticket URL: <https://code.djangoproject.com/ticket/35440#comment:15>

[Django]

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

--------------------------------------+------------------------------------
Reporter: Natalia Bidart | Owner: (none)
Type: Cleanup/optimization | Status: assigned

Component: HTTP handling | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0

--------------------------------------+------------------------------------
Changes (by Natalia Bidart):

* owner: Khudyakov Artem => (none)

* status: new => assigned

--
Ticket URL: <https://code.djangoproject.com/ticket/35440#comment:16>

[Django]

#35440: Update parse_header_parameters to leverage the parsing logic from (stdlib)
email Message.
--------------------------------------+------------------------------------
Reporter: Natalia Bidart | Owner: (none)

Type: Cleanup/optimization | Status: new

Component: HTTP handling | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------
Changes (by Natalia Bidart):

* status: assigned => new

--
Ticket URL: <https://code.djangoproject.com/ticket/35440#comment:17>

[Django]

#35440: Update parse_header_parameters to leverage the parsing logic from (stdlib)
email Message.
--------------------------------------+------------------------------------
Reporter: Natalia Bidart | Owner: (none)
Type: Cleanup/optimization | Status: new
Component: HTTP handling | Version: dev
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
--------------------------------------+------------------------------------

Comment (by 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.
--
Ticket URL: <https://code.djangoproject.com/ticket/35440#comment:18>