http-server: guard against crashes from unhandled exceptions
What the developer wrote
Authored by furszy
http-server: guard against crashes from unhandled exceptions
Currently, if an exception is thrown at the top-level HTTP request
handler (prior to invoking the command), the program crashes.
Ideally, each handler should catch all exceptions internally and
be responsible for sanitizing them and crafting the client response.
This is because only the handler knows the correct response format,
which differs per server type. However, because this cannot always
be guaranteed, it is safer to also catch exceptions in the top-level
server code, log the unexpected error, and disconnect the socket.
This both guards against crashes caused by uncaught exceptions and
prevents the client from hanging indefinitely while waiting for a
response that will never arrive.
The following diff can be used to trigger the crash in master
(just run single node functional tests like feature_shutdown.py):
```
diff --git a/src/httprpc.cpp b/src/httprpc.cpp
--- a/src/httprpc.cpp
+++ b/src/httprpc.cpp
@@ -103,6 +103,9 @@
static bool HTTPReq_JSONRPC(const std::any& context, HTTPRequest* req)
{
+ static int i = 0; // skip initial requests as they are used in the RPC warmup phase.
+ if (i++ > 3) throw std::runtime_error("error from json rpc handler");
+
// JSONRPC handles only POST
if (req->GetRequestMethod() != HTTPRequest::POST) {
req->WriteReply(HTTP_BAD_METHOD, "JSONRPC server handles only POST requests");
```
Note:
This leaves a TODO in the code because error responses should eventually
be specialized per server type. REST clients expect plain text responses,
while JSON-RPC clients expect a JSON error object.
The TODO is there because this is not consistently enforced everywhere
in the current codebase, and we should tackle them all at once.
What changed, and why it matters
This change fixes a bug in Bitcoin Core's built-in web server where an unexpected error inside an HTTP request handler could crash the entire program. The patch wraps each request in a safety net: if something throws an error, it is logged, the client connection is closed, and the program keeps running instead of crashing. The commit message also includes a test snippet showing the crash can be triggered intentionally.
Apply the patch. It is a straightforward defensive hardening change. Review individual HTTP handlers separately to ensure they catch exceptions internally and produce format-appropriate error responses (JSON-RPC vs REST), as noted by the TODO.
Security signals we found
Denial-of-service via unhandled exception in HTTP request handler
Process crash from top-level exception propagation
Client hang due to missing response on handler failure
Defensive catch-all exception handling added at HTTP server boundary
Evidence from the diff
The commit modifies src/httpserver.cpp to wrap the HTTP handler invocation in a try/catch block inside the dispatcher lambda created in http_request_cb. Previously, an exception thrown from a handler (e.g., HTTPReq_JSONRPC in httprpc.cpp) would propagate out of the worker thread and terminate the process. Now, std::exception and catch-all (…) exceptions are caught, logged with LogWarning, the Connection: close header is written, and an HTTP 500 Internal Server Error reply is sent before returning false. The patch is defensive and does not address per-server-type error formatting, leaving a TODO for future work.
Changed components
src/httpserver.cppHTTP server request dispatcherHTTPWorkItem handler wrapperInspect captured patch +18 / −1
diff --git a/src/httpserver.cpp b/src/httpserver.cpp
index 71c6f5b1..61df454a 100644
--- a/src/httpserver.cpp
+++ b/src/httpserver.cpp
@@ -327,7 +327,24 @@ static void http_request_cb(struct evhttp_request* req, void* arg)
// Dispatch to worker thread
if (i != iend) {
- std::unique_ptr<HTTPWorkItem> item(new HTTPWorkItem(std::move(hreq), path, i->handler));
+ auto item = std::make_unique<HTTPWorkItem>(std::move(hreq), path, [fn = i->handler](HTTPRequest* req, const std::string& path_inner) {
+ std::string err_msg;
+ try {
+ return fn(req, path_inner);
+ } catch (const std::exception& e) {
+ LogWarning("Unexpected error while processing request for '%s'. Error msg: '%s'", req->GetURI(), e.what());
+ err_msg = e.what();
+ } catch (...) {
+ LogWarning("Unknown error while processing request for '%s'", req->GetURI());
+ err_msg = "unknown error";
+ }
+ // Reply so the client doesn't hang waiting for the response.
+ req->WriteHeader("Connection", "close");
+ // TODO: Implement specific error formatting for the REST and JSON-RPC servers responses.
+ req->WriteReply(HTTP_INTERNAL_SERVER_ERROR, err_msg);
+ return false;
+ });
+
assert(g_work_queue);
if (g_work_queue->Enqueue(item.get())) {
(void)item.release(); /* if true, queue took ownership */
Why this scored 62/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.