diff --git a/httplib.h b/httplib.h index 80d90a1..ae6ca59 100644 --- a/httplib.h +++ b/httplib.h @@ -971,20 +971,30 @@ enum StatusCode { NetworkAuthenticationRequired_511 = 511, }; -// RFC 9110 5.3: the order in which header fields with the same field name are -// received is significant, and a proxy must not reorder them. Neither -// std::unordered_multimap (no ordering guarantee at all for equivalent keys: -// libstdc++ yields reverse insertion order, libc++ insertion order) nor -// std::multimap (sorts by field name, so control data such as Host no longer -// leads the message) can express that, so header fields are kept in a flat -// vector in the order they were received or set. Lookup is a linear scan, -// which beats hashing for the at most CPPHTTPLIB_HEADER_MAX_COUNT fields a -// message carries. -class Headers { +namespace detail { + +// A multimap that keeps its entries in the order they were inserted. +// +// HTTP needs that order in two places. RFC 9110 5.3 makes the order of header +// fields sharing a field name significant and forbids a proxy from reordering +// them, and a query string's parameters are meaningful in the order the caller +// wrote them. Neither standard container expresses it: std::unordered_multimap +// gives no ordering guarantee at all for equivalent keys (libstdc++ yields +// reverse insertion order, libc++ insertion order), and std::multimap sorts by +// key, which would drop control data such as Host behind whatever else the +// message carries and alphabetise a query string. +// +// Entries are therefore kept in a flat vector, in order. Lookup is a linear +// scan, which beats hashing for the handful of entries a message carries +// (headers are capped at CPPHTTPLIB_HEADER_MAX_COUNT). +// +// KeyEqual compares keys; it is what makes Headers case-insensitive and +// Params, whose parameter names are case-sensitive, not. +template class insertion_ordered_multimap { public: using key_type = std::string; - using mapped_type = std::string; - using value_type = std::pair; + using mapped_type = Mapped; + using value_type = std::pair; using size_type = std::size_t; using difference_type = std::ptrdiff_t; using reference = value_type &; @@ -993,16 +1003,20 @@ public: private: static size_type npos() { return static_cast(-1); } - // Iterating a Headers yields every field in insertion order, but - // equal_range() and find() have to walk only the fields sharing one name, - // which are not adjacent. Both are the same iterator type: key_idx_ selects - // between the two traversals, and since equality compares only the position, - // an iterator restricted to one name still compares equal to end(). + static bool keys_equal(const std::string &a, const std::string &b) { + return KeyEqual()(a, b); + } + + // Iterating yields every entry in insertion order, but equal_range() and + // find() have to walk only the entries sharing one key, which are not + // adjacent. Both are the same iterator type: key_idx_ selects between the + // two traversals, and since equality compares only the position, an iterator + // restricted to one key still compares equal to end(). template class iterator_t { public: using iterator_category = std::bidirectional_iterator_tag; - using value_type = Headers::value_type; - using difference_type = Headers::difference_type; + using value_type = insertion_ordered_multimap::value_type; + using difference_type = insertion_ordered_multimap::difference_type; using pointer = V *; using reference = V &; @@ -1019,8 +1033,8 @@ private: pointer operator->() const { return data_ + idx_; } iterator_t &operator++() { - // Saturating, so that advancing past the last field of a name (which - // get_header_value() does when asked for an out-of-range id) stays at + // Saturating, so that advancing past the last entry of a key (which + // get_multimap_value() does when asked for an out-of-range id) stays at // end() instead of running off the container. if (idx_ >= size_) { return *this; } ++idx_; @@ -1064,14 +1078,14 @@ private: } private: - friend class Headers; + friend class insertion_ordered_multimap; template friend class iterator_t; iterator_t(V *data, size_type idx, size_type size, size_type key_idx) : data_(data), idx_(idx), size_(size), key_idx_(key_idx) {} bool matches(size_type i) const { - return detail::case_ignore::equal(data_[i].first, data_[key_idx_].first); + return keys_equal(data_[i].first, data_[key_idx_].first); } V *data_; @@ -1084,10 +1098,12 @@ public: using iterator = iterator_t; using const_iterator = iterator_t; - Headers() = default; - Headers(std::initializer_list il) : entries_(il) {} + insertion_ordered_multimap() = default; + insertion_ordered_multimap(std::initializer_list il) + : entries_(il) {} template - Headers(InputIt first, InputIt last) : entries_(first, last) {} + insertion_ordered_multimap(InputIt first, InputIt last) + : entries_(first, last) {} iterator begin() { return make_iter(0, npos()); } iterator end() { return make_iter(entries_.size(), npos()); } @@ -1099,7 +1115,7 @@ public: bool empty() const { return entries_.empty(); } size_type size() const { return entries_.size(); } void clear() { entries_.clear(); } - void swap(Headers &rhs) { entries_.swap(rhs.entries_); } + void swap(insertion_ordered_multimap &rhs) { entries_.swap(rhs.entries_); } iterator insert(const value_type &val) { entries_.push_back(val); @@ -1116,7 +1132,8 @@ public: return make_iter(entries_.size() - 1, npos()); } - // RFC 9110 5.3 recommends sending control data such as Host first. + // For entries that have to lead the message, such as the Host header field + // (RFC 9110 5.3 recommends sending control data first). template iterator emplace_front(Args &&...args) { entries_.emplace(entries_.begin(), std::forward(args)...); return make_iter(0, npos()); @@ -1135,7 +1152,7 @@ public: size_type count(const std::string &key) const { size_type n = 0; for (const auto &entry : entries_) { - if (detail::case_ignore::equal(entry.first, key)) { n++; } + if (keys_equal(entry.first, key)) { n++; } } return n; } @@ -1157,8 +1174,7 @@ public: auto before = entries_.size(); entries_.erase(std::remove_if(entries_.begin(), entries_.end(), [&](const value_type &entry) { - return detail::case_ignore::equal( - entry.first, key); + return keys_equal(entry.first, key); }), entries_.end()); return before - entries_.size(); @@ -1170,7 +1186,7 @@ public: } // Erases what iterating [first, last) would actually visit, so erasing an - // equal_range() removes only the fields with that name, not everything + // equal_range() removes only the entries with that key, not everything // positioned between them. iterator erase(const_iterator first, const_iterator last) { auto from = first.idx_; @@ -1187,7 +1203,7 @@ public: auto key = entries_[first.key_idx_].first; auto keep = from_it; for (auto it = from_it; it != to_it; ++it) { - if (!detail::case_ignore::equal(it->first, key)) { + if (!keys_equal(it->first, key)) { if (keep != it) { *keep = std::move(*it); } ++keep; } @@ -1202,18 +1218,20 @@ public: return make_iter(from, npos()); } - friend bool operator==(const Headers &lhs, const Headers &rhs) { + friend bool operator==(const insertion_ordered_multimap &lhs, + const insertion_ordered_multimap &rhs) { return lhs.entries_ == rhs.entries_; } - friend bool operator!=(const Headers &lhs, const Headers &rhs) { + friend bool operator!=(const insertion_ordered_multimap &lhs, + const insertion_ordered_multimap &rhs) { return !(lhs == rhs); } private: size_type index_of(const std::string &key) const { for (size_type i = 0; i < entries_.size(); i++) { - if (detail::case_ignore::equal(entries_[i].first, key)) { return i; } + if (keys_equal(entries_[i].first, key)) { return i; } } return npos(); } @@ -1229,7 +1247,15 @@ private: std::vector entries_; }; -using Params = std::multimap; +} // namespace detail + +using Headers = + detail::insertion_ordered_multimap; + +// Query parameter names are case-sensitive, unlike header field names. +using Params = + detail::insertion_ordered_multimap>; using Match = std::smatch; using DownloadProgress = std::function; @@ -8288,8 +8314,11 @@ inline void parse_query_text(const std::string &s, Params ¶ms) { // Normalize a query string by decoding and re-encoding each key/value pair // while preserving the original parameter order. This avoids double-encoding -// and ensures consistent encoding without reordering (unlike Params which -// uses std::multimap and sorts keys). +// and ensures consistent encoding. It works on the raw string rather than +// parsing into Params and re-serializing, because that round trip cannot +// reproduce the input: params_to_query_str() always emits '=', so a bare +// "flag" would come back as "flag=", and parse_query_text() drops exactly +// duplicated pairs. inline std::string normalize_query_string(const std::string &query) { std::string result; split(query.data(), query.data() + query.size(), '&', diff --git a/test/test.cc b/test/test.cc index c641f6c..e397ace 100644 --- a/test/test.cc +++ b/test/test.cc @@ -1235,6 +1235,87 @@ TEST(ParamsToQueryTest, ConvertParamsToQuery) { EXPECT_EQ(detail::params_to_query_str(dic), "key1=val1&key2=val2&key3=val3"); } +// Joins every entry of a Headers or Params into "key=value " so a whole +// traversal can be compared in one assertion. Both are instantiations of the +// same insertion-ordered container, so one helper serves both. +template static std::string entries_to_string(const Map &m) { + std::string s; + for (const auto &entry : m) { + s += entry.first + "=" + entry.second + " "; + } + return s; +} + +TEST(ParamsOrderTest, QueryIsBuiltInInsertionOrderNotSorted) { + // Params used to be a std::multimap, which sorted by name and handed the + // caller's query back alphabetised. A client signing its query string could + // not reproduce the order it asked for. + Params dic; + dic.emplace("zulu", "1"); + dic.emplace("alpha", "2"); + dic.emplace("mike", "3"); + + EXPECT_EQ("zulu=1&alpha=2&mike=3", detail::params_to_query_str(dic)); + EXPECT_EQ("/p?zulu=1&alpha=2&mike=3", append_query_params("/p", dic)); +} + +TEST(ParamsOrderTest, ParsingKeepsTheOrderReceived) { + Params dic; + detail::parse_query_text("zulu=1&alpha=2&mike=3", dic); + + EXPECT_EQ("zulu=1 alpha=2 mike=3 ", entries_to_string(dic)); +} + +TEST(ParamsOrderTest, RepeatedNamesKeepTheirOrder) { + Request req; + detail::parse_query_text("tag=first&other=x&tag=second&tag=third", + req.params); + + EXPECT_EQ(3U, req.get_param_value_count("tag")); + EXPECT_EQ("first", req.get_param_value("tag")); + EXPECT_EQ("first", req.get_param_value("tag", 0)); + EXPECT_EQ("second", req.get_param_value("tag", 1)); + EXPECT_EQ("third", req.get_param_value("tag", 2)); + EXPECT_EQ("", req.get_param_value("tag", 3)); +} + +TEST(ParamsOrderTest, LookupStaysCaseSensitive) { + // Params shares its container with Headers, which matches field names + // case-insensitively. Parameter names must not pick that up. + Params dic; + dic.emplace("Key", "upper"); + dic.emplace("key", "lower"); + + EXPECT_EQ(2U, dic.size()); + EXPECT_EQ(1U, dic.count("Key")); + EXPECT_EQ(1U, dic.count("key")); + EXPECT_EQ("upper", dic.find("Key")->second); + EXPECT_EQ("lower", dic.find("key")->second); + EXPECT_TRUE(dic.find("KEY") == dic.end()); +} + +TEST(ParamsOrderTest, ServerSeesTheOrderTheClientSent) { + Server svr; + std::string order; + svr.Get("/order", [&](const Request &req, Response &res) { + order = entries_to_string(req.params); + res.set_content("ok", "text/plain"); + }); + + thread t = thread([&] { svr.listen(HOST, PORT); }); + auto se = detail::scope_exit([&] { + svr.stop(); + t.join(); + ASSERT_FALSE(svr.is_running()); + }); + svr.wait_until_ready(); + + Client cli(HOST, PORT); + auto res = cli.Get("/order?zulu=1&alpha=2&tag=x&mike=3&tag=y"); + ASSERT_TRUE(res); + EXPECT_EQ("zulu=1 alpha=2 tag=x mike=3 tag=y ", order); +} + TEST(ParseMultipartBoundaryTest, DefaultValue) { string content_type = "multipart/form-data; boundary=something"; string boundary; @@ -1377,16 +1458,6 @@ TEST(GetHeaderValueTest, Range) { } } -// Joins every field of a Headers into "name=value " so a whole traversal can -// be compared in one assertion. -static std::string headers_to_string(const Headers &headers) { - std::string s; - for (const auto &header : headers) { - s += header.first + "=" + header.second + " "; - } - return s; -} - TEST(HeadersOrderTest, DuplicateFieldsKeepInsertionOrder) { // RFC 9110 5.3: the order of fields sharing a name is significant. This used // to depend on the standard library (libstdc++ handed back duplicates in @@ -1422,7 +1493,7 @@ TEST(HeadersOrderTest, TraversalFollowsInsertionOrder) { EXPECT_EQ("Host=example.com Accept-Encoding=gzip User-Agent=test " "Accept-Encoding=deflate ", - headers_to_string(headers)); + entries_to_string(headers)); } TEST(HeadersOrderTest, LookupIsCaseInsensitiveAndPicksTheFirstField) { @@ -1444,7 +1515,7 @@ TEST(HeadersOrderTest, ErasingAnEqualRangeSparesInterleavedFields) { auto rng = headers.equal_range("a"); headers.erase(rng.first, rng.second); - EXPECT_EQ("X=x Y=y Z=z ", headers_to_string(headers)); + EXPECT_EQ("X=x Y=y Z=z ", entries_to_string(headers)); } TEST(HeadersOrderTest, ErasingByNameKeepsTheRemainingOrder) { @@ -1453,7 +1524,7 @@ TEST(HeadersOrderTest, ErasingByNameKeepsTheRemainingOrder) { EXPECT_EQ(3U, headers.erase("A")); EXPECT_EQ(0U, headers.count("A")); - EXPECT_EQ("B=2 C=4 ", headers_to_string(headers)); + EXPECT_EQ("B=2 C=4 ", entries_to_string(headers)); } TEST(HeadersOrderTest, EmplaceFrontPrepends) { @@ -1461,7 +1532,7 @@ TEST(HeadersOrderTest, EmplaceFrontPrepends) { headers.emplace_front("Host", "example.com"); EXPECT_EQ("Host=example.com Accept=*/* User-Agent=test ", - headers_to_string(headers)); + entries_to_string(headers)); } TEST(ParseHeaderValueTest, Range) { @@ -8397,7 +8468,7 @@ TEST(HeadersOrderTest, ReceivedFieldsKeepTheirOrder) { Server svr; std::string received; svr.Get("/order", [&](const Request &req, Response &res) { - received = headers_to_string(req.headers); + received = entries_to_string(req.headers); res.set_content("ok", "text/plain"); });