define HTTP request methods at module level outside of class

This is a refactor to prepare for matching the API of HTTPRequest
definitions in both namespaces http_bitcoin and http_libevent. In
particular, to provide a consistent return type for GetRequestMethod()
in both classes.
This commit is contained in:
Matthew Zipkin
2024-12-11 13:21:31 -05:00
parent 7ee7df988e
commit dd11b5e01b
5 changed files with 45 additions and 36 deletions

View File

@@ -198,7 +198,7 @@ UniValue ExecuteHTTPRPC(const UniValue& valRequest, JSONRPCRequest& jreq, HTTPSt
static void HTTPReq_JSONRPC(const std::any& context, HTTPRequest* req)
{
// JSONRPC handles only POST
if (req->GetRequestMethod() != HTTPRequest::POST) {
if (req->GetRequestMethod() != HTTPRequestMethod::POST) {
req->WriteReply(HTTP_BAD_METHOD, "JSONRPC server handles only POST requests");
return;
}

View File

@@ -183,19 +183,15 @@ static bool InitHTTPAllowList()
}
/** HTTP request method as string - use for logging only */
std::string RequestMethodString(HTTPRequest::RequestMethod m)
std::string_view RequestMethodString(HTTPRequestMethod m)
{
switch (m) {
case HTTPRequest::GET:
return "GET";
case HTTPRequest::POST:
return "POST";
case HTTPRequest::HEAD:
return "HEAD";
case HTTPRequest::PUT:
return "PUT";
case HTTPRequest::UNKNOWN:
return "unknown";
using enum HTTPRequestMethod;
case GET: return "GET";
case POST: return "POST";
case HEAD: return "HEAD";
case PUT: return "PUT";
case UNKNOWN: return "unknown";
} // no default case, so the compiler can warn about missing cases
assert(false);
}
@@ -237,7 +233,7 @@ static void http_request_cb(struct evhttp_request* req, void* arg)
}
// Early reject unknown HTTP methods
if (hreq->GetRequestMethod() == HTTPRequest::UNKNOWN) {
if (hreq->GetRequestMethod() == HTTPRequestMethod::UNKNOWN) {
LogDebug(BCLog::HTTP, "HTTP request from %s rejected: Unknown HTTP request method\n",
hreq->GetPeer().ToStringAddrPort());
hreq->WriteReply(HTTP_BAD_METHOD);
@@ -655,19 +651,19 @@ std::string HTTPRequest::GetURI() const
return evhttp_request_get_uri(req);
}
HTTPRequest::RequestMethod HTTPRequest::GetRequestMethod() const
HTTPRequestMethod HTTPRequest::GetRequestMethod() const
{
switch (evhttp_request_get_command(req)) {
case EVHTTP_REQ_GET:
return GET;
return HTTPRequestMethod::GET;
case EVHTTP_REQ_POST:
return POST;
return HTTPRequestMethod::POST;
case EVHTTP_REQ_HEAD:
return HEAD;
return HTTPRequestMethod::HEAD;
case EVHTTP_REQ_PUT:
return PUT;
return HTTPRequestMethod::PUT;
default:
return UNKNOWN;
return HTTPRequestMethod::UNKNOWN;
}
}
@@ -849,7 +845,19 @@ bool HTTPRequest::LoadControlData(LineReader& reader)
const std::vector<std::string_view> parts{Split<std::string_view>(request_line, " ")};
if (parts.size() != 3) throw std::runtime_error("HTTP request line malformed");
m_method = parts[0];
if (parts[0] == "GET") {
m_method = HTTPRequestMethod::GET;
} else if (parts[0] == "POST") {
m_method = HTTPRequestMethod::POST;
} else if (parts[0] == "HEAD") {
m_method = HTTPRequestMethod::HEAD;
} else if (parts[0] == "PUT") {
m_method = HTTPRequestMethod::PUT;
} else {
m_method = HTTPRequestMethod::UNKNOWN;
}
m_target = parts[1];
if (parts[2].rfind("HTTP/") != 0) throw std::runtime_error("HTTP request line malformed");
@@ -1411,7 +1419,7 @@ void HTTPServer::MaybeDispatchRequestsFromClient(const std::shared_ptr<HTTPRemot
LogDebug(
BCLog::HTTP,
"Received a %s request for %s from %s (id=%llu)",
req->m_method,
RequestMethodString(req->m_method),
req->m_target,
client->m_origin,
client->m_id);

View File

@@ -45,6 +45,14 @@ struct evhttp_request;
struct event_base;
class CService;
enum class HTTPRequestMethod {
UNKNOWN,
GET,
POST,
HEAD,
PUT
};
namespace http_libevent {
class HTTPRequest;
@@ -96,14 +104,6 @@ public:
explicit HTTPRequest(struct evhttp_request* req, const util::SignalInterrupt& interrupt, bool replySent = false);
~HTTPRequest();
enum RequestMethod {
UNKNOWN,
GET,
POST,
HEAD,
PUT
};
/** Get requested URI.
*/
std::string GetURI() const;
@@ -114,7 +114,7 @@ public:
/** Get request method.
*/
RequestMethod GetRequestMethod() const;
HTTPRequestMethod GetRequestMethod() const;
/** Get the query parameter value from request uri for a specified key, or std::nullopt if the
* key is not found.
@@ -287,7 +287,7 @@ class HTTPRemoteClient;
class HTTPRequest
{
public:
std::string m_method;
HTTPRequestMethod m_method;
std::string m_target;
HTTPVersion m_version;
HTTPHeaders m_headers;

View File

@@ -22,12 +22,13 @@
extern "C" int evhttp_parse_firstline_(struct evhttp_request*, struct evbuffer*);
extern "C" int evhttp_parse_headers_(struct evhttp_request*, struct evbuffer*);
std::string_view RequestMethodString(HTTPRequestMethod m);
FUZZ_TARGET(http_request)
{
using http_libevent::HTTPRequest;
std::string RequestMethodString(HTTPRequest::RequestMethod m);
FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()};
evhttp_request* evreq = evhttp_request_new(nullptr, nullptr);
assert(evreq != nullptr);
@@ -51,7 +52,7 @@ FUZZ_TARGET(http_request)
util::SignalInterrupt interrupt;
HTTPRequest http_request{evreq, interrupt, true};
const HTTPRequest::RequestMethod request_method = http_request.GetRequestMethod();
const HTTPRequestMethod request_method = http_request.GetRequestMethod();
(void)RequestMethodString(request_method);
(void)http_request.GetURI();
(void)http_request.GetHeader("Host");

View File

@@ -189,7 +189,7 @@ BOOST_AUTO_TEST_CASE(http_request_tests)
BOOST_CHECK(req.LoadControlData(reader));
BOOST_CHECK(req.LoadHeaders(reader));
BOOST_CHECK(req.LoadBody(reader));
BOOST_CHECK_EQUAL(req.m_method, "POST");
BOOST_CHECK_EQUAL(req.m_method, HTTPRequestMethod::POST);
BOOST_CHECK_EQUAL(req.m_target, "/");
BOOST_CHECK_EQUAL(req.m_version.major, 1);
BOOST_CHECK_EQUAL(req.m_version.minor, 1);
@@ -294,7 +294,7 @@ BOOST_AUTO_TEST_CASE(http_request_tests)
BOOST_CHECK(req.LoadControlData(reader));
BOOST_CHECK(req.LoadHeaders(reader));
BOOST_CHECK(req.LoadBody(reader));
BOOST_CHECK_EQUAL(req.m_method, "GET");
BOOST_CHECK_EQUAL(req.m_method, HTTPRequestMethod::GET);
BOOST_CHECK_EQUAL(req.m_target, "/");
BOOST_CHECK_EQUAL(req.m_version.major, 1);
BOOST_CHECK_EQUAL(req.m_version.minor, 0);