nghttpx: Normalize path when setting it to Downstream

src/http2.h

@@ -334,6 +334,21 @@ std::string normalize_path(InputIt first, InputIt last) {
                    nullptr, 0);
 }
 
+template <typename InputIt>
+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

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

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
 

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",

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<DownstreamAddrGroup> &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<DownstreamAddrGroup> &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

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 + "/".

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,

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 {

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)) {