Add helper methods to HTTPRequest to match original API

These methods are called by http_request_cb() and are present in the
original http_libevent::HTTPRequest.
This commit is contained in:
Matthew Zipkin 2024-12-12 11:44:52 -05:00 committed by Matthew Zipkin
parent 5b61d65034
commit 58fe646d3c
No known key found for this signature in database
GPG Key ID: E7E2984B6289C93A
3 changed files with 124 additions and 4 deletions

View File

@ -7,6 +7,7 @@
#include <chainparamsbase.h>
#include <common/args.h>
#include <common/messages.h>
#include <common/url.h>
#include <compat/compat.h>
#include <logging.h>
#include <netbase.h>
@ -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<std::string> 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<std::string> 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<std::string> 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<bool, std::string> HTTPRequest::GetHeader(const std::string& hdr) const
{
std::optional<std::string> 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<const std::byte> reply_body)
{
HTTPResponse res;

View File

@ -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<std::string> GetQueryParameter(const std::string& key) const;
std::pair<bool, std::string> 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<const std::byte> reply_body = {});
@ -277,6 +287,8 @@ public:
}
};
std::optional<std::string> GetQueryParameterFromUri(const std::string& uri, const std::string& key);
class HTTPServer;
class HTTPClient

View File

@ -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 <typename func>
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");