From b4ec1bb1defccd40931bcb82e81c2c8f92298661 Mon Sep 17 00:00:00 2001 From: yhirose Date: Thu, 27 Aug 2026 17:18:29 -0400 Subject: [PATCH] Respect quoted-strings when splitting header parameters (Fix #2568) (#2573) parse_disposition_params() and extract_media_type() both split on every ';' and then on every '=', with no idea that a parameter value can be a quoted-string. RFC 9110 5.6.6 allows ';' and '=' inside one, so filename="report=v2.pdf" came out as v2.pdf", and filename="a;b.txt" was truncated at the semicolon and left a bogus parameter behind. The same defect reached the boundary. RFC 2046 5.1.1 allows '=' in a boundary, which forces a sender to quote it, so the common MIME form boundary="----=_NextPart_000_0000_01D9" parsed as _NextPart_000_0000_01D9". Add split_unquoted(), which is split() with the one extra rule that a delimiter inside a quoted-string is not a delimiter, and route both parameter parsers through it. The key/value split, duplicated verbatim in the two of them, moves into divide_param_pair(). That one divides at the first '=' without tracking quotes: 5.6.6 makes the key a token, so no quote can precede the separator, and reusing divide() keeps this off the per-byte scan. A backslash stays an ordinary character here. Both browsers and httplib's own sender percent-encode '"' rather than escaping it, and recognizing a quoted-pair without also unescaping it would just trade one wrong value for another. --- httplib.h | 107 +++++++++++++++++++++++++++++++++++---------------- test/test.cc | 106 ++++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 180 insertions(+), 33 deletions(-) diff --git a/httplib.h b/httplib.h index 9d5d18df..68660859 100644 --- a/httplib.h +++ b/httplib.h @@ -3619,6 +3619,8 @@ bool parse_range_header(const std::string &s, Ranges &ranges); bool parse_accept_header(const std::string &s, std::vector &content_types); +void parse_disposition_params(const std::string &s, Params ¶ms); + ssize_t send_socket(socket_t sock, const void *ptr, size_t size, int flags); ssize_t read_socket(socket_t sock, void *ptr, size_t size, int flags); @@ -5885,6 +5887,55 @@ inline void split(const char *b, const char *e, char d, size_t m, } } +// Same contract as split(), except that a delimiter inside a quoted-string is +// not a delimiter. RFC 9110 Section 5.6.6 lets a parameter value be a +// quoted-string, and ';' and '=' are legal characters inside one. +inline void split_unquoted(const char *b, const char *e, char d, size_t m, + std::function fn) { + size_t i = 0; + size_t beg = 0; + size_t count = 1; + auto in_quotes = false; + + while (e ? (b + i < e) : (b[i] != '\0')) { + if (b[i] == '"') { + in_quotes = !in_quotes; + } else if (b[i] == d && !in_quotes && count < m) { + auto r = trim(b, e, beg, i); + if (r.first < r.second) { fn(&b[r.first], &b[r.second]); } + beg = i + 1; + count++; + } + i++; + } + + if (i) { + auto r = trim(b, e, beg, i); + if (r.first < r.second) { fn(&b[r.first], &b[r.second]); } + } +} + +inline void split_unquoted(const char *b, const char *e, char d, + std::function fn) { + return split_unquoted(b, e, d, (std::numeric_limits::max)(), + std::move(fn)); +} + +// Divide a header parameter at its first '='. RFC 9110 Section 5.6.6 makes the +// key a token, so the first '=' is the separator even when the value is a +// quoted-string carrying more of them. +inline void divide_param_pair(const char *b, const char *e, std::string &key, + std::string &val) { + divide( + b, static_cast(e - b), '=', + [&](const char *kb, std::size_t klen, const char *vb, std::size_t vlen) { + const auto kr = trim(kb, kb + klen, 0, klen); + key.assign(kb + kr.first, kb + kr.second); + const auto vr = trim(vb, vb + vlen, 0, vlen); + val.assign(vb + vr.first, vb + vr.second); + }); +} + inline bool split_find(const char *b, const char *e, char d, size_t m, std::function fn) { size_t i = 0; @@ -7303,21 +7354,16 @@ extract_media_type(const std::string &content_type, if (params) { // Parse parameters: key=value pairs separated by ';' - split(param_str.data(), param_str.data() + param_str.size(), ';', - [&](const char *b, const char *e) { - std::string key; - std::string val; - split(b, e, '=', [&](const char *b2, const char *e2) { - if (key.empty()) { - key.assign(b2, e2); - } else { - val.assign(b2, e2); - } - }); - if (!key.empty()) { - params->emplace(trim_copy(key), trim_double_quotes_copy(val)); - } - }); + split_unquoted(param_str.data(), param_str.data() + param_str.size(), ';', + [&](const char *b, const char *e) { + std::string key; + std::string val; + divide_param_pair(b, e, key, val); + if (!key.empty()) { + params->emplace(trim_copy(key), + trim_double_quotes_copy(val)); + } + }); } } @@ -8760,26 +8806,21 @@ inline bool parse_multipart_boundary(const std::string &content_type, inline void parse_disposition_params(const std::string &s, Params ¶ms) { std::set cache; - split(s.data(), s.data() + s.size(), ';', [&](const char *b, const char *e) { - std::string kv(b, e); - if (cache.find(kv) != cache.end()) { return; } - cache.insert(kv); + split_unquoted(s.data(), s.data() + s.size(), ';', + [&](const char *b, const char *e) { + std::string kv(b, e); + if (cache.find(kv) != cache.end()) { return; } + cache.insert(kv); - std::string key; - std::string val; - split(b, e, '=', [&](const char *b2, const char *e2) { - if (key.empty()) { - key.assign(b2, e2); - } else { - val.assign(b2, e2); - } - }); + std::string key; + std::string val; + divide_param_pair(b, e, key, val); - if (!key.empty()) { - params.emplace(trim_double_quotes_copy((key)), - trim_double_quotes_copy((val))); - } - }); + if (!key.empty()) { + params.emplace(trim_double_quotes_copy(key), + trim_double_quotes_copy(val)); + } + }); } #ifdef CPPHTTPLIB_NO_EXCEPTIONS diff --git a/test/test.cc b/test/test.cc index 0a7094b2..083cc2df 100644 --- a/test/test.cc +++ b/test/test.cc @@ -1624,6 +1624,112 @@ TEST(ParseMultipartBoundaryTest, QuotedValueIsMeasuredAfterUnquoting) { EXPECT_FALSE(detail::parse_multipart_boundary(content_type, parsed)); } +TEST(ParseMultipartBoundaryTest, QuotedValueWithEqualsSign) { + // RFC 2046 5.1.1 allows '=' in a boundary, so a MIME sender has to quote it. + // The parameter parser must not treat that '=' as the key/value separator. + string content_type = + "multipart/mixed; boundary=\"----=_NextPart_000_0000_01D9\"; " + "charset=UTF-8"; + string parsed; + EXPECT_TRUE(detail::parse_multipart_boundary(content_type, parsed)); + EXPECT_EQ(parsed, "----=_NextPart_000_0000_01D9"); +} + +TEST(ParseMultipartBoundaryTest, QuotedValueWithSemicolon) { + // A ';' inside the quoted-string is part of the value, not a parameter + // separator, so it must not truncate the boundary. + string content_type = "multipart/mixed; boundary=\"a;b\"; charset=UTF-8"; + string parsed; + EXPECT_TRUE(detail::parse_multipart_boundary(content_type, parsed)); + EXPECT_EQ(parsed, "a;b"); +} + +TEST(ParseDispositionParamsTest, QuotedValues) { + // RFC 9110 Section 5.6.6: a parameter value may be a quoted-string, and + // ';' and '=' are ordinary characters inside one. + struct { + const char *value; + std::vector> expected; + } cases[] = { + {"name=\"file1\"; filename=\"a.txt\"", + {{"name", "file1"}, {"filename", "a.txt"}}}, + // Whitespace around '=' and ';' is still trimmed + {"name=\"file2\" ;filename = \"a.html\"", + {{"name", "file2"}, {"filename", "a.html"}}}, + // '=' inside the value used to leave only the text after the last one + {"name=\"file1\"; filename=\"report=v2.pdf\"", + {{"name", "file1"}, {"filename", "report=v2.pdf"}}}, + {"name=\"x=y\"", {{"name", "x=y"}}}, + // ';' inside the value used to truncate the pair and leave a bogus one + {"name=\"file3\"; filename=\"a;b.txt\"", + {{"name", "file3"}, {"filename", "a;b.txt"}}}, + // An unquoted value keeps working, and so does filename* + {"filename*=UTF-8''%41.txt; filename=\"a.txt\"", + {{"filename*", "UTF-8''%41.txt"}, {"filename", "a.txt"}}}, + // A parameter with no key is dropped rather than turned into one whose + // key is the value + {"=nokey; name=\"file5\"", {{"name", "file5"}}}, + }; + + for (const auto &c : cases) { + Params params; + detail::parse_disposition_params(c.value, params); + for (const auto &kv : c.expected) { + auto it = params.find(kv.first); + ASSERT_NE(it, params.end()) + << "value: " << c.value << ", key: " << kv.first; + EXPECT_EQ(kv.second, it->second) << "value: " << c.value; + } + EXPECT_EQ(c.expected.size(), params.size()) << "value: " << c.value; + } +} + +TEST(ParseDispositionParamsTest, QuotedValuesSurviveTheRoundTrip) { + // escape_multipart_field() only escapes '"', CR and LF, so a name or + // filename holding '=' or ';' reaches the server's parameter parser as is. + Server svr; + std::string field_value, file_name, file_filename; + bool has_field = false, has_file = false; + + svr.Post("/quoted", [&](const Request &req, Response &res) { + has_field = req.form.has_field("x=y"); + field_value = req.form.get_field("x=y"); + + has_file = req.form.has_file("file1"); + const auto &file = req.form.get_file("file1"); + file_name = file.name; + file_filename = file.filename; + + res.set_content("ok", "text/plain"); + }); + + auto port = svr.bind_to_any_port(HOST); + thread t = thread([&] { svr.listen_after_bind(); }); + auto se = detail::scope_exit([&] { + svr.stop(); + t.join(); + ASSERT_FALSE(svr.is_running()); + }); + svr.wait_until_ready(); + + UploadFormDataItems items = { + {"x=y", "field-value", "", ""}, + {"file1", "pdf-bytes", "a;b=report.pdf", "application/pdf"}, + }; + + Client cli(HOST, port); + auto res = cli.Post("/quoted", items); + ASSERT_TRUE(res) << "Error: " << to_string(res.error()); + ASSERT_EQ(StatusCode::OK_200, res->status); + + EXPECT_TRUE(has_field); + EXPECT_EQ("field-value", field_value); + + EXPECT_TRUE(has_file); + EXPECT_EQ("file1", file_name); + EXPECT_EQ("a;b=report.pdf", file_filename); +} + TEST(GetHeaderValueTest, DefaultValue) { Headers headers = {{"Dummy", "Dummy"}}; auto val = detail::get_header_value(headers, "Content-Type", "text/plain", 0);