highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-11-11 14:38:29 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#27101: Support JSON-RPC 2.0 when requested by client

Browse Source 
cbc6c440e3 doc: add comments and release-notes for JSON-RPC 2.0 (Matthew Zipkin)
e7ee80dcf2 rpc: JSON-RPC 2.0 should not respond to "notifications" (Matthew Zipkin)
bf1a1f1662 rpc: Avoid returning HTTP errors for JSON-RPC 2.0 requests (Matthew Zipkin)
466b90562f rpc: Add "jsonrpc" field and drop null "result"/"error" fields (Matthew Zipkin)
2ca1460ae3 rpc: identify JSON-RPC 2.0 requests (Matthew Zipkin)
a64a2b77e0 rpc: refactor single/batch requests (Matthew Zipkin)
df6e3756d6 rpc: Avoid copies in JSONRPCReplyObj() (Matthew Zipkin)
09416f9ec4 test: cover JSONRPC 2.0 requests, batches, and notifications (Matthew Zipkin)
4202c170da test: refactor interface_rpc.py (Matthew Zipkin)

Pull request description:

  Closes https://github.com/bitcoin/bitcoin/issues/2960

  Bitcoin Core's JSONRPC server behaves with a special blend of 1.0, 1.1 and 2.0 behaviors. This introduces compliance issues with more strict clients. There are the major misbehaviors that I found:
  - returning non-200 HTTP codes for RPC errors like "Method not found" (this is not a server error or an HTTP error)
  - returning both `"error"` and `"result"` fields together in a response object.
  - different error-handling behavior for single and batched RPC requests (batches contain errors in the response but single requests will actually throw HTTP errors)

  https://github.com/bitcoin/bitcoin/pull/15495 added regression tests after a discussion in https://github.com/bitcoin/bitcoin/pull/15381 to kinda lock in our RPC behavior to preserve backwards compatibility.

  https://github.com/bitcoin/bitcoin/pull/12435 was an attempt to allow strict 2.0 compliance behind a flag, but was abandoned.

  The approach in this PR is not strict and preserves backwards compatibility in a familiar bitcoin-y way: all old behavior is preserved, but new rules are applied to clients that opt in. One of the rules in the [JSON RPC 2.0 spec](https://www.jsonrpc.org/specification#request_object) is that the kv pair `"jsonrpc": "2.0"` must be present in the request. Well, let's just use that to trigger strict 2.0 behavior! When that kv pair is included in a request object, the [response will adhere to strict JSON-RPC 2.0 rules](https://www.jsonrpc.org/specification#response_object), essentially:

  - always return HTTP 200 "OK" unless there really is a server error or malformed request
  - either return `"error"` OR `"result"` but never both
  - same behavior for single and batch requests

  If this is merged next steps can be:

  - Refactor bitcoin-cli to always use strict 2.0
  - Refactor the python test framework to always use strict 2.0 for everything
  - Begin deprecation process for 1.0/1.1 behavior (?)

  If we can one day remove the old 1.0/1.1 behavior we can clean up the rpc code quite a bit.

ACKs for top commit:
  cbergqvist:
    re ACK cbc6c440e3
  ryanofsky:
    Code review ACK cbc6c440e3. Just suggested changes since the last review: changing uncaught exception error code from PARSE_ERROR to MISC_ERROR, renaming a few things, and adding comments.
  tdb3:
    re ACK for cbc6c440e3

Tree-SHA512: 0b702ed32368b34b29ad570d090951a7aeb56e3b0f2baf745bd32fdc58ef68fee6b0b8fad901f1ca42573ed714b150303829cddad4a34ca7ad847350feeedb36
This commit is contained in:
Ryan Ofsky
2024-05-16 09:41:46 -04:00
parent dd42a5ddea cbc6c440e3
commit 75118a608f
13 changed files with 349 additions and 95 deletions
Download Patch File Download Diff File Expand all files Collapse all files

6
src/test/rpc_tests.cpp
View File

@@ -552,7 +552,7 @@ BOOST_AUTO_TEST_CASE(help_example)
// test different argument types
const RPCArgList& args = {{"foo", "bar"}, {"b", true}, {"n", 1}};
BOOST_CHECK_EQUAL(HelpExampleCliNamed("test", args), "> bitcoin-cli -named test foo=bar b=true n=1\n");
BOOST_CHECK_EQUAL(HelpExampleRpcNamed("test", args), "> curl --user myusername --data-binary '{\"jsonrpc\": \"1.0\", \"id\": \"curltest\", \"method\": \"test\", \"params\": {\"foo\":\"bar\",\"b\":true,\"n\":1}}' -H 'content-type: text/plain;' http://127.0.0.1:8332/\n");
BOOST_CHECK_EQUAL(HelpExampleRpcNamed("test", args), "> curl --user myusername --data-binary '{\"jsonrpc\": \"2.0\", \"id\": \"curltest\", \"method\": \"test\", \"params\": {\"foo\":\"bar\",\"b\":true,\"n\":1}}' -H 'content-type: text/plain;' http://127.0.0.1:8332/\n");
// test shell escape
BOOST_CHECK_EQUAL(HelpExampleCliNamed("test", {{"foo", "b'ar"}}), "> bitcoin-cli -named test foo='b'''ar'\n");
@@ -565,7 +565,7 @@ BOOST_AUTO_TEST_CASE(help_example)
obj_value.pushKV("b", false);
obj_value.pushKV("n", 1);
BOOST_CHECK_EQUAL(HelpExampleCliNamed("test", {{"name", obj_value}}), "> bitcoin-cli -named test name='{\"foo\":\"bar\",\"b\":false,\"n\":1}'\n");
BOOST_CHECK_EQUAL(HelpExampleRpcNamed("test", {{"name", obj_value}}), "> curl --user myusername --data-binary '{\"jsonrpc\": \"1.0\", \"id\": \"curltest\", \"method\": \"test\", \"params\": {\"name\":{\"foo\":\"bar\",\"b\":false,\"n\":1}}}' -H 'content-type: text/plain;' http://127.0.0.1:8332/\n");
BOOST_CHECK_EQUAL(HelpExampleRpcNamed("test", {{"name", obj_value}}), "> curl --user myusername --data-binary '{\"jsonrpc\": \"2.0\", \"id\": \"curltest\", \"method\": \"test\", \"params\": {\"name\":{\"foo\":\"bar\",\"b\":false,\"n\":1}}}' -H 'content-type: text/plain;' http://127.0.0.1:8332/\n");
// test array params
UniValue arr_value(UniValue::VARR);
@@ -573,7 +573,7 @@ BOOST_AUTO_TEST_CASE(help_example)
arr_value.push_back(false);
arr_value.push_back(1);
BOOST_CHECK_EQUAL(HelpExampleCliNamed("test", {{"name", arr_value}}), "> bitcoin-cli -named test name='[\"bar\",false,1]'\n");
BOOST_CHECK_EQUAL(HelpExampleRpcNamed("test", {{"name", arr_value}}), "> curl --user myusername --data-binary '{\"jsonrpc\": \"1.0\", \"id\": \"curltest\", \"method\": \"test\", \"params\": {\"name\":[\"bar\",false,1]}}' -H 'content-type: text/plain;' http://127.0.0.1:8332/\n");
BOOST_CHECK_EQUAL(HelpExampleRpcNamed("test", {{"name", arr_value}}), "> curl --user myusername --data-binary '{\"jsonrpc\": \"2.0\", \"id\": \"curltest\", \"method\": \"test\", \"params\": {\"name\":[\"bar\",false,1]}}' -H 'content-type: text/plain;' http://127.0.0.1:8332/\n");
// test types don't matter for shell
BOOST_CHECK_EQUAL(HelpExampleCliNamed("foo", {{"arg", true}}), HelpExampleCliNamed("foo", {{"arg", "true"}}));