gh-150499: http.server: enforce RFC 7230 framing rules for keep-alive connections#150500
gh-150499: http.server: enforce RFC 7230 framing rules for keep-alive connections#150500tonghuaroot wants to merge 1 commit into
Conversation
| self.assertIn(b"POST body=ABCD", data) | ||
|
|
||
| def test_transfer_encoding_rejected(self): | ||
| # RFC 7230 section 3.3.3 rule 3 plus no chunked decoder. |
There was a problem hiding this comment.
Will swap, both in parse_request and in the test class. TE reframes the body as chunked-framed and invalidates CL, so the duplicate-CL check is dead work when both headers are present.
| return False | ||
|
|
||
| # RFC 7230 section 3.3.3 rule 3: this handler does not implement | ||
| # a chunked decoder, so any Transfer-Encoding is rejected. |
There was a problem hiding this comment.
Will swap, both in parse_request and in the test class. TE reframes the body as chunked-framed and invalidates CL, so the duplicate-CL check is dead work when both headers are present.
| if not self.raw_requestline: | ||
| self.close_connection = True | ||
| return | ||
| # Wrap rfile to track body consumption for RFC 7230 section 6.3. |
There was a problem hiding this comment.
This must be a separate PR I think.
There was a problem hiding this comment.
Agreed. The §6.3 piece (_ReadCountingReader, _drain_request_body, the handle_one_request wrap) is now in #150508, a follow-up against the same issue. This PR is now scoped to §3.3.3 rules 3 and 4, the matching tests, and a trimmed news entry.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
|
How much was written by an AI? |
Add two framing checks to BaseHTTPRequestHandler so the handler does not desynchronise on a persistent connection: * RFC 7230 section 3.3.3 rule 3: reject any Transfer-Encoding header with 400 Bad Request and close, because http.server does not implement a chunked decoder. Once a decoder is added this check can be narrowed to anything other than 'identity'. * RFC 7230 section 3.3.3 rule 4: reject duplicate Content-Length headers whose values disagree, with 400 Bad Request and close. Identical values are still accepted. The §6.3 keep-alive body-drain piece is split into a follow-up PR. Add regression tests in Lib/test/test_httpservers.py covering the two defects plus a pipelined keep-alive POST regression test. Signed-off-by: tonghuaroot <tonghuaroot@gmail.com>
tonghuaroot
force-pushed
the
gh-150499-httpserver-framing-validation
branch
from
09be334 to
4275fa7
Compare
|
Thanks.
Will swap, both in
Agreed. The §6.3 piece (
English isn't my first language. I drafted the code and the prose then leaned on the LLM to polish the English across the PR body, code comments, docstrings, and the news entry. The RFC reading, the defect classification, and the design choices are mine. Happy to walk through any specific block. I have made the requested changes; please review again. |
|
Thanks for making the requested changes! @picnixz: please review the changes made to this pull request. |
2 participants
Refs gh-150499 (paired with a follow-up PR for the §6.3 body-drain piece).
Summary
BaseHTTPRequestHandlerinLib/http/server.pydoes not enforce twoRFC 7230 §3.3.3 framing-validation rules, so a non-conforming request can
desynchronise the keep-alive loop. This PR fixes both in a single
class:
Transfer-Encodingheader is nowrejected with
400 Bad RequestandConnection: close, becausehttp.serverdoes not implement a chunked decoder. Once a decoderis added this check can be narrowed to anything other than
identity.Content-Lengthheaderswith disagreeing values are now rejected with
400 Bad RequestandConnection: close. Identical values are still accepted.The §6.3 keep-alive body-drain piece is split into a separate
follow-up PR against the same issue.
Backwards compatibility
The module's class docstring already warns that
http.serveris notintended for production use. The two checks tighten conformance to
RFC 7230 and only change behaviour for non-conforming clients:
Content-Lengthstill works.Transfer-Encodingwas previously accepted and silently ignored,which is a request-smuggling primitive; clients sending TE must now
either dechunk themselves or stop sending it.
Tests
Lib/test/test_httpservers.pyaddsRFC7230FramingTestCasewith:test_transfer_encoding_rejected— rule 3, plus assertion that thesmuggled trailing request is not dispatched.
test_duplicate_content_length_rejected— rule 4.test_duplicate_content_length_same_value_accepted— confirms thesame-value case is unchanged.
test_keep_alive_post_pipeline— regression test for two pipelinedPOSTs with correct
Content-Lengthon the same keep-aliveconnection.
All four tests pass against
mainwith the patched module. Fulltest_httpservers(106 tests) andtest_wsgiref(38 tests) suitesalso pass with no regression.
News entry
Misc/NEWS.d/next/Library/2026-05-27-00-43-54.gh-issue-150499.ykruLi.rst.Backport
This PR targets
mainonly; per CPython practice the backport policyis left to the core dev review.