diff --git a/src/httpserver.cpp b/src/httpserver.cpp index bc1d2157df3..a54ca1325a6 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -7,6 +7,7 @@ #include #include #include +#include #include #include #include @@ -941,6 +942,69 @@ bool HTTPRequest::LoadBody(LineReader& reader) } } +CService HTTPRequest::GetPeer() const +{ + return m_client->m_addr; +} + +HTTPRequestMethod HTTPRequest::GetRequestMethod() const +{ + if (m_method == "GET") return HTTPRequestMethod::GET; + if (m_method == "POST") return HTTPRequestMethod::POST; + if (m_method == "HEAD") return HTTPRequestMethod::HEAD; + if (m_method == "PUT") return HTTPRequestMethod::PUT; + return HTTPRequestMethod::UNKNOWN; +} + +std::optional HTTPRequest::GetQueryParameter(const std::string& key) const +{ + return GetQueryParameterFromUri(GetURI(), key); +} + +// See libevent http.c evhttp_parse_query_impl() +// and https://www.rfc-editor.org/rfc/rfc3986#section-3.4 +std::optional GetQueryParameterFromUri(const std::string& uri, const std::string& key) +{ + // Handle %XX encoding + std::string decoded_uri{UrlDecode(uri)}; + + // find query in URI + size_t start = decoded_uri.find("?"); + if (start == std::string::npos) return std::nullopt; + size_t end = decoded_uri.find("#", start); + if (end == std::string::npos) { + end = decoded_uri.length(); + } + const std::string query{decoded_uri.substr(start + 1, end - start - 1)}; + // find requested parameter in query + const std::vector params{SplitString(query, "&")}; + for (const std::string& param : params) { + size_t delim = param.find("="); + if (key == param.substr(0, delim)) { + if (delim == std::string::npos) { + return ""; + } else { + return param.substr(delim + 1); + } + } + } + return std::nullopt; +} + +std::pair HTTPRequest::GetHeader(const std::string& hdr) const +{ + std::optional found{m_headers.Find(hdr)}; + if (found.has_value()) { + return std::make_pair(true, found.value()); + } else + return std::make_pair(false, ""); +} + +void HTTPRequest::WriteHeader(const std::string& hdr, const std::string& value) +{ + m_response_headers.Write(hdr, value); +} + void HTTPRequest::WriteReply(HTTPStatusCode status, std::span reply_body) { HTTPResponse res; diff --git a/src/httpserver.h b/src/httpserver.h index a5a9ba3bca1..2a5c124eb6b 100644 --- a/src/httpserver.h +++ b/src/httpserver.h @@ -267,6 +267,16 @@ public: bool LoadHeaders(LineReader& reader); bool LoadBody(LineReader& reader); + // These methods reimplement the API from http_libevent::HTTPRequest + // for downstream JSONRPC and REST modules. + std::string GetURI() const {return m_target;}; + CService GetPeer() const; + HTTPRequestMethod GetRequestMethod() const; + std::optional GetQueryParameter(const std::string& key) const; + std::pair GetHeader(const std::string& hdr) const; + std::string ReadBody() const {return m_body;}; + void WriteHeader(const std::string& hdr, const std::string& value); + // Response headers may be set in advance before response body is known HTTPHeaders m_response_headers; void WriteReply(HTTPStatusCode status, std::span reply_body = {}); @@ -277,6 +287,8 @@ public: } }; +std::optional GetQueryParameterFromUri(const std::string& uri, const std::string& key); + class HTTPServer; class HTTPClient diff --git a/src/test/httpserver_tests.cpp b/src/test/httpserver_tests.cpp index 5362a0806ec..0d58fe911a3 100644 --- a/src/test/httpserver_tests.cpp +++ b/src/test/httpserver_tests.cpp @@ -50,7 +50,37 @@ BOOST_FIXTURE_TEST_SUITE(httpserver_tests, HTTPTestingSetup) BOOST_AUTO_TEST_CASE(test_query_parameters) { - using http_libevent::GetQueryParameterFromUri; + // The legacy code that relied on libevent couldn't handle an invalid URI encoding. + // The new code is more tolerant and so we expect a difference in behavior. + // Re: libevent evhttp_uri_parse() see: + // "bugfix: rest: avoid segfault for invalid URI" https://github.com/bitcoin/bitcoin/pull/27468 + // "httpserver, rest: improving URI validation" https://github.com/bitcoin/bitcoin/pull/27253 + // Re: More tolerant URI decoding see: + // "refactor: Use our own implementation of urlDecode" https://github.com/bitcoin/bitcoin/pull/29904 + + std::string uri {}; + // This is an invalid URI because it contains a % that is not followed by two hex digits + uri = "/rest/endpoint/someresource.json?p1=v1&p2=v2%"; + // Old behavior: URI with invalid characters (%) raises a runtime error regardless of which query parameter is queried + BOOST_CHECK_EXCEPTION(http_libevent::GetQueryParameterFromUri(uri.c_str(), "p1"), std::runtime_error, HasReason("URI parsing failed, it likely contained RFC 3986 invalid characters")); + // New behavior: Tolerate as much as we can even + BOOST_CHECK_EQUAL(http_bitcoin::GetQueryParameterFromUri(uri.c_str(), "p1").value(), "v1"); + BOOST_CHECK_EQUAL(http_bitcoin::GetQueryParameterFromUri(uri.c_str(), "p2").value(), "v2%"); + + // This is a valid URI because the %XX encoding is correct: `?p1=v1&p2=100%` + uri = "/rest/endpoint/someresource.json%3Fp1%3Dv1%26p2%3D100%25"; + // Old behavior: libevent does not decode the URI before parsing, so it does not detect or return the query + // (libevent will parse the entire argument string as the uri path) + BOOST_CHECK(!http_libevent::GetQueryParameterFromUri(uri.c_str(), "p1").has_value()); + BOOST_CHECK(!http_libevent::GetQueryParameterFromUri(uri.c_str(), "p2").has_value()); + // New behavior: Decode before parsing the URI so reserved characters like ? & = are interpreted correctly + BOOST_CHECK_EQUAL(http_bitcoin::GetQueryParameterFromUri(uri.c_str(), "p1").value(), "v1"); + BOOST_CHECK_EQUAL(http_bitcoin::GetQueryParameterFromUri(uri.c_str(), "p2").value(), "100%"); +} + +// Ensure new behavior matches old behavior +template +void test_query_parameters(func GetQueryParameterFromUri) { std::string uri {}; // No parameters @@ -75,9 +105,20 @@ BOOST_AUTO_TEST_CASE(test_query_parameters) uri = "/rest/endpoint/someresource.json&p1=v1&p2=v2"; BOOST_CHECK(!GetQueryParameterFromUri(uri.c_str(), "p1").has_value()); - // URI with invalid characters (%) raises a runtime error regardless of which query parameter is queried - uri = "/rest/endpoint/someresource.json&p1=v1&p2=v2%"; - BOOST_CHECK_EXCEPTION(GetQueryParameterFromUri(uri.c_str(), "p1"), std::runtime_error, HasReason("URI parsing failed, it likely contained RFC 3986 invalid characters")); + // Multiple parameters, some characters encoded + uri = "/rest/endpoint/someresource.json?p1=v1%20&p2=100%25"; + BOOST_CHECK_EQUAL(GetQueryParameterFromUri(uri.c_str(), "p1").value(), "v1 "); + BOOST_CHECK_EQUAL(GetQueryParameterFromUri(uri.c_str(), "p2").value(), "100%"); +} + +BOOST_AUTO_TEST_CASE(test_query_parameters_libevent) +{ + test_query_parameters(http_libevent::GetQueryParameterFromUri); +} + +BOOST_AUTO_TEST_CASE(test_query_parameters_bitcoin) +{ + test_query_parameters(http_bitcoin::GetQueryParameterFromUri); } BOOST_AUTO_TEST_CASE(http_headers_tests) @@ -167,7 +208,9 @@ BOOST_AUTO_TEST_CASE(http_request_tests) BOOST_CHECK(req.LoadHeaders(reader)); BOOST_CHECK(req.LoadBody(reader)); BOOST_CHECK_EQUAL(req.m_method, "POST"); + BOOST_CHECK_EQUAL(req.GetRequestMethod(), HTTPRequestMethod::POST); BOOST_CHECK_EQUAL(req.m_target, "/"); + BOOST_CHECK_EQUAL(req.GetURI(), "/"); BOOST_CHECK_EQUAL(req.m_version_major, 1); BOOST_CHECK_EQUAL(req.m_version_minor, 1); BOOST_CHECK_EQUAL(req.m_headers.Find("Host").value(), "127.0.0.1"); @@ -355,6 +398,7 @@ BOOST_AUTO_TEST_CASE(http_client_server_tests) // Check the received request BOOST_CHECK_EQUAL(requests.front()->m_body, "{\"method\":\"getblockcount\",\"params\":[],\"id\":1}\n"); + BOOST_CHECK_EQUAL(requests.front()->GetPeer().ToStringAddrPort(), "5.5.5.5:6789"); // Respond to request requests.front()->WriteReply(HTTP_OK, "874140\n");