Added veryfication if path contains query params and add them to path…

… header (#6466)

* Description: Added veryfication if path contains query params and append path headers if it has them
Risk Level: medium (affects data plane, if only for fully qualified urls)
Testing: added new test cases
Docs Changes: n/a
Release Notes: n/a

Signed-off-by: Lukasz Dziedziak <lukasz.dziedziak@allegro.pl>

source/common/http/http1/codec_impl.cc

@@ -533,7 +533,7 @@ void ServerConnectionImpl::handlePath(HeaderMapImpl& headers, unsigned int metho
   // forward the received Host field-value.
   headers.insertHost().value(std::string(absolute_url.host_and_port()));
 
-  headers.insertPath().value(std::string(absolute_url.path()));
+  headers.insertPath().value(std::string(absolute_url.path_and_query_params()));
   active_request_->request_url_.clear();
 }
 

source/common/http/utility.cc

@@ -57,10 +57,19 @@ bool Utility::Url::initialize(absl::string_view absolute_url) {
   // RFC allows the absolute-uri to not end in /, but the absolute path form
   // must start with
   if ((u.field_set & (1 << UF_PATH)) == (1 << UF_PATH) && u.field_data[UF_PATH].len > 0) {
-    path_ = absl::string_view(absolute_url.data() + u.field_data[UF_PATH].off,
-                              u.field_data[UF_PATH].len);
+    uint64_t path_len = u.field_data[UF_PATH].len;
+    if ((u.field_set & (1 << UF_QUERY)) == (1 << UF_QUERY) && u.field_data[UF_QUERY].len > 0) {
+      path_len += 1 + u.field_data[UF_QUERY].len;
+    }
+    path_and_query_params_ =
+        absl::string_view(absolute_url.data() + u.field_data[UF_PATH].off, path_len);
+  } else if ((u.field_set & (1 << UF_QUERY)) == (1 << UF_QUERY) && u.field_data[UF_QUERY].len > 0) {
+    // Http parser skips question mark and starts count from first character after ?
+    // so we need to move left by one
+    path_and_query_params_ = absl::string_view(absolute_url.data() + u.field_data[UF_QUERY].off - 1,
+                                               u.field_data[UF_QUERY].len + 1);
   } else {
-    path_ = absl::string_view(kDefaultPath, 1);
+    path_and_query_params_ = absl::string_view(kDefaultPath, 1);
   }
   return true;
 }

source/common/http/utility.h

@@ -24,19 +24,19 @@ namespace Utility {
 
 /**
  * Given a fully qualified URL, splits the string_view provided into scheme,
- * host and path components.
+ * host and path with query parameters components.
  */
 class Url {
 public:
   bool initialize(absl::string_view absolute_url);
   absl::string_view scheme() { return scheme_; }
   absl::string_view host_and_port() { return host_and_port_; }
-  absl::string_view path() { return path_; }
+  absl::string_view path_and_query_params() { return path_and_query_params_; }
 
 private:
   absl::string_view scheme_;
   absl::string_view host_and_port_;
-  absl::string_view path_;
+  absl::string_view path_and_query_params_;
 };
 
 /**

source/common/router/router.cc

@@ -77,7 +77,7 @@ bool convertRequestHeadersForInternalRedirect(Http::HeaderMap& downstream_header
   // Replace the original host, scheme and path.
   downstream_headers.insertScheme().value(std::string(absolute_url.scheme()));
   downstream_headers.insertHost().value(std::string(absolute_url.host_and_port()));
-  downstream_headers.insertPath().value(std::string(absolute_url.path()));
+  downstream_headers.insertPath().value(std::string(absolute_url.path_and_query_params()));
 
   return true;
 }

test/common/http/utility_test.cc

@@ -754,7 +754,7 @@ void ValidateUrl(absl::string_view raw_url, absl::string_view expected_scheme,
   ASSERT_TRUE(url.initialize(raw_url)) << "Failed to initialize " << raw_url;
   EXPECT_EQ(url.scheme(), expected_scheme);
   EXPECT_EQ(url.host_and_port(), expected_host_port);
-  EXPECT_EQ(url.path(), expected_path);
+  EXPECT_EQ(url.path_and_query_params(), expected_path);
 }
 
 TEST(Url, ParsingTest) {
@@ -766,12 +766,43 @@ TEST(Url, ParsingTest) {
   ValidateUrl("http://www.host.com:80/", "http", "www.host.com:80", "/");
   ValidateUrl("http://www.host.com/", "http", "www.host.com", "/");
 
+  // Test url with "?".
+  ValidateUrl("http://www.host.com:80/?", "http", "www.host.com:80", "/");
+  ValidateUrl("http://www.host.com/?", "http", "www.host.com", "/");
+
+  // Test url with "?" but without slash.
+  ValidateUrl("http://www.host.com:80?", "http", "www.host.com:80", "/");
+  ValidateUrl("http://www.host.com?", "http", "www.host.com", "/");
+
   // Test url with multi-character path
   ValidateUrl("http://www.host.com:80/path", "http", "www.host.com:80", "/path");
   ValidateUrl("http://www.host.com/path", "http", "www.host.com", "/path");
 
+  // Test url with multi-character path and ? at the end
+  ValidateUrl("http://www.host.com:80/path?", "http", "www.host.com:80", "/path");
+  ValidateUrl("http://www.host.com/path?", "http", "www.host.com", "/path");
+
   // Test https scheme
   ValidateUrl("https://www.host.com", "https", "www.host.com", "/");
+
+  // Test url with query parameter
+  ValidateUrl("http://www.host.com:80/?query=param", "http", "www.host.com:80", "/?query=param");
+  ValidateUrl("http://www.host.com/?query=param", "http", "www.host.com", "/?query=param");
+
+  // Test url with query parameter but without slash
+  ValidateUrl("http://www.host.com:80?query=param", "http", "www.host.com:80", "?query=param");
+  ValidateUrl("http://www.host.com?query=param", "http", "www.host.com", "?query=param");
+
+  // Test url with multi-character path and query parameter
+  ValidateUrl("http://www.host.com:80/path?query=param", "http", "www.host.com:80",
+              "/path?query=param");
+  ValidateUrl("http://www.host.com/path?query=param", "http", "www.host.com", "/path?query=param");
+
+  // Test url with multi-character path and more than one query parameter
+  ValidateUrl("http://www.host.com:80/path?query=param&query2=param2", "http", "www.host.com:80",
+              "/path?query=param&query2=param2");
+  ValidateUrl("http://www.host.com/path?query=param&query2=param2", "http", "www.host.com",
+              "/path?query=param&query2=param2");
 }
 
 } // namespace Http