diff --git a/src/httprpc.cpp b/src/httprpc.cpp index 817c75f3b50..777ad32bbe2 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(); @@ -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 8e150ef2b75..a30fd3b7d06 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) +UniValue JSONRPCExec(const JSONRPCRequest& jreq, bool catch_errors) { - // 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 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 2761bcd9db1..c0e7bc04ad8 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(); diff --git a/test/functional/interface_rpc.py b/test/functional/interface_rpc.py index dc9fb5d40fa..824a7200cd4 100755 --- a/test/functional/interface_rpc.py +++ b/test/functional/interface_rpc.py @@ -188,9 +188,9 @@ class RPCInterfaceTest(BitcoinTestFramework): self.log.info("Testing HTTP status codes for JSON-RPC 2.0 requests...") # OK expect_http_rpc_status(200, None, self.nodes[0], "getblockhash", [0], 2, False) - # RPC errors and HTTP errors - expect_http_rpc_status(404, RPC_METHOD_NOT_FOUND, self.nodes[0], "invalidmethod", [], 2, False) - expect_http_rpc_status(500, RPC_INVALID_PARAMETER, self.nodes[0], "getblockhash", [42], 2, False) + # RPC errors but not HTTP errors + expect_http_rpc_status(200, RPC_METHOD_NOT_FOUND, self.nodes[0], "invalidmethod", [], 2, False) + expect_http_rpc_status(200, RPC_INVALID_PARAMETER, self.nodes[0], "getblockhash", [42], 2, False) # force-send invalidly formatted requests response, status = send_json_rpc(self.nodes[0], {"jsonrpc": 2, "method": "getblockcount"}) assert_equal(response, {"id": None, "result": None, "error": {"code": RPC_INVALID_REQUEST, "message": "jsonrpc field must be a string"}}) @@ -212,7 +212,7 @@ class RPCInterfaceTest(BitcoinTestFramework): expect_http_rpc_status(200, None, self.nodes[0], "generatetoaddress", [1, "bcrt1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqdku202"], 2, True) # The command worked even though there was no response assert_equal(block_count + 1, self.nodes[0].getblockcount()) - expect_http_rpc_status(500, RPC_INVALID_ADDRESS_OR_KEY, self.nodes[0], "generatetoaddress", [1, "invalid_address"], 2, True) + expect_http_rpc_status(200, RPC_INVALID_ADDRESS_OR_KEY, self.nodes[0], "generatetoaddress", [1, "invalid_address"], 2, True) # Sanity check: command was not executed assert_equal(block_count + 1, self.nodes[0].getblockcount())