From 6fbd01538b2d82e48761106ddd462d0459275c2e Mon Sep 17 00:00:00 2001 From: Liquid369 Date: Fri, 13 Sep 2024 11:28:14 -0500 Subject: [PATCH] boost::optional instead of std::optional, fix find_value calls and handle batches in HTTPReq_JSONRPC --- src/httprpc.cpp | 47 +++++++++++++++++++++++++++++--------------- src/rpc/protocol.cpp | 2 +- src/rpc/protocol.h | 4 ++-- src/rpc/server.cpp | 6 +++--- src/rpc/server.h | 4 ++-- 5 files changed, 39 insertions(+), 24 deletions(-) diff --git a/src/httprpc.cpp b/src/httprpc.cpp index dbbcd88734777..cd1074e4a0211 100644 --- a/src/httprpc.cpp +++ b/src/httprpc.cpp @@ -63,9 +63,6 @@ static std::unique_ptr httpRPCTimerInterface; static void JSONErrorReply(HTTPRequest* req, UniValue objError, const JSONRPCRequest& jreq) { - // Sending HTTP errors is a legacy JSON-RPC behavior. - Assume(jreq.m_json_version != JSONRPCVersion::V2); - // Send error reply from json-rpc error object int nStatus = HTTP_INTERNAL_SERVER_ERROR; int code = find_value(objError, "code").get_int(); @@ -180,22 +177,10 @@ static bool HTTPReq_JSONRPC(HTTPRequest* req, const std::string &) std::string strReply; UniValue reply; - bool user_has_whitelist = g_rpc_whitelist.count(jreq.authUser); - if (!user_has_whitelist && g_rpc_whitelist_default) { - LogPrintf("RPC User %s not allowed to call any methods\n", jreq.authUser); - req->WriteReply(HTTP_FORBIDDEN); - return false; // singleton request if (valRequest.isObject()) { jreq.parse(valRequest); - if (user_has_whitelist && !g_rpc_whitelist[jreq.authUser].count(jreq.strMethod)) { - LogPrintf("RPC User %s not allowed to call method %s\n", jreq.authUser, jreq.strMethod); - req->WriteReply(HTTP_FORBIDDEN); - return false; - } - - UniValue result = tableRPC.execute(jreq); // Legacy 1.0/1.1 behavior is for failed requests to throw // exceptions which return HTTP errors and RPC errors to the client. @@ -212,7 +197,37 @@ static bool HTTPReq_JSONRPC(HTTPRequest* req, const std::string &) // array of requests } else if (valRequest.isArray()) - strReply = JSONRPCExecBatch(valRequest.get_array()); + // Execute each request + reply = UniValue::VARR; + for (size_t i{0}; i < valRequest.size(); ++i) { + // Batches never throw HTTP errors, they are always just included + // in "HTTP OK" responses. Notifications never get any response. + UniValue response; + try { + jreq.parse(valRequest[i]); + response = JSONRPCExec(jreq, /*catch_errors=*/true); + } catch (UniValue& e) { + response = JSONRPCReplyObj(NullUniValue, std::move(e), jreq.id, jreq.m_json_version); + } catch (const std::exception& e) { + response = JSONRPCReplyObj(NullUniValue, JSONRPCError(RPC_PARSE_ERROR, e.what()), jreq.id, jreq.m_json_version); + } + if (!jreq.IsNotification()) { + reply.push_back(std::move(response)); + } + } + // Return no response for an all-notification batch, but only if the + // batch request is non-empty. Technically according to the JSON-RPC + // 2.0 spec, an empty batch request should also return no response, + // However, if the batch request is empty, it means the request did + // not contain any JSON-RPC version numbers, so returning an empty + // response could break backwards compatibility with old RPC clients + // relying on previous behavior. Return an empty array instead of an + // empty response in this case to favor being backwards compatible + // over complying with the JSON-RPC 2.0 spec in this case. + if (reply.size() == 0 && valRequest.size() > 0) { + req->WriteReply(HTTP_NO_CONTENT); + return true; + } else throw JSONRPCError(RPC_PARSE_ERROR, "Top-level object parse error"); diff --git a/src/rpc/protocol.cpp b/src/rpc/protocol.cpp index 07435bfab1d84..10a5036d21c2b 100644 --- a/src/rpc/protocol.cpp +++ b/src/rpc/protocol.cpp @@ -32,7 +32,7 @@ UniValue JSONRPCRequestObj(const std::string& strMethod, const UniValue& params, return request; } -UniValue JSONRPCReplyObj(UniValue result, UniValue error, std::optional id, JSONRPCVersion jsonrpc_version) +UniValue JSONRPCReplyObj(UniValue result, UniValue error, Optional id, JSONRPCVersion jsonrpc_version) { UniValue reply(UniValue::VOBJ); // Add JSON-RPC version number field in v2 only. diff --git a/src/rpc/protocol.h b/src/rpc/protocol.h index ab7f07b430d4d..ac1dbbc922c00 100644 --- a/src/rpc/protocol.h +++ b/src/rpc/protocol.h @@ -11,7 +11,7 @@ #include #include -#include +#include "optional.h" #include #include @@ -89,7 +89,7 @@ enum class JSONRPCVersion { }; UniValue JSONRPCRequestObj(const std::string& strMethod, const UniValue& params, const UniValue& id); -UniValue JSONRPCReplyObj(UniValue result, UniValue error, std::optional id, JSONRPCVersion jsonrpc_version); +UniValue JSONRPCReplyObj(UniValue result, UniValue error, Optional id, JSONRPCVersion jsonrpc_version); UniValue JSONRPCError(int code, const std::string& message); /** Get name of RPC authentication cookie file */ diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index 22baae2bd6324..23c703d6cd224 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -383,14 +383,14 @@ void JSONRPCRequest::parse(const UniValue& valRequest) // Parse id now so errors from here on will have the id if (request.exists("id")) { - id = request.find_value("id"); + id = find_value(request, "id"); } else { - id = std::nullopt; + id = nullopt; } // Check for JSON-RPC 2.0 (default 1.1) m_json_version = JSONRPCVersion::V1_LEGACY; - const UniValue& jsonrpc_version = request.find_value("jsonrpc"); + const UniValue& jsonrpc_version = find_value(request, "jsonrpc"); if (!jsonrpc_version.isNull()) { if (!jsonrpc_version.isStr()) { throw JSONRPCError(RPC_INVALID_REQUEST, "jsonrpc field must be a string"); diff --git a/src/rpc/server.h b/src/rpc/server.h index 4630b7010b87a..fc1a635bbc378 100644 --- a/src/rpc/server.h +++ b/src/rpc/server.h @@ -13,7 +13,7 @@ #include #include -#include +#include "optional.h" #include #include @@ -43,7 +43,7 @@ struct UniValueType { class JSONRPCRequest { public: - std::optional id = UniValue::VNULL; + Optional id = UniValue(UniValue::VNULL); std::string strMethod; UniValue params; bool fHelp;