define HTTP request methods at module level outside of class
What changed, and why it matters
This commit is a straightforward code cleanup: it moves the list of HTTP request methods (GET, POST, HEAD, PUT, UNKNOWN) out of a class and into a shared module-level type so two different parts of the codebase can use the same definitions. There is no security-relevant behavior change.
No security action needed; treat as normal refactoring review.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The patch refactors HTTPRequest::RequestMethod from a nested enum inside http_libevent::HTTPRequest into a top-level enum class HTTPRequestMethod. It updates all call sites, tests, and fuzz targets to use the new type. The function RequestMethodString now takes HTTPRequestMethod and returns std::string_view. The http_bitcoin::HTTPRequest::m_method field changes from std::string to HTTPRequestMethod. Logic for rejecting unknown methods and dispatching JSON-RPC only on POST remains identical.
Changed components
src/httpserver.hsrc/httpserver.cppsrc/httprpc.cppsrc/test/fuzz/http_request.cppsrc/test/httpserver_tests.cppInspect captured patch +45 / −36
diff --git a/src/httprpc.cpp b/src/httprpc.cpp
index 2fc7f093..1a4e9b9d 100644
--- a/src/httprpc.cpp
+++ b/src/httprpc.cpp
@@ -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;
}
diff --git a/src/httpserver.cpp b/src/httpserver.cpp
index ad6d020d..62e89598 100644
--- a/src/httpserver.cpp
+++ b/src/httpserver.cpp
@@ -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);
diff --git a/src/httpserver.h b/src/httpserver.h
index 339cd45f..4764ee56 100644
--- a/src/httpserver.h
+++ b/src/httpserver.h
@@ -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;
diff --git a/src/test/fuzz/http_request.cpp b/src/test/fuzz/http_request.cpp
index f64fb972..916fcb64 100644
--- a/src/test/fuzz/http_request.cpp
+++ b/src/test/fuzz/http_request.cpp
@@ -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");
diff --git a/src/test/httpserver_tests.cpp b/src/test/httpserver_tests.cpp
index c9b8eea5..46ac7195 100644
--- a/src/test/httpserver_tests.cpp
+++ b/src/test/httpserver_tests.cpp
@@ -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);
Why this scored 15/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.