From 52ab7e230b86e4631acd11380ca57a8144523781 Mon Sep 17 00:00:00 2001
From: Nick Travers <n.e.travers@gmail.com>
Date: Thu, 10 Sep 2020 17:41:39 -0700
Subject: [PATCH] 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 #13042.

Signed-off-by: Nick Travers <n.e.travers@gmail.com>
---
 source/common/router/config_impl.cc    |  5 +++--
 test/common/router/config_impl_test.cc | 29 ++++++++++++++++++++++----
 2 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/source/common/router/config_impl.cc b/source/common/router/config_impl.cc
index 1affb8d52d42..32bcd3cf13f3 100644
--- 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
index 7e5cb92cc983..80e295b9d81d 100644
--- 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) {