router: extend HTTP CONNECT route matching criteria

Currently, using `ConnectMatcher` as a `RouteMatch` will match only on an HTTP `:method` header of `CONNECT`. Extend the matching functionality to allow for use of the additional match criteria in `RouteMatch` - i.e. header matching, path matching, etc. - mimicking the existing behavior of the other matchers (prefix, path, etc.). Closes envoyproxy#13042. Signed-off-by: Nick Travers <[email protected]>
nicktrav · Sep 11, 2020 · 52ab7e2 · 52ab7e2
1 parent 5d43118
commit 52ab7e2
Show file tree

Hide file tree

Showing 2 changed files with 28 additions and 6 deletions.
diff --git a/source/common/router/config_impl.cc b/source/common/router/config_impl.cc
@@ -1056,9 +1056,10 @@ void ConnectRouteEntryImpl::rewritePathHeader(Http::RequestHeaderMap& headers,
 }
 
 RouteConstSharedPtr ConnectRouteEntryImpl::matches(const Http::RequestHeaderMap& headers,
-                                                   const StreamInfo::StreamInfo&,
+                                                   const StreamInfo::StreamInfo& stream_info,
                                                    uint64_t random_value) const {
-  if (Http::HeaderUtility::isConnect(headers)) {
+  if (Http::HeaderUtility::isConnect(headers) &&
+      RouteEntryImplBase::matchRoute(headers, stream_info, random_value)) {
     return clusterEntry(headers, random_value);
   }
   return nullptr;

diff --git a/test/common/router/config_impl_test.cc b/test/common/router/config_impl_test.cc
@@ -491,8 +491,8 @@ TEST_F(RouteMatcherTest, TestConnectRoutes) {
     route:
       cluster: connect_break
   - match:
-        connect_matcher:
-          {}
+      connect_matcher:
+        {}
     route:
       cluster: connect_match
       prefix_rewrite: "/rewrote"
@@ -507,9 +507,21 @@ TEST_F(RouteMatcherTest, TestConnectRoutes) {
   - bat4.com
   routes:
   - match:
-        connect_matcher:
-          {}
+      connect_matcher:
+        {}
     redirect: { path_redirect: /new_path }
+- name: connect3
+  domains:
+  - bat5.com
+  routes:
+  - match:
+      connect_matcher:
+        {}
+      headers:
+      - name: x-safe
+        exact_match: "safe"
+    route:
+      cluster: connect_header_match
 - name: default
   domains:
   - "*"
@@ -557,6 +569,15 @@ TEST_F(RouteMatcherTest, TestConnectRoutes) {
     redirect->rewritePathHeader(headers, true);
     EXPECT_EQ("http://bat4.com/new_path", redirect->newPath(headers));
   }
+
+  // Header matching (for HTTP/1.1)
+  EXPECT_EQ(
+      "connect_header_match",
+      config.route(genPathlessHeaders("bat5.com", "CONNECT"), 0)->routeEntry()->clusterName());
+
+  // Header matching (for HTTP/2)
+  EXPECT_EQ("connect_header_match",
+            config.route(genHeaders("bat5.com", " ", "CONNECT"), 0)->routeEntry()->clusterName());
 }
 
 TEST_F(RouteMatcherTest, TestRoutes) {