grpc_http1_bridge: add <ignore_query_params> option (#31110)

Some client requests' URLs may contain query params. gRPC upstream servers can not handle these requests, and may return error such as "unknown method". So we remove query params here.

Risk level: Low
Testing: Unit tests.

Signed-off-by: FHT <33562110+delphisfang@users.noreply.github.com>

api/envoy/extensions/filters/http/grpc_http1_bridge/v3/config.proto

@@ -26,4 +26,7 @@ message Config {
   // For the requests that went through this upgrade the filter will also strip the frame before forwarding the
   // response to the client.
   bool upgrade_protobuf_to_grpc = 1;
+
+  // If true then query parameters in request's URL path will be removed.
+  bool ignore_query_parameters = 2;
 }

changelogs/current.yaml

@@ -272,5 +272,10 @@ new_features:
 - area: redis
   change: |
     Added support for the watch command (aborts multi transactions if watched keys change).
+- area: grpc_http_bridge
+  change: |
+    added :ref:`ignore_query_parameters
+    <envoy_v3_api_field_extensions.filters.http.grpc_http1_bridge.v3.Config.ignore_query_parameters>` option for
+    automatically stripping query parameters in request URL path.
 
 deprecated:

docs/root/configuration/http/http_filters/grpc_http1_bridge_filter.rst

@@ -56,6 +56,13 @@ the size specified in the gRPC frame.
 The response body returned to the client will not contain the gRPC header frame for requests that are upgraded in this
 fashion, i.e. the body will contain only the encoded Protobuf.
 
+Ignore query parameters
+-----------------------
+
+Some client requests' URL path may contain query params, while gRPC upstream servers can not handle these requests,
+and may return error such as "unknown method". In order to solve this kind of problems, the filter will automatically strip query parameters in request's URL path if
+:ref:`ignore_query_parameters <envoy_v3_api_field_extensions.filters.http.grpc_http1_bridge.v3.Config.ignore_query_parameters>` is set.
+
 Statistics
 ----------
 

source/extensions/filters/http/grpc_http1_bridge/http1_bridge_filter.cc

@@ -19,6 +19,18 @@ namespace Extensions {
 namespace HttpFilters {
 namespace GrpcHttp1Bridge {
 
+// Some client requests' URLs may contain query params. gRPC upstream servers can not
+// handle these requests, and may return error such as "unknown method". So we remove
+// query params here.
+void Http1BridgeFilter::ignoreQueryParams(Http::RequestHeaderMap& headers) {
+  absl::string_view path = headers.getPathValue();
+  size_t pos = path.find("?");
+  if (pos != absl::string_view::npos) {
+    absl::string_view new_path = path.substr(0, pos);
+    headers.setPath(new_path);
+  }
+}
+
 Http::FilterHeadersStatus Http1BridgeFilter::decodeHeaders(Http::RequestHeaderMap& headers, bool) {
   const bool protobuf_request = Grpc::Common::isProtobufRequestHeaders(headers);
   if (upgrade_protobuf_ && protobuf_request) {
@@ -30,13 +42,15 @@ Http::FilterHeadersStatus Http1BridgeFilter::decodeHeaders(Http::RequestHeaderMa
   }
 
   const bool grpc_request = Grpc::Common::isGrpcRequestHeaders(headers);
-
   const absl::optional<Http::Protocol>& protocol = decoder_callbacks_->streamInfo().protocol();
   ASSERT(protocol);
   if (protocol.value() < Http::Protocol::Http2 && grpc_request) {
     do_bridging_ = true;
   }
 
+  if (do_bridging_ && ignore_query_parameters_) {
+    ignoreQueryParams(headers);
+  }
   return Http::FilterHeadersStatus::Continue;
 }
 

source/extensions/filters/http/grpc_http1_bridge/http1_bridge_filter.h

@@ -22,7 +22,8 @@ class Http1BridgeFilter : public Http::StreamFilter, Logger::Loggable<Logger::Id
   explicit Http1BridgeFilter(
       Grpc::Context& context,
       const envoy::extensions::filters::http::grpc_http1_bridge::v3::Config& proto_config)
-      : context_(context), upgrade_protobuf_(proto_config.upgrade_protobuf_to_grpc()) {}
+      : context_(context), upgrade_protobuf_(proto_config.upgrade_protobuf_to_grpc()),
+        ignore_query_parameters_(proto_config.ignore_query_parameters()) {}
 
   // Http::StreamFilterBase
   void onDestroy() override {}
@@ -56,6 +57,7 @@ class Http1BridgeFilter : public Http::StreamFilter, Logger::Loggable<Logger::Id
 
 private:
   void setupStatTracking(const Http::RequestHeaderMap& headers);
+  void ignoreQueryParams(Http::RequestHeaderMap& headers);
 
   Http::StreamDecoderFilterCallbacks* decoder_callbacks_{};
   Http::StreamEncoderFilterCallbacks* encoder_callbacks_{};
@@ -64,6 +66,7 @@ class Http1BridgeFilter : public Http::StreamFilter, Logger::Loggable<Logger::Id
   bool do_framing_{};
   Grpc::Context& context_;
   bool upgrade_protobuf_{};
+  bool ignore_query_parameters_{};
 };
 
 } // namespace GrpcHttp1Bridge

test/extensions/filters/http/grpc_http1_bridge/http1_bridge_filter_test.cc

@@ -31,9 +31,10 @@ namespace {
 
 class GrpcHttp1BridgeFilterTest : public testing::Test {
 protected:
-  void initialize(bool upgrade_protobuf = false) {
+  void initialize(bool upgrade_protobuf = false, bool ignore_query_params = false) {
     envoy::extensions::filters::http::grpc_http1_bridge::v3::Config config;
     config.set_upgrade_protobuf_to_grpc(upgrade_protobuf);
+    config.set_ignore_query_parameters(ignore_query_params);
     filter_ = std::make_unique<Http1BridgeFilter>(context_, config);
     filter_->setDecoderFilterCallbacks(decoder_callbacks_);
     filter_->setEncoderFilterCallbacks(encoder_callbacks_);
@@ -288,14 +289,24 @@ TEST_F(GrpcHttp1BridgeFilterTest, ProtobufUpgradedHeaderSanitized) {
   Http::TestRequestHeaderMapImpl request_headers{{"content-type", "application/x-protobuf"},
                                                  {"content-length", "5"},
                                                  {":path", "/v1/spotify.Concat/Concat"}};
-  Buffer::OwnedImpl data("hello");
 
   EXPECT_CALL(decoder_callbacks_.downstream_callbacks_, clearRouteCache());
   EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false));
   EXPECT_EQ(Http::Headers::get().ContentTypeValues.Grpc, request_headers.getContentTypeValue());
   EXPECT_EQ("", request_headers.getContentLengthValue());
 }
 
+// Verifies that the query params in URL are removed when ignore_query_parameters is enabled
+TEST_F(GrpcHttp1BridgeFilterTest, QueryParamsIgnored) {
+  initialize(false, true);
+  Http::TestRequestHeaderMapImpl request_headers{
+      {"content-type", "application/grpc"},
+      {":path", "/v1/spotify.Concat/Concat?timestamp=1701678591"}};
+
+  EXPECT_EQ(Http::FilterHeadersStatus::Continue, filter_->decodeHeaders(request_headers, false));
+  EXPECT_EQ("/v1/spotify.Concat/Concat", request_headers.getPathValue());
+}
+
 } // namespace
 } // namespace GrpcHttp1Bridge
 } // namespace HttpFilters