diff --git a/src/proxy/hdrs/MIME.cc b/src/proxy/hdrs/MIME.cc index 26aaeaa410b..f5d3e9b0ce0 100644 --- a/src/proxy/hdrs/MIME.cc +++ b/src/proxy/hdrs/MIME.cc @@ -2559,15 +2559,23 @@ mime_parser_parse(MIMEParser *parser, HdrHeap *heap, MIMEHdrImpl *mh, const char } field_name.rtrim_if(&ParseRules::is_ws); raw_print_field = false; - } else if (parsed.suffix(2) != "\r\n" || (parsed.size() > 2 && parsed[parsed.size() - 3] == '\r')) { - // Do not preserve malformed line endings when forwarding the field. - raw_print_field = false; } // find value first field_value.ltrim_if(&ParseRules::is_ws); field_value.rtrim_if(&ParseRules::is_wslfcr); + if (raw_print_field) { + // Raw printing copies the original input bytes instead of re-serializing, so it would + // replay this line ending verbatim. Everything between the end of the trimmed value and + // the end of the line must be optional whitespace followed by exactly one CRLF; a bare + // CR in there is malformed (RFC 9112 section 2.2) and must not be forwarded. + TextView tail{field_value.data() + field_value.size(), parsed.data() + parsed.size()}; + if (tail.suffix(2) != "\r\n" || tail.remove_suffix(2).find_first_of("\r\n") != TextView::npos) { + raw_print_field = false; + } + } + // Make sure the name + value is not longer than configured max_hdr_field_size if (field_name.size() + field_value.size() > max_hdr_field_size) { return ParseResult::ERROR; diff --git a/src/proxy/hdrs/unit_tests/test_Hdrs.cc b/src/proxy/hdrs/unit_tests/test_Hdrs.cc index f0a432fcb0c..7d4d44f2e29 100644 --- a/src/proxy/hdrs/unit_tests/test_Hdrs.cc +++ b/src/proxy/hdrs/unit_tests/test_Hdrs.cc @@ -3175,9 +3175,28 @@ TEST_CASE("HTTP parser tolerates high-bit bytes without UB", "[proxy][hdrtest]") req_hdr.destroy(); } -TEST_CASE("HTTP parser normalizes repeated carriage returns in header line endings", "[proxy][hdrtest]") +TEST_CASE("HTTP request parser normalizes bare carriage returns in header line endings", "[proxy][hdrtest]") { - constexpr std::string_view message = "GET / HTTP/1.1\r\nHost: example.com\r\nExtra-CRs: \r\r\r\r\n\r\n"sv; + struct Test { + std::string_view raw_value; + std::string_view normalized_field; + }; + + // The four-CR case from #13595 produces the maximum raw-print pad and is rejected by the + // existing pad-size limit alone. These smaller pads exercise the parser check independently. + static const std::vector tests = { + {"\r \r\n"sv, "Extra-CRs: \r\n"sv }, + {"\r\r \r\n"sv, "Extra-CRs: \r\n"sv }, + {"bar\r \r\n"sv, "Extra-CRs: bar\r\n"sv}, + {"\r\t\r\n"sv, "Extra-CRs: \r\n"sv }, + }; + + auto test = GENERATE(from_range(tests)); + CAPTURE(test.raw_value); + + std::string message = "GET / HTTP/1.1\r\nHost: example.com\r\nExtra-CRs: "; + message += test.raw_value; + message += "\r\n"; HTTPParser parser; http_parser_init(&parser); @@ -3186,7 +3205,7 @@ TEST_CASE("HTTP parser normalizes repeated carriage returns in header line endin HdrHeap *heap = new_HdrHeap(HdrHeap::DEFAULT_SIZE + 64); req_hdr.create(HTTPType::REQUEST, HTTP_1_1, heap); - auto start = message.data(); + const char *start = message.data(); REQUIRE(req_hdr.parse_req(&parser, &start, message.data() + message.size(), true) == ParseResult::DONE); std::string serialized(static_cast(req_hdr.length_get()), '\0'); @@ -3195,12 +3214,40 @@ TEST_CASE("HTTP parser normalizes repeated carriage returns in header line endin req_hdr.print(serialized.data(), static_cast(serialized.size()), &index, &offset); serialized.resize(static_cast(index)); - CHECK(serialized.find("Extra-CRs: \r\n") != std::string::npos); - CHECK(serialized.find("Extra-CRs: \r\r") == std::string::npos); + CHECK(serialized.find(test.normalized_field) != std::string::npos); + std::string raw_field = "Extra-CRs: "; + raw_field += test.raw_value; + CHECK(serialized.find(raw_field) == std::string::npos); req_hdr.destroy(); } +TEST_CASE("HTTP response parser normalizes bare carriage returns in header line endings", "[proxy][hdrtest]") +{ + constexpr std::string_view message = "HTTP/1.1 200 OK\r\nExtra-CRs: bar\r \r\n\r\n"sv; + + HTTPParser parser; + http_parser_init(&parser); + + HTTPHdr resp_hdr; + HdrHeap *heap = new_HdrHeap(HdrHeap::DEFAULT_SIZE + 64); + resp_hdr.create(HTTPType::RESPONSE, HTTP_1_1, heap); + + const char *start = message.data(); + REQUIRE(resp_hdr.parse_resp(&parser, &start, message.data() + message.size(), true) == ParseResult::DONE); + + std::string serialized(static_cast(resp_hdr.length_get()), '\0'); + int index = 0; + int offset = 0; + resp_hdr.print(serialized.data(), static_cast(serialized.size()), &index, &offset); + serialized.resize(static_cast(index)); + + CHECK(serialized.find("Extra-CRs: bar\r\n") != std::string::npos); + CHECK(serialized.find("Extra-CRs: bar\r \r\n") == std::string::npos); + + resp_hdr.destroy(); +} + TEST_CASE("HTTP response parser tolerates high-bit bytes without UB", "[proxy][hdrtest]") { struct Test {