From e96a52e9dd19b3e0724eb6153e05aecaa864307c Mon Sep 17 00:00:00 2001 From: yhirose Date: Wed, 26 Aug 2026 22:37:22 -0400 Subject: [PATCH] Ignore empty list elements in the Accept header (Fix #2567) parse_accept_header() rejected any Accept value with a leading, trailing or doubled comma, and Server::process_request() validates Accept before routing, so "Accept: text/html," was answered 400 Bad Request on every route. RFC 9110 Section 5.6.1.2 requires a recipient to parse and ignore empty list elements in a #rule list, so those values are legal. split() already trims each element and skips the empty ones, which made the guard inside the callback unreachable as well; drop both and let the empty elements fall away. The header length limit bounds how many a sender can send, so ignoring all of them cannot be used as a denial-of-service vector. get_combined_header_value() keeps skipping empty field lines, but that skip is no longer observable through a request now that a stray comma parses cleanly, so it gets its own test. --- httplib.h | 21 ++++++------------ test/test.cc | 60 +++++++++++++++++++++++++++++++++++++++++++--------- 2 files changed, 57 insertions(+), 24 deletions(-) diff --git a/httplib.h b/httplib.h index e22f2f15..89d15af4 100644 --- a/httplib.h +++ b/httplib.h @@ -7858,8 +7858,7 @@ inline std::string get_combined_header_value(const Headers &headers, for (auto it = rng.first; it != rng.second; ++it) { // RFC 9110 Section 5.6.1.2: a recipient has to parse and ignore empty list // elements, so an empty field line must not contribute a bare comma to the - // combined value. parse_accept_header() rejects a leading comma outright, - // which would turn a legal request into 400 Bad Request. + // combined value. if (it->second.empty()) { continue; } if (!combined.empty()) { combined += ", "; } combined += it->second; @@ -8836,12 +8835,6 @@ inline bool parse_accept_header(const std::string &s, // Empty string is considered valid (no preference) if (s.empty()) { return true; } - // Check for invalid patterns: leading/trailing commas or consecutive commas - if (s.front() == ',' || s.back() == ',' || - s.find(",,") != std::string::npos) { - return false; - } - struct AcceptEntry { std::string media_type; double quality; @@ -8852,16 +8845,16 @@ inline bool parse_accept_header(const std::string &s, int order = 0; bool has_invalid_entry = false; - // Split by comma and parse each entry + // Split by comma and parse each entry. RFC 9110 Section 5.6.1.2: a recipient + // has to parse and ignore empty list elements, so a leading, trailing or + // doubled comma must not turn a legal Accept value into 400 Bad Request. + // split() skips them, and the header length limit bounds how many a sender + // can send, so ignoring all of them cannot be used as a denial-of-service + // vector. split(s.data(), s.data() + s.size(), ',', [&](const char *b, const char *e) { std::string entry(b, e); entry = trim_copy(entry); - if (entry.empty()) { - has_invalid_entry = true; - return; - } - AcceptEntry accept_entry; accept_entry.order = order++; diff --git a/test/test.cc b/test/test.cc index ead102ac..23da37bc 100644 --- a/test/test.cc +++ b/test/test.cc @@ -1067,14 +1067,6 @@ TEST(ParseAcceptHeaderTest, InvalidCases) { EXPECT_FALSE( detail::parse_accept_header("invalidtype,application/json", result)); - // Empty media type - result.clear(); - EXPECT_FALSE(detail::parse_accept_header(",application/json", result)); - - // Only commas - result.clear(); - EXPECT_FALSE(detail::parse_accept_header(",,,", result)); - // Valid cases should still work EXPECT_TRUE(detail::parse_accept_header("*/*", result)); EXPECT_EQ(result.size(), 1U); @@ -1089,6 +1081,38 @@ TEST(ParseAcceptHeaderTest, InvalidCases) { EXPECT_EQ(result[0], "text/*"); } +// RFC 9110 Section 5.6.1.2: a recipient has to parse and ignore empty list +// elements, so a leading, trailing or doubled comma is a legal Accept value +// and must not be answered with 400 Bad Request. +TEST(ParseAcceptHeaderTest, EmptyListElementsAreIgnored) { + struct { + const char *value; + std::vector expected; + } cases[] = { + {",application/json", {"application/json"}}, + {"text/html,", {"text/html"}}, + {"text/html,,*/*", {"text/html", "*/*"}}, + // An empty element may be spelled with whitespace in it + {"text/html, , application/json", {"text/html", "application/json"}}, + // Nothing but empty elements is an empty list, which means the same + // thing as no Accept header at all + {",,,", {}}, + // Quality values still apply across ignored empty elements + {",text/html;q=0.5,,application/json", {"application/json", "text/html"}}, + }; + + for (const auto &c : cases) { + std::vector result; + EXPECT_TRUE(detail::parse_accept_header(c.value, result)) + << "value: " << c.value; + EXPECT_EQ(c.expected, result) << "value: " << c.value; + } + + // An invalid entry is still rejected when empty elements surround it + std::vector result; + EXPECT_FALSE(detail::parse_accept_header(",invalidtype,", result)); +} + TEST(ParseAcceptHeaderTest, ContentTypesPopulatedAndInvalidHeaderHandling) { Server svr; @@ -18519,8 +18543,6 @@ TEST(RepeatedFieldLinesTest, AcceptCombinesEveryFieldLine) { // RFC 9110 Section 5.6.1.2: empty list elements are parsed and ignored, so an // empty field line must not contribute a bare comma to the combined value. -// parse_accept_header() rejects a leading comma outright, so a stray one would -// turn a legal request into 400 Bad Request. TEST(RepeatedFieldLinesTest, EmptyFieldLineDoesNotInjectComma) { Server svr; @@ -18557,6 +18579,24 @@ TEST(RepeatedFieldLinesTest, EmptyFieldLineDoesNotInjectComma) { EXPECT_EQ("application/json", observed_types[0]); } +// The skip in get_combined_header_value() is what keeps an empty field line +// from contributing a bare comma. Only a trailing empty field line exercises +// it: a leading one is already covered by the "combined is still empty" check, +// and parse_accept_header() now ignores the empty element either way, so the +// request-level test above can no longer tell the two apart. +TEST(RepeatedFieldLinesTest, EmptyFieldLineIsNotCombined) { + Headers headers; + headers.emplace("Accept", "text/html"); + headers.emplace("Accept", ""); + EXPECT_EQ("text/html", detail::get_combined_header_value(headers, "Accept")); + + headers.clear(); + headers.emplace("Accept", ""); + headers.emplace("Accept", "application/json"); + EXPECT_EQ("application/json", + detail::get_combined_header_value(headers, "Accept")); +} + #ifdef CPPHTTPLIB_ZLIB_SUPPORT // An encoding offered on a later Accept-Encoding field line is still offered. TEST(RepeatedFieldLinesTest, AcceptEncodingCombinesEveryFieldLine) {