diff --git a/src/common/run_command.cpp b/src/common/run_command.cpp index 57683e03467..86f89e17f23 100644 --- a/src/common/run_command.cpp +++ b/src/common/run_command.cpp @@ -8,12 +8,13 @@ #include #include +#include #ifdef ENABLE_EXTERNAL_SIGNER #include #endif // ENABLE_EXTERNAL_SIGNER -UniValue RunCommandParseJSON(const std::string& str_command, const std::string& str_std_in) +UniValue RunCommandParseJSON(const std::vector& cmd_args, const std::string& str_std_in) { #ifdef ENABLE_EXTERNAL_SIGNER namespace sp = subprocess; @@ -22,9 +23,9 @@ UniValue RunCommandParseJSON(const std::string& str_command, const std::string& std::istringstream stdout_stream; std::istringstream stderr_stream; - if (str_command.empty()) return UniValue::VNULL; + if (cmd_args.empty()) return UniValue::VNULL; - auto c = sp::Popen(str_command, sp::input{sp::PIPE}, sp::output{sp::PIPE}, sp::error{sp::PIPE}); + auto c = sp::Popen(cmd_args, sp::input{sp::PIPE}, sp::output{sp::PIPE}, sp::error{sp::PIPE}); if (!str_std_in.empty()) { c.send(str_std_in); } @@ -38,7 +39,7 @@ UniValue RunCommandParseJSON(const std::string& str_command, const std::string& std::getline(stderr_stream, error); const int n_error = c.retcode(); - if (n_error) throw std::runtime_error(strprintf("RunCommandParseJSON error: process(%s) returned %d: %s\n", str_command, n_error, error)); + if (n_error) throw std::runtime_error(strprintf("RunCommandParseJSON error: process(%s) returned %d: %s\n", util::Join(cmd_args, " "), n_error, error)); if (!result_json.read(result)) throw std::runtime_error("Unable to parse JSON: " + result); return result_json; diff --git a/src/common/run_command.h b/src/common/run_command.h index 56c94f83bd9..9162c704561 100644 --- a/src/common/run_command.h +++ b/src/common/run_command.h @@ -6,16 +6,17 @@ #define BITCOIN_COMMON_RUN_COMMAND_H #include +#include class UniValue; /** * Execute a command which returns JSON, and parse the result. * - * @param str_command The command to execute, including any arguments + * @param cmd_args The command and arguments * @param str_std_in string to pass to stdin * @return parsed JSON */ -UniValue RunCommandParseJSON(const std::string& str_command, const std::string& str_std_in=""); +UniValue RunCommandParseJSON(const std::vector& cmd_args, const std::string& str_std_in = ""); #endif // BITCOIN_COMMON_RUN_COMMAND_H diff --git a/src/external_signer.cpp b/src/external_signer.cpp index 84d98a19906..3790f4d36f9 100644 --- a/src/external_signer.cpp +++ b/src/external_signer.cpp @@ -9,24 +9,27 @@ #include #include #include +#include #include #include #include #include -ExternalSigner::ExternalSigner(std::string command, std::string chain, std::string fingerprint, std::string name) +ExternalSigner::ExternalSigner(std::vector command, std::string chain, std::string fingerprint, std::string name) : m_command{std::move(command)}, m_chain{std::move(chain)}, m_fingerprint{std::move(fingerprint)}, m_name{std::move(name)} {} -std::string ExternalSigner::NetworkArg() const +std::vector ExternalSigner::NetworkArg() const { - return " --chain " + m_chain; + return {"--chain", m_chain}; } bool ExternalSigner::Enumerate(const std::string& command, std::vector& signers, const std::string& chain) { // Call enumerate - const UniValue result = RunCommandParseJSON(command + " enumerate"); + std::vector cmd_args = Cat(subprocess::util::split(command), {"enumerate"}); + + const UniValue result = RunCommandParseJSON(cmd_args, ""); if (!result.isArray()) { throw std::runtime_error(strprintf("'%s' received invalid response, expected array of signers", command)); } @@ -56,19 +59,19 @@ bool ExternalSigner::Enumerate(const std::string& command, std::vector command = Cat(m_command, Cat({"--stdin", "--fingerprint", m_fingerprint}, NetworkArg())); const std::string stdinStr = "signtx " + EncodeBase64(ssTx.str()); const UniValue signer_result = RunCommandParseJSON(command, stdinStr); diff --git a/src/external_signer.h b/src/external_signer.h index 1b36d49622e..5ba37c0626b 100644 --- a/src/external_signer.h +++ b/src/external_signer.h @@ -19,19 +19,19 @@ class ExternalSigner { private: //! The command which handles interaction with the external signer. - std::string m_command; + std::vector m_command; //! Bitcoin mainnet, testnet, etc std::string m_chain; - std::string NetworkArg() const; + std::vector NetworkArg() const; public: //! @param[in] command the command which handles interaction with the external signer //! @param[in] fingerprint master key fingerprint of the signer //! @param[in] chain "main", "test", "regtest" or "signet" //! @param[in] name device name - ExternalSigner(std::string command, std::string chain, std::string fingerprint, std::string name); + ExternalSigner(std::vector command, std::string chain, std::string fingerprint, std::string name); //! Master key fingerprint of the signer std::string m_fingerprint; diff --git a/src/test/system_tests.cpp b/src/test/system_tests.cpp index 0b7cc5b21f1..0ba58bcc6da 100644 --- a/src/test/system_tests.cpp +++ b/src/test/system_tests.cpp @@ -4,9 +4,11 @@ // #include // IWYU pragma: keep -#include + #include +#include #include +#include #ifdef ENABLE_EXTERNAL_SIGNER #include @@ -21,14 +23,14 @@ BOOST_FIXTURE_TEST_SUITE(system_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(run_command) { { - const UniValue result = RunCommandParseJSON(""); + const UniValue result = RunCommandParseJSON({}); BOOST_CHECK(result.isNull()); } { #ifdef WIN32 - const UniValue result = RunCommandParseJSON("cmd.exe /c echo {\"success\": true}"); + const UniValue result = RunCommandParseJSON({"cmd.exe", "/c", "echo", "{\"success\":", "true}"}); // The command is intentionally split "incorrectly", to exactly preserve previous behavior. This is due to the cmd.exe internal echo quoting strings with spaces in it, unlike the normal 'echo' below. #else - const UniValue result = RunCommandParseJSON("echo {\"success\": true}"); + const UniValue result = RunCommandParseJSON({"echo", "{\"success\": true}"}); #endif BOOST_CHECK(result.isObject()); const UniValue& success = result.find_value("success"); @@ -42,32 +44,32 @@ BOOST_AUTO_TEST_CASE(run_command) #else const std::string expected{"execve failed: "}; #endif - BOOST_CHECK_EXCEPTION(RunCommandParseJSON("invalid_command"), subprocess::CalledProcessError, HasReason(expected)); + BOOST_CHECK_EXCEPTION(RunCommandParseJSON({"invalid_command"}), subprocess::CalledProcessError, HasReason(expected)); } { // Return non-zero exit code, no output to stderr #ifdef WIN32 - const std::string command{"cmd.exe /c exit 1"}; + const std::vector command = {"cmd.exe", "/c", "exit 1"}; #else - const std::string command{"false"}; + const std::vector command = {"false"}; #endif BOOST_CHECK_EXCEPTION(RunCommandParseJSON(command), std::runtime_error, [&](const std::runtime_error& e) { const std::string what{e.what()}; - BOOST_CHECK(what.find(strprintf("RunCommandParseJSON error: process(%s) returned 1: \n", command)) != std::string::npos); + BOOST_CHECK(what.find(strprintf("RunCommandParseJSON error: process(%s) returned 1: \n", util::Join(command, " "))) != std::string::npos); return true; }); } { // Return non-zero exit code, with error message for stderr #ifdef WIN32 - const std::string command{"cmd.exe /c \"echo err 1>&2 && exit 1\""}; + const std::vector command = {"cmd.exe", "/c", "echo err 1>&2 && exit 1"}; #else - const std::string command{"sh -c 'echo err 1>&2 && false'"}; + const std::vector command = {"sh", "-c", "echo err 1>&2 && false"}; #endif const std::string expected{"err"}; BOOST_CHECK_EXCEPTION(RunCommandParseJSON(command), std::runtime_error, [&](const std::runtime_error& e) { const std::string what(e.what()); - BOOST_CHECK(what.find(strprintf("RunCommandParseJSON error: process(%s) returned", command)) != std::string::npos); + BOOST_CHECK(what.find(strprintf("RunCommandParseJSON error: process(%s) returned", util::Join(command, " "))) != std::string::npos); BOOST_CHECK(what.find(expected) != std::string::npos); return true; }); @@ -75,16 +77,16 @@ BOOST_AUTO_TEST_CASE(run_command) { // Unable to parse JSON #ifdef WIN32 - const std::string command{"cmd.exe /c echo {"}; + const std::vector command = {"cmd.exe", "/c", "echo {"}; #else - const std::string command{"echo {"}; + const std::vector command = {"echo", "{"}; #endif BOOST_CHECK_EXCEPTION(RunCommandParseJSON(command), std::runtime_error, HasReason("Unable to parse JSON: {")); } #ifndef WIN32 { // Test stdin - const UniValue result = RunCommandParseJSON("cat", "{\"success\": true}"); + const UniValue result = RunCommandParseJSON({"cat"}, "{\"success\": true}"); BOOST_CHECK(result.isObject()); const UniValue& success = result.find_value("success"); BOOST_CHECK(!success.isNull()); diff --git a/src/util/subprocess.h b/src/util/subprocess.h index e90c165205e..b4a99f08990 100644 --- a/src/util/subprocess.h +++ b/src/util/subprocess.h @@ -920,8 +920,6 @@ private: * API's provided by the class: * Popen({"cmd"}, output{..}, error{..}, ....) * Command provided as a sequence. - * Popen("cmd arg1", output{..}, error{..}, ....) - * Command provided in a single string. * wait() - Wait for the child to exit. * retcode() - The return code of the exited child. * send(...) - Send input to the input channel of the child. @@ -934,19 +932,6 @@ public: friend struct detail::ArgumentDeducer; friend class detail::Child; - template - Popen(const std::string& cmd_args, Args&& ...args): - args_(cmd_args) - { - vargs_ = util::split(cmd_args); - init_args(std::forward(args)...); - - // Setup the communication channels of the Popen class - stream_.setup_comm_channels(); - - execute_process(); - } - template Popen(std::initializer_list cmd_args, Args&& ...args) { @@ -1028,8 +1013,6 @@ private: std::string exe_name_; - // Command in string format - std::string args_; // Command provided as sequence std::vector vargs_; std::vector cargv_;