[http1] fix HttpRequest to support query params

markdroth · Nov 11, 2024 · 17d97a0 · 17d97a0
1 parent dc9af5a
commit 17d97a0
Show file tree

Hide file tree

Showing 3 changed files with 22 additions and 8 deletions.
diff --git a/src/core/util/http_client/httpcli.cc b/src/core/util/http_client/httpcli.cc
@@ -80,7 +80,8 @@ OrphanablePtr<HttpRequest> HttpRequest::Get(
   std::string name =
       absl::StrFormat("HTTP:GET:%s:%s", uri.authority(), uri.path());
   const grpc_slice request_text = grpc_httpcli_format_get_request(
-      request, uri.authority().c_str(), uri.path().c_str());
+      request, uri.authority().c_str(),
+      uri.EncodedPathAndQueryParams().c_str());
   return MakeOrphanable<HttpRequest>(
       std::move(uri), request_text, response, deadline, channel_args, on_done,
       pollent, name.c_str(), std::move(test_only_generate_response),
@@ -104,7 +105,8 @@ OrphanablePtr<HttpRequest> HttpRequest::Post(
   std::string name =
       absl::StrFormat("HTTP:POST:%s:%s", uri.authority(), uri.path());
   const grpc_slice request_text = grpc_httpcli_format_post_request(
-      request, uri.authority().c_str(), uri.path().c_str());
+      request, uri.authority().c_str(),
+      uri.EncodedPathAndQueryParams().c_str());
   return MakeOrphanable<HttpRequest>(
       std::move(uri), request_text, response, deadline, channel_args, on_done,
       pollent, name.c_str(), std::move(test_only_generate_response),
@@ -128,7 +130,7 @@ OrphanablePtr<HttpRequest> HttpRequest::Put(
   std::string name =
       absl::StrFormat("HTTP:PUT:%s:%s", uri.authority(), uri.path());
   const grpc_slice request_text = grpc_httpcli_format_put_request(
-      request, uri.authority().c_str(), uri.path().c_str());
+      request, uri.authority().c_str(), uri.PercentEncodeAuthority().c_str());
   return MakeOrphanable<HttpRequest>(
       std::move(uri), request_text, response, deadline, channel_args, on_done,
       pollent, name.c_str(), std::move(test_only_generate_response),
@@ -275,6 +277,8 @@ void HttpRequest::ContinueDoneWriteAfterScheduleOnExecCtx(
 }
 
 void HttpRequest::StartWrite() {
+  GRPC_TRACE_LOG(http1, INFO) << "Sending HTTP1 request: "
+                              << StringViewFromSlice(request_text_);
   CSliceRef(request_text_);
   grpc_slice_buffer_add(&outgoing_, request_text_);
   Ref().release();  // ref held by pending write

diff --git a/src/core/util/uri.cc b/src/core/util/uri.cc
@@ -352,6 +352,16 @@ std::string URI::ToString() const {
     parts.emplace_back("//");
     parts.emplace_back(PercentEncode(authority_, IsAuthorityChar));
   }
+  parts.emplace_back(EncodedPathAndQueryParams());
+  if (!fragment_.empty()) {
+    parts.push_back("#");
+    parts.push_back(PercentEncode(fragment_, IsQueryOrFragmentChar));
+  }
+  return absl::StrJoin(parts, "");
+}
+
+std::string URI::EncodedPathAndQueryParams() const {
+  std::vector<std::string> parts;
   if (!path_.empty()) {
     parts.emplace_back(PercentEncode(path_, IsPathChar));
   }
@@ -360,10 +370,6 @@ std::string URI::ToString() const {
     parts.push_back(
         absl::StrJoin(query_parameter_pairs_, "&", QueryParameterFormatter()));
   }
-  if (!fragment_.empty()) {
-    parts.push_back("#");
-    parts.push_back(PercentEncode(fragment_, IsQueryOrFragmentChar));
-  }
   return absl::StrJoin(parts, "");
 }
 

diff --git a/src/core/util/uri.h b/src/core/util/uri.h
@@ -76,7 +76,7 @@ class URI {
     return query_parameter_map_;
   }
   // A vector of key:value query parameter pairs, kept in order of appearance
-  // within the URI search string. Repeated keys are represented as separate
+  // within the URI string. Repeated keys are represented as separate
   // key:value elements.
   const std::vector<QueryParam>& query_parameter_pairs() const {
     return query_parameter_pairs_;
@@ -85,6 +85,10 @@ class URI {
 
   std::string ToString() const;
 
+  // Returns the encoded path and query params, such as would be used on
+  // the wire in an HTTP request.
+  std::string EncodedPathAndQueryParams() const;
+
  private:
   URI(std::string scheme, std::string authority, std::string path,
       std::vector<QueryParam> query_parameter_pairs, std::string fragment);