From fa8ec00061567e56333bb69c5623919d45a9a92d Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 17 Apr 2020 13:28:29 -0400 Subject: [PATCH 1/5] rpc: Check that left section is not multiline --- src/rpc/util.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index 54ea352a72c..d476feb9625 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -381,6 +381,9 @@ struct Sections { std::string ret; const size_t pad = m_max_pad + 4; for (const auto& s : m_sections) { + // The left part of a section is assumed to be a single line, usually it is the name of the JSON struct or a + // brace like {, }, [, or ] + CHECK_NONFATAL(s.m_left.find('\n') == std::string::npos); if (s.m_right.empty()) { ret += s.m_left; ret += "\n"; From faaeb2b0b347b40ce456a951eec5e820587e5b02 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 26 Jun 2020 12:16:22 -0400 Subject: [PATCH 2/5] rpc: Add CRPCCommand constructor which takes RPCHelpMan This allows the constructor to ask the rpc manager for the name of the rpc method or the rpc argument names instead of having it manually passed in. --- src/rpc/server.h | 15 ++++++++++++++- src/rpc/util.cpp | 14 ++++++++++++++ src/rpc/util.h | 12 +++++++++++- 3 files changed, 39 insertions(+), 2 deletions(-) diff --git a/src/rpc/server.h b/src/rpc/server.h index d7a04ff6e8b..1c587ae88f2 100644 --- a/src/rpc/server.h +++ b/src/rpc/server.h @@ -8,6 +8,7 @@ #include #include +#include #include #include @@ -85,6 +86,7 @@ void RPCUnsetTimerInterface(RPCTimerInterface *iface); void RPCRunLater(const std::string& name, std::function func, int64_t nSeconds); typedef UniValue(*rpcfn_type)(const JSONRPCRequest& jsonRequest); +typedef RPCHelpMan (*RpcMethodFnType)(); class CRPCCommand { @@ -101,6 +103,17 @@ public: { } + //! Simplified constructor taking plain RpcMethodFnType function pointer. + CRPCCommand(std::string category, std::string name_in, RpcMethodFnType fn, std::vector args_in) + : CRPCCommand( + category, + fn().m_name, + [fn](const JSONRPCRequest& request, UniValue& result, bool) { result = fn().HandleRequest(request); return true; }, + fn().GetArgNames(), + intptr_t(fn)) + { + } + //! Simplified constructor taking plain rpcfn_type function pointer. CRPCCommand(const char* category, const char* name, rpcfn_type fn, std::initializer_list args) : CRPCCommand(category, name, @@ -117,7 +130,7 @@ public: }; /** - * Bitcoin RPC command dispatcher. + * RPC command dispatcher. */ class CRPCTable { diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index d476feb9625..fbd51784a72 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -418,7 +418,11 @@ struct Sections { }; RPCHelpMan::RPCHelpMan(std::string name, std::string description, std::vector args, RPCResults results, RPCExamples examples) + : RPCHelpMan{std::move(name), std::move(description), std::move(args), std::move(results), std::move(examples), nullptr} {} + +RPCHelpMan::RPCHelpMan(std::string name, std::string description, std::vector args, RPCResults results, RPCExamples examples, RPCMethodImpl fun) : m_name{std::move(name)}, + m_fun{std::move(fun)}, m_description{std::move(description)}, m_args{std::move(args)}, m_results{std::move(results)}, @@ -467,6 +471,16 @@ bool RPCHelpMan::IsValidNumArgs(size_t num_args) const } return num_required_args <= num_args && num_args <= m_args.size(); } + +std::vector RPCHelpMan::GetArgNames() const +{ + std::vector ret; + for (const auto& arg : m_args) { + ret.emplace_back(arg.m_names); + } + return ret; +} + std::string RPCHelpMan::ToString() const { std::string ret; diff --git a/src/rpc/util.h b/src/rpc/util.h index 53dce2c3972..49f603c8a1f 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -326,8 +326,14 @@ class RPCHelpMan { public: RPCHelpMan(std::string name, std::string description, std::vector args, RPCResults results, RPCExamples examples); + using RPCMethodImpl = std::function; + RPCHelpMan(std::string name, std::string description, std::vector args, RPCResults results, RPCExamples examples, RPCMethodImpl fun); std::string ToString() const; + UniValue HandleRequest(const JSONRPCRequest& request) + { + return m_fun(*this, request); + } /** If the supplied number of args is neither too small nor too high */ bool IsValidNumArgs(size_t num_args) const; /** @@ -340,8 +346,12 @@ public: } } -private: + std::vector GetArgNames() const; + const std::string m_name; + +private: + const RPCMethodImpl m_fun; const std::string m_description; const std::vector m_args; const RPCResults m_results; From fa9708f94c01cb8bf2971bdf404af38c38fa341b Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 3 Jul 2020 10:13:18 -0400 Subject: [PATCH 3/5] rpc: Assert that passed arg names are equal to hardcoded ones --- src/rpc/server.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/rpc/server.h b/src/rpc/server.h index 1c587ae88f2..6da3e94ea28 100644 --- a/src/rpc/server.h +++ b/src/rpc/server.h @@ -112,6 +112,8 @@ public: fn().GetArgNames(), intptr_t(fn)) { + CHECK_NONFATAL(fn().m_name == name_in); + CHECK_NONFATAL(fn().GetArgNames() == args_in); } //! Simplified constructor taking plain rpcfn_type function pointer. From aaaaad562790cd4dce1568ae193f5393aacacedf Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 3 Jul 2020 10:10:26 -0400 Subject: [PATCH 4/5] rpc: Add option to hide RPCArg Also, update switch statments to our style guide --- src/rpc/util.cpp | 27 ++++++++------------------- src/rpc/util.h | 6 +++++- 2 files changed, 13 insertions(+), 20 deletions(-) diff --git a/src/rpc/util.cpp b/src/rpc/util.cpp index fbd51784a72..2076c9e64c9 100644 --- a/src/rpc/util.cpp +++ b/src/rpc/util.cpp @@ -368,9 +368,7 @@ struct Sections { PushSection({indent + "]" + (outer_type != OuterType::NONE ? "," : ""), ""}); break; } - - // no default case, so the compiler can warn about missing cases - } + } // no default case, so the compiler can warn about missing cases } /** @@ -489,6 +487,7 @@ std::string RPCHelpMan::ToString() const ret += m_name; bool was_optional{false}; for (const auto& arg : m_args) { + if (arg.m_hidden) continue; const bool optional = arg.IsOptional(); ret += " "; if (optional) { @@ -510,6 +509,7 @@ std::string RPCHelpMan::ToString() const Sections sections; for (size_t i{0}; i < m_args.size(); ++i) { const auto& arg = m_args.at(i); + if (arg.m_hidden) continue; if (i == 0) ret += "\nArguments:\n"; @@ -589,9 +589,7 @@ std::string RPCArg::ToDescriptionString() const ret += "json array"; break; } - - // no default case, so the compiler can warn about missing cases - } + } // no default case, so the compiler can warn about missing cases } if (m_fallback.which() == 1) { ret += ", optional, default=" + boost::get(m_fallback); @@ -609,9 +607,7 @@ std::string RPCArg::ToDescriptionString() const ret += ", required"; break; } - - // no default case, so the compiler can warn about missing cases - } + } // no default case, so the compiler can warn about missing cases } ret += ")"; ret += m_description.empty() ? "" : " " + m_description; @@ -706,10 +702,7 @@ void RPCResult::ToSections(Sections& sections, const OuterType outer_type, const sections.PushSection({indent + "}" + maybe_separator, ""}); return; } - - // no default case, so the compiler can warn about missing cases - } - + } // no default case, so the compiler can warn about missing cases CHECK_NONFATAL(false); } @@ -746,9 +739,7 @@ std::string RPCArg::ToStringObj(const bool oneline) const case Type::OBJ_USER_KEYS: // Currently unused, so avoid writing dead code CHECK_NONFATAL(false); - - // no default case, so the compiler can warn about missing cases - } + } // no default case, so the compiler can warn about missing cases CHECK_NONFATAL(false); } @@ -783,9 +774,7 @@ std::string RPCArg::ToString(const bool oneline) const } return "[" + res + "...]"; } - - // no default case, so the compiler can warn about missing cases - } + } // no default case, so the compiler can warn about missing cases CHECK_NONFATAL(false); } diff --git a/src/rpc/util.h b/src/rpc/util.h index 49f603c8a1f..f49fb1d6b1e 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -144,6 +144,7 @@ struct RPCArg { using Fallback = boost::variant; const std::string m_names; //!< The name of the arg (can be empty for inner args, can contain multiple aliases separated by | for named request arguments) const Type m_type; + const bool m_hidden; const std::vector m_inner; //!< Only used for arrays or dicts const Fallback m_fallback; const std::string m_description; @@ -156,9 +157,11 @@ struct RPCArg { const Fallback fallback, const std::string description, const std::string oneline_description = "", - const std::vector type_str = {}) + const std::vector type_str = {}, + const bool hidden = false) : m_names{std::move(name)}, m_type{std::move(type)}, + m_hidden{hidden}, m_fallback{std::move(fallback)}, m_description{std::move(description)}, m_oneline_description{std::move(oneline_description)}, @@ -177,6 +180,7 @@ struct RPCArg { const std::vector type_str = {}) : m_names{std::move(name)}, m_type{std::move(type)}, + m_hidden{false}, m_inner{std::move(inner)}, m_fallback{std::move(fallback)}, m_description{std::move(description)}, From fa7592bfa8691eb0289b21da3571709a18391b0f Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 26 Jun 2020 14:12:46 -0400 Subject: [PATCH 5/5] rpc: Update server to use new RPCHelpMan Also, move Check to inside HandleRequest --- src/rpc/server.cpp | 50 +++++++++++++++++++++++++--------------------- src/rpc/util.h | 1 + 2 files changed, 28 insertions(+), 23 deletions(-) diff --git a/src/rpc/server.cpp b/src/rpc/server.cpp index 844f62cbc67..e8ef3a7f3aa 100644 --- a/src/rpc/server.cpp +++ b/src/rpc/server.cpp @@ -130,11 +130,9 @@ std::string CRPCTable::help(const std::string& strCommand, const JSONRPCRequest& return strRet; } -UniValue help(const JSONRPCRequest& jsonRequest) +static RPCHelpMan help() { - if (jsonRequest.fHelp || jsonRequest.params.size() > 1) - throw std::runtime_error( - RPCHelpMan{"help", + return RPCHelpMan{"help", "\nList all commands, or get help for a specified command.\n", { {"command", RPCArg::Type::STR, /* default */ "all commands", "The command to get help on"}, @@ -143,32 +141,32 @@ UniValue help(const JSONRPCRequest& jsonRequest) RPCResult::Type::STR, "", "The help text" }, RPCExamples{""}, - }.ToString() - ); - + [&](const RPCHelpMan& self, const JSONRPCRequest& jsonRequest) -> UniValue +{ std::string strCommand; if (jsonRequest.params.size() > 0) strCommand = jsonRequest.params[0].get_str(); return tableRPC.help(strCommand, jsonRequest); +}, + }; } - -UniValue stop(const JSONRPCRequest& jsonRequest) +static RPCHelpMan stop() { static const std::string RESULT{PACKAGE_NAME " stopping"}; - // Accept the deprecated and ignored 'detach' boolean argument + return RPCHelpMan{"stop", // Also accept the hidden 'wait' integer argument (milliseconds) // For instance, 'stop 1000' makes the call wait 1 second before returning // to the client (intended for testing) - if (jsonRequest.fHelp || jsonRequest.params.size() > 1) - throw std::runtime_error( - RPCHelpMan{"stop", "\nRequest a graceful shutdown of " PACKAGE_NAME ".", - {}, + { + {"wait", RPCArg::Type::NUM, RPCArg::Optional::OMITTED_NAMED_ARG, "how long to wait in ms", "", {}, /* hidden */ true}, + }, RPCResult{RPCResult::Type::STR, "", "A string with the content '" + RESULT + "'"}, RPCExamples{""}, - }.ToString()); + [&](const RPCHelpMan& self, const JSONRPCRequest& jsonRequest) -> UniValue +{ // Event loop will exit after current HTTP requests have been handled, so // this reply will get back to the client. StartShutdown(); @@ -176,11 +174,13 @@ UniValue stop(const JSONRPCRequest& jsonRequest) UninterruptibleSleep(std::chrono::milliseconds{jsonRequest.params[0].get_int()}); } return RESULT; +}, + }; } -static UniValue uptime(const JSONRPCRequest& jsonRequest) +static RPCHelpMan uptime() { - RPCHelpMan{"uptime", + return RPCHelpMan{"uptime", "\nReturns the total uptime of the server.\n", {}, RPCResult{ @@ -190,14 +190,16 @@ static UniValue uptime(const JSONRPCRequest& jsonRequest) HelpExampleCli("uptime", "") + HelpExampleRpc("uptime", "") }, - }.Check(jsonRequest); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ return GetTime() - GetStartupTime(); } + }; +} -static UniValue getrpcinfo(const JSONRPCRequest& request) +static RPCHelpMan getrpcinfo() { - RPCHelpMan{"getrpcinfo", + return RPCHelpMan{"getrpcinfo", "\nReturns details of the RPC server.\n", {}, RPCResult{ @@ -217,8 +219,8 @@ static UniValue getrpcinfo(const JSONRPCRequest& request) RPCExamples{ HelpExampleCli("getrpcinfo", "") + HelpExampleRpc("getrpcinfo", "")}, - }.Check(request); - + [&](const RPCHelpMan& self, const JSONRPCRequest& request) -> UniValue +{ LOCK(g_rpc_server_info.mutex); UniValue active_commands(UniValue::VARR); for (const RPCCommandExecutionInfo& info : g_rpc_server_info.active_commands) { @@ -237,6 +239,8 @@ static UniValue getrpcinfo(const JSONRPCRequest& request) return result; } + }; +} // clang-format off static const CRPCCommand vRPCCommands[] = diff --git a/src/rpc/util.h b/src/rpc/util.h index f49fb1d6b1e..35e0d5e9ef9 100644 --- a/src/rpc/util.h +++ b/src/rpc/util.h @@ -336,6 +336,7 @@ public: std::string ToString() const; UniValue HandleRequest(const JSONRPCRequest& request) { + Check(request); return m_fun(*this, request); } /** If the supplied number of args is neither too small nor too high */