diff --git a/src/httprpc.cpp b/src/httprpc.cpp index 5d226cbaf8d..4cb901f3763 100644 --- a/src/httprpc.cpp +++ b/src/httprpc.cpp @@ -28,8 +28,8 @@ #include #include -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(); if (code == RPC_INVALID_REQUEST) diff --git a/src/httpserver.cpp b/src/httpserver.cpp index 87c4369f554..d9d4fef9578 100644 --- a/src/httpserver.cpp +++ b/src/httpserver.cpp @@ -46,7 +46,7 @@ #include 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 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(req, *static_cast(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([&](std::unique_ptr req){}); + // Create HTTPServer + g_http_server = std::make_unique(MaybeDispatchRequestToWorker); g_http_server->m_rpcservertimeout = std::chrono::seconds(gArgs.GetIntArg("-rpcservertimeout", DEFAULT_HTTP_SERVER_TIMEOUT)); diff --git a/src/httpserver.h b/src/httpserver.h index cff1f287a7a..876bb1046c8 100644 --- a/src/httpserver.h +++ b/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 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 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 byte_span(reinterpret_cast(reply_body_view.data()), reply_body_view.size()); WriteReply(status, byte_span); } + void WriteReply(HTTPStatusCode status, const std::string& reply_body) + { + std::span byte_span{reinterpret_cast(reply_body.data()), reply_body.size()}; + WriteReply(status, byte_span); + } }; std::optional 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 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 diff --git a/src/init.cpp b/src/init.cpp index 5ecb3e61c6c..15ee418e22b 100644 --- a/src/init.cpp +++ b/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; diff --git a/src/rest.cpp b/src/rest.cpp index 1b2e1e14e18..2c22d9a6fd1 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -37,7 +37,7 @@ #include -using http_libevent::HTTPRequest; +using http_bitcoin::HTTPRequest; using node::GetTransaction; using node::NodeContext; using util::SplitString; diff --git a/src/test/fuzz/http_request.cpp b/src/test/fuzz/http_request.cpp index 331f8f6c27c..eb97a57ff2d 100644 --- a/src/test/fuzz/http_request.cpp +++ b/src/test/fuzz/http_request.cpp @@ -10,17 +10,12 @@ #include #include -#include -#include -#include -#include - #include #include #include #include -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 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); } diff --git a/test/functional/interface_rest.py b/test/functional/interface_rest.py index 0e294696b0e..5458f70a6f1 100755 --- a/test/functional/interface_rest.py +++ b/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)