Merge branch 'nghttpx-rev-wildcard-router'

src/CMakeLists.txt

@@ -149,6 +149,7 @@ if(ENABLE_APP)
       shrpx_config_test.cc
       shrpx_worker_test.cc
       shrpx_http_test.cc
+      shrpx_router_test.cc
       http2_test.cc
       util_test.cc
       nghttp2_gzip_test.c

src/Makefile.am

@@ -175,6 +175,7 @@ nghttpx_unittest_SOURCES = shrpx-unittest.cc \
 	shrpx_config_test.cc shrpx_config_test.h \
 	shrpx_worker_test.cc shrpx_worker_test.h \
 	shrpx_http_test.cc shrpx_http_test.h \
+	shrpx_router_test.cc shrpx_router_test.h \
 	http2_test.cc http2_test.h \
 	util_test.cc util_test.h \
 	nghttp2_gzip_test.c nghttp2_gzip_test.h \

src/shrpx-unittest.cc

@@ -44,6 +44,7 @@
 #include "base64_test.h"
 #include "shrpx_config.h"
 #include "ssl.h"
+#include "shrpx_router_test.h"
 
 static int init_suite1(void) { return 0; }
 
@@ -125,6 +126,9 @@ int main(int argc, char *argv[]) {
                    shrpx::test_shrpx_http_create_forwarded) ||
       !CU_add_test(pSuite, "http_create_via_header_value",
                    shrpx::test_shrpx_http_create_via_header_value) ||
+      !CU_add_test(pSuite, "router_match", shrpx::test_shrpx_router_match) ||
+      !CU_add_test(pSuite, "router_match_prefix",
+                   shrpx::test_shrpx_router_match_prefix) ||
       !CU_add_test(pSuite, "util_streq", shrpx::test_util_streq) ||
       !CU_add_test(pSuite, "util_strieq", shrpx::test_util_strieq) ||
       !CU_add_test(pSuite, "util_inp_strlower",

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,21 +910,19 @@ 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 &wildcard_patterns = downstreamconf.wildcard_patterns;
+    auto &balloc = downstream->get_block_allocator();
+
     if (!req.authority.empty()) {
-      group_idx =
-          match_downstream_addr_group(router, wildcard_patterns, req.authority,
-                                      req.path, groups, catch_all);
+      group_idx = match_downstream_addr_group(
+          routerconf, req.authority, req.path, groups, catch_all, balloc);
     } else {
       auto h = req.fs.header(http2::HD_HOST);
       if (h) {
-        group_idx = match_downstream_addr_group(
-            router, wildcard_patterns, h->value, req.path, groups, catch_all);
+        group_idx = match_downstream_addr_group(routerconf, h->value, req.path,
+                                                groups, catch_all, balloc);
       } else {
-        group_idx =
-            match_downstream_addr_group(router, wildcard_patterns, StringRef{},
-                                        req.path, groups, catch_all);
+        group_idx = match_downstream_addr_group(
+            routerconf, StringRef{}, req.path, groups, catch_all, balloc);
       }
     }
   }

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; });
@@ -828,12 +831,19 @@ int parse_mapping(Config *config, DownstreamAddrConfig addr,
 
         auto &router = wildcard_patterns.back().router;
         router.add_route(path, idx);
+
+        auto rev_host = host.str();
+        std::reverse(std::begin(rev_host), std::end(rev_host));
+
+        rw_router.add_route(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;
 }
@@ -2805,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{};
@@ -2825,27 +2836,11 @@ 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));
-  } else {
-    auto &wildcard_patterns = downstreamconf.wildcard_patterns;
-    std::sort(std::begin(wildcard_patterns), std::end(wildcard_patterns),
-              [](const WildcardPattern &lhs, const WildcardPattern &rhs) {
-                return std::lexicographical_compare(
-                    rhs.host.rbegin(), rhs.host.rend(), lhs.host.rbegin(),
-                    lhs.host.rend());
-              });
-    if (LOG_ENABLED(INFO)) {
-      LOG(INFO) << "Reverse sorted wildcard hosts (compared from tail to head, "
-                   "and sorted in reverse order):";
-      for (auto &wp : wildcard_patterns) {
-        LOG(INFO) << wp.host;
-      }
-    }
   }
 
   // backward compatibility: override all SNI fields with the option

src/shrpx_config.h

@@ -599,7 +599,7 @@ struct RateLimitConfig {
 };
 
 // Wildcard host pattern routing.  We strips left most '*' from host
