Reject requests that have duplicate Content-Type / Host headers#3548
Reject requests that have duplicate Content-Type / Host headers#3548Cycloctane wants to merge 9 commits intobenoitc:masterfrom
Conversation
currently only Host, Content-Length and Content-Type are rejected as they are more likely to cause security issues. also move headers merging from wsgi to http header parser
since duplicated headers now are merged in parse_headers
tests/requests/valid/030 is exactly the same as 029
pajod
left a comment
pajod
left a comment
There was a problem hiding this comment.
This swaps mapping/merging order. This results in meaningful header content change for accepted requests while only justifying the decision to reject some requests. Some test like tests/requests/valid/040_compat.http - but actually testing the edge case - would need to be added.
benoitc
left a comment
benoitc
left a comment
There was a problem hiding this comment.
Thanks for your PR! Please check my comment. Also I think it should be an option. Some servers rely on multiple headers. Can you do the changes?
| def parse_headers(self, data, from_trailer=False): | ||
| cfg = self.cfg | ||
| headers = [] | ||
| headers = {} |
There was a problem hiding this comment.
thois would break the header order. If you want to check an unicity i would rather use a last seen pattern or equivalent like it's done for some headers. Also we need to keep compatibility with some weird behaviour. So I would just use it for content type and host headers
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
I've reconsidered my approach. "Last seen" pattern is indeed cleaner for this PR. I've updated my changes. Thanks for your review :)
|
@benoitc Sorry for the delay. It seems that the project was refactored. Should we add duplicated headers checks for the new http c parser to gunicorn_h1c? Then I would like to open a pr in that repository also. |
3 participants
Duplicate Content-Type and Host headers in requests are not allowed in RFC9110 and can cause security concerns. This PR makes
parse_headers()raiseInvalidHeaderon these malformed requests.Fixes #3366