#33697 closed Cleanup/optimization (fixed)
Cleanup duplication of HTTP header parsing in utils.http and multipart parser.
Reported by: | Carlton Gibson | Owned by: | Mehrdad Moradizadeh |
---|---|---|---|
Component: | Utilities | Version: | 4.0 |
Severity: | Normal | Keywords: | |
Cc: | Florian Apolloner | Triage Stage: | Ready for checkin |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | yes | UI/UX: | no |
Description
PY311 deprecated the cgi
module.
We removed usage of that in PR 15679.
There's similar logic already in place in multipartparser.py
, but header parameters are parsed slightly differently, and the return types of the helper functions used don't match exactly. (Comment on the PR linking to the code locations.)
Nonetheless it looks like we should be able to combine the two functions to have a single parsing routine.
Change History (17)
comment:1 by , 3 years ago
Triage Stage: | Unreviewed → Accepted |
---|
comment:2 by , 3 years ago
+1 Yes. (And there's the list vs scalar thing... but that's just needing to sit and think it through clearly.) — Ideal would be to unify on a single version in utils
and use in both places.
comment:3 by , 3 years ago
Hi,
I want to give this issue a try. Although I am not sure how to run and check what these functions are doing?
Would it be like creating a new django project and tests from where this function is called? Or something else?
comment:4 by , 3 years ago
Hi Kapil.
There's already some coverage, so making the change ensuring the existing tests pass would be a start.
On implementation, you may find you need to adjust tests, or add new ones. That's OK.
If we need extra tests after that, they can be suggested on review.
The Submitting patches guide should help you on your next steps.
Thanks!
comment:5 by , 2 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
comment:8 by , 2 years ago
Patch needs improvement: | set |
---|
comment:14 by , 2 years ago
Patch needs improvement: | unset |
---|---|
Triage Stage: | Accepted → Ready for checkin |
comment:16 by , 2 years ago
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
It looks that the only difference between
_parseparam()
anddjango.http.multipartparser._parse_header_params()
is that it works on strings (instead of bytes) and that it contains https://github.com/python/cpython/commit/1ef0c0349e8fdb5415e21231cb42edbf232b742a (which is desired in both places).