mirrors/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-01-19 05:45:05 +01:00
Code Issues Projects Releases Wiki Activity

rpc: JSON-RPC 2.0 should not respond to "notifications"

Browse Source 
For JSON-RPC 2.0 requests we need to distinguish between
a missing "id" field and "id":null. This is accomplished
by making the JSONRPCRequest id property a
std::optional<UniValue> with a default value of
UniValue::VNULL.

A side-effect of this change for non-2.0 requests is that request which do not
specify an "id" field will no longer return "id": null in the response.
This commit is contained in:
Matthew Zipkin 2023-07-07 15:06:35 -04:00
parent bf1a1f1662
commit e7ee80dcf2
No known key found for this signature in database
GPG Key ID: E7E2984B6289C93A
6 changed files with 67 additions and 17 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
src/httprpc.cpp
View File

@ -211,6 +211,12 @@ static bool HTTPReq_JSONRPC(const std::any& context, HTTPRequest* req)
const bool catch_errors{jreq.m_json_version == JSONRPCVersion::V2};
reply = JSONRPCExec(jreq, catch_errors);
if (jreq.IsNotification()) {
// Even though we do execute notifications, we do not respond to them
req->WriteReply(HTTP_NO_CONTENT);
return true;
}
// array of requests
} else if (valRequest.isArray()) {
// Check authorization for each request's method
@ -235,15 +241,32 @@ static bool HTTPReq_JSONRPC(const std::any& context, HTTPRequest* req)
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.
// in "HTTP OK" responses. Notifications never get any response.
UniValue response;
try {
jreq.parse(valRequest[i]);
reply.push_back(JSONRPCExec(jreq, /*catch_errors=*/true));
response = JSONRPCExec(jreq, /*catch_errors=*/true);
} catch (UniValue& e) {
reply.push_back(JSONRPCReplyObj(NullUniValue, std::move(e), jreq.id, jreq.m_json_version));
response = JSONRPCReplyObj(NullUniValue, std::move(e), jreq.id, jreq.m_json_version);
} catch (const std::exception& e) {
reply.push_back(JSONRPCReplyObj(NullUniValue, JSONRPCError(RPC_PARSE_ERROR, e.what()), jreq.id, jreq.m_json_version));
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

1
src/rpc/protocol.h
View File

@ -10,6 +10,7 @@
enum HTTPStatusCode
{
HTTP_OK = 200,
HTTP_NO_CONTENT = 204,
HTTP_BAD_REQUEST = 400,
HTTP_UNAUTHORIZED = 401,
HTTP_FORBIDDEN = 403,

10
src/rpc/request.cpp
View File

@ -37,7 +37,7 @@ UniValue JSONRPCRequestObj(const std::string& strMethod, const UniValue& params,
return request;
}
UniValue JSONRPCReplyObj(UniValue result, UniValue error, UniValue id, JSONRPCVersion jsonrpc_version)
UniValue JSONRPCReplyObj(UniValue result, UniValue error, std::optional<UniValue> id, JSONRPCVersion jsonrpc_version)
{
UniValue reply(UniValue::VOBJ);
// Add JSON-RPC version number field in v2 only.
@ -52,7 +52,7 @@ UniValue JSONRPCReplyObj(UniValue result, UniValue error, UniValue id, JSONRPCVe
if (jsonrpc_version == JSONRPCVersion::V1_LEGACY) reply.pushKV("result", NullUniValue);
reply.pushKV("error", std::move(error));
}
reply.pushKV("id", std::move(id));
if (id.has_value()) reply.pushKV("id", std::move(id.value()));
return reply;
}
@ -172,7 +172,11 @@ void JSONRPCRequest::parse(const UniValue& valRequest)
const UniValue& request = valRequest.get_obj();
// Parse id now so errors from here on will have the id
id = request.find_value("id");
if (request.exists("id")) {
id = request.find_value("id");
} else {
id = std::nullopt;
}
// Check for JSON-RPC 2.0 (default 1.1)
m_json_version = JSONRPCVersion::V1_LEGACY;

6
src/rpc/request.h
View File

@ -7,6 +7,7 @@
#define BITCOIN_RPC_REQUEST_H
#include <any>
#include <optional>
#include <string>
#include <univalue.h>
@ -17,7 +18,7 @@ enum class JSONRPCVersion {
};
UniValue JSONRPCRequestObj(const std::string& strMethod, const UniValue& params, const UniValue& id);
UniValue JSONRPCReplyObj(UniValue result, UniValue error, UniValue id, JSONRPCVersion jsonrpc_version);
UniValue JSONRPCReplyObj(UniValue result, UniValue error, std::optional<UniValue> id, JSONRPCVersion jsonrpc_version);
UniValue JSONRPCError(int code, const std::string& message);
/** Generate a new RPC authentication cookie and write it to disk */
@ -32,7 +33,7 @@ std::vector<UniValue> JSONRPCProcessBatchReply(const UniValue& in);
class JSONRPCRequest
{
public:
UniValue id;
std::optional<UniValue> id = UniValue::VNULL;
std::string strMethod;
UniValue params;
enum Mode { EXECUTE, GET_HELP, GET_ARGS } mode = EXECUTE;
@ -43,6 +44,7 @@ public:
JSONRPCVersion m_json_version = JSONRPCVersion::V1_LEGACY;
void parse(const UniValue& valRequest);
[[nodiscard]] bool IsNotification() const { return !id.has_value() && m_json_version == JSONRPCVersion::V2; };
};
#endif // BITCOIN_RPC_REQUEST_H

27
test/functional/interface_rpc.py
View File

@ -46,8 +46,11 @@ def format_request(options, idx, fields):
def format_response(options, idx, fields):
if options.version == 2 and options.notification:
return None
response = {}
response.update(id=None if options.notification else idx)
if not options.notification:
response.update(id=idx)
if options.version == 2:
response.update(jsonrpc="2.0")
else:
@ -129,11 +132,17 @@ class RPCInterfaceTest(BitcoinTestFramework):
if options is None:
continue
request.append(format_request(options, idx, call))
response.append(format_response(options, idx, result))
r = format_response(options, idx, result)
if r is not None:
response.append(r)
rpc_response, http_status = send_json_rpc(self.nodes[0], request)
assert_equal(http_status, 200)
assert_equal(rpc_response, response)
if len(response) == 0 and len(request) > 0:
assert_equal(http_status, 204)
assert_equal(rpc_response, None)
else:
assert_equal(http_status, 200)
assert_equal(rpc_response, response)
def test_batch_requests(self):
self.log.info("Testing empty batch request...")
@ -193,10 +202,10 @@ class RPCInterfaceTest(BitcoinTestFramework):
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"}})
assert_equal(response, {"result": None, "error": {"code": RPC_INVALID_REQUEST, "message": "jsonrpc field must be a string"}})
assert_equal(status, 400)
response, status = send_json_rpc(self.nodes[0], {"jsonrpc": "3.0", "method": "getblockcount"})
assert_equal(response, {"id": None, "result": None, "error": {"code": RPC_INVALID_REQUEST, "message": "JSON-RPC version not supported"}})
assert_equal(response, {"result": None, "error": {"code": RPC_INVALID_REQUEST, "message": "JSON-RPC version not supported"}})
assert_equal(status, 400)
self.log.info("Testing HTTP status codes for JSON-RPC 2.0 notifications...")
@ -209,10 +218,12 @@ class RPCInterfaceTest(BitcoinTestFramework):
# Not notification: has "id" field
expect_http_rpc_status(200, None, self.nodes[0], "getblockcount", [], 2, False)
block_count = self.nodes[0].getblockcount()
expect_http_rpc_status(200, None, self.nodes[0], "generatetoaddress", [1, "bcrt1qqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqqdku202"], 2, True)
# Notification response status code: HTTP_NO_CONTENT
expect_http_rpc_status(204, 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(200, RPC_INVALID_ADDRESS_OR_KEY, self.nodes[0], "generatetoaddress", [1, "invalid_address"], 2, True)
# No error response for notifications even if they are invalid
expect_http_rpc_status(204, None, self.nodes[0], "generatetoaddress", [1, "invalid_address"], 2, True)
# Sanity check: command was not executed
assert_equal(block_count + 1, self.nodes[0].getblockcount())

9
test/functional/test_framework/authproxy.py
View File

@ -160,6 +160,15 @@ class AuthServiceProxy():
raise JSONRPCException({
'code': -342, 'message': 'missing HTTP response from server'})
# Check for no-content HTTP status code, which can be returned when an
# RPC client requests a JSON-RPC 2.0 "notification" with no response.
# Currently this is only possible if clients call the _request() method
# directly to send a raw request.
if http_response.status == HTTPStatus.NO_CONTENT:
if len(http_response.read()) != 0:
raise JSONRPCException({'code': -342, 'message': 'Content received with NO CONTENT status code'})
return None, http_response.status
content_type = http_response.getheader('Content-Type')
if content_type != 'application/json':
raise JSONRPCException(