diff --git a/CMakeLists.txt b/CMakeLists.txt index 99490f742a6..bb711eace95 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -142,7 +142,7 @@ if(WITH_USDT) find_package(USDT MODULE REQUIRED) endif() -cmake_dependent_option(ENABLE_EXTERNAL_SIGNER "Enable external signer support." ON "NOT WIN32" OFF) +option(ENABLE_EXTERNAL_SIGNER "Enable external signer support." ON) cmake_dependent_option(WITH_QRENCODE "Enable QR code support." ON "BUILD_GUI" OFF) if(WITH_QRENCODE) diff --git a/ci/test/03_test_script.sh b/ci/test/03_test_script.sh index a71565ba98e..3394c50b8bd 100755 --- a/ci/test/03_test_script.sh +++ b/ci/test/03_test_script.sh @@ -118,7 +118,7 @@ BASE_BUILD_DIR=${BASE_BUILD_DIR:-$BASE_SCRATCH_DIR/build-$HOST} mkdir -p "${BASE_BUILD_DIR}" cd "${BASE_BUILD_DIR}" -BITCOIN_CONFIG_ALL="$BITCOIN_CONFIG_ALL -DENABLE_EXTERNAL_SIGNER=ON -DCMAKE_INSTALL_PREFIX=$BASE_OUTDIR" +BITCOIN_CONFIG_ALL="$BITCOIN_CONFIG_ALL -DCMAKE_INSTALL_PREFIX=$BASE_OUTDIR" if [[ "${RUN_TIDY}" == "true" ]]; then BITCOIN_CONFIG_ALL="$BITCOIN_CONFIG_ALL -DCMAKE_EXPORT_COMPILE_COMMANDS=ON" diff --git a/src/test/system_tests.cpp b/src/test/system_tests.cpp index a5d9be07d52..dec4d418290 100644 --- a/src/test/system_tests.cpp +++ b/src/test/system_tests.cpp @@ -25,7 +25,11 @@ BOOST_AUTO_TEST_CASE(run_command) BOOST_CHECK(result.isNull()); } { +#ifdef WIN32 + const UniValue result = RunCommandParseJSON("cmd.exe /c echo {\"success\": true}"); +#else const UniValue result = RunCommandParseJSON("echo {\"success\": true}"); +#endif BOOST_CHECK(result.isObject()); const UniValue& success = result.find_value("success"); BOOST_CHECK(!success.isNull()); @@ -33,12 +37,20 @@ BOOST_AUTO_TEST_CASE(run_command) } { // An invalid command is handled by cpp-subprocess +#ifdef WIN32 + const std::string expected{"CreateProcess failed: "}; +#else const std::string expected{"execve failed: "}; +#endif 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"}; +#else const std::string 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); @@ -47,7 +59,11 @@ BOOST_AUTO_TEST_CASE(run_command) } { // 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\""}; +#else const std::string 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()); @@ -58,17 +74,23 @@ BOOST_AUTO_TEST_CASE(run_command) } { // Unable to parse JSON +#ifdef WIN32 + const std::string command{"cmd.exe /c echo {"}; +#else const std::string command{"echo {"}; +#endif BOOST_CHECK_EXCEPTION(RunCommandParseJSON(command), std::runtime_error, HasReason("Unable to parse JSON: {")); } - // Test std::in +#ifndef WIN32 { + // Test stdin const UniValue result = RunCommandParseJSON("cat", "{\"success\": true}"); BOOST_CHECK(result.isObject()); const UniValue& success = result.find_value("success"); BOOST_CHECK(!success.isNull()); BOOST_CHECK_EQUAL(success.get_bool(), true); } +#endif } #endif // ENABLE_EXTERNAL_SIGNER diff --git a/src/util/subprocess.h b/src/util/subprocess.h index 3449fa3b1ba..54dc8a6da47 100644 --- a/src/util/subprocess.h +++ b/src/util/subprocess.h @@ -55,23 +55,15 @@ Documentation for C++ subprocessing library. #include #include -#if (defined _MSC_VER) || (defined __MINGW32__) - #define __USING_WINDOWS__ -#endif - -#ifdef __USING_WINDOWS__ +#ifdef WIN32 #include #endif extern "C" { -#ifdef __USING_WINDOWS__ - #include +#ifdef WIN32 + #include #include #include - - #define close _close - #define open _open - #define fileno _fileno #else #include #include @@ -81,6 +73,22 @@ extern "C" { #include } +// The Microsoft C++ compiler issues deprecation warnings +// for the standard POSIX function names. +// Its preferred implementations have a leading underscore. +// See: https://learn.microsoft.com/en-us/cpp/c-runtime-library/compatibility. +#if (defined _MSC_VER) + #define subprocess_close _close + #define subprocess_fileno _fileno + #define subprocess_open _open + #define subprocess_write _write +#else + #define subprocess_close close + #define subprocess_fileno fileno + #define subprocess_open open + #define subprocess_write write +#endif + /*! * Getting started with reading this source code. * The source is mainly divided into four parts: @@ -159,6 +167,7 @@ public: //-------------------------------------------------------------------- namespace util { +#ifdef WIN32 inline void quote_argument(const std::wstring &argument, std::wstring &command_line, bool force) { @@ -169,7 +178,7 @@ namespace util // if (force == false && argument.empty() == false && - argument.find_first_of(L" \t\n\v\"") == argument.npos) { + argument.find_first_of(L" \t\n\v") == argument.npos) { command_line.append(argument); } else { @@ -219,7 +228,6 @@ namespace util } } -#ifdef __USING_WINDOWS__ inline std::string get_last_error(DWORD errorMessageID) { if (errorMessageID == 0) @@ -264,7 +272,7 @@ namespace util FILE *fp = _fdopen(os_fhandle, mode); if (fp == 0) { - _close(os_fhandle); + subprocess_close(os_fhandle); throw OSError("_fdopen", 0); } @@ -320,7 +328,7 @@ namespace util } -#ifndef __USING_WINDOWS__ +#ifndef WIN32 /*! * Function: set_clo_on_exec * Sets/Resets the FD_CLOEXEC flag on the provided file descriptor @@ -383,7 +391,7 @@ namespace util { size_t nwritten = 0; while (nwritten < length) { - int written = write(fd, buf + nwritten, length - nwritten); + int written = subprocess_write(fd, buf + nwritten, length - nwritten); if (written == -1) return -1; nwritten += written; } @@ -408,10 +416,10 @@ namespace util static inline int read_atmost_n(FILE* fp, char* buf, size_t read_upto) { -#ifdef __USING_WINDOWS__ +#ifdef WIN32 return (int)fread(buf, 1, read_upto, fp); #else - int fd = fileno(fp); + int fd = subprocess_fileno(fp); int rbytes = 0; int eintr_cnter = 0; @@ -481,7 +489,7 @@ namespace util return total_bytes_read; } -#ifndef __USING_WINDOWS__ +#ifndef WIN32 /*! * Function: wait_for_child_exit * Waits for the process with pid `pid` to exit @@ -527,7 +535,7 @@ struct string_arg { string_arg(const char* arg): arg_value(arg) {} string_arg(std::string&& arg): arg_value(std::move(arg)) {} - string_arg(std::string arg): arg_value(std::move(arg)) {} + string_arg(const std::string& arg): arg_value(arg) {} std::string arg_value; }; @@ -573,16 +581,16 @@ struct input explicit input(int fd): rd_ch_(fd) {} // FILE pointer. - explicit input (FILE* fp):input(fileno(fp)) { assert(fp); } + explicit input (FILE* fp):input(subprocess_fileno(fp)) { assert(fp); } explicit input(const char* filename) { - int fd = open(filename, O_RDONLY); + int fd = subprocess_open(filename, O_RDONLY); if (fd == -1) throw OSError("File not found: ", errno); rd_ch_ = fd; } explicit input(IOTYPE typ) { assert (typ == PIPE && "STDOUT/STDERR not allowed"); -#ifndef __USING_WINDOWS__ +#ifndef WIN32 std::tie(rd_ch_, wr_ch_) = util::pipe_cloexec(); #endif } @@ -606,16 +614,16 @@ struct output { explicit output(int fd): wr_ch_(fd) {} - explicit output (FILE* fp):output(fileno(fp)) { assert(fp); } + explicit output (FILE* fp):output(subprocess_fileno(fp)) { assert(fp); } explicit output(const char* filename) { - int fd = open(filename, O_APPEND | O_CREAT | O_RDWR, 0640); + int fd = subprocess_open(filename, O_APPEND | O_CREAT | O_RDWR, 0640); if (fd == -1) throw OSError("File not found: ", errno); wr_ch_ = fd; } explicit output(IOTYPE typ) { assert (typ == PIPE && "STDOUT/STDERR not allowed"); -#ifndef __USING_WINDOWS__ +#ifndef WIN32 std::tie(rd_ch_, wr_ch_) = util::pipe_cloexec(); #endif } @@ -637,17 +645,17 @@ struct error { explicit error(int fd): wr_ch_(fd) {} - explicit error(FILE* fp):error(fileno(fp)) { assert(fp); } + explicit error(FILE* fp):error(subprocess_fileno(fp)) { assert(fp); } explicit error(const char* filename) { - int fd = open(filename, O_APPEND | O_CREAT | O_RDWR, 0640); + int fd = subprocess_open(filename, O_APPEND | O_CREAT | O_RDWR, 0640); if (fd == -1) throw OSError("File not found: ", errno); wr_ch_ = fd; } explicit error(IOTYPE typ) { assert ((typ == PIPE || typ == STDOUT) && "STDERR not allowed"); if (typ == PIPE) { -#ifndef __USING_WINDOWS__ +#ifndef WIN32 std::tie(rd_ch_, wr_ch_) = util::pipe_cloexec(); #endif } else { @@ -803,28 +811,28 @@ public: void cleanup_fds() { if (write_to_child_ != -1 && read_from_parent_ != -1) { - close(write_to_child_); + subprocess_close(write_to_child_); } if (write_to_parent_ != -1 && read_from_child_ != -1) { - close(read_from_child_); + subprocess_close(read_from_child_); } if (err_write_ != -1 && err_read_ != -1) { - close(err_read_); + subprocess_close(err_read_); } } void close_parent_fds() { - if (write_to_child_ != -1) close(write_to_child_); - if (read_from_child_ != -1) close(read_from_child_); - if (err_read_ != -1) close(err_read_); + if (write_to_child_ != -1) subprocess_close(write_to_child_); + if (read_from_child_ != -1) subprocess_close(read_from_child_); + if (err_read_ != -1) subprocess_close(err_read_); } void close_child_fds() { - if (write_to_parent_ != -1) close(write_to_parent_); - if (read_from_parent_ != -1) close(read_from_parent_); - if (err_write_ != -1) close(err_write_); + if (write_to_parent_ != -1) subprocess_close(write_to_parent_); + if (read_from_parent_ != -1) subprocess_close(read_from_parent_); + if (err_write_ != -1) subprocess_close(err_write_); } FILE* input() { return input_.get(); } @@ -858,7 +866,7 @@ public:// Yes they are public std::shared_ptr output_ = nullptr; std::shared_ptr error_ = nullptr; -#ifdef __USING_WINDOWS__ +#ifdef WIN32 HANDLE g_hChildStd_IN_Rd = nullptr; HANDLE g_hChildStd_IN_Wr = nullptr; HANDLE g_hChildStd_OUT_Rd = nullptr; @@ -999,7 +1007,7 @@ private: private: detail::Streams stream_; -#ifdef __USING_WINDOWS__ +#ifdef WIN32 HANDLE process_handle_; std::future cleanup_future_; #endif @@ -1040,10 +1048,22 @@ inline void Popen::populate_c_argv() inline int Popen::wait() noexcept(false) { -#ifdef __USING_WINDOWS__ +#ifdef WIN32 int ret = WaitForSingleObject(process_handle_, INFINITE); - return 0; + // WaitForSingleObject with INFINITE should only return when process has signaled + if (ret != WAIT_OBJECT_0) { + throw OSError("Unexpected return code from WaitForSingleObject", 0); + } + + DWORD dretcode_; + + if (FALSE == GetExitCodeProcess(process_handle_, &dretcode_)) + throw OSError("Failed during call to GetExitCodeProcess", 0); + + CloseHandle(process_handle_); + + return (int)dretcode_; #else int ret, status; std::tie(ret, status) = util::wait_for_child_exit(child_pid_); @@ -1061,7 +1081,7 @@ inline int Popen::wait() noexcept(false) inline void Popen::execute_process() noexcept(false) { -#ifdef __USING_WINDOWS__ +#ifdef WIN32 if (exe_name_.length()) { this->vargs_.insert(this->vargs_.begin(), this->exe_name_); this->populate_c_argv(); @@ -1156,8 +1176,8 @@ inline void Popen::execute_process() noexcept(false) child_pid_ = fork(); if (child_pid_ < 0) { - close(err_rd_pipe); - close(err_wr_pipe); + subprocess_close(err_rd_pipe); + subprocess_close(err_wr_pipe); throw OSError("fork failed", errno); } @@ -1167,14 +1187,14 @@ inline void Popen::execute_process() noexcept(false) stream_.close_parent_fds(); //Close the read end of the error pipe - close(err_rd_pipe); + subprocess_close(err_rd_pipe); detail::Child chld(this, err_wr_pipe); chld.execute_child(); } else { - close (err_wr_pipe);// close child side of pipe, else get stuck in read below + subprocess_close(err_wr_pipe);// close child side of pipe, else get stuck in read below stream_.close_child_fds(); @@ -1185,7 +1205,7 @@ inline void Popen::execute_process() noexcept(false) fdopen(err_rd_pipe, "r"), err_buf, SP_MAX_ERR_BUF_SIZ); - close(err_rd_pipe); + subprocess_close(err_rd_pipe); if (read_bytes || strlen(err_buf)) { // Call waitpid to reap the child process @@ -1235,7 +1255,7 @@ namespace detail { inline void Child::execute_child() { -#ifndef __USING_WINDOWS__ +#ifndef WIN32 int sys_ret = -1; auto& stream = parent_->stream_; @@ -1271,13 +1291,13 @@ namespace detail { // Close the duped descriptors if (stream.read_from_parent_ != -1 && stream.read_from_parent_ > 2) - close(stream.read_from_parent_); + subprocess_close(stream.read_from_parent_); if (stream.write_to_parent_ != -1 && stream.write_to_parent_ > 2) - close(stream.write_to_parent_); + subprocess_close(stream.write_to_parent_); if (stream.err_write_ != -1 && stream.err_write_ > 2) - close(stream.err_write_); + subprocess_close(stream.err_write_); // Replace the current image with the executable sys_ret = execvp(parent_->exe_name_.c_str(), parent_->cargv_.data()); @@ -1301,18 +1321,18 @@ namespace detail { inline void Streams::setup_comm_channels() { -#ifdef __USING_WINDOWS__ +#ifdef WIN32 util::configure_pipe(&this->g_hChildStd_IN_Rd, &this->g_hChildStd_IN_Wr, &this->g_hChildStd_IN_Wr); this->input(util::file_from_handle(this->g_hChildStd_IN_Wr, "w")); - this->write_to_child_ = _fileno(this->input()); + this->write_to_child_ = subprocess_fileno(this->input()); util::configure_pipe(&this->g_hChildStd_OUT_Rd, &this->g_hChildStd_OUT_Wr, &this->g_hChildStd_OUT_Rd); this->output(util::file_from_handle(this->g_hChildStd_OUT_Rd, "r")); - this->read_from_child_ = _fileno(this->output()); + this->read_from_child_ = subprocess_fileno(this->output()); util::configure_pipe(&this->g_hChildStd_ERR_Rd, &this->g_hChildStd_ERR_Wr, &this->g_hChildStd_ERR_Rd); this->error(util::file_from_handle(this->g_hChildStd_ERR_Rd, "r")); - this->err_read_ = _fileno(this->error()); + this->err_read_ = subprocess_fileno(this->error()); #else if (write_to_child_ != -1) input(fdopen(write_to_child_, "wb"));