diff --git a/src/httprpc.cpp b/src/httprpc.cpp index d8869432df36..d1e6b32ef476 100644 --- a/src/httprpc.cpp +++ b/src/httprpc.cpp @@ -61,7 +61,7 @@ static std::string strRPCUserColonPass; /* Stored RPC timer interface (for unregistration) */ static std::unique_ptr 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; diff --git a/src/rpc/protocol.cpp b/src/rpc/protocol.cpp index bb001cbe5c2a..10a5036d21c2 100644 --- a/src/rpc/protocol.cpp +++ b/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 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) diff --git a/src/rpc/protocol.h b/src/rpc/protocol.h index 585d986f1746..266ce84424ff 100644 --- a/src/rpc/protocol.h +++ b/src/rpc/protocol.h @@ -11,6 +11,7 @@ #include #include +#include "optional.h" #include #include @@ -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 id, JSONRPCVersion jsonrpc_version); UniValue JSONRPCError(int code, const std::string& message); /** Get name of RPC authentication cookie file */ diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index 133f57854890..23c703d6cd22 100644 --- a/src/rpc/server.cpp +++ b/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"; } diff --git a/src/rpc/server.h b/src/rpc/server.h index 331c7faf1510..9c1e4a1f7076 100644 --- a/src/rpc/server.h +++ b/src/rpc/server.h @@ -13,6 +13,7 @@ #include #include +#include "optional.h" #include #include @@ -42,15 +43,17 @@ struct UniValueType { class JSONRPCRequest { public: - UniValue id; + Optional 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 diff --git a/test/functional/test_framework/authproxy.py b/test/functional/test_framework/authproxy.py index 1de4b7889a9b..645be9d37259 100644 --- a/test/functional/test_framework/authproxy.py +++ b/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)