From 288449b9bc0eb8a3c710f738a494041100ec9c91 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Fri, 10 Jun 2016 23:13:40 +0900 Subject: [PATCH 1/4] nghttpx: Rewrite wildcard router --- src/shrpx_client_handler.cc | 18 +++---- src/shrpx_config.cc | 21 +++------ src/shrpx_config.h | 8 +++- src/shrpx_router.cc | 51 ++++++++++++++++++++ src/shrpx_router.h | 3 ++ src/shrpx_worker.cc | 49 ++++++++++--------- src/shrpx_worker.h | 3 +- src/shrpx_worker_test.cc | 94 ++++++++++++++++++++----------------- 8 files changed, 158 insertions(+), 89 deletions(-) diff --git a/src/shrpx_client_handler.cc b/src/shrpx_client_handler.cc index cfe95dfe..912c712a 100644 --- a/src/shrpx_client_handler.cc +++ b/src/shrpx_client_handler.cc @@ -910,20 +910,22 @@ ClientHandler::get_downstream_connection(Downstream *downstream) { 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, wildcard_patterns, req.authority, - req.path, groups, catch_all); + group_idx = match_downstream_addr_group(router, rev_wildcard_router, + 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, wildcard_patterns, h->value, req.path, groups, catch_all); + group_idx = match_downstream_addr_group(router, rev_wildcard_router, + wildcard_patterns, h->value, + req.path, groups, catch_all); } else { - group_idx = - match_downstream_addr_group(router, wildcard_patterns, StringRef{}, - req.path, groups, catch_all); + group_idx = match_downstream_addr_group(router, rev_wildcard_router, + wildcard_patterns, StringRef{}, + req.path, groups, catch_all); } } } diff --git a/src/shrpx_config.cc b/src/shrpx_config.cc index d1cf8a23..9f31f3f3 100644 --- a/src/shrpx_config.cc +++ b/src/shrpx_config.cc @@ -828,6 +828,12 @@ 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)); + + downstreamconf.rev_wildcard_router.add_route( + StringRef{rev_host}, wildcard_patterns.size() - 1); } else { (*it).router.add_route(path, idx); } @@ -2831,21 +2837,6 @@ int configure_downstream_group(Config *config, bool http2_proxy, router = Router(); 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 diff --git a/src/shrpx_config.h b/src/shrpx_config.h index 7bfe2990..ec258cbf 100644 --- a/src/shrpx_config.h +++ b/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) @@ -616,6 +616,12 @@ struct DownstreamConfig { ev_tstamp idle_read; } timeout; 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 wildcard_patterns; std::vector addr_groups; // The index of catch-all group in downstream_addr_groups. diff --git a/src/shrpx_router.cc b/src/shrpx_router.cc index 86d5f02f..7e875aef 100644 --- a/src/shrpx_router.cc +++ b/src/shrpx_router.cc @@ -279,6 +279,57 @@ ssize_t Router::match(const StringRef &host, const StringRef &path) const { return node->index; } +namespace { +const RNode *match_prefix(const RNode *node, const char *first, + const char *last) { + if (first == last) { + return nullptr; + } + + auto p = first; + + const RNode *ans = nullptr; + + for (;;) { + auto next_node = find_next_node(node, *p); + if (next_node == nullptr) { + return ans; + } + + node = next_node; + + auto n = std::min(node->len, static_cast(last - p)); + if (memcmp(node->s, p, n) != 0) { + return ans; + } + + p += n; + + if (p != last) { + if (node->index != -1) { + ans = node; + } + continue; + } + + if (node->len == n) { + return node; + } + + return ans; + } +} +} // namespace + +ssize_t Router::match_prefix(const StringRef &s) const { + auto node = ::shrpx::match_prefix(&root_, std::begin(s), std::end(s)); + if (node == nullptr) { + return -1; + } + + return node->index; +} + namespace { void dump_node(const RNode *node, int depth) { fprintf(stderr, "%*ss='%.*s', len=%zu, index=%zd\n", depth, "", diff --git a/src/shrpx_router.h b/src/shrpx_router.h index 2381a96c..7d3a588e 100644 --- a/src/shrpx_router.h +++ b/src/shrpx_router.h @@ -67,6 +67,9 @@ 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. + ssize_t match_prefix(const StringRef &s) const; void add_node(RNode *node, const char *pattern, size_t patlen, size_t index); diff --git a/src/shrpx_worker.cc b/src/shrpx_worker.cc index c0f9ee01..b5651f75 100644 --- a/src/shrpx_worker.cc +++ b/src/shrpx_worker.cc @@ -456,7 +456,8 @@ ConnectionHandler *Worker::get_connection_handler() const { namespace { size_t match_downstream_addr_group_host( - const Router &router, const std::vector &wildcard_patterns, + const Router &router, const Router &rev_wildcard_router, + const std::vector &wildcard_patterns, const StringRef &host, const StringRef &path, const std::vector> &groups, size_t catch_all) { @@ -486,23 +487,24 @@ 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 = std::string{std::begin(host) + 1, std::end(host)}; + std::reverse(std::begin(rev_host), std::end(rev_host)); + + auto wcidx = rev_wildcard_router.match_prefix( + StringRef{std::begin(rev_host), std::end(rev_host)}); + if (wcidx != -1) { + 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; + } + return group; } - return group; } } @@ -523,7 +525,8 @@ size_t match_downstream_addr_group_host( } // namespace size_t match_downstream_addr_group( - const Router &router, const std::vector &wildcard_patterns, + const Router &router, const Router &rev_wildcard_router, + const std::vector &wildcard_patterns, const StringRef &hostport, const StringRef &raw_path, const std::vector> &groups, size_t catch_all) { @@ -539,8 +542,9 @@ 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(router, rev_wildcard_router, + wildcard_patterns, hostport, path, + groups, catch_all); } StringRef host; @@ -570,8 +574,9 @@ size_t match_downstream_addr_group( util::inp_strlower(low_host); host = StringRef{low_host}; } - return match_downstream_addr_group_host(router, wildcard_patterns, host, path, - groups, catch_all); + return match_downstream_addr_group_host(router, rev_wildcard_router, + wildcard_patterns, host, path, groups, + catch_all); } void downstream_failure(DownstreamAddr *addr) { diff --git a/src/shrpx_worker.h b/src/shrpx_worker.h index 2a0a2965..3dbd31e6 100644 --- a/src/shrpx_worker.h +++ b/src/shrpx_worker.h @@ -280,7 +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 std::vector &wildcard_patterns, + const Router &router, const Router &rev_wildcard_router, + const std::vector &wildcard_patterns, const StringRef &hostport, const StringRef &path, const std::vector> &groups, size_t catch_all); diff --git a/src/shrpx_worker_test.cc b/src/shrpx_worker_test.cc index c4f60b4f..7c941d77 100644 --- a/src/shrpx_worker_test.cc +++ b/src/shrpx_worker_test.cc @@ -50,6 +50,7 @@ void test_shrpx_worker_match_downstream_addr_group(void) { } Router router; + Router wcrouter; for (size_t i = 0; i < groups.size(); ++i) { auto &g = groups[i]; @@ -59,122 +60,124 @@ void test_shrpx_worker_match_downstream_addr_group(void) { std::vector wp; CU_ASSERT(0 == match_downstream_addr_group( - router, wp, StringRef::from_lit("nghttp2.org"), + router, wcrouter, wp, StringRef::from_lit("nghttp2.org"), StringRef::from_lit("/"), groups, 255)); // port is removed - CU_ASSERT(0 == match_downstream_addr_group( - router, wp, StringRef::from_lit("nghttp2.org:8080"), - StringRef::from_lit("/"), groups, 255)); + CU_ASSERT(0 == + match_downstream_addr_group(router, wcrouter, wp, + StringRef::from_lit("nghttp2.org:8080"), + StringRef::from_lit("/"), groups, 255)); // host is case-insensitive CU_ASSERT(4 == match_downstream_addr_group( - router, wp, StringRef::from_lit("WWW.nghttp2.org"), + router, wcrouter, wp, + StringRef::from_lit("WWW.nghttp2.org"), StringRef::from_lit("/alpha"), groups, 255)); CU_ASSERT(1 == match_downstream_addr_group( - router, wp, StringRef::from_lit("nghttp2.org"), + router, wcrouter, wp, 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, wp, StringRef::from_lit("nghttp2.org"), + router, wcrouter, wp, 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, wp, StringRef::from_lit("nghttp2.org"), + router, wcrouter, wp, StringRef::from_lit("nghttp2.org"), StringRef::from_lit("/Alpha/bravo"), groups, 255)); CU_ASSERT(1 == match_downstream_addr_group( - router, wp, StringRef::from_lit("nghttp2.org"), + router, wcrouter, wp, StringRef::from_lit("nghttp2.org"), StringRef::from_lit("/alpha/bravo/charlie"), groups, 255)); CU_ASSERT(2 == match_downstream_addr_group( - router, wp, StringRef::from_lit("nghttp2.org"), + router, wcrouter, wp, 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, wp, StringRef::from_lit("nghttp2.org"), + router, wcrouter, wp, StringRef::from_lit("nghttp2.org"), StringRef::from_lit("/alpha/charlie/"), groups, 255)); CU_ASSERT(255 == match_downstream_addr_group( - router, wp, StringRef::from_lit("example.org"), + router, wcrouter, wp, StringRef::from_lit("example.org"), StringRef::from_lit("/"), groups, 255)); - CU_ASSERT(255 == - match_downstream_addr_group(router, wp, StringRef::from_lit(""), - StringRef::from_lit("/"), groups, 255)); + CU_ASSERT(255 == match_downstream_addr_group( + router, wcrouter, wp, StringRef::from_lit(""), + StringRef::from_lit("/"), groups, 255)); CU_ASSERT(255 == match_downstream_addr_group( - router, wp, StringRef::from_lit(""), + router, wcrouter, wp, StringRef::from_lit(""), StringRef::from_lit("alpha"), groups, 255)); CU_ASSERT(255 == match_downstream_addr_group( - router, wp, StringRef::from_lit("foo/bar"), + router, wcrouter, wp, 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, wp, StringRef::from_lit("nghttp2.org"), + router, wcrouter, wp, StringRef::from_lit("nghttp2.org"), StringRef::from_lit("*"), groups, 255)); - CU_ASSERT( - 5 == match_downstream_addr_group(router, wp, StringRef::from_lit("[::1]"), - StringRef::from_lit("/"), groups, 255)); CU_ASSERT(5 == match_downstream_addr_group( - router, wp, StringRef::from_lit("[::1]:8080"), + router, wcrouter, wp, StringRef::from_lit("[::1]"), + 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(router, wp, StringRef::from_lit("[::1"), - StringRef::from_lit("/"), groups, 255)); CU_ASSERT(255 == match_downstream_addr_group( - router, wp, StringRef::from_lit("[::1]8000"), + router, wcrouter, wp, StringRef::from_lit("[::1"), + 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, wp, StringRef::from_lit("nghttp2.org"), + router, wcrouter, wp, StringRef::from_lit("nghttp2.org"), StringRef::from_lit("/alpha/bravo/delta"), groups, 255)); CU_ASSERT(1 == match_downstream_addr_group( - router, wp, StringRef::from_lit("nghttp2.org"), + router, wcrouter, wp, 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, wp, StringRef::from_lit("example.com"), + router, wcrouter, wp, StringRef::from_lit("example.com"), StringRef::from_lit("/alpha/bravo"), groups, 255)); CU_ASSERT(255 == match_downstream_addr_group( - router, wp, StringRef::from_lit("example.com"), + router, wcrouter, wp, StringRef::from_lit("example.com"), StringRef::from_lit("/alpha/bravo/"), groups, 255)); CU_ASSERT(255 == match_downstream_addr_group( - router, wp, StringRef::from_lit("example.com"), + router, wcrouter, wp, 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, wp, StringRef::from_lit("192.168.0.1"), + router, wcrouter, wp, StringRef::from_lit("192.168.0.1"), StringRef::from_lit("/alpha"), groups, 255)); CU_ASSERT(8 == match_downstream_addr_group( - router, wp, StringRef::from_lit("192.168.0.1"), + router, wcrouter, wp, StringRef::from_lit("192.168.0.1"), StringRef::from_lit("/alpha/"), groups, 255)); CU_ASSERT(8 == match_downstream_addr_group( - router, wp, StringRef::from_lit("192.168.0.1"), + router, wcrouter, wp, StringRef::from_lit("192.168.0.1"), StringRef::from_lit("/alpha/bravo"), groups, 255)); CU_ASSERT(255 == match_downstream_addr_group( - router, wp, StringRef::from_lit("192.168.0.1"), + router, wcrouter, wp, StringRef::from_lit("192.168.0.1"), StringRef::from_lit("/alph"), groups, 255)); CU_ASSERT(255 == match_downstream_addr_group( - router, wp, StringRef::from_lit("192.168.0.1"), + router, wcrouter, wp, StringRef::from_lit("192.168.0.1"), StringRef::from_lit("/"), groups, 255)); // Test for wildcard hosts @@ -187,34 +190,41 @@ void test_shrpx_worker_match_downstream_addr_group(void) { groups.push_back(std::move(g2)); wp.emplace_back(StringRef::from_lit("git.nghttp2.org")); + wcrouter.add_route(StringRef::from_lit("gro.2ptthgn.tig"), 0); wp.back().router.add_route(StringRef::from_lit("/echo/"), 10); wp.emplace_back(StringRef::from_lit(".nghttp2.org")); + wcrouter.add_route(StringRef::from_lit("gro.2ptthgn."), 1); wp.back().router.add_route(StringRef::from_lit("/echo/"), 11); wp.back().router.add_route(StringRef::from_lit("/echo/foxtrot"), 12); CU_ASSERT(11 == match_downstream_addr_group( - router, wp, StringRef::from_lit("git.nghttp2.org"), + router, wcrouter, wp, + StringRef::from_lit("git.nghttp2.org"), StringRef::from_lit("/echo"), groups, 255)); CU_ASSERT(10 == match_downstream_addr_group( - router, wp, StringRef::from_lit("0git.nghttp2.org"), + router, wcrouter, wp, + StringRef::from_lit("0git.nghttp2.org"), StringRef::from_lit("/echo"), groups, 255)); CU_ASSERT(11 == match_downstream_addr_group( - router, wp, StringRef::from_lit("it.nghttp2.org"), + router, wcrouter, wp, + StringRef::from_lit("it.nghttp2.org"), StringRef::from_lit("/echo"), groups, 255)); CU_ASSERT(255 == match_downstream_addr_group( - router, wp, StringRef::from_lit(".nghttp2.org"), + router, wcrouter, wp, + StringRef::from_lit(".nghttp2.org"), StringRef::from_lit("/echo/foxtrot"), groups, 255)); CU_ASSERT(9 == match_downstream_addr_group( - router, wp, StringRef::from_lit("alpha.nghttp2.org"), + router, wcrouter, wp, + StringRef::from_lit("alpha.nghttp2.org"), StringRef::from_lit("/golf"), groups, 255)); CU_ASSERT(0 == match_downstream_addr_group( - router, wp, StringRef::from_lit("nghttp2.org"), + router, wcrouter, wp, StringRef::from_lit("nghttp2.org"), StringRef::from_lit("/echo"), groups, 255)); } From 084206baceacbf3244c3047af1a51f69b0ec74ff Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 11 Jun 2016 13:31:04 +0900 Subject: [PATCH 2/4] nghttpx: Handle edge case wildcard pattern and add tests Suppose the wildcard patterns follows: - *.nghttp2.org/foo - *.img.nghttp2.org/bar Previously, s.img.nghttp2.org/foo does not match anything. Now it matches first pattern. --- src/CMakeLists.txt | 1 + src/Makefile.am | 1 + src/shrpx-unittest.cc | 4 ++ src/shrpx_router.cc | 26 +++++--- src/shrpx_router.h | 11 +++- src/shrpx_router_test.cc | 129 +++++++++++++++++++++++++++++++++++++++ src/shrpx_router_test.h | 39 ++++++++++++ src/shrpx_worker.cc | 28 +++++++-- 8 files changed, 222 insertions(+), 17 deletions(-) create mode 100644 src/shrpx_router_test.cc create mode 100644 src/shrpx_router_test.h diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index e3822289..41fbb68f 100644 --- a/src/CMakeLists.txt +++ b/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 diff --git a/src/Makefile.am b/src/Makefile.am index 1d671c48..58f5f564 100644 --- a/src/Makefile.am +++ b/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 \ diff --git a/src/shrpx-unittest.cc b/src/shrpx-unittest.cc index df6d6b9c..12f7e84e 100644 --- a/src/shrpx-unittest.cc +++ b/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", diff --git a/src/shrpx_router.cc b/src/shrpx_router.cc index 7e875aef..caf8383a 100644 --- a/src/shrpx_router.cc +++ b/src/shrpx_router.cc @@ -280,7 +280,7 @@ ssize_t Router::match(const StringRef &host, const StringRef &path) const { } namespace { -const RNode *match_prefix(const RNode *node, const char *first, +const RNode *match_prefix(size_t *nread, const RNode *node, const char *first, const char *last) { if (first == last) { return nullptr; @@ -288,45 +288,53 @@ const RNode *match_prefix(const RNode *node, const char *first, auto p = first; - const RNode *ans = nullptr; - for (;;) { auto next_node = find_next_node(node, *p); if (next_node == nullptr) { - return ans; + return nullptr; } node = next_node; auto n = std::min(node->len, static_cast(last - p)); if (memcmp(node->s, p, n) != 0) { - return ans; + return nullptr; } p += n; if (p != last) { if (node->index != -1) { - ans = node; + *nread = p - first; + return node; } continue; } if (node->len == n) { + *nread = p - first; return node; } - return ans; + return nullptr; } } } // namespace -ssize_t Router::match_prefix(const StringRef &s) const { - auto node = ::shrpx::match_prefix(&root_, std::begin(s), std::end(s)); +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; } diff --git a/src/shrpx_router.h b/src/shrpx_router.h index 7d3a588e..b193e64a 100644 --- a/src/shrpx_router.h +++ b/src/shrpx_router.h @@ -68,8 +68,15 @@ public: // 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. - ssize_t match_prefix(const StringRef &s) const; + // |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); diff --git a/src/shrpx_router_test.cc b/src/shrpx_router_test.cc new file mode 100644 index 00000000..0248181c --- /dev/null +++ b/src/shrpx_router_test.cc @@ -0,0 +1,129 @@ +/* + * 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 + +#include "shrpx_router.h" + +namespace shrpx { + +struct Pattern { + StringRef pattern; + size_t idx; +}; + +void test_shrpx_router_match(void) { + auto patterns = std::vector{ + {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{ + {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 diff --git a/src/shrpx_router_test.h b/src/shrpx_router_test.h new file mode 100644 index 00000000..9f4ba665 --- /dev/null +++ b/src/shrpx_router_test.h @@ -0,0 +1,39 @@ +/* + * 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 +#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 diff --git a/src/shrpx_worker.cc b/src/shrpx_worker.cc index b5651f75..d05d2922 100644 --- a/src/shrpx_worker.cc +++ b/src/shrpx_worker.cc @@ -488,12 +488,23 @@ size_t match_downstream_addr_group_host( } if (!wildcard_patterns.empty() && !host.empty()) { - auto rev_host = std::string{std::begin(host) + 1, std::end(host)}; - std::reverse(std::begin(rev_host), std::end(rev_host)); + auto rev_host_src = std::string{std::begin(host) + 1, std::end(host)}; + std::reverse(std::begin(rev_host_src), std::end(rev_host_src)); + + ssize_t best_group = -1; + const RNode *last_node = nullptr; + auto rev_host = StringRef{std::begin(rev_host_src), std::end(rev_host_src)}; + + for (;;) { + size_t nread = 0; + auto wcidx = + rev_wildcard_router.match_prefix(&nread, &last_node, rev_host); + if (wcidx == -1) { + break; + } + + rev_host = StringRef{std::begin(rev_host) + nread, std::end(rev_host)}; - auto wcidx = rev_wildcard_router.match_prefix( - StringRef{std::begin(rev_host), std::end(rev_host)}); - if (wcidx != -1) { auto &wc = wildcard_patterns[wcidx]; auto group = wc.router.match(StringRef{}, path); if (group != -1) { @@ -503,9 +514,14 @@ size_t match_downstream_addr_group_host( LOG(INFO) << "Found wildcard pattern with query " << host << path << ", matched pattern=" << groups[group]->pattern; } - return group; + + best_group = group; } } + + if (best_group != -1) { + return best_group; + } } group = router.match(StringRef::from_lit(""), path); From a809da68a3361b6ea85ad12112f664b713903697 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 11 Jun 2016 18:21:37 +0900 Subject: [PATCH 3/4] nghttpx: Aggregate router configuration into one struct --- src/shrpx_client_handler.cc | 15 ++---- src/shrpx_config.cc | 22 ++++---- src/shrpx_config.h | 17 +++--- src/shrpx_worker.cc | 23 +++++---- src/shrpx_worker.h | 5 +- src/shrpx_worker_test.cc | 100 +++++++++++++++++------------------- 6 files changed, 90 insertions(+), 92 deletions(-) diff --git a/src/shrpx_client_handler.cc b/src/shrpx_client_handler.cc index 912c712a..33cb1225 100644 --- a/src/shrpx_client_handler.cc +++ b/src/shrpx_client_handler.cc @@ -889,6 +889,7 @@ std::unique_ptr 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, - wildcard_patterns, req.authority, + group_idx = match_downstream_addr_group(routerconf, 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, - wildcard_patterns, h->value, - req.path, groups, catch_all); + group_idx = match_downstream_addr_group(routerconf, h->value, req.path, + groups, catch_all); } else { - group_idx = match_downstream_addr_group(router, rev_wildcard_router, - wildcard_patterns, StringRef{}, + group_idx = match_downstream_addr_group(routerconf, StringRef{}, req.path, groups, catch_all); } } diff --git a/src/shrpx_config.cc b/src/shrpx_config.cc index 9f31f3f3..1a21e34d 100644 --- a/src/shrpx_config.cc +++ b/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( - StringRef{rev_host}, wildcard_patterns.size() - 1); + 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; } @@ -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().swap(downstreamconf.wildcard_patterns); std::vector().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)); } diff --git a/src/shrpx_config.h b/src/shrpx_config.h index ec258cbf..1165de4a 100644 --- a/src/shrpx_config.h +++ b/src/shrpx_config.h @@ -609,12 +609,8 @@ struct WildcardPattern { Router router; }; -struct DownstreamConfig { - struct { - ev_tstamp read; - ev_tstamp write; - ev_tstamp idle_read; - } timeout; +// 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() @@ -623,6 +619,15 @@ struct DownstreamConfig { // The index stored in this router is index of wildcard_patterns. Router rev_wildcard_router; std::vector wildcard_patterns; +}; + +struct DownstreamConfig { + struct { + ev_tstamp read; + ev_tstamp write; + ev_tstamp idle_read; + } timeout; + RouterConfig router; std::vector addr_groups; // The index of catch-all group in downstream_addr_groups. size_t addr_group_catch_all; diff --git a/src/shrpx_worker.cc b/src/shrpx_worker.cc index d05d2922..3e7b47df 100644 --- a/src/shrpx_worker.cc +++ b/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 std::vector &wildcard_patterns, - const StringRef &host, const StringRef &path, + const RouterConfig &routerconf, const StringRef &host, + const StringRef &path, const std::vector> &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 std::vector &wildcard_patterns, - const StringRef &hostport, const StringRef &raw_path, + const RouterConfig &routerconf, const StringRef &hostport, + const StringRef &raw_path, const std::vector> &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, - wildcard_patterns, hostport, path, - groups, catch_all); + return match_downstream_addr_group_host(routerconf, hostport, path, 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, - wildcard_patterns, host, path, groups, + return match_downstream_addr_group_host(routerconf, host, path, groups, catch_all); } diff --git a/src/shrpx_worker.h b/src/shrpx_worker.h index 3dbd31e6..7e506029 100644 --- a/src/shrpx_worker.h +++ b/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 std::vector &wildcard_patterns, - const StringRef &hostport, const StringRef &path, + const RouterConfig &routerconfig, const StringRef &hostport, + const StringRef &path, const std::vector> &groups, size_t catch_all); diff --git a/src/shrpx_worker_test.cc b/src/shrpx_worker_test.cc index 7c941d77..5294433f 100644 --- a/src/shrpx_worker_test.cc +++ b/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; - Router wcrouter; + RouterConfig routerconf; + + 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 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 == - match_downstream_addr_group(router, wcrouter, wp, - StringRef::from_lit("nghttp2.org:8080"), - StringRef::from_lit("/"), groups, 255)); + CU_ASSERT(0 == match_downstream_addr_group( + routerconf, StringRef::from_lit("nghttp2.org:8080"), + StringRef::from_lit("/"), groups, 255)); // host is case-insensitive CU_ASSERT(4 == match_downstream_addr_group( - router, wcrouter, wp, - StringRef::from_lit("WWW.nghttp2.org"), + routerconf, 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( - router, wcrouter, wp, StringRef::from_lit(""), - StringRef::from_lit("/"), groups, 255)); + CU_ASSERT(255 == + match_downstream_addr_group(routerconf, StringRef::from_lit(""), + 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]"), - StringRef::from_lit("/"), groups, 255)); - CU_ASSERT(5 == match_downstream_addr_group( - router, wcrouter, wp, StringRef::from_lit("[::1]:8080"), + routerconf, 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"), - StringRef::from_lit("/"), groups, 255)); - CU_ASSERT(255 == match_downstream_addr_group( - router, wcrouter, wp, StringRef::from_lit("[::1]8000"), + routerconf, 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, - StringRef::from_lit("git.nghttp2.org"), + routerconf, StringRef::from_lit("git.nghttp2.org"), StringRef::from_lit("/echo"), groups, 255)); CU_ASSERT(10 == match_downstream_addr_group( - router, wcrouter, wp, - StringRef::from_lit("0git.nghttp2.org"), + routerconf, StringRef::from_lit("0git.nghttp2.org"), StringRef::from_lit("/echo"), groups, 255)); CU_ASSERT(11 == match_downstream_addr_group( - router, wcrouter, wp, - StringRef::from_lit("it.nghttp2.org"), + routerconf, StringRef::from_lit("it.nghttp2.org"), StringRef::from_lit("/echo"), groups, 255)); CU_ASSERT(255 == match_downstream_addr_group( - router, wcrouter, wp, - StringRef::from_lit(".nghttp2.org"), + routerconf, StringRef::from_lit(".nghttp2.org"), StringRef::from_lit("/echo/foxtrot"), groups, 255)); CU_ASSERT(9 == match_downstream_addr_group( - router, wcrouter, wp, - StringRef::from_lit("alpha.nghttp2.org"), + routerconf, 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)); } From c06e8c89ff1a9035d5c885cb869ee4006853f567 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 11 Jun 2016 18:41:43 +0900 Subject: [PATCH 4/4] nghttpx: Use BlockAllocator in match_downstream_addr_group --- src/shrpx_client_handler.cc | 12 +++-- src/shrpx_worker.cc | 25 +++++----- src/shrpx_worker.h | 2 +- src/shrpx_worker_test.cc | 91 ++++++++++++++++++++----------------- 4 files changed, 72 insertions(+), 58 deletions(-) diff --git a/src/shrpx_client_handler.cc b/src/shrpx_client_handler.cc index 33cb1225..d1e4c25d 100644 --- a/src/shrpx_client_handler.cc +++ b/src/shrpx_client_handler.cc @@ -910,17 +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 &balloc = downstream->get_block_allocator(); + if (!req.authority.empty()) { - group_idx = match_downstream_addr_group(routerconf, 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(routerconf, h->value, req.path, - groups, catch_all); + groups, catch_all, balloc); } else { - group_idx = match_downstream_addr_group(routerconf, StringRef{}, - req.path, groups, catch_all); + group_idx = match_downstream_addr_group( + routerconf, StringRef{}, req.path, groups, catch_all, balloc); } } } diff --git a/src/shrpx_worker.cc b/src/shrpx_worker.cc index 3e7b47df..d5c6967f 100644 --- a/src/shrpx_worker.cc +++ b/src/shrpx_worker.cc @@ -459,7 +459,7 @@ size_t match_downstream_addr_group_host( const RouterConfig &routerconf, const StringRef &host, const StringRef &path, const std::vector> &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; @@ -492,12 +492,14 @@ size_t match_downstream_addr_group_host( } if (!wildcard_patterns.empty() && !host.empty()) { - auto rev_host_src = std::string{std::begin(host) + 1, std::end(host)}; - std::reverse(std::begin(rev_host_src), std::end(rev_host_src)); + 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; - auto rev_host = StringRef{std::begin(rev_host_src), std::end(rev_host_src)}; for (;;) { size_t nread = 0; @@ -548,7 +550,7 @@ size_t match_downstream_addr_group( const RouterConfig &routerconf, const StringRef &hostport, const StringRef &raw_path, const std::vector> &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 @@ -562,7 +564,7 @@ size_t match_downstream_addr_group( if (hostport.empty()) { return match_downstream_addr_group_host(routerconf, hostport, path, groups, - catch_all); + catch_all, balloc); } StringRef host; @@ -584,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(routerconf, host, path, groups, - catch_all); + catch_all, balloc); } void downstream_failure(DownstreamAddr *addr) { diff --git a/src/shrpx_worker.h b/src/shrpx_worker.h index 7e506029..c729af1e 100644 --- a/src/shrpx_worker.h +++ b/src/shrpx_worker.h @@ -283,7 +283,7 @@ size_t match_downstream_addr_group( const RouterConfig &routerconfig, const StringRef &hostport, const StringRef &path, const std::vector> &groups, - size_t catch_all); + size_t catch_all, BlockAllocator &balloc); void downstream_failure(DownstreamAddr *addr); diff --git a/src/shrpx_worker_test.cc b/src/shrpx_worker_test.cc index 5294433f..2e419416 100644 --- a/src/shrpx_worker_test.cc +++ b/src/shrpx_worker_test.cc @@ -49,6 +49,7 @@ void test_shrpx_worker_match_downstream_addr_group(void) { groups.push_back(std::move(g)); } + BlockAllocator balloc(1024, 1024); RouterConfig routerconf; auto &router = routerconf.router; @@ -62,122 +63,129 @@ void test_shrpx_worker_match_downstream_addr_group(void) { CU_ASSERT(0 == match_downstream_addr_group( routerconf, StringRef::from_lit("nghttp2.org"), - StringRef::from_lit("/"), groups, 255)); + StringRef::from_lit("/"), groups, 255, balloc)); // port is removed CU_ASSERT(0 == match_downstream_addr_group( routerconf, StringRef::from_lit("nghttp2.org:8080"), - StringRef::from_lit("/"), groups, 255)); + StringRef::from_lit("/"), groups, 255, balloc)); // host is case-insensitive CU_ASSERT(4 == match_downstream_addr_group( routerconf, StringRef::from_lit("WWW.nghttp2.org"), - StringRef::from_lit("/alpha"), groups, 255)); + StringRef::from_lit("/alpha"), groups, 255, balloc)); CU_ASSERT(1 == match_downstream_addr_group( routerconf, StringRef::from_lit("nghttp2.org"), - StringRef::from_lit("/alpha/bravo/"), groups, 255)); + StringRef::from_lit("/alpha/bravo/"), groups, 255, + balloc)); // /alpha/bravo also matches /alpha/bravo/ CU_ASSERT(1 == match_downstream_addr_group( routerconf, StringRef::from_lit("nghttp2.org"), - StringRef::from_lit("/alpha/bravo"), groups, 255)); + StringRef::from_lit("/alpha/bravo"), groups, 255, balloc)); // path part is case-sensitive CU_ASSERT(0 == match_downstream_addr_group( routerconf, StringRef::from_lit("nghttp2.org"), - StringRef::from_lit("/Alpha/bravo"), groups, 255)); + StringRef::from_lit("/Alpha/bravo"), groups, 255, balloc)); CU_ASSERT(1 == match_downstream_addr_group( routerconf, StringRef::from_lit("nghttp2.org"), - StringRef::from_lit("/alpha/bravo/charlie"), groups, 255)); + StringRef::from_lit("/alpha/bravo/charlie"), groups, 255, + balloc)); CU_ASSERT(2 == match_downstream_addr_group( routerconf, StringRef::from_lit("nghttp2.org"), - StringRef::from_lit("/alpha/charlie"), groups, 255)); + StringRef::from_lit("/alpha/charlie"), groups, 255, + balloc)); // 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( routerconf, StringRef::from_lit("nghttp2.org"), - StringRef::from_lit("/alpha/charlie/"), groups, 255)); + StringRef::from_lit("/alpha/charlie/"), groups, 255, + balloc)); CU_ASSERT(255 == match_downstream_addr_group( routerconf, StringRef::from_lit("example.org"), - StringRef::from_lit("/"), groups, 255)); - - CU_ASSERT(255 == - match_downstream_addr_group(routerconf, StringRef::from_lit(""), - StringRef::from_lit("/"), groups, 255)); + StringRef::from_lit("/"), groups, 255, balloc)); CU_ASSERT(255 == match_downstream_addr_group( routerconf, StringRef::from_lit(""), - StringRef::from_lit("alpha"), groups, 255)); + StringRef::from_lit("/"), groups, 255, balloc)); + + CU_ASSERT(255 == match_downstream_addr_group( + routerconf, StringRef::from_lit(""), + StringRef::from_lit("alpha"), groups, 255, balloc)); CU_ASSERT(255 == match_downstream_addr_group( routerconf, StringRef::from_lit("foo/bar"), - StringRef::from_lit("/"), groups, 255)); + StringRef::from_lit("/"), groups, 255, balloc)); // If path is StringRef::from_lit("*", only match with host + "/"). CU_ASSERT(0 == match_downstream_addr_group( routerconf, StringRef::from_lit("nghttp2.org"), - StringRef::from_lit("*"), groups, 255)); + StringRef::from_lit("*"), groups, 255, balloc)); - CU_ASSERT( - 5 == match_downstream_addr_group(routerconf, StringRef::from_lit("[::1]"), - StringRef::from_lit("/"), groups, 255)); + CU_ASSERT(5 == match_downstream_addr_group( + routerconf, StringRef::from_lit("[::1]"), + StringRef::from_lit("/"), groups, 255, balloc)); CU_ASSERT(5 == match_downstream_addr_group( routerconf, 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)); + StringRef::from_lit("/"), groups, 255, balloc)); + CU_ASSERT(255 == match_downstream_addr_group( + routerconf, StringRef::from_lit("[::1"), + StringRef::from_lit("/"), groups, 255, balloc)); CU_ASSERT(255 == match_downstream_addr_group( routerconf, StringRef::from_lit("[::1]8000"), - StringRef::from_lit("/"), groups, 255)); + StringRef::from_lit("/"), groups, 255, balloc)); // Check the case where adding route extends tree CU_ASSERT(6 == match_downstream_addr_group( routerconf, StringRef::from_lit("nghttp2.org"), - StringRef::from_lit("/alpha/bravo/delta"), groups, 255)); + StringRef::from_lit("/alpha/bravo/delta"), groups, 255, + balloc)); CU_ASSERT(1 == match_downstream_addr_group( routerconf, StringRef::from_lit("nghttp2.org"), - StringRef::from_lit("/alpha/bravo/delta/"), groups, 255)); + StringRef::from_lit("/alpha/bravo/delta/"), groups, 255, + balloc)); // Check the case where query is done in a single node CU_ASSERT(7 == match_downstream_addr_group( routerconf, StringRef::from_lit("example.com"), - StringRef::from_lit("/alpha/bravo"), groups, 255)); + StringRef::from_lit("/alpha/bravo"), groups, 255, balloc)); CU_ASSERT(255 == match_downstream_addr_group( routerconf, StringRef::from_lit("example.com"), - StringRef::from_lit("/alpha/bravo/"), groups, 255)); + StringRef::from_lit("/alpha/bravo/"), groups, 255, + balloc)); CU_ASSERT(255 == match_downstream_addr_group( routerconf, StringRef::from_lit("example.com"), - StringRef::from_lit("/alpha"), groups, 255)); + StringRef::from_lit("/alpha"), groups, 255, balloc)); // Check the case where quey is done in a single node CU_ASSERT(8 == match_downstream_addr_group( routerconf, StringRef::from_lit("192.168.0.1"), - StringRef::from_lit("/alpha"), groups, 255)); + StringRef::from_lit("/alpha"), groups, 255, balloc)); CU_ASSERT(8 == match_downstream_addr_group( routerconf, StringRef::from_lit("192.168.0.1"), - StringRef::from_lit("/alpha/"), groups, 255)); + StringRef::from_lit("/alpha/"), groups, 255, balloc)); CU_ASSERT(8 == match_downstream_addr_group( routerconf, StringRef::from_lit("192.168.0.1"), - StringRef::from_lit("/alpha/bravo"), groups, 255)); + StringRef::from_lit("/alpha/bravo"), groups, 255, balloc)); CU_ASSERT(255 == match_downstream_addr_group( routerconf, StringRef::from_lit("192.168.0.1"), - StringRef::from_lit("/alph"), groups, 255)); + StringRef::from_lit("/alph"), groups, 255, balloc)); CU_ASSERT(255 == match_downstream_addr_group( routerconf, StringRef::from_lit("192.168.0.1"), - StringRef::from_lit("/"), groups, 255)); + StringRef::from_lit("/"), groups, 255, balloc)); // Test for wildcard hosts auto g1 = std::make_shared(); @@ -199,27 +207,28 @@ void test_shrpx_worker_match_downstream_addr_group(void) { CU_ASSERT(11 == match_downstream_addr_group( routerconf, StringRef::from_lit("git.nghttp2.org"), - StringRef::from_lit("/echo"), groups, 255)); + StringRef::from_lit("/echo"), groups, 255, balloc)); CU_ASSERT(10 == match_downstream_addr_group( routerconf, StringRef::from_lit("0git.nghttp2.org"), - StringRef::from_lit("/echo"), groups, 255)); + StringRef::from_lit("/echo"), groups, 255, balloc)); CU_ASSERT(11 == match_downstream_addr_group( routerconf, StringRef::from_lit("it.nghttp2.org"), - StringRef::from_lit("/echo"), groups, 255)); + StringRef::from_lit("/echo"), groups, 255, balloc)); CU_ASSERT(255 == match_downstream_addr_group( routerconf, StringRef::from_lit(".nghttp2.org"), - StringRef::from_lit("/echo/foxtrot"), groups, 255)); + StringRef::from_lit("/echo/foxtrot"), groups, 255, + balloc)); CU_ASSERT(9 == match_downstream_addr_group( routerconf, StringRef::from_lit("alpha.nghttp2.org"), - StringRef::from_lit("/golf"), groups, 255)); + StringRef::from_lit("/golf"), groups, 255, balloc)); CU_ASSERT(0 == match_downstream_addr_group( routerconf, StringRef::from_lit("nghttp2.org"), - StringRef::from_lit("/echo"), groups, 255)); + StringRef::from_lit("/echo"), groups, 255, balloc)); } } // namespace shrpx