diff --git a/src/http2.cc b/src/http2.cc index 28314134..3a4474b8 100644 --- a/src/http2.cc +++ b/src/http2.cc @@ -1742,7 +1742,7 @@ StringRef normalize_path(BlockAllocator &balloc, const StringRef &path, auto p = result.base; auto it = std::begin(path); - for (; it + 2 != std::end(path);) { + for (; it + 2 < std::end(path);) { if (*it == '%') { if (util::is_hex_digit(*(it + 1)) && util::is_hex_digit(*(it + 2))) { auto c = @@ -1773,6 +1773,12 @@ StringRef normalize_path(BlockAllocator &balloc, const StringRef &path, query); } +std::string normalize_path(const StringRef &path, const StringRef &query) { + BlockAllocator balloc(1024, 1024); + + return normalize_path(balloc, path, query).str(); +} + StringRef rewrite_clean_path(BlockAllocator &balloc, const StringRef &src) { if (src.empty() || src[0] != '/') { return src; diff --git a/src/http2.h b/src/http2.h index 9b8b0bbb..cc295079 100644 --- a/src/http2.h +++ b/src/http2.h @@ -364,58 +364,11 @@ int lookup_method_token(const StringRef &name); // StringRef is guaranteed to be NULL-terminated. StringRef to_method_string(int method_token); -template -std::string normalize_path(InputIt first, InputIt last) { - // First, decode %XX for unreserved characters, then do - // http2::join_path - std::string result; - // We won't find %XX if length is less than 3. - if (last - first < 3) { - result.assign(first, last); - } else { - for (; first < last - 2;) { - if (*first == '%') { - if (util::is_hex_digit(*(first + 1)) && - util::is_hex_digit(*(first + 2))) { - auto c = (util::hex_to_uint(*(first + 1)) << 4) + - util::hex_to_uint(*(first + 2)); - if (util::in_rfc3986_unreserved_chars(c)) { - result += c; - first += 3; - continue; - } - result += '%'; - result += util::upcase(*(first + 1)); - result += util::upcase(*(first + 2)); - first += 3; - continue; - } - } - result += *first++; - } - result.append(first, last); - } - return path_join(StringRef{}, StringRef{}, StringRef{result}, StringRef{}); -} - -template -std::string rewrite_clean_path(InputIt first, InputIt last) { - if (first == last || *first != '/') { - return std::string(first, last); - } - // probably, not necessary most of the case, but just in case. - auto fragment = std::find(first, last, '#'); - auto query = std::find(first, fragment, '?'); - auto path = normalize_path(first, query); - if (query != fragment) { - path.append(query, fragment); - } - return path; -} - StringRef normalize_path(BlockAllocator &balloc, const StringRef &path, const StringRef &query); +std::string normalize_path(const StringRef &path, const StringRef &query); + StringRef rewrite_clean_path(BlockAllocator &balloc, const StringRef &src); // Returns path component of |uri|. The returned path does not diff --git a/src/http2_test.cc b/src/http2_test.cc index 4d4c7669..5ac726aa 100644 --- a/src/http2_test.cc +++ b/src/http2_test.cc @@ -46,7 +46,7 @@ using namespace nghttp2; namespace shrpx { namespace { -void check_nv(const Header &a, const nghttp2_nv *b) { +void check_nv(const HeaderRef &a, const nghttp2_nv *b) { CU_ASSERT(a.name.size() == b->namelen); CU_ASSERT(a.value.size() == b->valuelen); CU_ASSERT(memcmp(a.name.c_str(), b->name, b->namelen) == 0); @@ -134,20 +134,24 @@ void test_http2_get_header(void) { } namespace { -auto headers = - Headers{{"alpha", "0", true}, - {"bravo", "1"}, - {"connection", "2", false, http2::HD_CONNECTION}, - {"connection", "3", false, http2::HD_CONNECTION}, - {"delta", "4"}, - {"expect", "5"}, - {"foxtrot", "6"}, - {"tango", "7"}, - {"te", "8", false, http2::HD_TE}, - {"te", "9", false, http2::HD_TE}, - {"x-forwarded-proto", "10", false, http2::HD_X_FORWARDED_FOR}, - {"x-forwarded-proto", "11", false, http2::HD_X_FORWARDED_FOR}, - {"zulu", "12"}}; +auto headers = HeaderRefs{ + {StringRef::from_lit("alpha"), StringRef::from_lit("0"), true}, + {StringRef::from_lit("bravo"), StringRef::from_lit("1")}, + {StringRef::from_lit("connection"), StringRef::from_lit("2"), false, + http2::HD_CONNECTION}, + {StringRef::from_lit("connection"), StringRef::from_lit("3"), false, + http2::HD_CONNECTION}, + {StringRef::from_lit("delta"), StringRef::from_lit("4")}, + {StringRef::from_lit("expect"), StringRef::from_lit("5")}, + {StringRef::from_lit("foxtrot"), StringRef::from_lit("6")}, + {StringRef::from_lit("tango"), StringRef::from_lit("7")}, + {StringRef::from_lit("te"), StringRef::from_lit("8"), false, http2::HD_TE}, + {StringRef::from_lit("te"), StringRef::from_lit("9"), false, http2::HD_TE}, + {StringRef::from_lit("x-forwarded-proto"), StringRef::from_lit("10"), false, + http2::HD_X_FORWARDED_FOR}, + {StringRef::from_lit("x-forwarded-proto"), StringRef::from_lit("11"), false, + http2::HD_X_FORWARDED_FOR}, + {StringRef::from_lit("zulu"), StringRef::from_lit("12")}}; } // namespace void test_http2_copy_headers_to_nva(void) { @@ -209,10 +213,12 @@ void check_rewrite_location_uri(const std::string &want, const std::string &uri, const std::string &match_host, const std::string &req_authority, const std::string &upstream_scheme) { + BlockAllocator balloc(4096, 4096); http_parser_url u{}; CU_ASSERT(0 == http_parser_parse_url(uri.c_str(), uri.size(), 0, &u)); - auto got = http2::rewrite_location_uri(uri, u, match_host, req_authority, - upstream_scheme); + auto got = http2::rewrite_location_uri( + balloc, StringRef{uri}, u, StringRef{match_host}, + StringRef{req_authority}, StringRef{upstream_scheme}); CU_ASSERT(want == got); } } // namespace @@ -245,13 +251,13 @@ void test_http2_rewrite_location_uri(void) { } void test_http2_parse_http_status_code(void) { - CU_ASSERT(200 == http2::parse_http_status_code("200")); - CU_ASSERT(102 == http2::parse_http_status_code("102")); - CU_ASSERT(-1 == http2::parse_http_status_code("099")); - CU_ASSERT(-1 == http2::parse_http_status_code("99")); - CU_ASSERT(-1 == http2::parse_http_status_code("-1")); - CU_ASSERT(-1 == http2::parse_http_status_code("20a")); - CU_ASSERT(-1 == http2::parse_http_status_code("")); + CU_ASSERT(200 == http2::parse_http_status_code(StringRef::from_lit("200"))); + CU_ASSERT(102 == http2::parse_http_status_code(StringRef::from_lit("102"))); + CU_ASSERT(-1 == http2::parse_http_status_code(StringRef::from_lit("099"))); + CU_ASSERT(-1 == http2::parse_http_status_code(StringRef::from_lit("99"))); + CU_ASSERT(-1 == http2::parse_http_status_code(StringRef::from_lit("-1"))); + CU_ASSERT(-1 == http2::parse_http_status_code(StringRef::from_lit("20a"))); + CU_ASSERT(-1 == http2::parse_http_status_code(StringRef{})); } void test_http2_index_header(void) { @@ -814,137 +820,135 @@ void test_http2_path_join(void) { } void test_http2_normalize_path(void) { - std::string src; - - src = "/alpha/bravo/../charlie"; CU_ASSERT("/alpha/charlie" == - http2::normalize_path(std::begin(src), std::end(src))); + http2::normalize_path( + StringRef::from_lit("/alpha/bravo/../charlie"), StringRef{})); - src = "/a%6c%70%68%61"; - CU_ASSERT("/alpha" == http2::normalize_path(std::begin(src), std::end(src))); + CU_ASSERT("/alpha" == + http2::normalize_path(StringRef::from_lit("/a%6c%70%68%61"), + StringRef{})); - src = "/alpha%2f%3a"; - CU_ASSERT("/alpha%2F%3A" == - http2::normalize_path(std::begin(src), std::end(src))); + CU_ASSERT( + "/alpha%2F%3A" == + http2::normalize_path(StringRef::from_lit("/alpha%2f%3a"), StringRef{})); - src = "%2f"; - CU_ASSERT("/%2F" == http2::normalize_path(std::begin(src), std::end(src))); + CU_ASSERT("/%2F" == + http2::normalize_path(StringRef::from_lit("%2f"), StringRef{})); - src = "%f"; - CU_ASSERT("/%f" == http2::normalize_path(std::begin(src), std::end(src))); + CU_ASSERT("/%f" == + http2::normalize_path(StringRef::from_lit("%f"), StringRef{})); - src = "%"; - CU_ASSERT("/%" == http2::normalize_path(std::begin(src), std::end(src))); + CU_ASSERT("/%" == + http2::normalize_path(StringRef::from_lit("%"), StringRef{})); - src = ""; - CU_ASSERT("/" == http2::normalize_path(std::begin(src), std::end(src))); + CU_ASSERT("/" == http2::normalize_path(StringRef{}, StringRef{})); + + CU_ASSERT("/alpha?bravo" == + http2::normalize_path(StringRef::from_lit("/alpha"), + StringRef::from_lit("bravo"))); } void test_http2_rewrite_clean_path(void) { - std::string src; + BlockAllocator balloc(4096, 4096); // unreserved characters - src = "/alpha/%62ravo/"; CU_ASSERT("/alpha/bravo/" == - http2::rewrite_clean_path(std::begin(src), std::end(src))); + http2::rewrite_clean_path(balloc, + StringRef::from_lit("/alpha/%62ravo/"))); // percent-encoding is converted to upper case. - src = "/delta%3a"; - CU_ASSERT("/delta%3A" == - http2::rewrite_clean_path(std::begin(src), std::end(src))); + CU_ASSERT("/delta%3A" == http2::rewrite_clean_path( + balloc, StringRef::from_lit("/delta%3a"))); // path component is normalized before mathcing - src = "/alpha/charlie/%2e././bravo/delta/.."; - CU_ASSERT("/alpha/bravo/" == - http2::rewrite_clean_path(std::begin(src), std::end(src))); + CU_ASSERT( + "/alpha/bravo/" == + http2::rewrite_clean_path( + balloc, StringRef::from_lit("/alpha/charlie/%2e././bravo/delta/.."))); - src = "alpha%3a"; - CU_ASSERT(src == http2::rewrite_clean_path(std::begin(src), std::end(src))); + CU_ASSERT("alpha%3a" == + http2::rewrite_clean_path(balloc, StringRef::from_lit("alpha%3a"))); - src = ""; - CU_ASSERT(src == http2::rewrite_clean_path(std::begin(src), std::end(src))); + CU_ASSERT("" == http2::rewrite_clean_path(balloc, StringRef{})); } void test_http2_get_pure_path_component(void) { - std::string path; + CU_ASSERT("/" == http2::get_pure_path_component(StringRef::from_lit("/"))); - path = "/"; - CU_ASSERT("/" == http2::get_pure_path_component(path)); + CU_ASSERT("/foo" == + http2::get_pure_path_component(StringRef::from_lit("/foo"))); - path = "/foo"; - CU_ASSERT("/foo" == http2::get_pure_path_component(path)); + CU_ASSERT("/bar" == http2::get_pure_path_component( + StringRef::from_lit("https://example.org/bar"))); - path = "https://example.org/bar"; - CU_ASSERT("/bar" == http2::get_pure_path_component(path)); + CU_ASSERT("/alpha" == http2::get_pure_path_component(StringRef::from_lit( + "https://example.org/alpha?q=a"))); - path = "https://example.org/alpha?q=a"; - CU_ASSERT("/alpha" == http2::get_pure_path_component(path)); + CU_ASSERT("/bravo" == http2::get_pure_path_component(StringRef::from_lit( + "https://example.org/bravo?q=a#fragment"))); - path = "https://example.org/bravo?q=a#fragment"; - CU_ASSERT("/bravo" == http2::get_pure_path_component(path)); - - path = "\x01\x02"; - CU_ASSERT("" == http2::get_pure_path_component(path)); + CU_ASSERT("" == + http2::get_pure_path_component(StringRef::from_lit("\x01\x02"))); } void test_http2_construct_push_component(void) { + BlockAllocator balloc(4096, 4096); StringRef base, uri; - std::string scheme, authority, path; + StringRef scheme, authority, path; base = StringRef::from_lit("/b/"); - uri = StringRef::from_lit("https://example.org/foo"); - CU_ASSERT( - 0 == http2::construct_push_component(scheme, authority, path, base, uri)); + CU_ASSERT(0 == http2::construct_push_component(balloc, scheme, authority, + path, base, uri)); CU_ASSERT("https" == scheme); CU_ASSERT("example.org" == authority); CU_ASSERT("/foo" == path); - scheme.clear(); - authority.clear(); - path.clear(); + scheme = StringRef{}; + authority = StringRef{}; + path = StringRef{}; uri = StringRef::from_lit("/foo/bar?q=a"); - CU_ASSERT( - 0 == http2::construct_push_component(scheme, authority, path, base, uri)); + CU_ASSERT(0 == http2::construct_push_component(balloc, scheme, authority, + path, base, uri)); CU_ASSERT("" == scheme); CU_ASSERT("" == authority); CU_ASSERT("/foo/bar?q=a" == path); - scheme.clear(); - authority.clear(); - path.clear(); + scheme = StringRef{}; + authority = StringRef{}; + path = StringRef{}; uri = StringRef::from_lit("foo/../bar?q=a"); - CU_ASSERT( - 0 == http2::construct_push_component(scheme, authority, path, base, uri)); + CU_ASSERT(0 == http2::construct_push_component(balloc, scheme, authority, + path, base, uri)); CU_ASSERT("" == scheme); CU_ASSERT("" == authority); CU_ASSERT("/b/bar?q=a" == path); - scheme.clear(); - authority.clear(); - path.clear(); + scheme = StringRef{}; + authority = StringRef{}; + path = StringRef{}; uri = StringRef{}; - CU_ASSERT( - 0 == http2::construct_push_component(scheme, authority, path, base, uri)); + CU_ASSERT(0 == http2::construct_push_component(balloc, scheme, authority, + path, base, uri)); CU_ASSERT("" == scheme); CU_ASSERT("" == authority); CU_ASSERT("/" == path); - scheme.clear(); - authority.clear(); - path.clear(); + scheme = StringRef{}; + authority = StringRef{}; + path = StringRef{}; uri = StringRef::from_lit("?q=a"); - CU_ASSERT( - 0 == http2::construct_push_component(scheme, authority, path, base, uri)); + CU_ASSERT(0 == http2::construct_push_component(balloc, scheme, authority, + path, base, uri)); CU_ASSERT("" == scheme); CU_ASSERT("" == authority); CU_ASSERT("/b/?q=a" == path); diff --git a/src/shrpx-unittest.cc b/src/shrpx-unittest.cc index bebc0f28..8cd75690 100644 --- a/src/shrpx-unittest.cc +++ b/src/shrpx-unittest.cc @@ -142,6 +142,7 @@ int main(int argc, char *argv[]) { !CU_add_test(pSuite, "util_select_h2", shrpx::test_util_select_h2) || !CU_add_test(pSuite, "util_ipv6_numeric_addr", shrpx::test_util_ipv6_numeric_addr) || + !CU_add_test(pSuite, "util_utos", shrpx::test_util_utos) || !CU_add_test(pSuite, "util_utos_unit", shrpx::test_util_utos_unit) || !CU_add_test(pSuite, "util_utos_funit", shrpx::test_util_utos_funit) || !CU_add_test(pSuite, "util_parse_uint_with_unit", diff --git a/src/shrpx_config.cc b/src/shrpx_config.cc index 1046fdf0..a2ad7c54 100644 --- a/src/shrpx_config.cc +++ b/src/shrpx_config.cc @@ -624,7 +624,8 @@ int parse_mapping(const DownstreamAddrConfig &addr, } else { pattern.assign(std::begin(raw_pattern), slash); util::inp_strlower(pattern); - pattern += http2::normalize_path(slash, std::end(raw_pattern)); + pattern += http2::normalize_path(StringRef{slash, std::end(raw_pattern)}, + StringRef{}); } for (auto &g : addr_groups) { if (g.pattern == pattern) { diff --git a/src/shrpx_downstream.cc b/src/shrpx_downstream.cc index f5e4f084..eadfed39 100644 --- a/src/shrpx_downstream.cc +++ b/src/shrpx_downstream.cc @@ -257,8 +257,7 @@ std::string Downstream::assemble_request_cookie() const { std::string cookie; cookie = ""; for (auto &kv : req_.fs.headers()) { - if (kv.name.size() != 6 || kv.name[5] != 'e' || - !util::streq_l("cooki", kv.name.c_str(), 5)) { + if (kv.token != http2::HD_COOKIE) { continue; } @@ -273,7 +272,7 @@ std::string Downstream::assemble_request_cookie() const { if (c == ' ' || c == ';') { continue; } - end = it + 1; + end = it; break; } diff --git a/src/shrpx_downstream_test.cc b/src/shrpx_downstream_test.cc index d9989a72..5bfcda67 100644 --- a/src/shrpx_downstream_test.cc +++ b/src/shrpx_downstream_test.cc @@ -33,7 +33,8 @@ namespace shrpx { void test_downstream_field_store_add_header_lower(void) { - FieldStore fs(0); + BlockAllocator balloc(4096, 4096); + FieldStore fs(balloc, 0); fs.add_header_lower(StringRef::from_lit("1"), StringRef::from_lit("0"), false); fs.add_header_lower(StringRef::from_lit("2"), StringRef::from_lit("1"), @@ -51,19 +52,21 @@ void test_downstream_field_store_add_header_lower(void) { fs.add_header_lower(StringRef::from_lit(":authority"), StringRef::from_lit("7"), false); - auto ans = Headers{{"1", "0"}, - {"2", "1"}, - {"charlie", "2"}, - {"alpha", "3"}, - {"delta", "4"}, - {"bravo", "5"}, - {":method", "6"}, - {":authority", "7"}}; + auto ans = + HeaderRefs{{StringRef::from_lit("1"), StringRef::from_lit("0")}, + {StringRef::from_lit("2"), StringRef::from_lit("1")}, + {StringRef::from_lit("charlie"), StringRef::from_lit("2")}, + {StringRef::from_lit("alpha"), StringRef::from_lit("3")}, + {StringRef::from_lit("delta"), StringRef::from_lit("4")}, + {StringRef::from_lit("bravo"), StringRef::from_lit("5")}, + {StringRef::from_lit(":method"), StringRef::from_lit("6")}, + {StringRef::from_lit(":authority"), StringRef::from_lit("7")}}; CU_ASSERT(ans == fs.headers()); } void test_downstream_field_store_header(void) { - FieldStore fs(0); + BlockAllocator balloc(4096, 4096); + FieldStore fs(balloc, 0); fs.add_header_token(StringRef::from_lit("alpha"), StringRef::from_lit("0"), false, -1); fs.add_header_token(StringRef::from_lit(":authority"), @@ -73,11 +76,13 @@ void test_downstream_field_store_header(void) { http2::HD_CONTENT_LENGTH); // By token - CU_ASSERT(Header(":authority", "1") == *fs.header(http2::HD__AUTHORITY)); + CU_ASSERT(HeaderRef(StringRef{":authority"}, StringRef{"1"}) == + *fs.header(http2::HD__AUTHORITY)); CU_ASSERT(nullptr == fs.header(http2::HD__METHOD)); // By name - CU_ASSERT(Header("alpha", "0") == *fs.header(StringRef::from_lit("alpha"))); + CU_ASSERT(HeaderRef(StringRef{"alpha"}, StringRef{"0"}) == + *fs.header(StringRef::from_lit("alpha"))); CU_ASSERT(nullptr == fs.header(StringRef::from_lit("bravo"))); } @@ -105,19 +110,20 @@ void test_downstream_crumble_request_cookie(void) { CU_ASSERT(5 == nva.size()); CU_ASSERT(5 == num_cookies); - Headers cookies; + HeaderRefs cookies; std::transform(std::begin(nva), std::end(nva), std::back_inserter(cookies), [](const nghttp2_nv &nv) { - return Header(std::string(nv.name, nv.name + nv.namelen), - std::string(nv.value, nv.value + nv.valuelen), - nv.flags & NGHTTP2_NV_FLAG_NO_INDEX); + return HeaderRef(StringRef{nv.name, nv.namelen}, + StringRef{nv.value, nv.valuelen}, + nv.flags & NGHTTP2_NV_FLAG_NO_INDEX); }); - Headers ans = {{"cookie", "alpha"}, - {"cookie", "bravo"}, - {"cookie", "charlie"}, - {"cookie", "delta"}, - {"cookie", "echo"}}; + HeaderRefs ans = { + {StringRef::from_lit("cookie"), StringRef::from_lit("alpha")}, + {StringRef::from_lit("cookie"), StringRef::from_lit("bravo")}, + {StringRef::from_lit("cookie"), StringRef::from_lit("charlie")}, + {StringRef::from_lit("cookie"), StringRef::from_lit("delta")}, + {StringRef::from_lit("cookie"), StringRef::from_lit("echo")}}; CU_ASSERT(ans == cookies); CU_ASSERT(cookies[0].no_index); @@ -128,6 +134,7 @@ void test_downstream_crumble_request_cookie(void) { void test_downstream_assemble_request_cookie(void) { Downstream d(nullptr, nullptr, 0); auto &req = d.request(); + req.fs.add_header_token(StringRef::from_lit(":method"), StringRef::from_lit("get"), false, -1); req.fs.add_header_token(StringRef::from_lit(":path"), @@ -151,12 +158,12 @@ void test_downstream_rewrite_location_response_header(void) { Downstream d(nullptr, nullptr, 0); auto &req = d.request(); auto &resp = d.response(); - d.set_request_downstream_host("localhost2"); - req.authority = "localhost:8443"; + d.set_request_downstream_host(StringRef::from_lit("localhost2")); + req.authority = StringRef::from_lit("localhost:8443"); resp.fs.add_header_token(StringRef::from_lit("location"), StringRef::from_lit("http://localhost2:3000/"), false, http2::HD_LOCATION); - d.rewrite_location_response_header("https"); + d.rewrite_location_response_header(StringRef::from_lit("https")); auto location = resp.fs.header(http2::HD_LOCATION); CU_ASSERT("https://localhost:8443/" == (*location).value); } diff --git a/src/util.h b/src/util.h index f24f8140..90f03f1f 100644 --- a/src/util.h +++ b/src/util.h @@ -398,10 +398,11 @@ template OutputIt utos(OutputIt dst, T n) { ; --i; auto p = dst + i; + auto res = p + 1; for (; n; --i, n /= 10) { *p-- = (n % 10) + '0'; } - return dst + i + 1; + return res; } template std::string utos_unit(T n) { diff --git a/src/util_test.cc b/src/util_test.cc index cd8e2b02..d99c3a96 100644 --- a/src/util_test.cc +++ b/src/util_test.cc @@ -234,6 +234,15 @@ void test_util_ipv6_numeric_addr(void) { CU_ASSERT(!util::ipv6_numeric_addr("localhost")); } +void test_util_utos(void) { + uint8_t buf[32]; + + CU_ASSERT(("0" == StringRef{buf, util::utos(buf, 0)})); + CU_ASSERT(("123" == StringRef{buf, util::utos(buf, 123)})); + CU_ASSERT(("9223372036854775808" == + StringRef{buf, util::utos(buf, 9223372036854775808ULL)})); +} + void test_util_utos_unit(void) { CU_ASSERT("0" == util::utos_unit(0)); CU_ASSERT("1023" == util::utos_unit(1023)); diff --git a/src/util_test.h b/src/util_test.h index 1b79bbae..f51692aa 100644 --- a/src/util_test.h +++ b/src/util_test.h @@ -44,6 +44,7 @@ void test_util_utox(void); void test_util_http_date(void); void test_util_select_h2(void); void test_util_ipv6_numeric_addr(void); +void test_util_utos(void); void test_util_utos_unit(void); void test_util_utos_funit(void); void test_util_parse_uint_with_unit(void);