diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index cc195226e9e..3455aea73b9 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -57,6 +57,7 @@ using CliClock = std::chrono::system_clock; const TranslateFn G_TRANSLATION_FUN{nullptr}; static const char DEFAULT_RPCCONNECT[] = "127.0.0.1"; +static constexpr const char* DEFAULT_RPC_REQ_ID{"1"}; static const int DEFAULT_HTTP_CLIENT_TIMEOUT=900; static constexpr int DEFAULT_WAIT_CLIENT_TIMEOUT = 0; static const bool DEFAULT_NAMED=false; @@ -100,6 +101,7 @@ static void SetupCliArgs(ArgsManager& argsman) SetupChainParamsBaseOptions(argsman); argsman.AddArg("-color=", strprintf("Color setting for CLI output (default: %s). Valid values: always, auto (add color codes when standard output is connected to a terminal and OS is not WIN32), never. Only applies to the output of -getinfo.", DEFAULT_COLOR_SETTING), ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_NEGATION, OptionsCategory::OPTIONS); argsman.AddArg("-named", strprintf("Pass named instead of positional arguments (default: %s)", DEFAULT_NAMED), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); + argsman.AddArg("-rpcid=", strprintf("Set a custom JSON-RPC request ID string (default: %s)", DEFAULT_RPC_REQ_ID), ArgsManager::ALLOW_ANY | ArgsManager::DISALLOW_NEGATION | ArgsManager::DISALLOW_ELISION, OptionsCategory::OPTIONS); argsman.AddArg("-rpcclienttimeout=", strprintf("Timeout in seconds during HTTP requests, or 0 for no timeout. (default: %d)", DEFAULT_HTTP_CLIENT_TIMEOUT), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-rpcconnect=", strprintf("Send commands to node running on (default: %s)", DEFAULT_RPCCONNECT), ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); argsman.AddArg("-rpccookiefile=", "Location of the auth cookie. Relative paths will be prefixed by a net-specific datadir location. (default: data dir)", ArgsManager::ALLOW_ANY, OptionsCategory::OPTIONS); @@ -787,7 +789,8 @@ struct DefaultRequestHandler : BaseRequestHandler { } else { params = RPCConvertValues(method, args); } - return JSONRPCRequestObj(method, params, 1); + UniValue id{UniValue::VSTR, gArgs.GetArg("-rpcid", DEFAULT_RPC_REQ_ID)}; + return JSONRPCRequestObj(method, params, id); } UniValue ProcessReply(const UniValue &reply) override diff --git a/src/rpc/request.cpp b/src/rpc/request.cpp index 34a9ff75535..d162263e67d 100644 --- a/src/rpc/request.cpp +++ b/src/rpc/request.cpp @@ -233,11 +233,13 @@ void JSONRPCRequest::parse(const UniValue& valRequest) if (!valMethod.isStr()) throw JSONRPCError(RPC_INVALID_REQUEST, "Method must be a string"); strMethod = valMethod.get_str(); + const std::string log_id{id && !id->isNull() ? SanitizeString(id->getValStr()) : ""}; if (fLogIPs) - LogDebug(BCLog::RPC, "ThreadRPCServer method=%s user=%s peeraddr=%s\n", SanitizeString(strMethod), - this->authUser, this->peerAddr); + LogDebug(BCLog::RPC, "ThreadRPCServer method=%s user=%s peeraddr=%s id=%s", SanitizeString(strMethod), + this->authUser, this->peerAddr, log_id); else - LogDebug(BCLog::RPC, "ThreadRPCServer method=%s user=%s\n", SanitizeString(strMethod), this->authUser); + LogDebug(BCLog::RPC, "ThreadRPCServer method=%s user=%s id=%s", SanitizeString(strMethod), this->authUser, + log_id); // Parse params const UniValue& valParams{request.find_value("params")}; diff --git a/test/functional/interface_bitcoin_cli.py b/test/functional/interface_bitcoin_cli.py index 953eea3d63c..e908ba8a5e0 100755 --- a/test/functional/interface_bitcoin_cli.py +++ b/test/functional/interface_bitcoin_cli.py @@ -421,6 +421,18 @@ class TestBitcoinCli(BitcoinTestFramework): self.test_netinfo() + self.log.info("Test -rpcid option sets custom JSON-RPC request ID") + with self.nodes[0].assert_debug_log(expected_msgs=['id=myrpcid']): + self.nodes[0].cli('-rpcid=myrpcid').getblockcount() + + self.log.info("Test default request logs default id=1") + with self.nodes[0].assert_debug_log(expected_msgs=["ThreadRPCServer method=getblockcount", "id=1"]): + self.nodes[0].cli.getblockcount() + + self.log.info("Test that request ids with unsafe characters are sanitized in the log") + with self.nodes[0].assert_debug_log(expected_msgs=["ThreadRPCServer method=getblockcount", "id=abcdef"]): + self.nodes[0].cli('-rpcid=abc<\n>def').getblockcount() + self.log.info("Test -version with node stopped") self.stop_node(0) cli_response = self.nodes[0].cli('-version').send_cli()