nghttpx: Aggregate router configuration into one struct

src/shrpx_client_handler.cc

@@ -889,6 +889,7 @@ std::unique_ptr<DownstreamConnection>
 ClientHandler::get_downstream_connection(Downstream *downstream) {
   size_t group_idx;
   auto &downstreamconf = *worker_->get_downstream_config();
+  auto &routerconf = downstreamconf.router;
 
   auto catch_all = downstreamconf.addr_group_catch_all;
   auto &groups = worker_->get_downstream_addr_groups();
@@ -909,22 +910,16 @@ ClientHandler::get_downstream_connection(Downstream *downstream) {
     //  have dealt with proxy case already, just use catch-all group.
     group_idx = catch_all;
   } else {
-    auto &router = downstreamconf.router;
-    auto &rev_wildcard_router = downstreamconf.rev_wildcard_router;
-    auto &wildcard_patterns = downstreamconf.wildcard_patterns;
     if (!req.authority.empty()) {
-      group_idx = match_downstream_addr_group(router, rev_wildcard_router,
+      group_idx = match_downstream_addr_group(routerconf, req.authority,
-                                              wildcard_patterns, req.authority,
                                               req.path, groups, catch_all);
     } else {
       auto h = req.fs.header(http2::HD_HOST);
       if (h) {
-        group_idx = match_downstream_addr_group(router, rev_wildcard_router,
+        group_idx = match_downstream_addr_group(routerconf, h->value, req.path,
-                                                wildcard_patterns, h->value,
+                                                groups, catch_all);
-                                                req.path, groups, catch_all);
       } else {
-        group_idx = match_downstream_addr_group(router, rev_wildcard_router,
+        group_idx = match_downstream_addr_group(routerconf, StringRef{},
-                                                wildcard_patterns, StringRef{},
                                                 req.path, groups, catch_all);
       }
     }

src/shrpx_config.cc

@@ -772,6 +772,11 @@ int parse_mapping(Config *config, DownstreamAddrConfig addr,
   addr.tls = params.tls;
   addr.sni = ImmutableString{std::begin(params.sni), std::end(params.sni)};
 
+  auto &routerconf = downstreamconf.router;
+  auto &router = routerconf.router;
+  auto &rw_router = routerconf.rev_wildcard_router;
+  auto &wildcard_patterns = routerconf.wildcard_patterns;
+
   for (const auto &raw_pattern : mapping) {
     auto done = false;
     std::string pattern;
@@ -817,8 +822,6 @@ int parse_mapping(Config *config, DownstreamAddrConfig addr,
       auto host = StringRef{std::begin(g.pattern) + 1, path_first};
       auto path = StringRef{path_first, std::end(g.pattern)};
 
-      auto &wildcard_patterns = downstreamconf.wildcard_patterns;
-
       auto it = std::find_if(
           std::begin(wildcard_patterns), std::end(wildcard_patterns),
           [&host](const WildcardPattern &wp) { return wp.host == host; });
@@ -832,14 +835,15 @@ int parse_mapping(Config *config, DownstreamAddrConfig addr,
         auto rev_host = host.str();
         std::reverse(std::begin(rev_host), std::end(rev_host));
 
-        downstreamconf.rev_wildcard_router.add_route(
+        rw_router.add_route(StringRef{rev_host}, wildcard_patterns.size() - 1);
-            StringRef{rev_host}, wildcard_patterns.size() - 1);
       } else {
         (*it).router.add_route(path, idx);
       }
-    } else {
+
-      downstreamconf.router.add_route(StringRef{g.pattern}, idx);
+      continue;
     }
+
+    router.add_route(StringRef{g.pattern}, idx);
   }
   return 0;
 }
@@ -2811,7 +2815,8 @@ int configure_downstream_group(Config *config, bool http2_proxy,
                                const TLSConfig &tlsconf) {
   auto &downstreamconf = *config->conn.downstream;
   auto &addr_groups = downstreamconf.addr_groups;
-  auto &router = downstreamconf.router;
+  auto &routerconf = downstreamconf.router;
+  auto &router = routerconf.router;
 
   if (addr_groups.empty()) {
     DownstreamAddrConfig addr{};
@@ -2831,10 +2836,9 @@ int configure_downstream_group(Config *config, bool http2_proxy,
       std::move(std::begin(g.addrs), std::end(g.addrs),
                 std::back_inserter(catch_all.addrs));
     }
-    std::vector<WildcardPattern>().swap(downstreamconf.wildcard_patterns);
     std::vector<DownstreamAddrGroupConfig>().swap(addr_groups);
     // maybe not necessary?
-    router = Router();
+    routerconf = RouterConfig{};
     router.add_route(StringRef{catch_all.pattern}, addr_groups.size());
     addr_groups.push_back(std::move(catch_all));
   }

src/shrpx_config.h

@@ -609,12 +609,8 @@ struct WildcardPattern {
   Router router;
 };
 
-struct DownstreamConfig {
+// Configuration to select backend to forward request
-  struct {
+struct RouterConfig {
-    ev_tstamp read;
-    ev_tstamp write;
-    ev_tstamp idle_read;
-  } timeout;
   Router router;
   // Router for reversed wildcard hosts.  Since this router has
   // wildcard hosts reversed without '*', one should call match()
@@ -623,6 +619,15 @@ struct DownstreamConfig {
   // The index stored in this router is index of wildcard_patterns.
   Router rev_wildcard_router;
   std::vector<WildcardPattern> wildcard_patterns;
+};
+
+struct DownstreamConfig {
+  struct {
+    ev_tstamp read;
+    ev_tstamp write;
+    ev_tstamp idle_read;
+  } timeout;
+  RouterConfig router;
   std::vector<DownstreamAddrGroupConfig> addr_groups;
   // The index of catch-all group in downstream_addr_groups.
   size_t addr_group_catch_all;

src/shrpx_worker.cc

@@ -456,11 +456,15 @@ ConnectionHandler *Worker::get_connection_handler() const {
 
 namespace {
 size_t match_downstream_addr_group_host(
-    const Router &router, const Router &rev_wildcard_router,
+    const RouterConfig &routerconf, const StringRef &host,
-    const std::vector<WildcardPattern> &wildcard_patterns,
+    const StringRef &path,
-    const StringRef &host, const StringRef &path,
     const std::vector<std::shared_ptr<DownstreamAddrGroup>> &groups,
     size_t catch_all) {
+
+  const auto &router = routerconf.router;
+  const auto &rev_wildcard_router = routerconf.rev_wildcard_router;
+  const auto &wildcard_patterns = routerconf.wildcard_patterns;
+
   if (path.empty() || path[0] != '/') {
     auto group = router.match(host, StringRef::from_lit("/"));
     if (group != -1) {
@@ -541,9 +545,8 @@ size_t match_downstream_addr_group_host(
 } // namespace
 
 size_t match_downstream_addr_group(
-    const Router &router, const Router &rev_wildcard_router,
+    const RouterConfig &routerconf, const StringRef &hostport,
-    const std::vector<WildcardPattern> &wildcard_patterns,
+    const StringRef &raw_path,
-    const StringRef &hostport, const StringRef &raw_path,
     const std::vector<std::shared_ptr<DownstreamAddrGroup>> &groups,
     size_t catch_all) {
   if (std::find(std::begin(hostport), std::end(hostport), '/') !=
@@ -558,9 +561,8 @@ size_t match_downstream_addr_group(
   auto path = StringRef{std::begin(raw_path), query};
 
   if (hostport.empty()) {
-    return match_downstream_addr_group_host(router, rev_wildcard_router,
+    return match_downstream_addr_group_host(routerconf, hostport, path, groups,
-                                            wildcard_patterns, hostport, path,
+                                            catch_all);
-                                            groups, catch_all);
   }
 
   StringRef host;
@@ -590,8 +592,7 @@ size_t match_downstream_addr_group(
     util::inp_strlower(low_host);
     host = StringRef{low_host};
   }
-  return match_downstream_addr_group_host(router, rev_wildcard_router,
+  return match_downstream_addr_group_host(routerconf, host, path, groups,
-                                          wildcard_patterns, host, path, groups,
                                           catch_all);
 }
 

src/shrpx_worker.h

@@ -280,9 +280,8 @@ private:
 // group.  The catch-all group index is given in |catch_all|.  All
 // patterns are given in |groups|.
 size_t match_downstream_addr_group(
-    const Router &router, const Router &rev_wildcard_router,
+    const RouterConfig &routerconfig, const StringRef &hostport,
-    const std::vector<WildcardPattern> &wildcard_patterns,
+    const StringRef &path,
-    const StringRef &hostport, const StringRef &path,
     const std::vector<std::shared_ptr<DownstreamAddrGroup>> &groups,
     size_t catch_all);
 

src/shrpx_worker_test.cc

@@ -49,135 +49,134 @@ void test_shrpx_worker_match_downstream_addr_group(void) {
     groups.push_back(std::move(g));
   }
 
-  Router router;
+  RouterConfig routerconf;
-  Router wcrouter;
+
+  auto &router = routerconf.router;
+  auto &wcrouter = routerconf.rev_wildcard_router;
+  auto &wp = routerconf.wildcard_patterns;
 
   for (size_t i = 0; i < groups.size(); ++i) {
     auto &g = groups[i];
     router.add_route(StringRef{g->pattern}, i);
   }
 
-  std::vector<WildcardPattern> wp;
-
   CU_ASSERT(0 == match_downstream_addr_group(
-                     router, wcrouter, wp, StringRef::from_lit("nghttp2.org"),
+                     routerconf, StringRef::from_lit("nghttp2.org"),
                      StringRef::from_lit("/"), groups, 255));
 
   // port is removed
-  CU_ASSERT(0 ==
+  CU_ASSERT(0 == match_downstream_addr_group(
-            match_downstream_addr_group(router, wcrouter, wp,
+                     routerconf, StringRef::from_lit("nghttp2.org:8080"),
-                                        StringRef::from_lit("nghttp2.org:8080"),
+                     StringRef::from_lit("/"), groups, 255));
-                                        StringRef::from_lit("/"), groups, 255));
 
   // host is case-insensitive
   CU_ASSERT(4 == match_downstream_addr_group(
-                     router, wcrouter, wp,
+                     routerconf, StringRef::from_lit("WWW.nghttp2.org"),
-                     StringRef::from_lit("WWW.nghttp2.org"),
                      StringRef::from_lit("/alpha"), groups, 255));
 
   CU_ASSERT(1 == match_downstream_addr_group(
-                     router, wcrouter, wp, StringRef::from_lit("nghttp2.org"),
+                     routerconf, StringRef::from_lit("nghttp2.org"),
                      StringRef::from_lit("/alpha/bravo/"), groups, 255));
 
   // /alpha/bravo also matches /alpha/bravo/
   CU_ASSERT(1 == match_downstream_addr_group(
-                     router, wcrouter, wp, StringRef::from_lit("nghttp2.org"),
+                     routerconf, StringRef::from_lit("nghttp2.org"),
                      StringRef::from_lit("/alpha/bravo"), groups, 255));
 
   // path part is case-sensitive
   CU_ASSERT(0 == match_downstream_addr_group(
-                     router, wcrouter, wp, StringRef::from_lit("nghttp2.org"),
+                     routerconf, StringRef::from_lit("nghttp2.org"),
                      StringRef::from_lit("/Alpha/bravo"), groups, 255));
 
   CU_ASSERT(1 == match_downstream_addr_group(
-                     router, wcrouter, wp, StringRef::from_lit("nghttp2.org"),
+                     routerconf, StringRef::from_lit("nghttp2.org"),
                      StringRef::from_lit("/alpha/bravo/charlie"), groups, 255));
 
   CU_ASSERT(2 == match_downstream_addr_group(
-                     router, wcrouter, wp, StringRef::from_lit("nghttp2.org"),
+                     routerconf, StringRef::from_lit("nghttp2.org"),
                      StringRef::from_lit("/alpha/charlie"), groups, 255));
 
   // pattern which does not end with '/' must match its entirely.  So
   // this matches to group 0, not group 2.
   CU_ASSERT(0 == match_downstream_addr_group(
-                     router, wcrouter, wp, StringRef::from_lit("nghttp2.org"),
+                     routerconf, StringRef::from_lit("nghttp2.org"),
                      StringRef::from_lit("/alpha/charlie/"), groups, 255));
 
   CU_ASSERT(255 == match_downstream_addr_group(
-                       router, wcrouter, wp, StringRef::from_lit("example.org"),
+                       routerconf, StringRef::from_lit("example.org"),
                        StringRef::from_lit("/"), groups, 255));
 
-  CU_ASSERT(255 == match_downstream_addr_group(
+  CU_ASSERT(255 ==
-                       router, wcrouter, wp, StringRef::from_lit(""),
+            match_downstream_addr_group(routerconf, StringRef::from_lit(""),
-                       StringRef::from_lit("/"), groups, 255));
+                                        StringRef::from_lit("/"), groups, 255));
 
   CU_ASSERT(255 == match_downstream_addr_group(
-                       router, wcrouter, wp, StringRef::from_lit(""),
+                       routerconf, StringRef::from_lit(""),
                        StringRef::from_lit("alpha"), groups, 255));
 
   CU_ASSERT(255 == match_downstream_addr_group(
-                       router, wcrouter, wp, StringRef::from_lit("foo/bar"),
+                       routerconf, StringRef::from_lit("foo/bar"),
                        StringRef::from_lit("/"), groups, 255));
 
   // If path is StringRef::from_lit("*", only match with host + "/").
   CU_ASSERT(0 == match_downstream_addr_group(
-                     router, wcrouter, wp, StringRef::from_lit("nghttp2.org"),
+                     routerconf, StringRef::from_lit("nghttp2.org"),
                      StringRef::from_lit("*"), groups, 255));
 
+  CU_ASSERT(
+      5 == match_downstream_addr_group(routerconf, StringRef::from_lit("[::1]"),
+                                       StringRef::from_lit("/"), groups, 255));
   CU_ASSERT(5 == match_downstream_addr_group(
-                     router, wcrouter, wp, StringRef::from_lit("[::1]"),
+                     routerconf, StringRef::from_lit("[::1]:8080"),
-                     StringRef::from_lit("/"), groups, 255));
-  CU_ASSERT(5 == match_downstream_addr_group(
-                     router, wcrouter, wp, StringRef::from_lit("[::1]:8080"),
                      StringRef::from_lit("/"), groups, 255));
+  CU_ASSERT(255 ==
+            match_downstream_addr_group(routerconf, StringRef::from_lit("[::1"),
+                                        StringRef::from_lit("/"), groups, 255));
   CU_ASSERT(255 == match_downstream_addr_group(
-                       router, wcrouter, wp, StringRef::from_lit("[::1"),
+                       routerconf, StringRef::from_lit("[::1]8000"),
-                       StringRef::from_lit("/"), groups, 255));
-  CU_ASSERT(255 == match_downstream_addr_group(
-                       router, wcrouter, wp, StringRef::from_lit("[::1]8000"),
                        StringRef::from_lit("/"), groups, 255));
 
   // Check the case where adding route extends tree
   CU_ASSERT(6 == match_downstream_addr_group(
-                     router, wcrouter, wp, StringRef::from_lit("nghttp2.org"),
+                     routerconf, StringRef::from_lit("nghttp2.org"),
                      StringRef::from_lit("/alpha/bravo/delta"), groups, 255));
 
   CU_ASSERT(1 == match_downstream_addr_group(
-                     router, wcrouter, wp, StringRef::from_lit("nghttp2.org"),
+                     routerconf, StringRef::from_lit("nghttp2.org"),
                      StringRef::from_lit("/alpha/bravo/delta/"), groups, 255));
 
   // Check the case where query is done in a single node
   CU_ASSERT(7 == match_downstream_addr_group(
-                     router, wcrouter, wp, StringRef::from_lit("example.com"),
+                     routerconf, StringRef::from_lit("example.com"),
                      StringRef::from_lit("/alpha/bravo"), groups, 255));
 
   CU_ASSERT(255 == match_downstream_addr_group(
-                       router, wcrouter, wp, StringRef::from_lit("example.com"),
+                       routerconf, StringRef::from_lit("example.com"),
                        StringRef::from_lit("/alpha/bravo/"), groups, 255));
 
   CU_ASSERT(255 == match_downstream_addr_group(
-                       router, wcrouter, wp, StringRef::from_lit("example.com"),
+                       routerconf, StringRef::from_lit("example.com"),
                        StringRef::from_lit("/alpha"), groups, 255));
 
   // Check the case where quey is done in a single node
   CU_ASSERT(8 == match_downstream_addr_group(
-                     router, wcrouter, wp, StringRef::from_lit("192.168.0.1"),
+                     routerconf, StringRef::from_lit("192.168.0.1"),
                      StringRef::from_lit("/alpha"), groups, 255));
 
   CU_ASSERT(8 == match_downstream_addr_group(
-                     router, wcrouter, wp, StringRef::from_lit("192.168.0.1"),
+                     routerconf, StringRef::from_lit("192.168.0.1"),
                      StringRef::from_lit("/alpha/"), groups, 255));
 
   CU_ASSERT(8 == match_downstream_addr_group(
-                     router, wcrouter, wp, StringRef::from_lit("192.168.0.1"),
+                     routerconf, StringRef::from_lit("192.168.0.1"),
                      StringRef::from_lit("/alpha/bravo"), groups, 255));
 
   CU_ASSERT(255 == match_downstream_addr_group(
-                       router, wcrouter, wp, StringRef::from_lit("192.168.0.1"),
+                       routerconf, StringRef::from_lit("192.168.0.1"),
                        StringRef::from_lit("/alph"), groups, 255));
 
   CU_ASSERT(255 == match_downstream_addr_group(
-                       router, wcrouter, wp, StringRef::from_lit("192.168.0.1"),
+                       routerconf, StringRef::from_lit("192.168.0.1"),
                        StringRef::from_lit("/"), groups, 255));
 
   // Test for wildcard hosts
@@ -199,32 +198,27 @@ void test_shrpx_worker_match_downstream_addr_group(void) {
   wp.back().router.add_route(StringRef::from_lit("/echo/foxtrot"), 12);
 
   CU_ASSERT(11 == match_downstream_addr_group(
-                      router, wcrouter, wp,
+                      routerconf, StringRef::from_lit("git.nghttp2.org"),
-                      StringRef::from_lit("git.nghttp2.org"),
                       StringRef::from_lit("/echo"), groups, 255));
 
   CU_ASSERT(10 == match_downstream_addr_group(
-                      router, wcrouter, wp,
+                      routerconf, StringRef::from_lit("0git.nghttp2.org"),
-                      StringRef::from_lit("0git.nghttp2.org"),
                       StringRef::from_lit("/echo"), groups, 255));
 
   CU_ASSERT(11 == match_downstream_addr_group(
-                      router, wcrouter, wp,
+                      routerconf, StringRef::from_lit("it.nghttp2.org"),
-                      StringRef::from_lit("it.nghttp2.org"),
                       StringRef::from_lit("/echo"), groups, 255));
 
   CU_ASSERT(255 == match_downstream_addr_group(
-                       router, wcrouter, wp,
+                       routerconf, StringRef::from_lit(".nghttp2.org"),
-                       StringRef::from_lit(".nghttp2.org"),
                        StringRef::from_lit("/echo/foxtrot"), groups, 255));
 
   CU_ASSERT(9 == match_downstream_addr_group(
-                     router, wcrouter, wp,
+                     routerconf, StringRef::from_lit("alpha.nghttp2.org"),
-                     StringRef::from_lit("alpha.nghttp2.org"),
                      StringRef::from_lit("/golf"), groups, 255));
 
   CU_ASSERT(0 == match_downstream_addr_group(
-                     router, wcrouter, wp, StringRef::from_lit("nghttp2.org"),
+                     routerconf, StringRef::from_lit("nghttp2.org"),
                      StringRef::from_lit("/echo"), groups, 255));
 }