Merge branch 'nghttpx-refactor-downstream-addr-group'

src/shrpx.cc

@@ -2109,14 +2109,14 @@ void process_options(
     addr.host = ImmutableString::from_lit(DEFAULT_DOWNSTREAM_HOST);
     addr.port = DEFAULT_DOWNSTREAM_PORT;
 
-    DownstreamAddrGroup g("/");
+    DownstreamAddrGroup g(StringRef::from_lit("/"));
     g.addrs.push_back(std::move(addr));
-    mod_config()->router.add_route(g.pattern.get(), 1, addr_groups.size());
+    mod_config()->router.add_route(StringRef{g.pattern}, addr_groups.size());
     addr_groups.push_back(std::move(g));
   } else if (get_config()->http2_proxy || get_config()->client_proxy) {
     // We don't support host mapping in these cases.  Move all
     // non-catch-all patterns to catch-all pattern.
-    DownstreamAddrGroup catch_all("/");
+    DownstreamAddrGroup catch_all(StringRef::from_lit("/"));
     for (auto &g : addr_groups) {
       std::move(std::begin(g.addrs), std::end(g.addrs),
                 std::back_inserter(catch_all.addrs));
@@ -2124,7 +2124,7 @@ void process_options(
     std::vector<DownstreamAddrGroup>().swap(addr_groups);
     // maybe not necessary?
     mod_config()->router = Router();
-    mod_config()->router.add_route(catch_all.pattern.get(), 1,
+    mod_config()->router.add_route(StringRef{catch_all.pattern},
                                    addr_groups.size());
     addr_groups.push_back(std::move(catch_all));
   }
@@ -2136,11 +2136,11 @@ void process_options(
   ssize_t catch_all_group = -1;
   for (size_t i = 0; i < addr_groups.size(); ++i) {
     auto &g = addr_groups[i];
-    if (util::streq(g.pattern.get(), "/")) {
+    if (g.pattern == "/") {
       catch_all_group = i;
     }
     if (LOG_ENABLED(INFO)) {
-      LOG(INFO) << "Host-path pattern: group " << i << ": '" << g.pattern.get()
+      LOG(INFO) << "Host-path pattern: group " << i << ": '" << g.pattern
                 << "'";
       for (auto &addr : g.addrs) {
         LOG(INFO) << "group " << i << " -> " << addr.host.c_str()

src/shrpx_client_handler.cc

@@ -681,16 +681,19 @@ ClientHandler::get_downstream_connection(Downstream *downstream) {
   } else {
     auto &router = get_config()->router;
     if (!req.authority.empty()) {
-      group = match_downstream_addr_group(router, req.authority, req.path,
-                                          groups, catch_all);
+      group =
+          match_downstream_addr_group(router, StringRef{req.authority},
+                                      StringRef{req.path}, groups, catch_all);
     } else {
       auto h = req.fs.header(http2::HD_HOST);
       if (h) {
-        group = match_downstream_addr_group(router, h->value, req.path, groups,
-                                            catch_all);
+        group =
+            match_downstream_addr_group(router, StringRef{h->value},
+                                        StringRef{req.path}, groups, catch_all);
       } else {
-        group = match_downstream_addr_group(router, "", req.path, groups,
-                                            catch_all);
+        group =
+            match_downstream_addr_group(router, StringRef::from_lit(""),
+                                        StringRef{req.path}, groups, catch_all);
       }
     }
   }

src/shrpx_config.cc

@@ -78,21 +78,6 @@ TicketKeys::~TicketKeys() {
   }
 }
 
-DownstreamAddrGroup::DownstreamAddrGroup(const DownstreamAddrGroup &other)
-    : pattern(strcopy(other.pattern)), addrs(other.addrs) {}
-
-DownstreamAddrGroup &DownstreamAddrGroup::
-operator=(const DownstreamAddrGroup &other) {
-  if (this == &other) {
-    return *this;
-  }
-
-  pattern = strcopy(other.pattern);
-  addrs = other.addrs;
-
-  return *this;
-}
-
 namespace {
 int split_host_port(char *host, size_t hostlen, uint16_t *port_ptr,
                     const char *hostport, size_t hostportlen) {
@@ -612,7 +597,7 @@ void parse_mapping(const DownstreamAddr &addr, const char *src) {
       pattern += http2::normalize_path(slash, raw_pattern.second);
     }
     for (auto &g : addr_groups) {
-      if (g.pattern.get() == pattern) {
+      if (g.pattern == pattern) {
         g.addrs.push_back(addr);
         done = true;
         break;
@@ -621,11 +606,10 @@ void parse_mapping(const DownstreamAddr &addr, const char *src) {
     if (done) {
       continue;
     }
-    DownstreamAddrGroup g(pattern);
+    DownstreamAddrGroup g(StringRef{pattern});
     g.addrs.push_back(addr);
 
-    mod_config()->router.add_route(g.pattern.get(), strlen(g.pattern.get()),
-                                   addr_groups.size());
+    mod_config()->router.add_route(StringRef{g.pattern}, addr_groups.size());
 
     addr_groups.push_back(std::move(g));
   }
@@ -2519,17 +2503,15 @@ int int_syslog_facility(const char *strfacility) {
 }
 
 namespace {
-size_t
-match_downstream_addr_group_host(const Router &router, const std::string &host,
-                                 const char *path, size_t pathlen,
-                                 const std::vector<DownstreamAddrGroup> &groups,
-                                 size_t catch_all) {
-  if (pathlen == 0 || *path != '/') {
-    auto group = router.match(host, "/", 1);
+size_t match_downstream_addr_group_host(
+    const Router &router, const StringRef &host, const StringRef &path,
+    const std::vector<DownstreamAddrGroup> &groups, size_t catch_all) {
+  if (path.empty() || path[0] != '/') {
+    auto group = router.match(host, StringRef::from_lit("/"));
     if (group != -1) {
       if (LOG_ENABLED(INFO)) {
         LOG(INFO) << "Found pattern with query " << host
-                  << ", matched pattern=" << groups[group].pattern.get();
+                  << ", matched pattern=" << groups[group].pattern;
       }
       return group;
     }
@@ -2538,24 +2520,23 @@ match_downstream_addr_group_host(const Router &router, const std::string &host,
 
   if (LOG_ENABLED(INFO)) {
     LOG(INFO) << "Perform mapping selection, using host=" << host
-              << ", path=" << std::string(path, pathlen);
+              << ", path=" << path;
   }
 
-  auto group = router.match(host, path, pathlen);
+  auto group = router.match(host, path);
   if (group != -1) {
     if (LOG_ENABLED(INFO)) {
-      LOG(INFO) << "Found pattern with query " << host
-                << std::string(path, pathlen)
-                << ", matched pattern=" << groups[group].pattern.get();
+      LOG(INFO) << "Found pattern with query " << host << path
+                << ", matched pattern=" << groups[group].pattern;
     }
     return group;
   }
 
-  group = router.match("", path, pathlen);
+  group = router.match("", path);
   if (group != -1) {
     if (LOG_ENABLED(INFO)) {
-      LOG(INFO) << "Found pattern with query " << std::string(path, pathlen)
-                << ", matched pattern=" << groups[group].pattern.get();
+      LOG(INFO) << "Found pattern with query " << path
+                << ", matched pattern=" << groups[group].pattern;
     }
     return group;
   }
@@ -2567,11 +2548,9 @@ match_downstream_addr_group_host(const Router &router, const std::string &host,
 }
 } // namespace
 
-size_t
-match_downstream_addr_group(const Router &router, const std::string &hostport,
-                            const std::string &raw_path,
-                            const std::vector<DownstreamAddrGroup> &groups,
-                            size_t catch_all) {
+size_t match_downstream_addr_group(
+    const Router &router, const StringRef &hostport, const StringRef &raw_path,
+    const std::vector<DownstreamAddrGroup> &groups, size_t catch_all) {
   if (std::find(std::begin(hostport), std::end(hostport), '/') !=
       std::end(hostport)) {
     // We use '/' specially, and if '/' is included in host, it breaks
@@ -2581,12 +2560,11 @@ match_downstream_addr_group(const Router &router, const std::string &hostport,
 
   auto fragment = std::find(std::begin(raw_path), std::end(raw_path), '#');
   auto query = std::find(std::begin(raw_path), fragment, '?');
-  auto path = raw_path.c_str();
-  auto pathlen = query - std::begin(raw_path);
+  auto path = StringRef{std::begin(raw_path), query};
 
   if (hostport.empty()) {
-    return match_downstream_addr_group_host(router, hostport, path, pathlen,
-                                            groups, catch_all);
+    return match_downstream_addr_group_host(router, hostport, path, groups,
+                                            catch_all);
   }
 
   std::string host;
@@ -2609,7 +2587,7 @@ match_downstream_addr_group(const Router &router, const std::string &hostport,
   }
 
   util::inp_strlower(host);
-  return match_downstream_addr_group_host(router, host, path, pathlen, groups,
+  return match_downstream_addr_group_host(router, StringRef{host}, path, groups,
                                           catch_all);
 }
 

src/shrpx_config.h

@@ -301,13 +301,10 @@ struct DownstreamAddr {
 };
 
 struct DownstreamAddrGroup {
-  DownstreamAddrGroup(const std::string &pattern) : pattern(strcopy(pattern)) {}
-  DownstreamAddrGroup(const DownstreamAddrGroup &other);
-  DownstreamAddrGroup(DownstreamAddrGroup &&) = default;
-  DownstreamAddrGroup &operator=(const DownstreamAddrGroup &other);
-  DownstreamAddrGroup &operator=(DownstreamAddrGroup &&) = default;
+  DownstreamAddrGroup(const StringRef &pattern)
+      : pattern(pattern.c_str(), pattern.size()) {}
 
-  std::unique_ptr<char[]> pattern;
+  ImmutableString pattern;
   std::vector<DownstreamAddr> addrs;
 };
 
@@ -655,7 +652,7 @@ read_tls_ticket_key_file(const std::vector<std::string> &files,
 // 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::string &hostport, const std::string &path,
+    const Router &router, const StringRef &hostport, const StringRef &path,
     const std::vector<DownstreamAddrGroup> &groups, size_t catch_all);
 
 } // namespace shrpx

src/shrpx_config_test.cc

@@ -256,7 +256,7 @@ void test_shrpx_config_match_downstream_addr_group(void) {
 
   for (size_t i = 0; i < groups.size(); ++i) {
     auto &g = groups[i];
-    router.add_route(g.pattern.get(), strlen(g.pattern.get()), i);
+    router.add_route(StringRef{g.pattern}, i);
   }
 
   CU_ASSERT(0 == match_downstream_addr_group(router, "nghttp2.org", "/", groups,

src/shrpx_router.cc

@@ -66,21 +66,21 @@ void Router::add_node(RNode *node, const char *pattern, size_t patlen,
   add_next_node(node, std::move(new_node));
 }
 
-bool Router::add_route(const char *pattern, size_t patlen, size_t index) {
+bool Router::add_route(const StringRef &pattern, size_t index) {
   auto node = &root_;
   size_t i = 0;
 
   for (;;) {
     auto next_node = find_next_node(node, pattern[i]);
     if (next_node == nullptr) {
-      add_node(node, pattern + i, patlen - i, index);
+      add_node(node, pattern.c_str() + i, pattern.size() - i, index);
       return true;
     }
 
     node = next_node;
 
-    auto slen = patlen - i;
-    auto s = pattern + i;
+    auto slen = pattern.size() - i;
+    auto s = pattern.c_str() + i;
     auto n = std::min(node->len, slen);
     size_t j;
     for (j = 0; j < n && node->s[j] == s[j]; ++j)
@@ -125,8 +125,8 @@ bool Router::add_route(const char *pattern, size_t patlen, size_t index) {
 
     i += j;
 
-    assert(patlen > i);
-    add_node(node, pattern + i, patlen - i, index);
+    assert(pattern.size() > i);
+    add_node(node, pattern.c_str() + i, pattern.size() - i, index);
 
     return true;
   }
@@ -259,18 +259,16 @@ const RNode *match_partial(const RNode *node, size_t offset, const char *first,
 }
 } // namespace
 
-ssize_t Router::match(const std::string &host, const char *path,
-                      size_t pathlen) const {
+ssize_t Router::match(const StringRef &host, const StringRef &path) const {
   const RNode *node;
   size_t offset;
 
-  node =
-      match_complete(&offset, &root_, host.c_str(), host.c_str() + host.size());
+  node = match_complete(&offset, &root_, std::begin(host), std::end(host));
   if (node == nullptr) {
     return -1;
   }
 
-  node = match_partial(node, offset, path, path + pathlen);
+  node = match_partial(node, offset, std::begin(path), std::end(path));
   if (node == nullptr || node == &root_) {
     return -1;
   }

src/shrpx_router.h

@@ -55,11 +55,10 @@ struct RNode {
 class Router {
 public:
   Router();
-  // Adds route |pattern| of size |patlen| with its |index|.
-  bool add_route(const char *pattern, size_t patlen, size_t index);
+  // Adds route |pattern| with its |index|.
+  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 std::string &host, const char *path,
-                size_t pathlen) const;
+  ssize_t match(const StringRef &host, const StringRef &path) const;
 
   void add_node(RNode *node, const char *pattern, size_t patlen, size_t index);
 

src/template.h

@@ -397,6 +397,9 @@ public:
       : base(reinterpret_cast<const char *>(s)), len(n) {}
   template <typename InputIt>
   StringRef(InputIt first, InputIt last)
+      : base(&*first), len(std::distance(first, last)) {}
+  template <typename InputIt>
+  StringRef(InputIt *first, InputIt *last)
       : base(first), len(std::distance(first, last)) {}
   template <typename CharT, size_t N>
   constexpr static StringRef from_lit(const CharT(&s)[N]) {