mirror of
https://github.com/yhirose/cpp-httplib.git
synced 2026-09-30 20:52:31 +07:00
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.
This commit is contained in:
@@ -7858,8 +7858,7 @@ inline std::string get_combined_header_value(const Headers &headers,
|
|||||||
for (auto it = rng.first; it != rng.second; ++it) {
|
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
|
// 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
|
// elements, so an empty field line must not contribute a bare comma to the
|
||||||
// combined value. parse_accept_header() rejects a leading comma outright,
|
// combined value.
|
||||||
// which would turn a legal request into 400 Bad Request.
|
|
||||||
if (it->second.empty()) { continue; }
|
if (it->second.empty()) { continue; }
|
||||||
if (!combined.empty()) { combined += ", "; }
|
if (!combined.empty()) { combined += ", "; }
|
||||||
combined += it->second;
|
combined += it->second;
|
||||||
@@ -8836,12 +8835,6 @@ inline bool parse_accept_header(const std::string &s,
|
|||||||
// Empty string is considered valid (no preference)
|
// Empty string is considered valid (no preference)
|
||||||
if (s.empty()) { return true; }
|
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 {
|
struct AcceptEntry {
|
||||||
std::string media_type;
|
std::string media_type;
|
||||||
double quality;
|
double quality;
|
||||||
@@ -8852,16 +8845,16 @@ inline bool parse_accept_header(const std::string &s,
|
|||||||
int order = 0;
|
int order = 0;
|
||||||
bool has_invalid_entry = false;
|
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) {
|
split(s.data(), s.data() + s.size(), ',', [&](const char *b, const char *e) {
|
||||||
std::string entry(b, e);
|
std::string entry(b, e);
|
||||||
entry = trim_copy(entry);
|
entry = trim_copy(entry);
|
||||||
|
|
||||||
if (entry.empty()) {
|
|
||||||
has_invalid_entry = true;
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
|
|
||||||
AcceptEntry accept_entry;
|
AcceptEntry accept_entry;
|
||||||
accept_entry.order = order++;
|
accept_entry.order = order++;
|
||||||
|
|
||||||
|
|||||||
+50
-10
@@ -1067,14 +1067,6 @@ TEST(ParseAcceptHeaderTest, InvalidCases) {
|
|||||||
EXPECT_FALSE(
|
EXPECT_FALSE(
|
||||||
detail::parse_accept_header("invalidtype,application/json", result));
|
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
|
// Valid cases should still work
|
||||||
EXPECT_TRUE(detail::parse_accept_header("*/*", result));
|
EXPECT_TRUE(detail::parse_accept_header("*/*", result));
|
||||||
EXPECT_EQ(result.size(), 1U);
|
EXPECT_EQ(result.size(), 1U);
|
||||||
@@ -1089,6 +1081,38 @@ TEST(ParseAcceptHeaderTest, InvalidCases) {
|
|||||||
EXPECT_EQ(result[0], "text/*");
|
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<std::string> 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<std::string> 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<std::string> result;
|
||||||
|
EXPECT_FALSE(detail::parse_accept_header(",invalidtype,", result));
|
||||||
|
}
|
||||||
|
|
||||||
TEST(ParseAcceptHeaderTest, ContentTypesPopulatedAndInvalidHeaderHandling) {
|
TEST(ParseAcceptHeaderTest, ContentTypesPopulatedAndInvalidHeaderHandling) {
|
||||||
Server svr;
|
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
|
// 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.
|
// 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) {
|
TEST(RepeatedFieldLinesTest, EmptyFieldLineDoesNotInjectComma) {
|
||||||
Server svr;
|
Server svr;
|
||||||
|
|
||||||
@@ -18557,6 +18579,24 @@ TEST(RepeatedFieldLinesTest, EmptyFieldLineDoesNotInjectComma) {
|
|||||||
EXPECT_EQ("application/json", observed_types[0]);
|
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
|
#ifdef CPPHTTPLIB_ZLIB_SUPPORT
|
||||||
// An encoding offered on a later Accept-Encoding field line is still offered.
|
// An encoding offered on a later Accept-Encoding field line is still offered.
|
||||||
TEST(RepeatedFieldLinesTest, AcceptEncodingCombinesEveryFieldLine) {
|
TEST(RepeatedFieldLinesTest, AcceptEncodingCombinesEveryFieldLine) {
|
||||||
|
|||||||
Reference in New Issue
Block a user