lishuxu commented on code in PR #616:
URL: https://github.com/apache/iceberg-cpp/pull/616#discussion_r3084504021
##########
src/iceberg/catalog/rest/http_client.cc:
##########
@@ -68,27 +70,37 @@ namespace {
/// \brief Default error type for unparseable REST responses.
constexpr std::string_view kRestExceptionType = "RESTException";
-/// \brief Prepare headers for an HTTP request.
-Result<cpr::Header> BuildHeaders(
- const std::unordered_map<std::string, std::string>& request_headers,
+/// \brief Merge default headers with per-request headers (per-request wins).
+std::unordered_map<std::string, std::string> MergeHeaders(
const std::unordered_map<std::string, std::string>& default_headers,
- auth::AuthSession& session) {
- std::unordered_map<std::string, std::string> headers(default_headers);
+ const std::unordered_map<std::string, std::string>& request_headers) {
+ std::unordered_map<std::string, std::string> merged(default_headers);
for (const auto& [key, val] : request_headers) {
- headers.insert_or_assign(key, val);
+ merged.insert_or_assign(key, val);
}
- ICEBERG_RETURN_UNEXPECTED(session.Authenticate(headers));
- return cpr::Header(headers.begin(), headers.end());
+ return merged;
+}
+
+cpr::Header ToCprHeader(const auth::HTTPRequest& request) {
+ return {request.headers.begin(), request.headers.end()};
}
-/// \brief Converts a map of string key-value pairs to cpr::Parameters.
-cpr::Parameters GetParameters(
+/// \brief Append URL-encoded query parameters to a URL, sorted by key.
+std::string AppendQueryString(
+ const std::string& base_url,
const std::unordered_map<std::string, std::string>& params) {
- cpr::Parameters cpr_params;
- for (const auto& [key, val] : params) {
- cpr_params.Add({key, val});
+ if (params.empty()) return base_url;
+ std::map<std::string, std::string> sorted(params.begin(), params.end());
+ std::string url = base_url + "?";
+ bool first = true;
+ for (const auto& [k, v] : sorted) {
+ if (!first) url += "&";
+ auto ek = EncodeString(k);
+ auto ev = EncodeString(v);
+ url += (ek ? *ek : k) + "=" + (ev ? *ev : v);
Review Comment:
AppendQueryString silently falls back to the raw key/value when EncodeString
fails. If encoding fails, the URL passed to Authenticate would differ from what
the server receives, causing signature verification to fail. Consider
propagating the error instead:
ICEBERG_ASSIGN_OR_RAISE(auto ek, EncodeString(k));
ICEBERG_ASSIGN_OR_RAISE(auto ev, EncodeString(v));
url += ek + "=" + ev;
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
