From c795ad1c326b4b63ef2da40befd5fcaf8b74e1bd Mon Sep 17 00:00:00 2001 From: yhirose Date: Fri, 5 Dec 2025 21:39:40 -0500 Subject: [PATCH] Fix #2259. Add query string normalization to preserve parameter order in requests --- httplib.h | 59 ++++++++++++++++++++++++++++++++++++++++++++++++---- test/test.cc | 34 +++++++++++++++++++++++++++++- 2 files changed, 88 insertions(+), 5 deletions(-) diff --git a/httplib.h b/httplib.h index 31f6ba3..49eb719 100644 --- a/httplib.h +++ b/httplib.h @@ -5717,6 +5717,38 @@ inline void parse_query_text(const std::string &s, Params ¶ms) { parse_query_text(s.data(), s.size(), params); } +// 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). +inline std::string normalize_query_string(const std::string &query) { + std::string result; + split(query.data(), query.data() + query.size(), '&', + [&](const char *b, const char *e) { + std::string key; + std::string val; + divide(b, static_cast(e - b), '=', + [&](const char *lhs_data, std::size_t lhs_size, + const char *rhs_data, std::size_t rhs_size) { + key.assign(lhs_data, lhs_size); + val.assign(rhs_data, rhs_size); + }); + + if (!key.empty()) { + auto dec_key = decode_query_component(key); + auto dec_val = decode_query_component(val); + + if (!result.empty()) { result += '&'; } + result += encode_query_component(dec_key); + if (!val.empty() || std::find(b, e, '=') != e) { + result += '='; + result += encode_query_component(dec_val); + } + } + }); + return result; +} + inline bool parse_multipart_boundary(const std::string &content_type, std::string &boundary) { auto boundary_keyword = "boundary="; @@ -10204,13 +10236,32 @@ inline bool ClientImpl::write_request(Stream &strm, Request &req, query_part = ""; } - // Encode path and query + // Encode path part. If the original `req.path` already contained a + // query component, preserve its raw query string (including parameter + // order) instead of reparsing and reassembling it which may reorder + // parameters due to container ordering (e.g. `Params` uses + // `std::multimap`). When there is no query in `req.path`, fall back to + // building a query from `req.params` so existing callers that pass + // `Params` continue to work. auto path_with_query = path_encode_ ? detail::encode_path(path_part) : path_part; - detail::parse_query_text(query_part, req.params); - if (!req.params.empty()) { - path_with_query = append_query_params(path_with_query, req.params); + if (!query_part.empty()) { + // Normalize the query string (decode then re-encode) while preserving + // the original parameter order. + auto normalized = detail::normalize_query_string(query_part); + if (!normalized.empty()) { path_with_query += '?' + normalized; } + + // Still populate req.params for handlers/users who read them. + detail::parse_query_text(query_part, req.params); + } else { + // No query in path; parse any query_part (empty) and append params + // from `req.params` when present (preserves prior behavior for + // callers who provide Params separately). + detail::parse_query_text(query_part, req.params); + if (!req.params.empty()) { + path_with_query = append_query_params(path_with_query, req.params); + } } // Write request line and headers diff --git a/test/test.cc b/test/test.cc index 9001aae..e8fe6fd 100644 --- a/test/test.cc +++ b/test/test.cc @@ -399,6 +399,38 @@ TEST(EncodeQueryParamTest, ParseReservedCharactersTest) { "%3B%2C%2F%3F%3A%40%26%3D%2B%24"); } +TEST(ClientQueryOrder, PreserveOrder) { + // This test reproduces Issue #2259: client may reorder query parameters + // when sending a GET request. The expected behavior is that the client + // preserves the original query string order when the caller supplied it + // as part of the path. + Server svr; + svr.Get("/", [&](const Request &req, Response &res) { + // Echo back the raw target so the test can assert ordering + res.set_content(req.target, "text/plain"); + }); + + std::thread t{[&] { 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); + ASSERT_TRUE(cli.is_valid()); + + const std::string original = "/?z=1&y=2&x=3&c=7&b=8&a=9"; + auto res = cli.Get(original); + ASSERT_TRUE(res); + + // Expect the echoed target to exactly match the original path (order + // preserved) + EXPECT_EQ(res->body, original); +} + TEST(EncodeQueryParamTest, TestUTF8Characters) { string chineseCharacters = u8"中国語"; string russianCharacters = u8"дом"; @@ -13221,4 +13253,4 @@ TEST(ETagTest, NegativeFileModificationTime) { svr.stop(); t.join(); std::remove(fname); -} \ No newline at end of file +}