diff options
author | Matthew Zipkin <pinheadmz@gmail.com> | 2023-07-07 14:41:23 -0400 |
---|---|---|
committer | Matthew Zipkin <pinheadmz@gmail.com> | 2024-05-14 11:15:54 -0400 |
commit | bf1a1f1662427fbf1a43bb951364eface469bdb7 (patch) | |
tree | 40c6da6c106ed68bf368cc2e52496f492866c563 /src | |
parent | 466b90562f4785de74b548f7c4a256069e2aaf43 (diff) | |
download | bitcoin-bf1a1f1662427fbf1a43bb951364eface469bdb7.tar.xz |
rpc: Avoid returning HTTP errors for JSON-RPC 2.0 requests
Avoid returning HTTP status errors for non-batch JSON-RPC 2.0 requests if the
RPC method failed but the HTTP request was otherwise valid. Batch requests
already did not return HTTP errors previously.
Diffstat (limited to 'src')
-rw-r--r-- | src/httprpc.cpp | 15 | ||||
-rw-r--r-- | src/rpc/server.cpp | 19 | ||||
-rw-r--r-- | src/rpc/server.h | 2 |
3 files changed, 27 insertions, 9 deletions
diff --git a/src/httprpc.cpp b/src/httprpc.cpp index 817c75f3b5..777ad32bbe 100644 --- a/src/httprpc.cpp +++ b/src/httprpc.cpp @@ -75,6 +75,9 @@ static bool g_rpc_whitelist_default = false; 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 = objError.find_value("code").getInt<int>(); @@ -201,7 +204,12 @@ static bool HTTPReq_JSONRPC(const std::any& context, HTTPRequest* req) return false; } - reply = JSONRPCExec(jreq); + // Legacy 1.0/1.1 behavior is for failed requests to throw + // exceptions which return HTTP errors and RPC errors to the client. + // 2.0 behavior is to catch exceptions and return HTTP success with + // RPC errors, as long as there is not an actual HTTP server error. + const bool catch_errors{jreq.m_json_version == JSONRPCVersion::V2}; + reply = JSONRPCExec(jreq, catch_errors); // array of requests } else if (valRequest.isArray()) { @@ -226,10 +234,11 @@ static bool HTTPReq_JSONRPC(const std::any& context, HTTPRequest* req) // Execute each request reply = UniValue::VARR; for (size_t i{0}; i < valRequest.size(); ++i) { - // Batches include errors in the batch response, they do not throw + // Batches never throw HTTP errors, they are always just included + // in "HTTP OK" responses. try { jreq.parse(valRequest[i]); - reply.push_back(JSONRPCExec(jreq)); + reply.push_back(JSONRPCExec(jreq, /*catch_errors=*/true)); } catch (UniValue& e) { reply.push_back(JSONRPCReplyObj(NullUniValue, std::move(e), jreq.id, jreq.m_json_version)); } catch (const std::exception& e) { diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index 8e150ef2b7..a30fd3b7d0 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -358,11 +358,20 @@ bool IsDeprecatedRPCEnabled(const std::string& method) return find(enabled_methods.begin(), enabled_methods.end(), method) != enabled_methods.end(); } -UniValue JSONRPCExec(const JSONRPCRequest& jreq) -{ - // Might throw exception. Single requests will throw and send HTTP error codes - // but inside a batch, we just include the error object and return HTTP 200 - UniValue result = tableRPC.execute(jreq); +UniValue JSONRPCExec(const JSONRPCRequest& jreq, bool catch_errors) +{ + UniValue result; + if (catch_errors) { + try { + result = tableRPC.execute(jreq); + } catch (UniValue& e) { + return JSONRPCReplyObj(NullUniValue, std::move(e), jreq.id, jreq.m_json_version); + } catch (const std::exception& e) { + return JSONRPCReplyObj(NullUniValue, JSONRPCError(RPC_MISC_ERROR, e.what()), jreq.id, jreq.m_json_version); + } + } else { + result = tableRPC.execute(jreq); + } return JSONRPCReplyObj(std::move(result), NullUniValue, jreq.id, jreq.m_json_version); } diff --git a/src/rpc/server.h b/src/rpc/server.h index 2761bcd9db..c0e7bc04ad 100644 --- a/src/rpc/server.h +++ b/src/rpc/server.h @@ -181,7 +181,7 @@ extern CRPCTable tableRPC; void StartRPC(); void InterruptRPC(); void StopRPC(); -UniValue JSONRPCExec(const JSONRPCRequest& jreq); +UniValue JSONRPCExec(const JSONRPCRequest& jreq, bool catch_errors); // Drop witness when serializing for RPC? bool RPCSerializationWithoutWitness(); |