From 19e47a192237faf8619c883b60b24a1a23bf4ac4 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Sat, 11 Jul 2015 16:12:35 +0900 Subject: [PATCH] nghttpx: Normalize path when setting it to Downstream --- src/http2.h | 15 +++++++++++++++ src/http2_test.cc | 25 +++++++++++++++++++++++++ src/http2_test.h | 1 + src/shrpx-unittest.cc | 2 ++ src/shrpx_config.cc | 25 ++++++++++++++----------- src/shrpx_config_test.cc | 17 +++-------------- src/shrpx_http2_upstream.cc | 9 +++++++-- src/shrpx_https_upstream.cc | 6 +++++- src/shrpx_spdy_upstream.cc | 3 ++- 9 files changed, 74 insertions(+), 29 deletions(-) diff --git a/src/http2.h b/src/http2.h index be79caca..182c45d9 100644 --- a/src/http2.h +++ b/src/http2.h @@ -334,6 +334,21 @@ std::string normalize_path(InputIt first, InputIt last) { nullptr, 0); } +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; +} + } // namespace http2 } // namespace nghttp2 diff --git a/src/http2_test.cc b/src/http2_test.cc index 1937bfaf..88339045 100644 --- a/src/http2_test.cc +++ b/src/http2_test.cc @@ -856,4 +856,29 @@ void test_http2_normalize_path(void) { CU_ASSERT("/" == http2::normalize_path(std::begin(src), std::end(src))); } +void test_http2_rewrite_clean_path(void) { + std::string src; + + // unreserved characters + src = "/alpha/%62ravo/"; + CU_ASSERT("/alpha/bravo/" == + http2::rewrite_clean_path(std::begin(src), std::end(src))); + + // percent-encoding is converted to upper case. + src = "/delta%3a"; + CU_ASSERT("/delta%3A" == + http2::rewrite_clean_path(std::begin(src), std::end(src))); + + // 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))); + + src = "alpha%3a"; + CU_ASSERT(src == http2::rewrite_clean_path(std::begin(src), std::end(src))); + + src = ""; + CU_ASSERT(src == http2::rewrite_clean_path(std::begin(src), std::end(src))); +} + } // namespace shrpx diff --git a/src/http2_test.h b/src/http2_test.h index 193f23e3..bc65d453 100644 --- a/src/http2_test.h +++ b/src/http2_test.h @@ -46,6 +46,7 @@ void test_http2_mandatory_request_headers_presence(void); void test_http2_parse_link_header(void); void test_http2_path_join(void); void test_http2_normalize_path(void); +void test_http2_rewrite_clean_path(void); } // namespace shrpx diff --git a/src/shrpx-unittest.cc b/src/shrpx-unittest.cc index 7030aa3d..83695204 100644 --- a/src/shrpx-unittest.cc +++ b/src/shrpx-unittest.cc @@ -98,6 +98,8 @@ int main(int argc, char *argv[]) { !CU_add_test(pSuite, "http2_path_join", shrpx::test_http2_path_join) || !CU_add_test(pSuite, "http2_normalize_path", shrpx::test_http2_normalize_path) || + !CU_add_test(pSuite, "http2_rewrite_clean_path", + shrpx::test_http2_rewrite_clean_path) || !CU_add_test(pSuite, "downstream_index_request_headers", shrpx::test_downstream_index_request_headers) || !CU_add_test(pSuite, "downstream_index_response_headers", diff --git a/src/shrpx_config.cc b/src/shrpx_config.cc index 66b10ccf..ca130e77 100644 --- a/src/shrpx_config.cc +++ b/src/shrpx_config.cc @@ -1438,9 +1438,9 @@ ssize_t match(const std::string &path, namespace { size_t match_downstream_addr_group_host( - const std::string &host, const std::string &raw_path, + const std::string &host, const std::string &path, const std::vector &groups, size_t catch_all) { - if (raw_path == "*") { + if (path.empty() || path[0] != '/') { auto group = match(host + "/", groups); if (group != -1) { if (LOG_ENABLED(INFO)) { @@ -1452,11 +1452,6 @@ size_t match_downstream_addr_group_host( return catch_all; } - // probably, not necessary most of the case, but just in case. - auto fragment = std::find(std::begin(raw_path), std::end(raw_path), '#'); - auto query = std::find(std::begin(raw_path), fragment, '?'); - auto path = http2::normalize_path(std::begin(raw_path), query); - if (LOG_ENABLED(INFO)) { LOG(INFO) << "Perform mapping selection, using host=" << host << ", path=" << path; @@ -1490,13 +1485,21 @@ size_t match_downstream_addr_group_host( size_t match_downstream_addr_group( const std::string &hostport, const std::string &raw_path, const std::vector &groups, size_t catch_all) { - if (hostport.empty() || - std::find(std::begin(hostport), std::end(hostport), '/') != - std::end(hostport)) { + if (std::find(std::begin(hostport), std::end(hostport), '/') != + std::end(hostport)) { // We use '/' specially, and if '/' is included in host, it breaks // our code. Select catch-all case. return catch_all; } + + auto fragment = std::find(std::begin(raw_path), std::end(raw_path), '#'); + auto query = std::find(std::begin(raw_path), fragment, '?'); + auto path = std::string(std::begin(raw_path), query); + + if (hostport.empty()) { + return match_downstream_addr_group_host(hostport, path, groups, catch_all); + } + std::string host; if (hostport[0] == '[') { // assume this is IPv6 numeric address @@ -1517,7 +1520,7 @@ size_t match_downstream_addr_group( } util::inp_strlower(host); - return match_downstream_addr_group_host(host, raw_path, groups, catch_all); + return match_downstream_addr_group_host(host, path, groups, catch_all); } } // namespace shrpx diff --git a/src/shrpx_config_test.cc b/src/shrpx_config_test.cc index 9f458d43..ab6a0f27 100644 --- a/src/shrpx_config_test.cc +++ b/src/shrpx_config_test.cc @@ -203,12 +203,8 @@ void test_shrpx_config_match_downstream_addr_group(void) { CU_ASSERT(0 == match_downstream_addr_group("nghttp2.org", "/Alpha/bravo", groups, 255)); - // unreserved characters are decoded before matching - CU_ASSERT(1 == match_downstream_addr_group("nghttp2.org", "/alpha/%62ravo/", - groups, 255)); - CU_ASSERT(1 == match_downstream_addr_group( - "nghttp2.org", "/alpha/%62ravo/charlie", groups, 255)); + "nghttp2.org", "/alpha/bravo/charlie", groups, 255)); CU_ASSERT(2 == match_downstream_addr_group("nghttp2.org", "/alpha/charlie", groups, 255)); @@ -218,20 +214,13 @@ void test_shrpx_config_match_downstream_addr_group(void) { CU_ASSERT(0 == match_downstream_addr_group("nghttp2.org", "/alpha/charlie/", groups, 255)); - // percent-encoding is normalized to upper case hex digits. - CU_ASSERT(3 == match_downstream_addr_group("nghttp2.org", "/delta%3a", groups, - 255)); - - // path component is normalized before mathcing - CU_ASSERT(1 == match_downstream_addr_group( - "nghttp2.org", "/alpha/charlie/%2e././bravo/delta/..", - groups, 255)); - CU_ASSERT(255 == match_downstream_addr_group("example.org", "/", groups, 255)); CU_ASSERT(255 == match_downstream_addr_group("", "/", groups, 255)); + CU_ASSERT(255 == match_downstream_addr_group("", "alpha", groups, 255)); + CU_ASSERT(255 == match_downstream_addr_group("foo/bar", "/", groups, 255)); // If path is "*", only match with host + "/". diff --git a/src/shrpx_http2_upstream.cc b/src/shrpx_http2_upstream.cc index 59553292..f0b3b213 100644 --- a/src/shrpx_http2_upstream.cc +++ b/src/shrpx_http2_upstream.cc @@ -300,7 +300,11 @@ int Http2Upstream::on_request_headers(Downstream *downstream, downstream->set_request_method(method_token); downstream->set_request_http2_scheme(http2::value_to_str(scheme)); downstream->set_request_http2_authority(http2::value_to_str(authority)); - downstream->set_request_path(http2::value_to_str(path)); + if (path) { + auto &value = path->value; + downstream->set_request_path( + http2::rewrite_clean_path(std::begin(value), std::end(value))); + } if (!(frame->hd.flags & NGHTTP2_FLAG_END_STREAM)) { downstream->set_request_http2_expect_body(true); @@ -541,7 +545,8 @@ int on_frame_send_callback(nghttp2_session *session, const nghttp2_frame *frame, {nv.value, nv.value + nv.valuelen}); break; case http2::HD__PATH: - downstream->set_request_path({nv.value, nv.value + nv.valuelen}); + downstream->set_request_path( + http2::rewrite_clean_path(nv.value, nv.value + nv.valuelen)); break; } downstream->add_request_header(nv.name, nv.namelen, nv.value, nv.valuelen, diff --git a/src/shrpx_https_upstream.cc b/src/shrpx_https_upstream.cc index 0fef7e6f..d1732e7a 100644 --- a/src/shrpx_https_upstream.cc +++ b/src/shrpx_https_upstream.cc @@ -218,7 +218,8 @@ void rewrite_request_host_path_from_uri(Downstream *downstream, const char *uri, path += '?'; path.append(uri + fdata.off, fdata.len); } - downstream->set_request_path(std::move(path)); + downstream->set_request_path( + http2::rewrite_clean_path(std::begin(path), std::end(path))); std::string scheme; http2::copy_url_component(scheme, &u, UF_SCHEMA, uri); @@ -286,6 +287,9 @@ int htp_hdrs_completecb(http_parser *htp) { return -1; } + downstream->set_request_path( + http2::rewrite_clean_path(std::begin(uri), std::end(uri))); + if (upstream->get_client_handler()->get_ssl()) { downstream->set_request_http2_scheme("https"); } else { diff --git a/src/shrpx_spdy_upstream.cc b/src/shrpx_spdy_upstream.cc index eb23e107..d6ac4f6c 100644 --- a/src/shrpx_spdy_upstream.cc +++ b/src/shrpx_spdy_upstream.cc @@ -229,7 +229,8 @@ void on_ctrl_recv_callback(spdylay_session *session, spdylay_frame_type type, } else { downstream->set_request_http2_scheme(scheme->value); downstream->set_request_http2_authority(host->value); - downstream->set_request_path(path->value); + downstream->set_request_path(http2::rewrite_clean_path( + std::begin(path->value), std::end(path->value))); } if (!(frame->syn_stream.hd.flags & SPDYLAY_CTRL_FLAG_FIN)) {