http: switch servers from libevent to bitcoin

src/httprpc.cpp

@@ -28,8 +28,8 @@
 #include <string>
 #include <vector>
 
-using http_libevent::HTTPRequest;
 using node::NodeContext;
+using http_bitcoin::HTTPRequest;
 using util::SplitString;
 using util::TrimStringView;
 
@@ -83,7 +83,7 @@ static void JSONErrorReply(HTTPRequest* req, UniValue objError, const JSONRPCReq
     Assume(jreq.m_json_version != JSONRPCVersion::V2);
 
     // Send error reply from json-rpc error object
-    int nStatus = HTTP_INTERNAL_SERVER_ERROR;
+    HTTPStatusCode nStatus = HTTP_INTERNAL_SERVER_ERROR;
     int code = objError.find_value("code").getInt<int>();
 
     if (code == RPC_INVALID_REQUEST)

src/httpserver.cpp

@@ -46,7 +46,7 @@
 #include <support/events.h>
 
 using common::InvalidPortErrMsg;
-using http_libevent::HTTPRequest;
+using http_bitcoin::HTTPRequest;
 
 /** Maximum size of http request (request line + headers) */
 static const size_t MAX_HEADERS_SIZE = 8192;
@@ -281,9 +281,6 @@ static void MaybeDispatchRequestToWorker(std::unique_ptr<HTTPRequest> hreq)
         return;
     }
 
-    LogDebug(BCLog::HTTP, "Received a %s request for %s from %s\n",
-             RequestMethodString(hreq->GetRequestMethod()), SanitizeString(hreq->GetURI(), SAFE_CHARS_URI).substr(0, 100), hreq->GetPeer().ToStringAddrPort());
-
     // Find registered handler for prefix
     std::string strURI = hreq->GetURI();
     std::string path;
@@ -350,7 +347,11 @@ static void http_request_cb(struct evhttp_request* req, void* arg)
         }
     }
     auto hreq{std::make_unique<http_libevent::HTTPRequest>(req, *static_cast<const util::SignalInterrupt*>(arg))};
-    MaybeDispatchRequestToWorker(std::move(hreq));
+
+    // Disabled now that http_libevent is deprecated, or code won't compile.
+    // This line is currently unreachable and will be cleaned up in a future commit.
+    // MaybeDispatchRequestToWorker(std::move(hreq));
+    Assume(false);
 }
 
 /** Callback to reject HTTP requests after shutdown. */
@@ -1368,8 +1369,8 @@ bool InitHTTPServer(const util::SignalInterrupt& interrupt)
     if (!InitHTTPAllowList())
         return false;
 
-    // Create HTTPServer, using a dummy request handler just for this commit
-    g_http_server = std::make_unique<HTTPServer>([&](std::unique_ptr<HTTPRequest> req){});
+    // Create HTTPServer
+    g_http_server = std::make_unique<HTTPServer>(MaybeDispatchRequestToWorker);
 
     g_http_server->m_rpcservertimeout = std::chrono::seconds(gArgs.GetIntArg("-rpcservertimeout", DEFAULT_HTTP_SERVER_TIMEOUT));
 

src/httpserver.h

@@ -46,6 +46,8 @@ enum HTTPRequestMethod {
     PUT
 };
 
+namespace http_bitcoin {};
+
 namespace http_libevent {
 class HTTPRequest;
 
@@ -67,16 +69,6 @@ void StopHTTPServer();
 void UpdateHTTPServerLogging(bool enable);
 } // namespace http_libevent
 
-/** Handler for requests to a certain HTTP path */
-typedef std::function<bool(http_libevent::HTTPRequest* req, const std::string&)> HTTPRequestHandler;
-/** Register handler for prefix.
- * If multiple handlers match a prefix, the first-registered one will
- * be invoked.
- */
-void RegisterHTTPHandler(const std::string &prefix, bool exactMatch, const HTTPRequestHandler &handler);
-/** Unregister handler for prefix */
-void UnregisterHTTPHandler(const std::string &prefix, bool exactMatch);
-
 /** Return evhttp event base. This can be used by submodules to
  * queue timers or custom events.
  */
@@ -281,11 +273,17 @@ public:
     // 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 = {});
-    void WriteReply(HTTPStatusCode status, const char* reply_body) {
+    void WriteReply(HTTPStatusCode status, const char* reply_body)
+    {
         auto reply_body_view = std::string_view(reply_body);
         std::span<const std::byte> byte_span(reinterpret_cast<const std::byte*>(reply_body_view.data()), reply_body_view.size());
         WriteReply(status, byte_span);
     }
+    void WriteReply(HTTPStatusCode status, const std::string& reply_body)
+    {
+        std::span<const std::byte> byte_span{reinterpret_cast<const std::byte*>(reply_body.data()), reply_body.size()};
+        WriteReply(status, byte_span);
+    }
 };
 
 std::optional<std::string> GetQueryParameterFromUri(const std::string& uri, const std::string& key);
@@ -463,4 +461,14 @@ void InterruptHTTPServer();
 void StopHTTPServer();
 } // namespace http_bitcoin
 
+/** Handler for requests to a certain HTTP path */
+typedef std::function<bool(http_bitcoin::HTTPRequest* req, const std::string&)> HTTPRequestHandler;
+/** Register handler for prefix.
+ * If multiple handlers match a prefix, the first-registered one will
+ * be invoked.
+ */
+void RegisterHTTPHandler(const std::string &prefix, bool exactMatch, const HTTPRequestHandler &handler);
+/** Unregister handler for prefix */
+void UnregisterHTTPHandler(const std::string &prefix, bool exactMatch);
+
 #endif // BITCOIN_HTTPSERVER_H

