highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-06-23 15:22:46 +02:00
Code Issues Packages Projects Releases Wiki Activity

rpc: Avoid returning HTTP errors for JSON-RPC 2.0 requests

Browse Source 
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.
This commit is contained in:
Matthew Zipkin 2023-07-07 14:41:23 -04:00
parent 466b90562f
commit bf1a1f1662
No known key found for this signature in database
GPG Key ID: E7E2984B6289C93A
4 changed files with 30 additions and 12 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
src/httprpc.cpp
View File

@ -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) {

17
src/rpc/server.cpp
View File

@ -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);
}

2
src/rpc/server.h
View File

@ -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();

8
test/functional/interface_rpc.py
View File

@ -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())