-// field.  router includes all path pattern sharing same wildcard
+// field.  router includes all path patterns sharing the same wildcard
 // host.
 struct WildcardPattern {
   WildcardPattern(const StringRef &host)
@@ -609,14 +609,25 @@ struct WildcardPattern {
   Router router;
 };
 
+// Configuration to select backend to forward request
+struct RouterConfig {
+  Router router;
+  // Router for reversed wildcard hosts.  Since this router has
+  // wildcard hosts reversed without '*', one should call match()
+  // function with reversed host stripping last character.  This is
+  // because we require at least one character must match for '*'.
+  // 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;
-  Router router;
-  std::vector<WildcardPattern> wildcard_patterns;
+  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_router.cc

@@ -279,6 +279,65 @@ ssize_t Router::match(const StringRef &host, const StringRef &path) const {
   return node->index;
 }
 
+namespace {
+const RNode *match_prefix(size_t *nread, const RNode *node, const char *first,
+                          const char *last) {
+  if (first == last) {
+    return nullptr;
+  }
+
+  auto p = first;
+
+  for (;;) {
+    auto next_node = find_next_node(node, *p);
+    if (next_node == nullptr) {
+      return nullptr;
+    }
+
+    node = next_node;
+
+    auto n = std::min(node->len, static_cast<size_t>(last - p));
+    if (memcmp(node->s, p, n) != 0) {
+      return nullptr;
+    }
+
+    p += n;
+
+    if (p != last) {
+      if (node->index != -1) {
+        *nread = p - first;
+        return node;
+      }
+      continue;
+    }
+
+    if (node->len == n) {
+      *nread = p - first;
+      return node;
+    }
+
+    return nullptr;
+  }
+}
+} // namespace
+
+ssize_t Router::match_prefix(size_t *nread, const RNode **last_node,
+                             const StringRef &s) const {
+  if (*last_node == nullptr) {
+    *last_node = &root_;
+  }
+
+  auto node =
+      ::shrpx::match_prefix(nread, *last_node, std::begin(s), std::end(s));
+  if (node == nullptr) {
+    return -1;
+  }
+
+  *last_node = node;
+
+  return node->index;
+}
+
 namespace {
 void dump_node(const RNode *node, int depth) {
   fprintf(stderr, "%*ss='%.*s', len=%zu, index=%zd\n", depth, "",

src/shrpx_router.h

@@ -67,6 +67,16 @@ public:
   bool add_route(const StringRef &pattern, size_t index);
   // Returns the matched index of pattern.  -1 if there is no match.
   ssize_t match(const StringRef &host, const StringRef &path) const;
+  // Returns the matched index of pattern if a pattern is a suffix of
+  // |s|, otherwise -1.  If |*last_node| is not nullptr, it specifies
+  // the first node to start matching.  If it is nullptr, match will
+  // start from scratch.  When the match was found (the return value
+  // is not -1), |*nread| has the number of bytes matched in |s|, and
+  // |*last_node| has the last matched node.  One can continue to
+  // match the longer pattern using the returned |*last_node| to the
+  // another invocation of this function until it returns -1.
+  ssize_t match_prefix(size_t *nread, const RNode **last_node,
+                       const StringRef &s) const;
 
   void add_node(RNode *node, const char *pattern, size_t patlen, size_t index);
 

src/shrpx_router_test.cc

+/*
+ * nghttp2 - HTTP/2 C Library
+ *
+ * Copyright (c) 2016 Tatsuhiro Tsujikawa
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining
+ * a copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sublicense, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE
+ * LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
+ * OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
+ * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+#include "shrpx_router_test.h"
+
+#include <CUnit/CUnit.h>
+
+#include "shrpx_router.h"
+
+namespace shrpx {
+
+struct Pattern {
+  StringRef pattern;
+  size_t idx;
+};
+
+void test_shrpx_router_match(void) {
+  auto patterns = std::vector<Pattern>{
+      {StringRef::from_lit("nghttp2.org/"), 0},
+      {StringRef::from_lit("nghttp2.org/alpha"), 1},
+      {StringRef::from_lit("nghttp2.org/alpha/"), 2},
+      {StringRef::from_lit("nghttp2.org/alpha/bravo/"), 3},
+      {StringRef::from_lit("www.nghttp2.org/alpha/"), 4},
+      {StringRef::from_lit("/alpha"), 5},
+      {StringRef::from_lit("example.com/alpha/"), 6},
+  };
+
+  Router router;
+
+  for (auto &p : patterns) {
+    router.add_route(p.pattern, p.idx);
+  }
+
+  ssize_t idx;
+
+  idx = router.match(StringRef::from_lit("nghttp2.org"),
+                     StringRef::from_lit("/"));
+
+  CU_ASSERT(0 == idx);
+
+  idx = router.match(StringRef::from_lit("nghttp2.org"),
+                     StringRef::from_lit("/alpha"));
+
+  CU_ASSERT(1 == idx);
+
+  idx = router.match(StringRef::from_lit("nghttp2.org"),
+                     StringRef::from_lit("/alpha/"));
+
+  CU_ASSERT(2 == idx);
+
+  idx = router.match(StringRef::from_lit("nghttp2.org"),
+                     StringRef::from_lit("/alpha/charlie"));
+
+  CU_ASSERT(2 == idx);
+
+  idx = router.match(StringRef::from_lit("nghttp2.org"),
+                     StringRef::from_lit("/alpha/bravo/"));
+
+  CU_ASSERT(3 == idx);
+
+  // matches pattern when last '/' is missing in path
+  idx = router.match(StringRef::from_lit("nghttp2.org"),
+                     StringRef::from_lit("/alpha/bravo"));
+
+  idx = router.match(StringRef{}, StringRef::from_lit("/alpha"));
+
+  CU_ASSERT(5 == idx);
+}
+
+void test_shrpx_router_match_prefix(void) {
+  auto patterns = std::vector<Pattern>{
+      {StringRef::from_lit("gro.2ptthgn."), 0},
+      {StringRef::from_lit("gro.2ptthgn.www."), 1},
+      {StringRef::from_lit("gro.2ptthgn.gmi."), 2},
+      {StringRef::from_lit("gro.2ptthgn.gmi.ahpla."), 3},
+  };
+
+  Router router;
+
+  for (auto &p : patterns) {
+    router.add_route(p.pattern, p.idx);
+  }
+
+  ssize_t idx;
+  const RNode *node;
+  size_t nread;
+
+  node = nullptr;
+
+  idx = router.match_prefix(&nread, &node,
+                            StringRef::from_lit("gro.2ptthgn.gmi.ahpla.ovarb"));
+
+  CU_ASSERT(0 == idx);
+  CU_ASSERT(12 == nread);
+
+  idx = router.match_prefix(&nread, &node,
+                            StringRef::from_lit("gmi.ahpla.ovarb"));
+
+  CU_ASSERT(2 == idx);
+  CU_ASSERT(4 == nread);
+
+  idx = router.match_prefix(&nread, &node, StringRef::from_lit("ahpla.ovarb"));
+
+  CU_ASSERT(3 == idx);
+  CU_ASSERT(6 == nread);
+}
+
+} // namespace shrpx

src/shrpx_router_test.h

+/*
+ * nghttp2 - HTTP/2 C Library
+ *
+ * Copyright (c) 2016 Tatsuhiro Tsujikawa
+ *
+ * Permission is hereby granted, free of charge, to any person obtaining
+ * a copy of this software and associated documentation files (the
+ * "Software"), to deal in the Software without restriction, including
+ * without limitation the rights to use, copy, modify, merge, publish,
+ * distribute, sublicense, and/or sell copies of the Software, and to
+ * permit persons to whom the Software is furnished to do so, subject to
+ * the following conditions:
+ *
+ * The above copyright notice and this permission notice shall be
+ * included in all copies or substantial portions of the Software.
+ *
+ * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND,
+ * EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF
+ * MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND
+ * NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE
+ * LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION
+ * OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION
+ * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
+ */
+#ifndef SHRPX_ROUTER_TEST_H
+#define SHRPX_ROUTER_TEST_H
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif // HAVE_CONFIG_H
+
+namespace shrpx {
+
+void test_shrpx_router_match(void);
+void test_shrpx_router_match_prefix(void);
+
+} // namespace shrpx
+
+#endif // SHRPX_ROUTER_TEST_H

src/shrpx_worker.cc

@@ -456,10 +456,15 @@ ConnectionHandler *Worker::get_connection_handler() const {
 
 namespace {
 size_t match_downstream_addr_group_host(
-    const Router &router, const std::vector<WildcardPattern> &wildcard_patterns,
-    const StringRef &host, const StringRef &path,
+    const RouterConfig &routerconf, const StringRef &host,
+    const StringRef &path,
     const std::vector<std::shared_ptr<DownstreamAddrGroup>> &groups,
-    size_t catch_all) {
+    size_t catch_all, BlockAllocator &balloc) {
+
+  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) {
@@ -486,23 +491,42 @@ size_t match_downstream_addr_group_host(
     return group;
   }
 
-  for (auto it = std::begin(wildcard_patterns);
-       it != std::end(wildcard_patterns); ++it) {
-    /* left most '*' must match at least one character */
-    if (host.size() <= (*it).host.size() ||
-        !util::ends_with(std::begin(host), std::end(host),
-                         std::begin((*it).host), std::end((*it).host))) {
-      continue;
-    }
-    auto group = (*it).router.match(StringRef{}, path);
-    if (group != -1) {
-      // We sorted wildcard_patterns in a way that first match is the
-      // longest host pattern.
-      if (LOG_ENABLED(INFO)) {
-        LOG(INFO) << "Found wildcard pattern with query " << host << path
-                  << ", matched pattern=" << groups[group]->pattern;
+  if (!wildcard_patterns.empty() && !host.empty()) {
+    auto rev_host_src = make_byte_ref(balloc, host.size() - 1);
+    auto ep =
+        std::copy(std::begin(host) + 1, std::end(host), rev_host_src.base);
+    std::reverse(rev_host_src.base, ep);
+    auto rev_host = StringRef{rev_host_src.base, ep};
+
+    ssize_t best_group = -1;
+    const RNode *last_node = nullptr;
+
+    for (;;) {
+      size_t nread = 0;
+      auto wcidx =
+          rev_wildcard_router.match_prefix(&nread, &last_node, rev_host);
+      if (wcidx == -1) {
+        break;
       }
-      return group;
+
+      rev_host = StringRef{std::begin(rev_host) + nread, std::end(rev_host)};
+
+      auto &wc = wildcard_patterns[wcidx];
+      auto group = wc.router.match(StringRef{}, path);
+      if (group != -1) {
+        // We sorted wildcard_patterns in a way that first match is the
+        // longest host pattern.
+        if (LOG_ENABLED(INFO)) {
+          LOG(INFO) << "Found wildcard pattern with query " << host << path
+                    << ", matched pattern=" << groups[group]->pattern;
+        }
+
+        best_group = group;
+      }
+    }
+
+    if (best_group != -1) {
+      return best_group;
     }
   }
 
@@ -523,10 +547,10 @@ size_t match_downstream_addr_group_host(
 } // namespace
 
 size_t match_downstream_addr_group(
-    const Router &router, const std::vector<WildcardPattern> &wildcard_patterns,
-    const StringRef &hostport, const StringRef &raw_path,
+    const RouterConfig &routerconf, const StringRef &hostport,
+    const StringRef &raw_path,
     const std::vector<std::shared_ptr<DownstreamAddrGroup>> &groups,
-    size_t catch_all) {
+    size_t catch_all, BlockAllocator &balloc) {
   if (std::find(std::begin(hostport), std::end(hostport), '/') !=
       std::end(hostport)) {
     // We use '/' specially, and if '/' is included in host, it breaks
@@ -539,8 +563,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, wildcard_patterns, hostport,
-                                            path, groups, catch_all);
+    return match_downstream_addr_group_host(routerconf, hostport, path, groups,
+                                            catch_all, balloc);
   }
 
   StringRef host;
@@ -562,16 +586,17 @@ size_t match_downstream_addr_group(
     host = StringRef{std::begin(hostport), p};
   }
 
-  std::string low_host;
   if (std::find_if(std::begin(host), std::end(host), [](char c) {
         return 'A' <= c || c <= 'Z';
       }) != std::end(host)) {
-    low_host = host.str();
-    util::inp_strlower(low_host);
-    host = StringRef{low_host};
+    auto low_host = make_byte_ref(balloc, host.size() + 1);
+    auto ep = std::copy(std::begin(host), std::end(host), low_host.base);
+    *ep = '\0';
+    util::inp_strlower(low_host.base, ep);
+    host = StringRef{low_host.base, ep};
   }
-  return match_downstream_addr_group_host(router, wildcard_patterns, host, path,
-                                          groups, catch_all);
+  return match_downstream_addr_group_host(routerconf, host, path, groups,
+                                          catch_all, balloc);
 }
 
 void downstream_failure(DownstreamAddr *addr) {

src/shrpx_worker.h

@@ -280,10 +280,10 @@ 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 std::vector<WildcardPattern> &wildcard_patterns,
-    const StringRef &hostport, const StringRef &path,
+    const RouterConfig &routerconfig, const StringRef &hostport,
+    const StringRef &path,
     const std::vector<std::shared_ptr<DownstreamAddrGroup>> &groups,
-    size_t catch_all);
+    size_t catch_all, BlockAllocator &balloc);
 
 void downstream_failure(DownstreamAddr *addr);