You've already forked cpp-httplib
Fix #2259. Add query string normalization to preserve parameter order in requests
This commit is contained in:
59
httplib.h
59
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<std::size_t>(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
|
||||
|
||||
34
test/test.cc
34
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);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user