Merge pull request #8081 from vespa-engine/havardpe/url-dequote-and-parse-query

Havardpe/url dequote and parse query

8 files changed, 255 insertions, 6 deletions

diff --git a/staging_vespalib/src/tests/state_server/state_server_test.cpp b/staging_vespalib/src/tests/state_server/state_server_test.cpp
index b688887c3fb..d4f665029cc 100644
--- a/staging_vespalib/src/tests/state_server/state_server_test.cpp
+++ b/staging_vespalib/src/tests/state_server/state_server_test.cpp
@@ -40,7 +40,7 @@ vespalib::string run_cmd(const vespalib::string &cmd) {
 }
 
 vespalib::string getPage(int port, const vespalib::string &path, const vespalib::string &extra_params = "") {
-    return run_cmd(make_string("curl -s %s http://localhost:%d%s", extra_params.c_str(), port, path.c_str()));
+    return run_cmd(make_string("curl -s %s 'http://localhost:%d%s'", extra_params.c_str(), port, path.c_str()));
 }
 
 vespalib::string getFull(int port, const vespalib::string &path) { return getPage(port, path, "-D -"); }
@@ -123,6 +123,50 @@ TEST_FF("require that host is passed correctly", EchoHost(), HttpServer(0)) {
     EXPECT_EQUAL(default_result, run_cmd(make_string("curl -s http://localhost:%d/my/path -H \"Host:\"", f2.port())));
 }
 
