refactor: split http_request_cb into libevent callback and dispatch
What the developer wrote
Authored by Matthew Zipkin
refactor: split http_request_cb into libevent callback and dispatch
The original function is passed to libevent as a callback when HTTP
requests are received and processed. It wrapped the libevent request
object in a http_libevent::HTTPRequest and then handed that off to
bitcoin for basic checks and finally dispatch to worker threads.
In this commit we split the function after the
http_libevent::HTTPRequest is created, and pass that object to a new
function that maintains the logic of checking and dispatching.
This will be the merge point for http_libevent and http_bitcoin,
where HTTPRequest objects from either namespace have the same
downstream lifecycle.
What changed, and why it matters
This is a pure code cleanup change. A single function that handles incoming HTTP requests was split into two functions: one that prepares the raw libevent request and wraps it, and another that performs the existing allow-checks and hands the request to worker threads. No behavior, security checks, or request handling logic was changed.
No security action needed. Treat as normal refactoring review.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The commit refactors http_request_cb in src/httpserver.cpp. The original callback performed libevent bookkeeping (tracking active requests, setting completion/close callbacks, applying a libevent read-disable workaround) and then wrapped evhttp_request in std::make_shared<HTTPRequest> and proceeded to client-allow checks and dispatch. The refactor moves the post-wrap logic into a new static function MaybeDispatchRequestToWorker(std::shared_ptr<HTTPRequest>), which http_request_cb calls. The diff shows the same code blocks moved verbatim; no logic, ordering, or data handling changed.
Changed components
src/httpserver.cppInspect captured patch +31 / −27
diff --git a/src/httpserver.cpp b/src/httpserver.cpp
index dec81f3a..9587a99b 100644
--- a/src/httpserver.cpp
+++ b/src/httpserver.cpp
@@ -197,34 +197,8 @@ std::string_view RequestMethodString(HTTPRequestMethod m)
assert(false);
}
-/** HTTP request callback */
-static void http_request_cb(struct evhttp_request* req, void* arg)
+static void MaybeDispatchRequestToWorker(std::shared_ptr<HTTPRequest> hreq)
{
- evhttp_connection* conn{evhttp_request_get_connection(req)};
- // Track active requests
- {
- g_requests.AddRequest(req);
- evhttp_request_set_on_complete_cb(req, [](struct evhttp_request* req, void*) {
- g_requests.RemoveRequest(req);
- }, nullptr);
- evhttp_connection_set_closecb(conn, [](evhttp_connection* conn, void* arg) {
- g_requests.RemoveConnection(conn);
- }, nullptr);
- }
-
- // Disable reading to work around a libevent bug, fixed in 2.1.9
- // See https://github.com/libevent/libevent/commit/5ff8eb26371c4dc56f384b2de35bea2d87814779
- // and https://github.com/bitcoin/bitcoin/pull/11593.
- if (event_get_version_number() >= 0x02010600 && event_get_version_number() < 0x02010900) {
- if (conn) {
- bufferevent* bev = evhttp_connection_get_bufferevent(conn);
- if (bev) {
- bufferevent_disable(bev, EV_READ);
- }
- }
- }
- auto hreq{std::make_shared<HTTPRequest>(req, *static_cast<const util::SignalInterrupt*>(arg))};
-
// Early address-based allow check
if (!ClientAllowed(hreq->GetPeer())) {
LogDebug(BCLog::HTTP, "HTTP request from %s rejected: Client network is not allowed RPC access\n",
@@ -300,6 +274,36 @@ static void http_request_cb(struct evhttp_request* req, void* arg)
}
}
+/** HTTP request callback */
+static void http_request_cb(struct evhttp_request* req, void* arg)
+{
+ evhttp_connection* conn{evhttp_request_get_connection(req)};
+ // Track active requests
+ {
+ g_requests.AddRequest(req);
+ evhttp_request_set_on_complete_cb(req, [](struct evhttp_request* req, void*) {
+ g_requests.RemoveRequest(req);
+ }, nullptr);
+ evhttp_connection_set_closecb(conn, [](evhttp_connection* conn, void* arg) {
+ g_requests.RemoveConnection(conn);
+ }, nullptr);
+ }
+
+ // Disable reading to work around a libevent bug, fixed in 2.1.9
+ // See https://github.com/libevent/libevent/commit/5ff8eb26371c4dc56f384b2de35bea2d87814779
+ // and https://github.com/bitcoin/bitcoin/pull/11593.
+ if (event_get_version_number() >= 0x02010600 && event_get_version_number() < 0x02010900) {
+ if (conn) {
+ bufferevent* bev = evhttp_connection_get_bufferevent(conn);
+ if (bev) {
+ bufferevent_disable(bev, EV_READ);
+ }
+ }
+ }
+ auto hreq{std::make_shared<HTTPRequest>(req, *static_cast<const util::SignalInterrupt*>(arg))};
+ MaybeDispatchRequestToWorker(std::move(hreq));
+}
+
/** Callback to reject HTTP requests after shutdown. */
static void http_reject_request_cb(struct evhttp_request* req, void*)
{
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.