From 4202c170da37a3203e05a9f39f303d7df19b6d81 Mon Sep 17 00:00:00 2001 From: Matthew Zipkin Date: Mon, 27 Feb 2023 13:00:37 -0500 Subject: test: refactor interface_rpc.py Refactors the helper functions in the test to provide more fine-grained control over RPC requests and responses than the usual AuthProxy methods. No change in test behavior, the same RPC requests are made. --- test/functional/interface_rpc.py | 112 +++++++++++++++++++++++++++++---------- 1 file changed, 83 insertions(+), 29 deletions(-) (limited to 'test') diff --git a/test/functional/interface_rpc.py b/test/functional/interface_rpc.py index e873e2da0b..501a841805 100755 --- a/test/functional/interface_rpc.py +++ b/test/functional/interface_rpc.py @@ -4,22 +4,72 @@ # file COPYING or http://www.opensource.org/licenses/mit-license.php. """Tests some generic aspects of the RPC interface.""" +import json import os -from test_framework.authproxy import JSONRPCException +from dataclasses import dataclass from test_framework.test_framework import BitcoinTestFramework from test_framework.util import assert_equal, assert_greater_than_or_equal from threading import Thread +from typing import Optional import subprocess -def expect_http_status(expected_http_status, expected_rpc_code, - fcn, *args): - try: - fcn(*args) - raise AssertionError(f"Expected RPC error {expected_rpc_code}, got none") - except JSONRPCException as exc: - assert_equal(exc.error["code"], expected_rpc_code) - assert_equal(exc.http_status, expected_http_status) +RPC_INVALID_PARAMETER = -8 +RPC_METHOD_NOT_FOUND = -32601 +RPC_INVALID_REQUEST = -32600 + + +@dataclass +class BatchOptions: + version: Optional[int] = None + notification: bool = False + request_fields: Optional[dict] = None + response_fields: Optional[dict] = None + + +def format_request(options, idx, fields): + request = {} + if options.version == 1: + request.update(version="1.1") + elif options.version == 2: + request.update(jsonrpc="2.0") + elif options.version is not None: + raise NotImplementedError(f"Unknown JSONRPC version {options.version}") + if not options.notification: + request.update(id=idx) + request.update(fields) + if options.request_fields: + request.update(options.request_fields) + return request + + +def format_response(options, idx, fields): + response = {} + response.update(id=None if options.notification else idx) + response.update(result=None, error=None) + response.update(fields) + if options.response_fields: + response.update(options.response_fields) + return response + + +def send_raw_rpc(node, raw_body: bytes) -> tuple[object, int]: + return node._request("POST", "/", raw_body) + + +def send_json_rpc(node, body: object) -> tuple[object, int]: + raw = json.dumps(body).encode("utf-8") + return send_raw_rpc(node, raw) + + +def expect_http_rpc_status(expected_http_status, expected_rpc_error_code, node, method, params, version=1, notification=False): + req = format_request(BatchOptions(version, notification), 0, {"method": method, "params": params}) + response, status = send_json_rpc(node, req) + + if expected_rpc_error_code is not None: + assert_equal(response["error"]["code"], expected_rpc_error_code) + + assert_equal(status, expected_http_status) def test_work_queue_getblock(node, got_exceeded_error): @@ -51,34 +101,38 @@ class RPCInterfaceTest(BitcoinTestFramework): def test_batch_request(self): self.log.info("Testing basic JSON-RPC batch request...") - results = self.nodes[0].batch([ + calls = [ # A basic request that will work fine. - {"method": "getblockcount", "id": 1}, + {"method": "getblockcount"}, # Request that will fail. The whole batch request should still # work fine. - {"method": "invalidmethod", "id": 2}, + {"method": "invalidmethod"}, # Another call that should succeed. - {"method": "getblockhash", "id": 3, "params": [0]}, - ]) - - result_by_id = {} - for res in results: - result_by_id[res["id"]] = res - - assert_equal(result_by_id[1]['error'], None) - assert_equal(result_by_id[1]['result'], 0) - - assert_equal(result_by_id[2]['error']['code'], -32601) - assert_equal(result_by_id[2]['result'], None) - - assert_equal(result_by_id[3]['error'], None) - assert result_by_id[3]['result'] is not None + {"method": "getblockhash", "params": [0]}, + ] + results = [ + {"result": 0}, + {"error": {"code": RPC_METHOD_NOT_FOUND, "message": "Method not found"}}, + {"result": "0f9188f13cb7b2c71f2a335e3a4fc328bf5beb436012afca590b1a11466e2206"}, + {"error": {"code": RPC_INVALID_REQUEST, "message": "Missing method"}}, + ] + + request = [] + response = [] + for idx, (call, result) in enumerate(zip(calls, results), 1): + options = BatchOptions() + request.append(format_request(options, idx, call)) + response.append(format_response(options, idx, result)) + + rpc_response, http_status = send_json_rpc(self.nodes[0], request) + assert_equal(http_status, 200) + assert_equal(rpc_response, response) def test_http_status_codes(self): self.log.info("Testing HTTP status codes for JSON-RPC requests...") - expect_http_status(404, -32601, self.nodes[0].invalidmethod) - expect_http_status(500, -8, self.nodes[0].getblockhash, 42) + expect_http_rpc_status(404, RPC_METHOD_NOT_FOUND, self.nodes[0], "invalidmethod", []) + expect_http_rpc_status(500, RPC_INVALID_PARAMETER, self.nodes[0], "getblockhash", [42]) def test_work_queue_exceeded(self): self.log.info("Testing work queue exceeded...") -- cgit v1.2.3 From 09416f9ec445e4d6bb277400758083b0b4e8b174 Mon Sep 17 00:00:00 2001 From: Matthew Zipkin Date: Sun, 31 Dec 2023 07:55:03 -0500 Subject: test: cover JSONRPC 2.0 requests, batches, and notifications --- test/functional/interface_rpc.py | 97 +++++++++++++++++++++++++++++++++++----- 1 file changed, 87 insertions(+), 10 deletions(-) (limited to 'test') diff --git a/test/functional/interface_rpc.py b/test/functional/interface_rpc.py index 501a841805..748d83858a 100755 --- a/test/functional/interface_rpc.py +++ b/test/functional/interface_rpc.py @@ -14,9 +14,11 @@ from typing import Optional import subprocess -RPC_INVALID_PARAMETER = -8 -RPC_METHOD_NOT_FOUND = -32601 -RPC_INVALID_REQUEST = -32600 +RPC_INVALID_ADDRESS_OR_KEY = -5 +RPC_INVALID_PARAMETER = -8 +RPC_METHOD_NOT_FOUND = -32601 +RPC_INVALID_REQUEST = -32600 +RPC_PARSE_ERROR = -32700 @dataclass @@ -98,9 +100,7 @@ class RPCInterfaceTest(BitcoinTestFramework): assert_greater_than_or_equal(command['duration'], 0) assert_equal(info['logpath'], os.path.join(self.nodes[0].chain_path, 'debug.log')) - def test_batch_request(self): - self.log.info("Testing basic JSON-RPC batch request...") - + def test_batch_request(self, call_options): calls = [ # A basic request that will work fine. {"method": "getblockcount"}, @@ -109,6 +109,8 @@ class RPCInterfaceTest(BitcoinTestFramework): {"method": "invalidmethod"}, # Another call that should succeed. {"method": "getblockhash", "params": [0]}, + # Invalid request format + {"pizza": "sausage"} ] results = [ {"result": 0}, @@ -120,7 +122,9 @@ class RPCInterfaceTest(BitcoinTestFramework): request = [] response = [] for idx, (call, result) in enumerate(zip(calls, results), 1): - options = BatchOptions() + options = call_options(idx) + if options is None: + continue request.append(format_request(options, idx, call)) response.append(format_response(options, idx, result)) @@ -128,11 +132,84 @@ class RPCInterfaceTest(BitcoinTestFramework): assert_equal(http_status, 200) assert_equal(rpc_response, response) - def test_http_status_codes(self): - self.log.info("Testing HTTP status codes for JSON-RPC requests...") + def test_batch_requests(self): + self.log.info("Testing empty batch request...") + self.test_batch_request(lambda idx: None) + + self.log.info("Testing basic JSON-RPC 2.0 batch request...") + self.test_batch_request(lambda idx: BatchOptions(version=2)) + + self.log.info("Testing JSON-RPC 2.0 batch with notifications...") + self.test_batch_request(lambda idx: BatchOptions(version=2, notification=idx < 2)) + + self.log.info("Testing JSON-RPC 2.0 batch of ALL notifications...") + self.test_batch_request(lambda idx: BatchOptions(version=2, notification=True)) + + # JSONRPC 1.1 does not support batch requests, but test them for backwards compatibility. + self.log.info("Testing nonstandard JSON-RPC 1.1 batch request...") + self.test_batch_request(lambda idx: BatchOptions(version=1)) + self.log.info("Testing nonstandard mixed JSON-RPC 1.1/2.0 batch request...") + self.test_batch_request(lambda idx: BatchOptions(version=2 if idx % 2 else 1)) + + self.log.info("Testing nonstandard batch request without version numbers...") + self.test_batch_request(lambda idx: BatchOptions()) + + self.log.info("Testing nonstandard batch request without version numbers or ids...") + self.test_batch_request(lambda idx: BatchOptions(notification=True)) + + self.log.info("Testing nonstandard jsonrpc 1.0 version number is accepted...") + self.test_batch_request(lambda idx: BatchOptions(request_fields={"jsonrpc": "1.0"})) + + self.log.info("Testing unrecognized jsonrpc version number is accepted...") + self.test_batch_request(lambda idx: BatchOptions(request_fields={"jsonrpc": "2.1"})) + + def test_http_status_codes(self): + self.log.info("Testing HTTP status codes for JSON-RPC 1.1 requests...") + # OK + expect_http_rpc_status(200, None, self.nodes[0], "getblockhash", [0]) + # Errors expect_http_rpc_status(404, RPC_METHOD_NOT_FOUND, self.nodes[0], "invalidmethod", []) expect_http_rpc_status(500, RPC_INVALID_PARAMETER, self.nodes[0], "getblockhash", [42]) + # force-send empty request + response, status = send_raw_rpc(self.nodes[0], b"") + assert_equal(response, {"id": None, "result": None, "error": {"code": RPC_PARSE_ERROR, "message": "Parse error"}}) + assert_equal(status, 500) + # force-send invalidly formatted request + response, status = send_raw_rpc(self.nodes[0], b"this is bad") + assert_equal(response, {"id": None, "result": None, "error": {"code": RPC_PARSE_ERROR, "message": "Parse error"}}) + assert_equal(status, 500) + + 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) + # force-send invalidly formatted requests + response, status = send_json_rpc(self.nodes[0], {"jsonrpc": 2, "method": "getblockcount"}) + assert_equal(response, {"error": None, "id": None, "result": 0}) + assert_equal(status, 200) + response, status = send_json_rpc(self.nodes[0], {"jsonrpc": "3.0", "method": "getblockcount"}) + assert_equal(response, {"error": None, "id": None, "result": 0}) + assert_equal(status, 200) + + self.log.info("Testing HTTP status codes for JSON-RPC 2.0 notifications...") + # Not notification: id exists + response, status = send_json_rpc(self.nodes[0], {"jsonrpc": "2.0", "id": None, "method": "getblockcount"}) + assert_equal(response["result"], 0) + assert_equal(status, 200) + # Not notification: JSON 1.1 + expect_http_rpc_status(200, None, self.nodes[0], "getblockcount", [], 1) + # 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) + # 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) + # Sanity check: command was not executed + assert_equal(block_count + 1, self.nodes[0].getblockcount()) def test_work_queue_exceeded(self): self.log.info("Testing work queue exceeded...") @@ -148,7 +225,7 @@ class RPCInterfaceTest(BitcoinTestFramework): def run_test(self): self.test_getrpcinfo() - self.test_batch_request() + self.test_batch_requests() self.test_http_status_codes() self.test_work_queue_exceeded() -- cgit v1.2.3 From 2ca1460ae3a7217eaa8c5972515bf622bedadfce Mon Sep 17 00:00:00 2001 From: Matthew Zipkin Date: Fri, 7 Jul 2023 14:31:18 -0400 Subject: rpc: identify JSON-RPC 2.0 requests --- test/functional/interface_rpc.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) (limited to 'test') diff --git a/test/functional/interface_rpc.py b/test/functional/interface_rpc.py index 748d83858a..075dd1c2bc 100755 --- a/test/functional/interface_rpc.py +++ b/test/functional/interface_rpc.py @@ -161,8 +161,10 @@ class RPCInterfaceTest(BitcoinTestFramework): self.log.info("Testing nonstandard jsonrpc 1.0 version number is accepted...") self.test_batch_request(lambda idx: BatchOptions(request_fields={"jsonrpc": "1.0"})) - self.log.info("Testing unrecognized jsonrpc version number is accepted...") - self.test_batch_request(lambda idx: BatchOptions(request_fields={"jsonrpc": "2.1"})) + self.log.info("Testing unrecognized jsonrpc version number is rejected...") + self.test_batch_request(lambda idx: BatchOptions( + request_fields={"jsonrpc": "2.1"}, + response_fields={"result": None, "error": {"code": RPC_INVALID_REQUEST, "message": "JSON-RPC version not supported"}})) def test_http_status_codes(self): self.log.info("Testing HTTP status codes for JSON-RPC 1.1 requests...") @@ -188,11 +190,11 @@ class RPCInterfaceTest(BitcoinTestFramework): expect_http_rpc_status(500, 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, {"error": None, "id": None, "result": 0}) - assert_equal(status, 200) + assert_equal(response, {"id": None, "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, {"error": None, "id": None, "result": 0}) - assert_equal(status, 200) + assert_equal(response, {"id": None, "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...") # Not notification: id exists -- cgit v1.2.3 From 466b90562f4785de74b548f7c4a256069e2aaf43 Mon Sep 17 00:00:00 2001 From: Matthew Zipkin Date: Fri, 7 Jul 2023 14:41:23 -0400 Subject: rpc: Add "jsonrpc" field and drop null "result"/"error" fields Only for JSON-RPC 2.0 requests. --- test/functional/interface_rpc.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'test') diff --git a/test/functional/interface_rpc.py b/test/functional/interface_rpc.py index 075dd1c2bc..dc9fb5d40f 100755 --- a/test/functional/interface_rpc.py +++ b/test/functional/interface_rpc.py @@ -48,7 +48,10 @@ def format_request(options, idx, fields): def format_response(options, idx, fields): response = {} response.update(id=None if options.notification else idx) - response.update(result=None, error=None) + if options.version == 2: + response.update(jsonrpc="2.0") + else: + response.update(result=None, error=None) response.update(fields) if options.response_fields: response.update(options.response_fields) -- cgit v1.2.3 From bf1a1f1662427fbf1a43bb951364eface469bdb7 Mon Sep 17 00:00:00 2001 From: Matthew Zipkin Date: Fri, 7 Jul 2023 14:41:23 -0400 Subject: 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. --- test/functional/interface_rpc.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'test') diff --git a/test/functional/interface_rpc.py b/test/functional/interface_rpc.py index dc9fb5d40f..824a7200cd 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()) -- cgit v1.2.3 From e7ee80dcf2b68684eae96070875ea13a60e3e7b0 Mon Sep 17 00:00:00 2001 From: Matthew Zipkin Date: Fri, 7 Jul 2023 15:06:35 -0400 Subject: rpc: JSON-RPC 2.0 should not respond to "notifications" 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 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. --- test/functional/interface_rpc.py | 27 +++++++++++++++++++-------- test/functional/test_framework/authproxy.py | 9 +++++++++ 2 files changed, 28 insertions(+), 8 deletions(-) (limited to 'test') diff --git a/test/functional/interface_rpc.py b/test/functional/interface_rpc.py index 824a7200cd..b08ca42796 100755 --- a/test/functional/interface_rpc.py +++ b/test/functional/interface_rpc.py @@ -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()) diff --git a/test/functional/test_framework/authproxy.py b/test/functional/test_framework/authproxy.py index 03042877b2..7edf9f3679 100644 --- a/test/functional/test_framework/authproxy.py +++ b/test/functional/test_framework/authproxy.py @@ -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( -- cgit v1.2.3