src/init.cpp

@@ -121,10 +121,10 @@ using common::AmountErrMsg;
 using common::InvalidPortErrMsg;
 using common::ResolveErrMsg;
 
-using http_libevent::InitHTTPServer;
-using http_libevent::InterruptHTTPServer;
-using http_libevent::StartHTTPServer;
-using http_libevent::StopHTTPServer;
+using http_bitcoin::InitHTTPServer;
+using http_bitcoin::InterruptHTTPServer;
+using http_bitcoin::StartHTTPServer;
+using http_bitcoin::StopHTTPServer;
 using node::ApplyArgsManOptions;
 using node::BlockManager;
 using node::CalculateCacheSizes;

src/rest.cpp

@@ -37,7 +37,7 @@
 
 #include <univalue.h>
 
-using http_libevent::HTTPRequest;
+using http_bitcoin::HTTPRequest;
 using node::GetTransaction;
 using node::NodeContext;
 using util::SplitString;

src/test/fuzz/http_request.cpp

@@ -10,17 +10,12 @@
 #include <util/signalinterrupt.h>
 #include <util/strencodings.h>
 
-#include <event2/buffer.h>
-#include <event2/event.h>
-#include <event2/http.h>
-#include <event2/http_struct.h>
-
 #include <cassert>
 #include <cstdint>
 #include <string>
 #include <vector>
 
-using http_libevent::HTTPRequest;
+using http_bitcoin::HTTPRequest;
 
 extern "C" int evhttp_parse_firstline_(struct evhttp_request*, struct evbuffer*);
 extern "C" int evhttp_parse_headers_(struct evhttp_request*, struct evbuffer*);
@@ -30,28 +25,9 @@ std::string RequestMethodString(HTTPRequestMethod m);
 FUZZ_TARGET(http_request)
 {
     FuzzedDataProvider fuzzed_data_provider{buffer.data(), buffer.size()};
-    evhttp_request* evreq = evhttp_request_new(nullptr, nullptr);
-    assert(evreq != nullptr);
-    evreq->kind = EVHTTP_REQUEST;
-    evbuffer* evbuf = evbuffer_new();
-    assert(evbuf != nullptr);
-    const std::vector<uint8_t> http_buffer = ConsumeRandomLengthByteVector(fuzzed_data_provider, 4096);
-    evbuffer_add(evbuf, http_buffer.data(), http_buffer.size());
-    // Avoid constructing requests that will be interpreted by libevent as PROXY requests to avoid triggering
-    // a nullptr dereference. The dereference (req->evcon->http_server) takes place in evhttp_parse_request_line
-    // and is a consequence of our hacky but necessary use of the internal function evhttp_parse_firstline_ in
-    // this fuzzing harness. The workaround is not aesthetically pleasing, but it successfully avoids the troublesome
-    // code path. " http:// HTTP/1.1\n" was a crashing input prior to this workaround.
-    const std::string http_buffer_str = ToLower(std::string{http_buffer.begin(), http_buffer.end()});
-    if (http_buffer_str.find(" http://") != std::string::npos || http_buffer_str.find(" https://") != std::string::npos ||
-        evhttp_parse_firstline_(evreq, evbuf) != 1 || evhttp_parse_headers_(evreq, evbuf) != 1) {
-        evbuffer_free(evbuf);
-        evhttp_request_free(evreq);
-        return;
-    }
 
     util::SignalInterrupt interrupt;
-    HTTPRequest http_request{evreq, interrupt, true};
+    HTTPRequest http_request;
     const HTTPRequestMethod request_method = http_request.GetRequestMethod();
     (void)RequestMethodString(request_method);
     (void)http_request.GetURI();
@@ -64,7 +40,4 @@ FUZZ_TARGET(http_request)
     assert(body.empty());
     const CService service = http_request.GetPeer();
     assert(service.ToStringAddrPort() == "[::]:0");
-
-    evbuffer_free(evbuf);
-    evhttp_request_free(evreq);
 }

test/functional/interface_rest.py

@@ -282,10 +282,12 @@ class RESTTest (BitcoinTestFramework):
         assert_equal(len(json_obj), 1)  # ensure that there is one header in the json response
         assert_equal(json_obj[0]['hash'], bb_hash)  # request/response hash should be the same
 
-        # Check invalid uri (% symbol at the end of the request)
-        for invalid_uri in [f"/headers/{bb_hash}%", f"/blockfilterheaders/basic/{bb_hash}%", "/mempool/contents.json?%"]:
+        # Check tolerance for invalid URI (% symbol at the end of the request)
+        for invalid_uri in [f"/headers/{bb_hash}%", f"/blockfilterheaders/basic/{bb_hash}%"]:
             resp = self.test_rest_request(invalid_uri, ret_type=RetType.OBJ, status=400)
-            assert_equal(resp.read().decode('utf-8').rstrip(), "URI parsing failed, it likely contained RFC 3986 invalid characters")
+            assert_equal(resp.read().decode('utf-8').rstrip(), f"Invalid hash: {bb_hash}%")
+        resp = self.test_rest_request("/mempool/contents.json?%", ret_type=RetType.OBJ, status=200)
+        assert_equal(resp.read().decode('utf-8').rstrip(), "{}")
 
         # Compare with normal RPC block response
         rpc_block_json = self.nodes[0].getblock(bb_hash)