From c11dece36fca97941220af9bd3f760cc7b6bb233 Mon Sep 17 00:00:00 2001 From: "Paul J. Dorn" Date: Mon, 29 Jan 2024 04:02:24 +0100 Subject: [PATCH 1/3] re-enable 4 tests, accidentally dropped I copied the wrong line, earlier. It is the (request) parser param that should have informed the xfail() decision. --- tests/test_http_parser.py | 51 +++++++++++++++++++++++++++++++++------ 1 file changed, 43 insertions(+), 8 deletions(-) diff --git a/tests/test_http_parser.py b/tests/test_http_parser.py index 5258b05387d..e7aabe90758 100644 --- a/tests/test_http_parser.py +++ b/tests/test_http_parser.py @@ -282,9 +282,20 @@ def test_parse_headers_longline(parser: Any) -> None: parser.feed_data(text) +@pytest.fixture +def xfail_c_parser_status(request) -> None: + if isinstance(request.getfixturevalue("parser"), HttpRequestParserPy): + return + request.node.add_marker( + pytest.mark.xfail( + reason="Regression test for Py parser. May match C behaviour later.", + raises=http_exceptions.BadStatusLine, + ) + ) + + +@pytest.mark.usefixtures("xfail_c_parser_status") def test_parse_unusual_request_line(parser: Any) -> None: - if not isinstance(response, HttpResponseParserPy): - pytest.xfail("Regression test for Py parser. May match C behaviour later.") text = b"#smol //a HTTP/1.3\r\n\r\n" messages, upgrade, tail = parser.feed_data(text) assert len(messages) == 1 @@ -611,9 +622,25 @@ def test_headers_content_length_err_2(parser: Any) -> None: } +@pytest.fixture +def xfail_c_parser_empty_header(request) -> None: + if not all( + (request.getfixturevalue(name) == b"") for name in ("pad1", "pad2", "hdr") + ): + return + if isinstance(request.getfixturevalue("parser"), HttpRequestParserPy): + return + request.node.add_marker( + pytest.mark.xfail( + reason="Regression test for Py parser. May match C behaviour later.", + ) + ) + + @pytest.mark.parametrize("hdr", [b"", b"foo"], ids=["name-empty", "with-name"]) @pytest.mark.parametrize("pad2", _pad.keys(), ids=["post-" + n for n in _pad.values()]) @pytest.mark.parametrize("pad1", _pad.keys(), ids=["pre-" + n for n in _pad.values()]) +@pytest.mark.usefixtures("xfail_c_parser_empty_header") def test_invalid_header_spacing( parser: Any, pad1: bytes, pad2: bytes, hdr: bytes ) -> None: @@ -622,15 +649,12 @@ def test_invalid_header_spacing( if pad1 == pad2 == b"" and hdr != b"": # one entry in param matrix is correct: non-empty name, not padded expectation = nullcontext() - if pad1 == pad2 == hdr == b"": - if not isinstance(response, HttpResponseParserPy): - pytest.xfail("Regression test for Py parser. May match C behaviour later.") with expectation: parser.feed_data(text) def test_empty_header_name(parser: Any) -> None: - if not isinstance(response, HttpResponseParserPy): + if not isinstance(parser, HttpRequestParserPy): pytest.xfail("Regression test for Py parser. May match C behaviour later.") text = b"GET /test HTTP/1.1\r\n" b":test\r\n\r\n" with pytest.raises(http_exceptions.BadHttpMessage): @@ -808,9 +832,20 @@ def test_http_request_upgrade(parser: Any) -> None: assert tail == b"some raw data" +@pytest.fixture +def xfail_c_parser_url(request) -> None: + if isinstance(request.getfixturevalue("parser"), HttpRequestParserPy): + return + request.node.add_marker( + pytest.mark.xfail( + reason="Regression test for Py parser. May match C behaviour later.", + raises=http_exceptions.InvalidURLError, + ) + ) + + +@pytest.mark.usefixtures("xfail_c_parser_url") def test_http_request_parser_utf8_request_line(parser: Any) -> None: - if not isinstance(response, HttpResponseParserPy): - pytest.xfail("Regression test for Py parser. May match C behaviour later.") messages, upgrade, tail = parser.feed_data( # note the truncated unicode sequence b"GET /P\xc3\xbcnktchen\xa0\xef\xb7 HTTP/1.1\r\n" + From 3f54d578c4212c71b152fcc2383d0fdb6686f782 Mon Sep 17 00:00:00 2001 From: "Paul J. Dorn" Date: Wed, 7 Feb 2024 02:20:30 +0100 Subject: [PATCH 2/3] test specifically for the yarl.URL usage we need previous variant failed on PyPy, but that is yarl.URL("invalid") behaviour we do not depend on --- tests/test_http_parser.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/test_http_parser.py b/tests/test_http_parser.py index e7aabe90758..cd0b8b6ef76 100644 --- a/tests/test_http_parser.py +++ b/tests/test_http_parser.py @@ -865,7 +865,9 @@ def test_http_request_parser_utf8_request_line(parser: Any) -> None: assert msg.compression is None assert not msg.upgrade assert not msg.chunked - assert msg.url.path == URL("/P%C3%BCnktchen\udca0\udcef\udcb7").path + # python HTTP parser depends on Cython and CPython URL to match + # .. but yarl.URL("/abs") is not equal to URL.build(path="/abs"), see #6409 + assert msg.url == URL.build(path="/Pünktchen\udca0\udcef\udcb7", encoded=True) def test_http_request_parser_utf8(parser: Any) -> None: From bc6f24e6cbbc5050605a375718fccdcbcb9cf401 Mon Sep 17 00:00:00 2001 From: "Paul J. Dorn" Date: Wed, 7 Feb 2024 02:40:54 +0100 Subject: [PATCH 3/3] change note for #8088 - enabled HTTP parser tests --- CHANGES/8088.contrib.rst | 1 + 1 file changed, 1 insertion(+) create mode 100644 CHANGES/8088.contrib.rst diff --git a/CHANGES/8088.contrib.rst b/CHANGES/8088.contrib.rst new file mode 100644 index 00000000000..b3aec71bdf7 --- /dev/null +++ b/CHANGES/8088.contrib.rst @@ -0,0 +1 @@ +Enabled HTTP parser tests originally intended for 3.9.2 release -- by :user:`pajod`.