+struct SamplingHandler : JsonGetHandler {
+    mutable std::mutex my_lock;
+    mutable vespalib::string my_host;
+    mutable vespalib::string my_path;
+    mutable std::map<vespalib::string,vespalib::string> my_params;
+    vespalib::string get(const vespalib::string &host, const vespalib::string &path,
+                         const std::map<vespalib::string,vespalib::string> &params) const override
+    {
+        {
+            auto guard = std::lock_guard(my_lock);
+            my_host = host;
+            my_path = path;
+            my_params = params;
+        }
+        return "[]";
+    }
+};
+
+TEST_FF("require that request parameters can be inspected", SamplingHandler(), HttpServer(0))
+{
+    auto token = f2.repo().bind("/foo", f1);
+    EXPECT_EQUAL("[]", getPage(f2.port(), "/foo?a=b&x=y&z"));
+    {
+        auto guard = std::lock_guard(f1.my_lock);
+        EXPECT_EQUAL(f1.my_path, "/foo");
+        EXPECT_EQUAL(f1.my_params.size(), 3u);
+        EXPECT_EQUAL(f1.my_params["a"], "b");
+        EXPECT_EQUAL(f1.my_params["x"], "y");
+        EXPECT_EQUAL(f1.my_params["z"], "");
+        EXPECT_EQUAL(f1.my_params.size(), 3u); // "z" was present
+    }
+}
+
+TEST_FF("require that request path is dequoted", SamplingHandler(), HttpServer(0))
+{
+    auto token = f2.repo().bind("/[foo]", f1);
+    EXPECT_EQUAL("[]", getPage(f2.port(), "/%5bfoo%5D"));
+    {
+        auto guard = std::lock_guard(f1.my_lock);
+        EXPECT_EQUAL(f1.my_path, "/[foo]");
+        EXPECT_EQUAL(f1.my_params.size(), 0u);
+    }
+}
+
 //-----------------------------------------------------------------------------
 
 TEST_FFFF("require that the state server wires the appropriate url prefixes",
diff --git a/staging_vespalib/src/vespa/vespalib/net/http_server.cpp b/staging_vespalib/src/vespa/vespalib/net/http_server.cpp
index 99a66fccde5..2a3bb5b3e0e 100644
--- a/staging_vespalib/src/vespa/vespalib/net/http_server.cpp
+++ b/staging_vespalib/src/vespa/vespalib/net/http_server.cpp
@@ -8,7 +8,7 @@ namespace vespalib {
 void
 HttpServer::get(Portal::GetRequest req)
 {
-    vespalib::string json_result = _handler_repo.get(req.get_host(), req.get_uri(), {});
+    vespalib::string json_result = _handler_repo.get(req.get_host(), req.get_path(), req.export_params());
     if (json_result.empty()) {
         req.respond_with_error(404, "Not Found");
     } else {
diff --git a/vespalib/src/tests/portal/http_request/http_request_test.cpp b/vespalib/src/tests/portal/http_request/http_request_test.cpp
index 6e1527efa4b..047fde5750c 100644
--- a/vespalib/src/tests/portal/http_request/http_request_test.cpp
+++ b/vespalib/src/tests/portal/http_request/http_request_test.cpp
@@ -2,6 +2,7 @@
 
 #include <vespa/vespalib/testkit/test_kit.h>
 #include <vespa/vespalib/portal/http_request.h>
+#include <vespa/vespalib/util/stringfmt.h>
 
 using namespace vespalib;
 using namespace vespalib::portal;
@@ -117,4 +118,58 @@ TEST("require that header line must contain separator") {
                                    "missing separator\r\n"));
 }
 
+TEST("require that uri parameters can be parsed") {
+    auto req = make_request("GET /my/path?foo=bar&baz HTTP/1.1\r\n\r\n");
+    EXPECT_EQUAL(req.get_uri(), "/my/path?foo=bar&baz");
+    EXPECT_EQUAL(req.get_path(), "/my/path");
+    EXPECT_TRUE(req.has_param("foo"));
+    EXPECT_TRUE(!req.has_param("bar"));
+    EXPECT_TRUE(req.has_param("baz"));
+    EXPECT_EQUAL(req.get_param("foo"), "bar");
+    EXPECT_EQUAL(req.get_param("bar"), "");
+    EXPECT_EQUAL(req.get_param("baz"), "");
+}
+
+TEST("require that byte values in uri segments (path, key, value) are dequoted as expected") {
+    vespalib::string str = "0123456789aBcDeF";
+    for (size_t a = 0; a < 16; ++a) {
+        for (size_t b = 0; b < 16; ++b) {
+            vespalib::string expect = " foo ";
+            expect.push_back((a * 16) + b);
+            expect.push_back((a * 16) + b);
+            expect.append(" bar ");
+            vespalib::string input = vespalib::make_string("+foo+%%%c%c%%%c%c+bar+",
+                    str[a], str[b], str[a], str[b]);
+            vespalib::string uri = vespalib::make_string("%s?%s=%s&extra=yes",
+                    input.c_str(), input.c_str(), input.c_str());
+            auto req = make_request(vespalib::make_string("GET %s HTTP/1.1\r\n\r\n",
+                            uri.c_str()));
+            EXPECT_EQUAL(req.get_uri(), uri);
+            EXPECT_EQUAL(req.get_path(), expect);
+            EXPECT_TRUE(req.has_param(expect));
+            EXPECT_EQUAL(req.get_param(expect), expect);
+            EXPECT_TRUE(req.has_param("extra"));
+            EXPECT_EQUAL(req.get_param("extra"), "yes");
+        }
+    }
+}
+
+TEST("require that percent character becomes plain if not followed by exactly 2 hex digits") {
+    auto req = make_request("GET %/5%5:%@5%5G%`5%5g%5?% HTTP/1.1\r\n\r\n");
+    EXPECT_EQUAL(req.get_path(), "%/5%5:%@5%5G%`5%5g%5");
+    EXPECT_TRUE(req.has_param("%"));
+}
+
+TEST("require that last character of uri segments (path, key, value) can be quoted") {
+    auto req = make_request("GET /%41?%42=%43 HTTP/1.1\r\n\r\n");
+    EXPECT_EQUAL(req.get_path(), "/A");
+    EXPECT_EQUAL(req.get_param("B"), "C");
+}
+
+TEST("require that additional query and key/value separators are not special") {
+    auto req = make_request("GET /?" "?== HTTP/1.1\r\n\r\n");
+    EXPECT_EQUAL(req.get_path(), "/");
+    EXPECT_EQUAL(req.get_param("?"), "=");
+}
+
 TEST_MAIN() { TEST_RUN_ALL(); }
diff --git a/vespalib/src/tests/portal/portal_test.cpp b/vespalib/src/tests/portal/portal_test.cpp
index 299340fd131..ee5d10a313a 100644
--- a/vespalib/src/tests/portal/portal_test.cpp
+++ b/vespalib/src/tests/portal/portal_test.cpp
@@ -346,4 +346,40 @@ TEST_MT_FFF("require that portal destruction waits for request completion", 3,
 
 //-----------------------------------------------------------------------------
 
+TEST("require that query parameters can be inspected") {
+    auto portal = Portal::create(null_crypto(), 0);
+    MyGetHandler handler([](Portal::GetRequest request)
+                         {
+                             EXPECT_EQUAL(request.get_uri(), "/test?a=b&x=y");
+                             EXPECT_EQUAL(request.get_path(), "/test");
+                             EXPECT_TRUE(request.has_param("a"));
+                             EXPECT_TRUE(request.has_param("x"));
+                             EXPECT_TRUE(!request.has_param("b"));
+                             EXPECT_EQUAL(request.get_param("a"), "b");
+                             EXPECT_EQUAL(request.get_param("x"), "y");
+                             EXPECT_EQUAL(request.get_param("b"), "");
+                             auto params = request.export_params();
+                             EXPECT_EQUAL(params.size(), 2u);
+                             EXPECT_EQUAL(params["a"], "b");
+                             EXPECT_EQUAL(params["x"], "y");
+                             request.respond_with_content("a", "b");
+                         });
+    auto bound = portal->bind("/test", handler);
+    auto result = fetch(portal->listen_port(), null_crypto(), "/test?a=b&x=y");
+    EXPECT_EQUAL(result, make_expected_response("a", "b"));
+}
+
+TEST("require that request path is dequoted before handler dispatching") {
+    auto portal = Portal::create(null_crypto(), 0);
+    MyGetHandler handler([](Portal::GetRequest request)
+                         {
+                             EXPECT_EQUAL(request.get_uri(), "/%5btest%5D");
+                             EXPECT_EQUAL(request.get_path(), "/[test]");
+                             request.respond_with_content("a", "b");
+                         });
+    auto bound = portal->bind("/[test]", handler);
+    auto result = fetch(portal->listen_port(), null_crypto(), "/%5btest%5D");
+    EXPECT_EQUAL(result, make_expected_response("a", "b"));
+}
+
 TEST_MAIN() { TEST_RUN_ALL(); }
diff --git a/vespalib/src/vespa/vespalib/portal/http_request.cpp b/vespalib/src/vespa/vespalib/portal/http_request.cpp
index d49fc2e70f4..abd690897c6 100644
--- a/vespalib/src/vespa/vespalib/portal/http_request.cpp
+++ b/vespalib/src/vespa/vespalib/portal/http_request.cpp
@@ -15,11 +15,11 @@ void strip_cr(vespalib::string &str) {
     }
 }
 
-std::vector<vespalib::string> split(vespalib::stringref str, vespalib::stringref sep) {
+std::vector<vespalib::string> split(vespalib::stringref str, char sep) {
     vespalib::string token;
     std::vector<vespalib::string> list;
     for (char c: str) {
-        if (sep.find(c) == vespalib::stringref::npos) {
+        if (c != sep) {
             token.push_back(c);
         } else if (!token.empty()) {
             list.push_back(token);
@@ -32,6 +32,49 @@ std::vector<vespalib::string> split(vespalib::stringref str, vespalib::stringref
     return list;
 }
 
+int decode_hex_digit(char c) {
+    if ((c >= '0') && (c <= '9')) {
+        return (c - '0');
+    }
+    if ((c >= 'a') && (c <= 'f')) {
+        return ((c - 'a') + 10);
+    }
+    if ((c >= 'A') && (c <= 'F')) {
+        return ((c - 'A') + 10);
+    }
+    return -1;
+}
+
+int decode_hex_num(vespalib::stringref src, size_t idx) {
+    if (src.size() < (idx + 2)) {
+        return -1;
+    }
+    int a = decode_hex_digit(src[idx]);
+    int b = decode_hex_digit(src[idx + 1]);
+    if ((a < 0) || (b < 0)) {
+        return -1;
+    }
+    return ((a << 4) | b);
+}
+
+vespalib::string dequote(vespalib::stringref src) {
+    vespalib::string dst;
+    for (size_t idx = 0; idx < src.size(); ++idx) {
+        char c = src[idx];
+        if (c == '+') {
+            c = ' ';
+        } else if (c == '%') {
+            int x = decode_hex_num(src, idx + 1);
+            if (x >= 0) {
+                c = x;
+                idx += 2;
+            }
+        }
+        dst.push_back(c);
+    }
+    return dst;
+}
+
 } // namespace vespalib::portal::<unnamed>
 
 void
@@ -49,13 +92,30 @@ HttpRequest::set_error()
 void
 HttpRequest::handle_request_line(const vespalib::string &line)
 {
-    auto parts = split(line, " ");
+    auto parts = split(line, ' ');
     if (parts.size() != 3) {
         return set_error(); // malformed request line
     }
     _method = parts[0];
     _uri = parts[1];
     _version = parts[2];
+    size_t query_sep = _uri.find("?");
+    if (query_sep == vespalib::string::npos) {
+        _path = dequote(_uri);
+    } else {
+        _path = dequote(_uri.substr(0, query_sep));
+        auto query = split(_uri.substr(query_sep + 1), '&');
+        for (const auto &param: query) {
+            size_t value_sep = param.find("=");
+            if (value_sep == vespalib::string::npos) {
+                _params[dequote(param)] = "";
+            } else {
+                auto key = param.substr(0, value_sep);
+                auto value = param.substr(value_sep + 1);
+                _params[dequote(key)] = dequote(value);
+            }
+        }
+    }
 }
 
 void
@@ -163,4 +223,20 @@ HttpRequest::get_header(const vespalib::string &name) const
     return pos->second;
 }
 
+bool
+HttpRequest::has_param(const vespalib::string &name) const
+{
+    return (_params.find(name) != _params.end());
+}
+
+const vespalib::string &
+HttpRequest::get_param(const vespalib::string &name) const
+{
+    auto pos = _params.find(name);
+    if (pos == _params.end()) {
+        return _empty;
+    }
+    return pos->second;
+}
+
 } // namespace vespalib::portal
diff --git a/vespalib/src/vespa/vespalib/portal/http_request.h b/vespalib/src/vespa/vespalib/portal/http_request.h
index 51c7ab08da9..39467c3b248 100644
--- a/vespalib/src/vespa/vespalib/portal/http_request.h
+++ b/vespalib/src/vespa/vespalib/portal/http_request.h
@@ -14,6 +14,8 @@ private:
     // http stuff
     vespalib::string _method;
     vespalib::string _uri;
+    vespalib::string _path;
+    std::map<vespalib::string, vespalib::string> _params;
     vespalib::string _version;
     std::map<vespalib::string, vespalib::string> _headers;
     vespalib::string _host;
@@ -43,6 +45,10 @@ public:
     const vespalib::string &get_header(const vespalib::string &name) const;
     const vespalib::string &get_host() const { return _host; }
     const vespalib::string &get_uri() const { return _uri; }
+    const vespalib::string &get_path() const { return _path; }
+    bool has_param(const vespalib::string &name) const;
+    const vespalib::string &get_param(const vespalib::string &name) const;
+    std::map<vespalib::string, vespalib::string> export_params() const { return _params; }
 };
 
 } // namespace vespalib::portal
diff --git a/vespalib/src/vespa/vespalib/portal/portal.cpp b/vespalib/src/vespa/vespalib/portal/portal.cpp
index 0d62d5728d1..ec2f1b78c03 100644
--- a/vespalib/src/vespa/vespalib/portal/portal.cpp
+++ b/vespalib/src/vespa/vespalib/portal/portal.cpp
@@ -48,6 +48,34 @@ Portal::GetRequest::get_uri() const
     return _conn->get_request().get_uri();
 }
 
+const vespalib::string &
+Portal::GetRequest::get_path() const
+{
+    assert(active());
+    return _conn->get_request().get_path();
+}
+
+bool
+Portal::GetRequest::has_param(const vespalib::string &name) const
+{
+    assert(active());
+    return _conn->get_request().has_param(name);
+}
+
+const vespalib::string &
+Portal::GetRequest::get_param(const vespalib::string &name) const
+{
+    assert(active());
+    return _conn->get_request().get_param(name);
+}
+
+std::map<vespalib::string, vespalib::string>
+Portal::GetRequest::export_params() const
+{
+    assert(active());
+    return _conn->get_request().export_params();
+}
+
 void
 Portal::GetRequest::respond_with_content(const vespalib::string &content_type,
                                  const vespalib::string &content)
@@ -131,7 +159,7 @@ Portal::handle_http(portal::HttpConnection *conn)
             conn->respond_with_error(501, "Not Implemented");
         } else {
             GetHandler *get_handler = nullptr;
-            auto guard = lookup_get_handler(conn->get_request().get_uri(), get_handler);
+            auto guard = lookup_get_handler(conn->get_request().get_path(), get_handler);
             if (guard.valid()) {
                 assert(get_handler != nullptr);
                 conn->resolve_host(_my_host);
diff --git a/vespalib/src/vespa/vespalib/portal/portal.h b/vespalib/src/vespa/vespalib/portal/portal.h
index 93424dda90c..fc3b81b37e6 100644
--- a/vespalib/src/vespa/vespalib/portal/portal.h
+++ b/vespalib/src/vespa/vespalib/portal/portal.h
@@ -60,6 +60,10 @@ public:
         const vespalib::string &get_header(const vespalib::string &name) const;
         const vespalib::string &get_host() const;
         const vespalib::string &get_uri() const;
+        const vespalib::string &get_path() const;
+        bool has_param(const vespalib::string &name) const;
+        const vespalib::string &get_param(const vespalib::string &name) const;
+        std::map<vespalib::string, vespalib::string> export_params() const;
         void respond_with_content(const vespalib::string &content_type,
                                   const vespalib::string &content);
         void respond_with_error(int code, const vespalib::string &msg);