[RPC][Backport] JSON-RPC 2.0 when Client requests

src/httprpc.cpp

@@ -61,7 +61,7 @@ static std::string strRPCUserColonPass;
 /* Stored RPC timer interface (for unregistration) */
 static std::unique_ptr<HTTPRPCTimerInterface> httpRPCTimerInterface;
 
-static void JSONErrorReply(HTTPRequest* req, const UniValue& objError, const UniValue& id)
+static void JSONErrorReply(HTTPRequest* req, UniValue objError, const JSONRPCRequest& jreq)
 {
     // Send error reply from json-rpc error object
     int nStatus = HTTP_INTERNAL_SERVER_ERROR;
@@ -72,7 +72,7 @@ static void JSONErrorReply(HTTPRequest* req, const UniValue& objError, const Uni
     else if (code == RPC_METHOD_NOT_FOUND)
         nStatus = HTTP_NOT_FOUND;
 
-    std::string strReply = JSONRPCReply(NullUniValue, objError, id);
+    std::string strReply = JSONRPCReplyObj(NullUniValue, std::move(objError), jreq.id, jreq.m_json_version).write() + "\n";
 
     req->WriteHeader("Content-Type", "application/json");
     req->WriteReply(nStatus, strReply);
@@ -176,28 +176,68 @@ static bool HTTPReq_JSONRPC(HTTPRequest* req, const std::string &)
         jreq.URI = req->GetURI();
 
         std::string strReply;
+        UniValue reply;
+
         // singleton request
         if (valRequest.isObject()) {
             jreq.parse(valRequest);
 
-            UniValue result = tableRPC.execute(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);
 
-            // Send reply
-            strReply = JSONRPCReply(result, NullUniValue, jreq.id);
+            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())
-            strReply = JSONRPCExecBatch(valRequest.get_array());
+            // Execute each request
+            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. Notifications never get any response.
+                UniValue response;
+                try {
+                    jreq.parse(valRequest[i]);
+                    response = JSONRPCExec(jreq, /*catch_errors=*/true);
+                } catch (UniValue& e) {
+                    response = JSONRPCReplyObj(NullUniValue, std::move(e), jreq.id, jreq.m_json_version);
+                } catch (const std::exception& e) {
+                    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
             throw JSONRPCError(RPC_PARSE_ERROR, "Top-level object parse error");
 
         req->WriteHeader("Content-Type", "application/json");
-        req->WriteReply(HTTP_OK, strReply);
-    } catch (const UniValue& objError) {
-        JSONErrorReply(req, objError, jreq.id);
+        req->WriteReply(HTTP_OK, reply.write() + "\n");
+    } catch (UniValue& e) {
+        JSONErrorReply(req, std::move(e), jreq);
         return false;
     } catch (const std::exception& e) {
-        JSONErrorReply(req, JSONRPCError(RPC_PARSE_ERROR, e.what()), jreq.id);
+        JSONErrorReply(req, JSONRPCError(RPC_PARSE_ERROR, e.what()), jreq);
         return false;
     }
     return true;

src/rpc/protocol.cpp

@@ -32,22 +32,23 @@ UniValue JSONRPCRequestObj(const std::string& strMethod, const UniValue& params,
     return request;
 }
 
-UniValue JSONRPCReplyObj(const UniValue& result, const UniValue& error, const UniValue& id)
+UniValue JSONRPCReplyObj(UniValue result, UniValue error, Optional<UniValue> id, JSONRPCVersion jsonrpc_version)
 {
     UniValue reply(UniValue::VOBJ);
-    if (!error.isNull())
-        reply.pushKV("result", NullUniValue);
-    else
-        reply.pushKV("result", result);
-    reply.pushKV("error", error);
-    reply.pushKV("id", id);
-    return reply;
-}
+    // Add JSON-RPC version number field in v2 only.
+    if (jsonrpc_version == JSONRPCVersion::V2) reply.pushKV("jsonrpc", "2.0");
 
-std::string JSONRPCReply(const UniValue& result, const UniValue& error, const UniValue& id)
-{
-    UniValue reply = JSONRPCReplyObj(result, error, id);
-    return reply.write() + "\n";
+    // Add both result and error fields in v1, even though one will be null.
+    // Omit the null field in v2.
+    if (error.isNull()) {
+        reply.pushKV("result", std::move(result));
+        if (jsonrpc_version == JSONRPCVersion::V1_LEGACY) reply.pushKV("error", NullUniValue);
+    } else {
+        if (jsonrpc_version == JSONRPCVersion::V1_LEGACY) reply.pushKV("result", NullUniValue);
+        reply.pushKV("error", std::move(error));
+    }
+    if (id.has_value()) reply.pushKV("id", std::move(id.value()));
+    return reply;
 }
 
 UniValue JSONRPCError(int code, const std::string& message)

src/rpc/protocol.h

@@ -11,6 +11,7 @@
 
 #include <list>
 #include <map>
+#include "optional.h"
 #include <stdint.h>
 #include <string>
 
@@ -19,6 +20,7 @@
 //! HTTP status codes
 enum HTTPStatusCode {
     HTTP_OK                    = 200,
+    HTTP_NO_CONTENT            = 204,
     HTTP_BAD_REQUEST           = 400,
     HTTP_UNAUTHORIZED          = 401,
     HTTP_FORBIDDEN             = 403,
@@ -80,9 +82,14 @@ enum RPCErrorCode {
     RPC_WALLET_NOT_SPECIFIED            = -19, //!< No wallet specified (error when there are multiple wallets loaded)
 };
 
+// JSON RPC Versions
+enum class JSONRPCVersion {
+    V1_LEGACY,
+    V2
+};
+
 UniValue JSONRPCRequestObj(const std::string& strMethod, const UniValue& params, const UniValue& id);
-UniValue JSONRPCReplyObj(const UniValue& result, const UniValue& error, const UniValue& id);
-std::string JSONRPCReply(const UniValue& result, const UniValue& error, const UniValue& id);
+UniValue JSONRPCReplyObj(UniValue result, UniValue error, Optional<UniValue> id, JSONRPCVersion jsonrpc_version);
 UniValue JSONRPCError(int code, const std::string& message);
 
 /** Get name of RPC authentication cookie file */

src/rpc/server.cpp

@@ -382,7 +382,30 @@ 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 = find_value(request, "id");
+    if (request.exists("id")) {
+        id = find_value(request, "id");
+    } else {
+        id = nullopt;
+    }
+
+    // Check for JSON-RPC 2.0 (default 1.1)
+    m_json_version = JSONRPCVersion::V1_LEGACY;
+    const UniValue& jsonrpc_version = find_value(request, "jsonrpc");
+    if (!jsonrpc_version.isNull()) {
+        if (!jsonrpc_version.isStr()) {
+            throw JSONRPCError(RPC_INVALID_REQUEST, "jsonrpc field must be a string");
+        }
+        // The "jsonrpc" key was added in the 2.0 spec, but some older documentation
+        // incorrectly included {"jsonrpc":"1.0"} in a request object, so we
+        // maintain that for backwards compatibility.
+        if (jsonrpc_version.get_str() == "1.0") {
+            m_json_version = JSONRPCVersion::V1_LEGACY;
+        } else if (jsonrpc_version.get_str() == "2.0") {
+            m_json_version = JSONRPCVersion::V2;
+        } else {
+            throw JSONRPCError(RPC_INVALID_REQUEST, "JSON-RPC version not supported");
+        }
+    }
 
     // Parse method
     UniValue valMethod = find_value(request, "method");
@@ -411,33 +434,22 @@ bool IsDeprecatedRPCEnabled(const std::string& method)
     return find(enabled_methods.begin(), enabled_methods.end(), method) != enabled_methods.end();
 }
 
-static UniValue JSONRPCExecOne(const UniValue& req)
+UniValue JSONRPCExec(const JSONRPCRequest& jreq, bool catch_errors)
 {
-    UniValue rpc_result(UniValue::VOBJ);
-
-    JSONRPCRequest jreq;
-    try {
-        jreq.parse(req);
-
-        UniValue result = tableRPC.execute(jreq);
-        rpc_result = JSONRPCReplyObj(result, NullUniValue, jreq.id);
-    } catch (const UniValue& objError) {
-        rpc_result = JSONRPCReplyObj(NullUniValue, objError, jreq.id);
-    } catch (const std::exception& e) {
-        rpc_result = JSONRPCReplyObj(NullUniValue,
-            JSONRPCError(RPC_PARSE_ERROR, e.what()), jreq.id);
+    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 rpc_result;
-}
-
-std::string JSONRPCExecBatch(const UniValue& vReq)
-{
-    UniValue ret(UniValue::VARR);
-    for (unsigned int reqIdx = 0; reqIdx < vReq.size(); reqIdx++)
-        ret.push_back(JSONRPCExecOne(vReq[reqIdx]));
-
-    return ret.write() + "\n";
+    return JSONRPCReplyObj(std::move(result), NullUniValue, jreq.id, jreq.m_json_version);
 }
 
 /**
@@ -531,7 +543,7 @@ std::string HelpExampleCli(std::string methodname, std::string args)
 
 std::string HelpExampleRpc(std::string methodname, std::string args)
 {
-    return "> curl --user myusername --data-binary '{\"jsonrpc\": \"1.0\", \"id\":\"curltest\", "
+    return "> curl --user myusername --data-binary '{\"jsonrpc\": \"2.0\", \"id\":\"curltest\", "
            "\"method\": \"" +
            methodname + "\", \"params\": [" + args + "] }' -H 'content-type: text/plain;' http://127.0.0.1:51473/\n";
 }

src/rpc/server.h

@@ -13,6 +13,7 @@
 
 #include <list>
 #include <map>
+#include "optional.h"
 #include <stdint.h>
 #include <string>
 
@@ -42,15 +43,17 @@ struct UniValueType {
 class JSONRPCRequest
 {
 public:
-    UniValue id;
+    Optional<UniValue> id = UniValue(UniValue::VNULL);
     std::string strMethod;
     UniValue params;
     bool fHelp;
     std::string URI;
     std::string authUser;
+    JSONRPCVersion m_json_version = JSONRPCVersion::V1_LEGACY;
 
     JSONRPCRequest() { id = NullUniValue; params = NullUniValue; fHelp = false; }
     void parse(const UniValue& valRequest);
+    [[nodiscard]] bool IsNotification() const { return !id.has_value() && m_json_version == JSONRPCVersion::V2; };
 };
 
 /** Query whether RPC is running */
@@ -200,7 +203,7 @@ extern std::string HelpExampleRpc(std::string methodname, std::string args);
 bool StartRPC();
 void InterruptRPC();
 void StopRPC();
-std::string JSONRPCExecBatch(const UniValue& vReq);
+UniValue JSONRPCExec(const JSONRPCRequest& jreq, bool catch_errors);
 void RPCNotifyBlockChange(bool fInitialDownload, const CBlockIndex* pindex);
 
 #endif // PIVX_RPC_SERVER_H

test/functional/test_framework/authproxy.py

@@ -26,7 +26,7 @@
 
 - HTTP connections persist for the life of the AuthServiceProxy object
   (if server supports HTTP/1.1)
-- sends protocol 'version', per JSON-RPC 1.1
+- sends "jsonrpc":"2.0", per JSON-RPC 2.0
 - sends proper, incrementing 'id'
 - sends Basic HTTP authentication headers
 - parses all JSON numbers that look like floats as Decimal
@@ -35,6 +35,7 @@
 
 import base64
 import decimal
+from http import HTTPStatus
 import http.client
 import json
 import logging
@@ -47,6 +48,7 @@
 
 log = logging.getLogger("BitcoinRPC")
 
+
 class JSONRPCException(Exception):
     def __init__(self, rpc_error):
         try:
@@ -62,6 +64,7 @@ def EncodeDecimal(o):
         return str(o)
     raise TypeError(repr(o) + " is not JSON serializable")
 
+
 class AuthServiceProxy():
     __id_count = 0
 
@@ -133,13 +136,28 @@ def get_request(self, *args, **argsn):
 
     def __call__(self, *args, **argsn):
         postdata = json.dumps(self.get_request(*args, **argsn), default=EncodeDecimal, ensure_ascii=self.ensure_ascii)
-        response = self._request('POST', self.__url.path, postdata.encode('utf-8'))
-        if response['error'] is not None:
-            raise JSONRPCException(response['error'])
+        response, status = self._request('POST', self.__url.path, postdata.encode('utf-8'))
+
+        error_response = response['error']
+        if 'code' not in error_response:
+            error_response['code'] = -1
+            raise JSONRPCException({'error': error_response, 'status': status})
         elif 'result' not in response:
             raise JSONRPCException({
                 'code': -343, 'message': 'missing JSON-RPC result'})
+        elif status != HTTPStatus.OK:
+            raise JSONRPCException({
+                'code': -342, 'message': 'non-200 HTTP status code but no JSON-RPC error'})
         else:
+            assert response['jsonrpc'] == '2.0'
+            if status != HTTPStatus.OK:
+                raise JSONRPCException({
+                    'code': -342, 'message': 'non-200 HTTP status code'})
+            if 'error' in response:
+                raise JSONRPCException({'error': response['error'], 'status': status})
+            elif 'result' not in response:
+                raise JSONRPCException({
+                    'code': -343, 'message': 'missing JSON-RPC 2.0 result and error'})
             return response['result']
 
     def batch(self, rpc_call_list):
@@ -162,6 +180,15 @@ def _get_response(self):
             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({
@@ -174,7 +201,8 @@ def _get_response(self):
             log.debug("<-%s- [%.6f] %s" % (response["id"], elapsed, json.dumps(response["result"], default=EncodeDecimal, ensure_ascii=self.ensure_ascii)))
         else:
             log.debug("<-- [%.6f] %s" % (elapsed, responsedata))
-        return response
+        return response, http_response.status
+
 
     def __truediv__(self, relative_uri):
         return AuthServiceProxy("{}/{}".format(self.__service_url, relative_uri), self._service_name, connection=self.__conn)