From 7739a9a747cd9ad78e9bd9e99c4adb311f9e4ec1 Mon Sep 17 00:00:00 2001 From: Rafi Indra Pramudhito Zuhayr Date: Tue, 15 Sep 2026 08:06:46 +0700 Subject: [PATCH 01/10] fix: track forbidden response body state --- src/http/response_parser.hpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/http/response_parser.hpp b/src/http/response_parser.hpp index e0a15b3..44385ca 100644 --- a/src/http/response_parser.hpp +++ b/src/http/response_parser.hpp @@ -50,6 +50,7 @@ class ResponseParser final { std::size_t content_length_remaining_{0}; bool connection_close_requested_{false}; bool close_delimited_{false}; + bool body_forbidden_{false}; }; } // namespace cpp_request::detail::http From 7f25134e8ef83b19496df40fc8826ea1d19b9600 Mon Sep 17 00:00:00 2001 From: Rafi Indra Pramudhito Zuhayr Date: Tue, 15 Sep 2026 08:07:23 +0700 Subject: [PATCH 02/10] fix: harden response framing precedence --- src/http/response_parser.cpp | 191 ++++++++++++++++++++++++----------- 1 file changed, 130 insertions(+), 61 deletions(-) diff --git a/src/http/response_parser.cpp b/src/http/response_parser.cpp index dd7a437..6075f47 100644 --- a/src/http/response_parser.cpp +++ b/src/http/response_parser.cpp @@ -18,6 +18,15 @@ struct ParsedHead { Headers headers; }; +struct FramingInfo { + std::size_t content_length_count{0}; + std::size_t content_length{0}; + std::size_t transfer_encoding_fields{0}; + std::size_t transfer_encoding_tokens{0}; + bool transfer_encoding_is_chunked{true}; + bool connection_close_requested{false}; +}; + [[nodiscard]] constexpr char ascii_lower(char ch) noexcept { return ch >= 'A' && ch <= 'Z' ? static_cast(ch + ('a' - 'A')) @@ -230,10 +239,66 @@ struct ParsedHead { return parsed.ec == std::errc{} && parsed.ptr == last; } -[[nodiscard]] bool is_no_body_status(int status_code) noexcept { - return status_code == 101 +[[nodiscard]] Result analyze_framing(const Headers& headers) { + FramingInfo framing; + + for (const auto& field : headers) { + if (ascii_iequals(field.name, "Connection") + && contains_token(field.value, "close")) { + framing.connection_close_requested = true; + } + + if (ascii_iequals(field.name, "Content-Length")) { + ++framing.content_length_count; + if (!parse_content_length(field.value, framing.content_length)) { + return Error{ErrorCode::InvalidContentLength}; + } + continue; + } + + if (!ascii_iequals(field.name, "Transfer-Encoding")) { + continue; + } + + ++framing.transfer_encoding_fields; + std::size_t token_begin = 0; + while (token_begin <= field.value.size()) { + const std::size_t comma = field.value.find(',', token_begin); + const std::size_t token_end = comma == std::string_view::npos + ? field.value.size() + : comma; + const std::string_view token = trim_ows(field.value.substr( + token_begin, + token_end - token_begin)); + if (token.empty()) { + return Error{ErrorCode::MalformedResponse}; + } + + ++framing.transfer_encoding_tokens; + if (!ascii_iequals(token, "chunked")) { + framing.transfer_encoding_is_chunked = false; + } + + if (comma == std::string_view::npos) { + break; + } + token_begin = comma + 1; + } + } + + if (framing.content_length_count > 1) { + return Error{ErrorCode::InvalidContentLength}; + } + + return framing; +} + +[[nodiscard]] bool is_header_terminated_response( + Method request_method, + int status_code) noexcept { + return request_method == Method::Head + || status_code == 101 || status_code == 204 - || status_code == 205 || status_code == 304; } @@ -241,6 +306,10 @@ struct ParsedHead { return status_code >= 100 && status_code < 200 && status_code != 101; } +[[nodiscard]] bool framing_header_forbidden_on_status(int status_code) noexcept { + return status_code == 101 || status_code == 204; +} + } // namespace ResponseParser::ResponseParser(Method request_method) noexcept @@ -270,88 +339,79 @@ Result ResponseParser::process_buffer() { buffer_.erase(0, head_end + 4); ParsedHead head = std::move(parsed).value(); + auto framing_result = analyze_framing(head.headers); + if (!framing_result) { + return framing_result.error(); + } + const FramingInfo framing = framing_result.value(); + if (is_interim_status(head.status_code)) { + if (framing.content_length_count != 0 + || framing.transfer_encoding_fields != 0) { + return Error{ErrorCode::MalformedResponse}; + } continue; } response_.status_code_ = head.status_code; response_.reason_ = std::move(head.reason); response_.headers_ = std::move(head.headers); + connection_close_requested_ = framing.connection_close_requested + || response_.status_code_ == 101; - connection_close_requested_ = response_.status_code_ == 101; - std::size_t content_length_count = 0; - std::size_t content_length = 0; - std::size_t transfer_encoding_fields = 0; - std::size_t transfer_encoding_tokens = 0; - bool transfer_encoding_is_chunked = true; - - for (const auto& field : response_.headers_) { - if (ascii_iequals(field.name, "Connection") - && contains_token(field.value, "close")) { - connection_close_requested_ = true; - } - - if (ascii_iequals(field.name, "Content-Length")) { - ++content_length_count; - if (!parse_content_length(field.value, content_length)) { - return Error{ErrorCode::InvalidContentLength}; - } - continue; - } - - if (ascii_iequals(field.name, "Transfer-Encoding")) { - ++transfer_encoding_fields; - std::size_t token_begin = 0; - while (token_begin <= field.value.size()) { - const std::size_t comma = field.value.find(',', token_begin); - const std::size_t token_end = comma == std::string_view::npos - ? field.value.size() - : comma; - const std::string_view token = trim_ows( - std::string_view{field.value.data(), field.value.size()}.substr( - token_begin, - token_end - token_begin)); - if (token.empty()) { - return Error{ErrorCode::MalformedResponse}; - } - ++transfer_encoding_tokens; - if (!ascii_iequals(token, "chunked")) { - transfer_encoding_is_chunked = false; - } - if (comma == std::string_view::npos) { - break; - } - token_begin = comma + 1; - } - } + if (framing.transfer_encoding_fields != 0 + && framing.content_length_count != 0) { + return Error{ErrorCode::ConflictingMessageFraming}; } - if (content_length_count > 1) { - return Error{ErrorCode::InvalidContentLength}; + if (framing_header_forbidden_on_status(response_.status_code_) + && (framing.content_length_count != 0 + || framing.transfer_encoding_fields != 0)) { + return Error{ErrorCode::MalformedResponse}; } - const bool no_body = request_method_ == Method::Head - || is_no_body_status(response_.status_code_); - if (no_body) { + if (is_header_terminated_response(request_method_, response_.status_code_)) { stage_ = Stage::Complete; return ResponseParseProgress::Complete; } - if (transfer_encoding_fields != 0 && content_length_count != 0) { - return Error{ErrorCode::ConflictingMessageFraming}; + if (response_.status_code_ == 205) { + body_forbidden_ = true; + + if (framing.transfer_encoding_fields != 0) { + if (framing.transfer_encoding_tokens != 1 + || !framing.transfer_encoding_is_chunked) { + return Error{ErrorCode::MalformedResponse}; + } + stage_ = Stage::ChunkedBody; + continue; + } + + if (framing.content_length_count == 1) { + if (framing.content_length != 0) { + return Error{ErrorCode::InvalidContentLength}; + } + stage_ = Stage::Complete; + return ResponseParseProgress::Complete; + } + + close_delimited_ = true; + stage_ = Stage::CloseDelimitedBody; + continue; } - if (transfer_encoding_fields != 0) { - if (transfer_encoding_tokens != 1 || !transfer_encoding_is_chunked) { + if (framing.transfer_encoding_fields != 0) { + if (framing.transfer_encoding_tokens != 1 + || !framing.transfer_encoding_is_chunked) { return Error{ErrorCode::MalformedResponse}; } stage_ = Stage::ChunkedBody; continue; } - if (content_length_count == 1) { - content_length_remaining_ = content_length; - response_.body_.reserve(content_length); + if (framing.content_length_count == 1) { + content_length_remaining_ = framing.content_length; + response_.body_.reserve(framing.content_length); if (content_length_remaining_ == 0) { stage_ = Stage::Complete; return ResponseParseProgress::Complete; @@ -383,6 +443,9 @@ Result ResponseParser::process_buffer() { } case Stage::CloseDelimitedBody: + if (body_forbidden_ && !buffer_.empty()) { + return Error{ErrorCode::MalformedResponse}; + } if (!buffer_.empty()) { response_.body_.append(buffer_); buffer_.clear(); @@ -394,6 +457,9 @@ Result ResponseParser::process_buffer() { if (!decoded) { return decoded.error(); } + if (body_forbidden_ && !response_.body_.empty()) { + return Error{ErrorCode::MalformedResponse}; + } if (decoded.value() == ChunkDecodeProgress::NeedMore) { return ResponseParseProgress::NeedMore; } @@ -410,6 +476,9 @@ Result ResponseParser::process_buffer() { Result ResponseParser::finish_eof() { switch (stage_) { case Stage::CloseDelimitedBody: + if (body_forbidden_ && !buffer_.empty()) { + return Error{ErrorCode::MalformedResponse}; + } if (!buffer_.empty()) { response_.body_.append(buffer_); buffer_.clear(); From 18a564c7d0db00a1df5f12a307c8a1273b4e90d9 Mon Sep 17 00:00:00 2001 From: Rafi Indra Pramudhito Zuhayr Date: Tue, 15 Sep 2026 08:07:47 +0700 Subject: [PATCH 03/10] fix: validate chunk extension grammar --- src/http/chunked_decoder.cpp | 123 +++++++++++++++++++++++++++++++++-- 1 file changed, 116 insertions(+), 7 deletions(-) diff --git a/src/http/chunked_decoder.cpp b/src/http/chunked_decoder.cpp index 84ec173..d3123ef 100644 --- a/src/http/chunked_decoder.cpp +++ b/src/http/chunked_decoder.cpp @@ -62,6 +62,10 @@ namespace { } } +[[nodiscard]] constexpr bool is_ows(unsigned char ch) noexcept { + return ch == ' ' || ch == '\t'; +} + [[nodiscard]] bool valid_field_name(std::string_view name) noexcept { if (name.empty()) { return false; @@ -87,27 +91,132 @@ namespace { return true; } -[[nodiscard]] bool valid_chunk_extension(std::string_view extension) noexcept { - for (const unsigned char ch : extension) { - if (ch == '\t') { +void skip_ows(std::string_view text, std::size_t& cursor) noexcept { + while (cursor < text.size() + && is_ows(static_cast(text[cursor]))) { + ++cursor; + } +} + +[[nodiscard]] bool parse_token( + std::string_view text, + std::size_t& cursor) noexcept { + const std::size_t begin = cursor; + while (cursor < text.size() + && is_tchar(static_cast(text[cursor]))) { + ++cursor; + } + return cursor != begin; +} + +[[nodiscard]] constexpr bool is_qdtext(unsigned char ch) noexcept { + return ch == '\t' + || ch == ' ' + || ch == 0x21 + || (ch >= 0x23 && ch <= 0x5b) + || (ch >= 0x5d && ch <= 0x7e) + || ch >= 0x80; +} + +[[nodiscard]] constexpr bool is_quoted_pair_char(unsigned char ch) noexcept { + return ch == '\t' + || ch == ' ' + || (ch >= 0x21 && ch <= 0x7e) + || ch >= 0x80; +} + +[[nodiscard]] bool parse_quoted_string( + std::string_view text, + std::size_t& cursor) noexcept { + if (cursor >= text.size() || text[cursor] != '"') { + return false; + } + ++cursor; + + while (cursor < text.size()) { + const unsigned char ch = static_cast(text[cursor]); + if (ch == '"') { + ++cursor; + return true; + } + + if (ch == '\\') { + ++cursor; + if (cursor >= text.size() + || !is_quoted_pair_char( + static_cast(text[cursor]))) { + return false; + } + ++cursor; continue; } - if (ch < 0x20 || ch == 0x7f) { + + if (!is_qdtext(ch)) { + return false; + } + ++cursor; + } + + return false; +} + +[[nodiscard]] bool valid_chunk_extensions(std::string_view extension) noexcept { + if (extension.empty()) { + return false; + } + + std::size_t cursor = 0; + while (true) { + skip_ows(extension, cursor); + if (!parse_token(extension, cursor)) { + return false; + } + + skip_ows(extension, cursor); + if (cursor < extension.size() && extension[cursor] == '=') { + ++cursor; + skip_ows(extension, cursor); + if (cursor >= extension.size()) { + return false; + } + + if (extension[cursor] == '"') { + if (!parse_quoted_string(extension, cursor)) { + return false; + } + } else if (!parse_token(extension, cursor)) { + return false; + } + skip_ows(extension, cursor); + } + + if (cursor == extension.size()) { + return true; + } + if (extension[cursor] != ';') { + return false; + } + ++cursor; + if (cursor == extension.size()) { return false; } } - return true; } [[nodiscard]] Result parse_chunk_size(std::string_view line) noexcept { const std::size_t semicolon = line.find(';'); - const std::string_view size_text = line.substr(0, semicolon); + std::string_view size_text = line.substr(0, semicolon); + while (!size_text.empty() + && is_ows(static_cast(size_text.back()))) { + size_text.remove_suffix(1); + } + if (size_text.empty()) { return Error{ErrorCode::InvalidChunkSize}; } if (semicolon != std::string_view::npos - && !valid_chunk_extension(line.substr(semicolon + 1))) { + && !valid_chunk_extensions(line.substr(semicolon + 1))) { return Error{ErrorCode::InvalidChunkFraming}; } From c55829987ece0018ab9b3dccaa519cf13434cc13 Mon Sep 17 00:00:00 2001 From: Rafi Indra Pramudhito Zuhayr Date: Tue, 15 Sep 2026 08:08:20 +0700 Subject: [PATCH 04/10] test: cover hardened response framing semantics --- tests/response_parser_test.cpp | 164 ++++++++++++++++++++++++++++++++- 1 file changed, 163 insertions(+), 1 deletion(-) diff --git a/tests/response_parser_test.cpp b/tests/response_parser_test.cpp index 342400c..9fb5c8b 100644 --- a/tests/response_parser_test.cpp +++ b/tests/response_parser_test.cpp @@ -76,6 +76,35 @@ TEST(ResponseParserTest, HeadCompletesAfterHeadersAndLeavesFollowingBytesPending EXPECT_TRUE(parser.connection_reusable()); } +TEST(ResponseParserTest, HeadRejectsTransferEncodingAndContentLengthTogether) { + ResponseParser parser{Method::Head}; + + auto result = parser.feed( + "HTTP/1.1 200 OK\r\n" + "Transfer-Encoding: chunked\r\n" + "Content-Length: 42\r\n" + "\r\n"); + + ASSERT_FALSE(result); + EXPECT_EQ(result.error().code, ErrorCode::ConflictingMessageFraming); +} + +TEST(ResponseParserTest, NotModifiedAllowsRepresentationContentLength) { + ResponseParser parser{Method::Get}; + + auto result = parser.feed( + "HTTP/1.1 304 Not Modified\r\n" + "Content-Length: 1234\r\n" + "\r\n" + "next-response-bytes"); + + ASSERT_TRUE(result); + EXPECT_EQ(result.value(), ResponseParseProgress::Complete); + EXPECT_TRUE(parser.response().body().empty()); + EXPECT_EQ(parser.pending_bytes(), "next-response-bytes"); + EXPECT_TRUE(parser.connection_reusable()); +} + TEST(ResponseParserTest, CloseDelimitedBodyCompletesOnlyAtEof) { ResponseParser parser{Method::Get}; @@ -246,7 +275,32 @@ TEST(ResponseParserTest, ConsumesInterimResponseBeforeFinalResponse) { EXPECT_EQ(parser.response().body(), "ok"); } -TEST(ResponseParserTest, NoBodyStatusCompletesAtHeaders) { +TEST(ResponseParserTest, RejectsFramingHeadersOnInterimResponse) { + ResponseParser parser{Method::Post}; + + auto result = parser.feed( + "HTTP/1.1 100 Continue\r\n" + "Content-Length: 0\r\n" + "\r\n"); + + ASSERT_FALSE(result); + EXPECT_EQ(result.error().code, ErrorCode::MalformedResponse); +} + +TEST(ResponseParserTest, NoContentCompletesAtHeaders) { + ResponseParser parser{Method::Get}; + + auto result = parser.feed( + "HTTP/1.1 204 No Content\r\n" + "\r\n"); + + ASSERT_TRUE(result); + EXPECT_EQ(result.value(), ResponseParseProgress::Complete); + EXPECT_TRUE(parser.response().body().empty()); + EXPECT_TRUE(parser.connection_reusable()); +} + +TEST(ResponseParserTest, RejectsContentLengthOnNoContent) { ResponseParser parser{Method::Get}; auto result = parser.feed( @@ -254,12 +308,120 @@ TEST(ResponseParserTest, NoBodyStatusCompletesAtHeaders) { "Content-Length: 0\r\n" "\r\n"); + ASSERT_FALSE(result); + EXPECT_EQ(result.error().code, ErrorCode::MalformedResponse); +} + +TEST(ResponseParserTest, RejectsTransferEncodingOnNoContent) { + ResponseParser parser{Method::Get}; + + auto result = parser.feed( + "HTTP/1.1 204 No Content\r\n" + "Transfer-Encoding: chunked\r\n" + "\r\n" + "0\r\n\r\n"); + + ASSERT_FALSE(result); + EXPECT_EQ(result.error().code, ErrorCode::MalformedResponse); +} + +TEST(ResponseParserTest, SwitchingProtocolsRejectsFramingHeaders) { + ResponseParser parser{Method::Get}; + + auto result = parser.feed( + "HTTP/1.1 101 Switching Protocols\r\n" + "Content-Length: 0\r\n" + "\r\n"); + + ASSERT_FALSE(result); + EXPECT_EQ(result.error().code, ErrorCode::MalformedResponse); +} + +TEST(ResponseParserTest, ResetContentWithZeroContentLengthIsReusable) { + ResponseParser parser{Method::Post}; + + auto result = parser.feed( + "HTTP/1.1 205 Reset Content\r\n" + "Content-Length: 0\r\n" + "\r\n"); + + ASSERT_TRUE(result); + EXPECT_EQ(result.value(), ResponseParseProgress::Complete); + EXPECT_TRUE(parser.response().body().empty()); + EXPECT_TRUE(parser.connection_reusable()); +} + +TEST(ResponseParserTest, ResetContentWithoutFramingCompletesAtEofAndIsNotReusable) { + ResponseParser parser{Method::Post}; + + auto result = parser.feed( + "HTTP/1.1 205 Reset Content\r\n" + "\r\n"); + + ASSERT_TRUE(result); + EXPECT_EQ(result.value(), ResponseParseProgress::NeedMore); + EXPECT_FALSE(parser.complete()); + + auto eof = parser.finish_eof(); + ASSERT_TRUE(eof); + EXPECT_EQ(eof.value(), ResponseParseProgress::Complete); + EXPECT_TRUE(parser.response().body().empty()); + EXPECT_FALSE(parser.connection_reusable()); +} + +TEST(ResponseParserTest, ResetContentRejectsNonZeroContentLength) { + ResponseParser parser{Method::Post}; + + auto result = parser.feed( + "HTTP/1.1 205 Reset Content\r\n" + "Content-Length: 1\r\n" + "\r\n"); + + ASSERT_FALSE(result); + EXPECT_EQ(result.error().code, ErrorCode::InvalidContentLength); +} + +TEST(ResponseParserTest, ResetContentAllowsZeroChunkedFraming) { + ResponseParser parser{Method::Post}; + + auto result = parser.feed( + "HTTP/1.1 205 Reset Content\r\n" + "Transfer-Encoding: chunked\r\n" + "\r\n" + "0\r\n\r\n"); + ASSERT_TRUE(result); EXPECT_EQ(result.value(), ResponseParseProgress::Complete); EXPECT_TRUE(parser.response().body().empty()); EXPECT_TRUE(parser.connection_reusable()); } +TEST(ResponseParserTest, ResetContentRejectsChunkedContent) { + ResponseParser parser{Method::Post}; + + auto result = parser.feed( + "HTTP/1.1 205 Reset Content\r\n" + "Transfer-Encoding: chunked\r\n" + "\r\n" + "1\r\nx\r\n" + "0\r\n\r\n"); + + ASSERT_FALSE(result); + EXPECT_EQ(result.error().code, ErrorCode::MalformedResponse); +} + +TEST(ResponseParserTest, ResetContentRejectsCloseDelimitedContent) { + ResponseParser parser{Method::Post}; + + auto result = parser.feed( + "HTTP/1.1 205 Reset Content\r\n" + "\r\n" + "unexpected"); + + ASSERT_FALSE(result); + EXPECT_EQ(result.error().code, ErrorCode::MalformedResponse); +} + TEST(ResponseParserTest, PrematureEofForContentLengthIsError) { ResponseParser parser{Method::Get}; From ba2d1b71f1ddea3060775a1a2c4afb84fed1291d Mon Sep 17 00:00:00 2001 From: Rafi Indra Pramudhito Zuhayr Date: Tue, 15 Sep 2026 08:08:41 +0700 Subject: [PATCH 05/10] test: cover strict chunk extension grammar --- tests/chunked_decoder_test.cpp | 50 +++++++++++++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/tests/chunked_decoder_test.cpp b/tests/chunked_decoder_test.cpp index 7b9c90d..048c60c 100644 --- a/tests/chunked_decoder_test.cpp +++ b/tests/chunked_decoder_test.cpp @@ -51,7 +51,7 @@ TEST(ChunkedDecoderTest, HandlesOneByteAtATime) { TEST(ChunkedDecoderTest, ConsumesExtensionsAndTrailersAndPreservesFollowingBytes) { ChunkedDecoder decoder; std::string input = - "4;foo=bar\r\nWiki\r\n" + "4;foo=bar; note=\"hello world\"\r\nWiki\r\n" "0;end=yes\r\n" "X-Checksum: abc123\r\n" "X-Trace: done\r\n" @@ -67,6 +67,21 @@ TEST(ChunkedDecoderTest, ConsumesExtensionsAndTrailersAndPreservesFollowingBytes EXPECT_EQ(input, "NEXT"); } +TEST(ChunkedDecoderTest, AcceptsBadWhitespaceBeforeExtensionDelimiter) { + ChunkedDecoder decoder; + std::string input = + "1 \t; foo=bar\r\n" + "x\r\n" + "0\r\n\r\n"; + std::string output; + + auto result = decoder.process(input, output); + + ASSERT_TRUE(result); + EXPECT_EQ(result.value(), ChunkDecodeProgress::Complete); + EXPECT_EQ(output, "x"); +} + TEST(ChunkedDecoderTest, RejectsInvalidChunkSize) { ChunkedDecoder decoder; std::string input = "xyz\r\n"; @@ -78,6 +93,39 @@ TEST(ChunkedDecoderTest, RejectsInvalidChunkSize) { EXPECT_EQ(result.error().code, ErrorCode::InvalidChunkSize); } +TEST(ChunkedDecoderTest, RejectsEmptyChunkExtensionName) { + ChunkedDecoder decoder; + std::string input = "1;\r\nx\r\n0\r\n\r\n"; + std::string output; + + auto result = decoder.process(input, output); + + ASSERT_FALSE(result); + EXPECT_EQ(result.error().code, ErrorCode::InvalidChunkFraming); +} + +TEST(ChunkedDecoderTest, RejectsMissingChunkExtensionValue) { + ChunkedDecoder decoder; + std::string input = "1;foo=\r\nx\r\n0\r\n\r\n"; + std::string output; + + auto result = decoder.process(input, output); + + ASSERT_FALSE(result); + EXPECT_EQ(result.error().code, ErrorCode::InvalidChunkFraming); +} + +TEST(ChunkedDecoderTest, RejectsUnterminatedQuotedChunkExtension) { + ChunkedDecoder decoder; + std::string input = "1;foo=\"bar\r\nx\r\n0\r\n\r\n"; + std::string output; + + auto result = decoder.process(input, output); + + ASSERT_FALSE(result); + EXPECT_EQ(result.error().code, ErrorCode::InvalidChunkFraming); +} + TEST(ChunkedDecoderTest, RejectsMissingChunkDataCrlf) { ChunkedDecoder decoder; std::string input = "1\r\naXY"; From a4351ca519b671aa94081c298066708cb16167c6 Mon Sep 17 00:00:00 2001 From: Rafi Indra Pramudhito Zuhayr Date: Tue, 15 Sep 2026 08:09:43 +0700 Subject: [PATCH 06/10] fix: emit zero Content-Length for content methods --- src/http/request_serializer.cpp | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/http/request_serializer.cpp b/src/http/request_serializer.cpp index 28c9734..2b66116 100644 --- a/src/http/request_serializer.cpp +++ b/src/http/request_serializer.cpp @@ -22,6 +22,12 @@ namespace { return {}; } +[[nodiscard]] constexpr bool method_has_content_semantics(Method method) noexcept { + return method == Method::Post + || method == Method::Put + || method == Method::Patch; +} + [[nodiscard]] constexpr char ascii_lower(char ch) noexcept { return ch >= 'A' && ch <= 'Z' ? static_cast(ch + ('a' - 'A')) @@ -299,7 +305,8 @@ Result serialize_request(const Request& request) { serialized.head.append("\r\n"); } - if (!request.body().empty() && content_length_count == 0) { + if (content_length_count == 0 + && (!request.body().empty() || method_has_content_semantics(request.method()))) { serialized.head.append("Content-Length: "); append_decimal(serialized.head, request.body().size()); serialized.head.append("\r\n"); From 128d8effdcc207a2709be8049e39a1963dcaa8e3 Mon Sep 17 00:00:00 2001 From: Rafi Indra Pramudhito Zuhayr Date: Tue, 15 Sep 2026 08:10:06 +0700 Subject: [PATCH 07/10] test: cover zero Content-Length policy --- tests/request_serializer_test.cpp | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/request_serializer_test.cpp b/tests/request_serializer_test.cpp index 1a77658..ece3dce 100644 --- a/tests/request_serializer_test.cpp +++ b/tests/request_serializer_test.cpp @@ -106,6 +106,30 @@ TEST(RequestSerializerTest, AddsContentLengthForNonEmptyBodyWithoutCopyingBody) EXPECT_EQ(result.value().body.data(), body.data()); } +TEST(RequestSerializerTest, AddsZeroContentLengthForEmptyContentMethods) { + const Method methods[] = {Method::Post, Method::Put, Method::Patch}; + + for (const Method method : methods) { + Request request{method, "http://example.com/empty"}; + auto result = serialize_request(request); + + ASSERT_TRUE(result); + EXPECT_NE(result.value().head.find("Content-Length: 0\r\n"), std::string::npos); + } +} + +TEST(RequestSerializerTest, DoesNotAddZeroContentLengthForEmptyGetHeadOrDelete) { + const Method methods[] = {Method::Get, Method::Head, Method::Delete}; + + for (const Method method : methods) { + Request request{method, "http://example.com/empty"}; + auto result = serialize_request(request); + + ASSERT_TRUE(result); + EXPECT_EQ(result.value().head.find("Content-Length:"), std::string::npos); + } +} + TEST(RequestSerializerTest, AcceptsMatchingCallerContentLength) { Request request{Method::Post, "http://example.com/upload"}; request.set_body("abc"); From 1c9d310b98b556eefa46b37d55cd7b2eac1683bd Mon Sep 17 00:00:00 2001 From: Rafi Indra Pramudhito Zuhayr Date: Tue, 15 Sep 2026 08:10:58 +0700 Subject: [PATCH 08/10] docs: record hardened HTTP framing semantics --- docs/architecture/http.md | 104 +++++++++++++++++++++++++++----------- 1 file changed, 75 insertions(+), 29 deletions(-) diff --git a/docs/architecture/http.md b/docs/architecture/http.md index fa51531..da51645 100644 --- a/docs/architecture/http.md +++ b/docs/architecture/http.md @@ -28,7 +28,17 @@ The serializer is responsible for producing valid HTTP/1.1 wire format, includin - terminating CRLF sequence - request body bytes when present -Automatically generated headers such as `Host` and `Content-Length` may be added when required and not supplied explicitly, subject to the later public API contract. +Automatically generated headers such as `Host` and `Content-Length` are added when required and not supplied explicitly. + +For v1 request framing: + +- caller-supplied `Transfer-Encoding` is rejected, +- duplicate `Host` is rejected, +- duplicate `Content-Length` is rejected, +- caller-supplied `Content-Length` must exactly match the in-memory body size, +- non-empty bodies receive an automatic `Content-Length` when absent, +- empty `POST`, `PUT`, and `PATCH` requests receive `Content-Length: 0`, +- empty `GET`, `HEAD`, and `DELETE` requests do not receive an automatic zero length. ## Response Processing @@ -48,39 +58,72 @@ Malformed protocol data transitions to an error result instead of continuing wit ## Body Framing -The parser must support v1 framing rules for: +The parser supports v1 framing rules for: -1. responses with no message body by HTTP semantics, including `HEAD`, +1. responses terminated by the header section because of request/status semantics, 2. `Transfer-Encoding: chunked`, 3. `Content-Length`, -4. connection-close-delimited body where valid/required by HTTP/1.1 behavior. +4. connection-close-delimited bodies where required by HTTP/1.1 framing. + +Framing precedence follows HTTP/1.1 semantics and unsafe ambiguity is rejected rather than guessed. + +The v1 hardening policy is: -Framing precedence must follow HTTP semantics; conflicting or invalid framing must not be guessed silently. +- responses to `HEAD` complete after the header section, +- `101`, `204`, and `304` are header-terminated responses, +- `1xx` responses other than `101` are interim and are consumed before the final response, +- framing fields on `1xx`/`204` responses are rejected, +- simultaneous `Transfer-Encoding` and `Content-Length` is rejected as conflicting framing, +- duplicate `Content-Length` fields are rejected rather than normalized, +- `304` and `HEAD` may carry representation-length/framing metadata while still having no response body, +- `205 Reset Content` is content-forbidden but not blindly treated as header-terminated: `Content-Length: 0` and zero-content chunked framing can complete on a persistent connection, while an unframed 205 is close-delimited and cannot be reused, +- any actual decoded/close-delimited content on a 205 is a malformed response. + +This conservative policy prioritizes unambiguous message boundaries and connection-reuse safety. ## Chunked Decoder The chunked decoder owns protocol state for: -- reading hexadecimal chunk size -- consuming chunk extensions without exposing them as body content when present -- consuming each chunk payload -- validating CRLF framing -- recognizing terminal zero-size chunk -- consuming trailer section as required for correct message completion +- reading hexadecimal chunk size, +- parsing chunk extensions, +- consuming each chunk payload, +- validating CRLF framing, +- recognizing the terminal zero-size chunk, +- consuming and validating the trailer section, +- preserving bytes that belong to a following response. + +Chunk extensions follow token / quoted-string syntax rather than accepting arbitrary printable bytes. Empty extension names, missing extension values, malformed quoted strings, invalid chunk sizes, and invalid data delimiters are rejected. + +Trailer fields are syntax-validated and consumed but are not merged into the public response-header collection in v1. Framing-critical trailer fields such as `Content-Length` and `Transfer-Encoding` are rejected. Decoded body bytes are accumulated into the v1 in-memory response body. ## Header Handling -Headers should be parsed without requiring per-character heap allocation. +Headers are parsed without requiring per-character heap allocation. -The architecture should permit: +The architecture permits: -- case-insensitive header-name lookup -- duplicate header fields where HTTP permits them -- preserving sufficient response data for user-facing access +- case-insensitive header-name lookup, +- duplicate header fields where HTTP permits them, +- preserving sufficient response data for user-facing access. -Exact container representation is deferred to the public API and performance design phase. +Obsolete folded header lines and invalid field-name/value bytes are rejected. + +## Timeout and EOF Semantics + +Timeouts remain stage-specific: + +- connect timeout bounds connection establishment across candidate endpoints, +- write timeout bounds the complete logical request transmission; segmented head/body writes share one deadline, +- read timeout bounds each wait for additional response data, acting as an inactivity/read-progress timeout rather than one deadline for the entire response. + +EOF is interpreted only with HTTP framing context: + +- incomplete `Content-Length` or chunked response at EOF is `UnexpectedEof`, +- a valid close-delimited response completes at EOF, +- a read timeout is not treated as EOF and therefore does not complete a close-delimited message. ## Connection Semantics @@ -88,26 +131,29 @@ After a response is complete, the HTTP layer reports whether protocol semantics Reasons to reject reuse include: -- `Connection: close` -- incomplete or malformed response framing -- transport termination before required body completion -- any parser state that leaves unread bytes ambiguous +- `Connection: close`, +- close-delimited framing, +- protocol upgrade, +- incomplete or malformed response framing, +- transport termination before required body completion, +- any parser state that leaves unread bytes ambiguous. + +The `Client` additionally refuses reuse when parser pending bytes remain after a completed non-pipelined request. ## Redirect Semantics Redirect handling is orchestrated above the low-level parser but uses parsed status and `Location` metadata from this layer. -The v1 redirect policy must: - -- respect the configured enable/disable option -- enforce a finite redirect count -- resolve relative locations -- reject redirects requiring unsupported HTTPS transport +The v1 redirect policy: -Method rewriting/preservation rules for redirect status codes must be specified before implementation of redirect execution. +- respects the configured enable/disable option, +- enforces a finite redirect count, +- resolves relative locations, +- rejects redirects requiring unsupported HTTPS transport, +- applies the documented 301/302/303/307/308 method/body policy. ## Parser Design Principle -The parser should support incremental input from `read_some()` rather than assuming one receive call contains a complete status line, header block, chunk, or body. +The parser supports incremental input from `read_some()` rather than assuming one receive call contains a complete status line, header block, chunk, or body. This is required for correctness over TCP and also enables focused parser benchmarks independent of live network latency. From dad340745544faa2b47aa091c531681185940bf4 Mon Sep 17 00:00:00 2001 From: Rafi Indra Pramudhito Zuhayr Date: Tue, 15 Sep 2026 08:11:34 +0700 Subject: [PATCH 09/10] docs: sync v0.8 HTTP correctness progress --- docs/roadmap.md | 81 ++++++++++++++++++++++++++++++------------------- 1 file changed, 50 insertions(+), 31 deletions(-) diff --git a/docs/roadmap.md b/docs/roadmap.md index f87b9ac..5b267fb 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -252,49 +252,68 @@ Exit criteria: **complete**. Goal: close known correctness gaps before treating the API as release-candidate quality. -## 1. `Result` hardening — 🚧 current work +## 1. `Result` hardening — ✅ complete -Required work: +Completed work: -- eliminate wrong-state type-based `std::get` behavior hidden behind `noexcept`. -- use explicit discriminant/index-based state access. -- keep accessor misuse as a documented programmer precondition violation rather than a network error. -- make `Result` capable of representing both success and failure unambiguously. -- preserve move-only payload support. -- keep `std::variant` as the baseline representation until measurement justifies custom storage. -- keep `Result` deferred until a concrete operation requires it. +- replaced type-based state lookup with explicit variant-index/discriminant access. +- removed hidden wrong-state `std::bad_variant_access` paths from `noexcept` accessors. +- retained accessor misuse as a documented programmer precondition violation. +- added explicit success/failure factories. +- made `Result` capable of representing success and failure unambiguously. +- preserved move-only payload support. +- retained `std::variant` as the baseline representation until measurement justifies custom storage. +- kept `Result` deferred until a concrete operation requires it. -Exit for this substep: +Historical reference: -- dedicated `Result` unit coverage passes on all CI matrices. -- `Result` success and failure are distinguishable. -- normal existing implicit error propagation remains source-compatible. +- PR #22 — `Result` state hardening -## 2. URL and query correctness — ⏳ next +Exit criteria: **complete**. -REQ-HTTP-004 requires query behavior beyond merely preserving an already encoded raw query. +## 2. URL and query correctness — ✅ complete -Planned work: +Completed work: + +- added owned `Request::add_query_param(name, value)` parameters. +- percent-encoded appended query parameters. +- preserved repeated query keys and insertion order. +- preserved an existing raw URL query before appended parameters. +- validated raw `%HH` escapes. +- replaced locale-sensitive scheme handling with ASCII-only normalization. +- strengthened bracketed IPv6 literals using numeric IPv6 validation. +- explicitly rejected unsupported v1 authority encodings / zone identifiers. +- kept redirect base URL resolution consistent with the effective serialized request URL. -- define the v1 query-parameter API or explicitly refine the requirement if raw URL input is the intended contract. -- percent-encode reserved characters where required. -- validate percent escapes. -- strengthen bracketed IPv6 literal validation. -- add query/encoding tests. +Historical reference: + +- PR #23 — URL parsing and query parameter hardening + +Exit criteria: **complete**. -## 3. HTTP correctness review — ⏳ +## 3. HTTP correctness review — 🚧 current work -Planned review: +Current hardening scope: + +- enforce safe response framing precedence. +- reject `Transfer-Encoding` + `Content-Length` ambiguity before connection reuse. +- reject framing fields where `1xx` / `204` semantics forbid them. +- keep `HEAD` / `304` header-terminated while preserving allowed representation metadata. +- correct `205 Reset Content` framing so unframed responses are close-delimited rather than incorrectly reusable. +- reject actual content in a 205 response. +- validate chunk extension token / quoted-string grammar instead of accepting arbitrary printable bytes. +- retain trailer validation and reject framing-critical trailer fields. +- emit `Content-Length: 0` for empty POST/PUT/PATCH requests while leaving empty GET/HEAD/DELETE unchanged. +- document timeout boundaries: connect budget across endpoint attempts, one write budget across request transmission, read timeout per wait for response progress. +- preserve context-sensitive EOF semantics: incomplete explicit framing is `UnexpectedEof`; valid close-delimited EOF completes normally; timeout never masquerades as EOF. + +Exit for this substep: -- response framing precedence and duplicate framing fields. -- status/no-body edge cases. -- header syntax edge cases. -- chunk extension/trailer correctness. -- request serializer Host/Content-Length policy. -- timeout boundary semantics. -- EOF classification. +- framing/status/chunk edge cases have deterministic parser tests. +- serializer framing policy is covered by tests. +- no reviewed HTTP message-boundary ambiguity remains known. -## 4. Resource-bound review — ⏳ +## 4. Resource-bound review — ⏳ next Because response bodies are memory-resident in v1, document or introduce practical defensive limits where appropriate: From 4396486776c19aa87bbdbea9f16a90e1333eb2c2 Mon Sep 17 00:00:00 2001 From: Rafi Indra Pramudhito Zuhayr Date: Tue, 15 Sep 2026 08:20:58 +0700 Subject: [PATCH 10/10] fix: avoid dangling transfer-encoding token view --- src/http/response_parser.cpp | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/http/response_parser.cpp b/src/http/response_parser.cpp index 6075f47..86edb5c 100644 --- a/src/http/response_parser.cpp +++ b/src/http/response_parser.cpp @@ -261,13 +261,14 @@ struct FramingInfo { } ++framing.transfer_encoding_fields; + const std::string_view field_value{field.value}; std::size_t token_begin = 0; - while (token_begin <= field.value.size()) { - const std::size_t comma = field.value.find(',', token_begin); + while (token_begin <= field_value.size()) { + const std::size_t comma = field_value.find(',', token_begin); const std::size_t token_end = comma == std::string_view::npos - ? field.value.size() + ? field_value.size() : comma; - const std::string_view token = trim_ows(field.value.substr( + const std::string_view token = trim_ows(field_value.substr( token_begin, token_end - token_begin)); if (token.empty()) {