diff --git a/configure.ac b/configure.ac index 0c97af58e657..e8c922437a9d 100644 --- a/configure.ac +++ b/configure.ac @@ -1966,7 +1966,6 @@ AM_CONDITIONAL([ENABLE_QT_TESTS], [test "$BUILD_TEST_QT" = "yes"]) AM_CONDITIONAL([ENABLE_BENCH], [test "$use_bench" = "yes"]) AM_CONDITIONAL([USE_QRCODE], [test "$use_qr" = "yes"]) AM_CONDITIONAL([USE_LCOV], [test "$use_lcov" = "yes"]) -AM_CONDITIONAL([USE_LIBEVENT], [test "$use_libevent" = "yes"]) AM_CONDITIONAL([HARDEN], [test "$use_hardening" = "yes"]) AM_CONDITIONAL([ENABLE_SSSE3], [test "$enable_ssse3" = "yes"]) AM_CONDITIONAL([ENABLE_SSE42], [test "$enable_sse42" = "yes"]) diff --git a/src/Makefile.am b/src/Makefile.am index 4f40714c7677..e8ec46b4a5c8 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -424,7 +424,7 @@ BITCOIN_CORE_H = \ util/sock.h \ util/string.h \ util/spanparsing.h \ - util/subprocess.hpp \ + util/subprocess.h \ util/syserror.h \ util/system.h \ util/time.h \ @@ -927,6 +927,7 @@ libbitcoin_common_a_SOURCES = \ coins.cpp \ common/bloom.cpp \ common/run_command.cpp \ + common/url.cpp \ compressor.cpp \ core_read.cpp \ core_write.cpp \ @@ -964,11 +965,6 @@ libbitcoin_common_a_SOURCES = \ script/standard.cpp \ warnings.cpp \ $(BITCOIN_CORE_H) - -if USE_LIBEVENT -libbitcoin_common_a_CPPFLAGS += $(EVENT_CFLAGS) -libbitcoin_common_a_SOURCES += common/url.cpp -endif # # util # diff --git a/src/Makefile.test.include b/src/Makefile.test.include index b7d7cc5cde14..78cd380bb958 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -100,6 +100,7 @@ BITCOIN_TESTS =\ test/cachemultimap_tests.cpp \ test/coins_tests.cpp \ test/coinstatsindex_tests.cpp \ + test/common_url_tests.cpp \ test/compilerbug_tests.cpp \ test/compress_tests.cpp \ test/crypto_tests.cpp \ diff --git a/src/bitcoin-cli.cpp b/src/bitcoin-cli.cpp index 3b0873970e9f..6c94f27b87d5 100644 --- a/src/bitcoin-cli.cpp +++ b/src/bitcoin-cli.cpp @@ -10,7 +10,6 @@ #include #include -#include #include #include #include @@ -49,7 +48,6 @@ using CliClock = std::chrono::system_clock; const std::function G_TRANSLATION_FUN = nullptr; -UrlDecodeFn* const URL_DECODE = urlDecode; static const char DEFAULT_RPCCONNECT[] = "127.0.0.1"; static const int DEFAULT_HTTP_CLIENT_TIMEOUT=900; diff --git a/src/bitcoin-wallet.cpp b/src/bitcoin-wallet.cpp index be9ba957ec4b..3b79b75cc46c 100644 --- a/src/bitcoin-wallet.cpp +++ b/src/bitcoin-wallet.cpp @@ -8,7 +8,6 @@ #include #include -#include #include #include #include @@ -25,7 +24,6 @@ #include #include const std::function G_TRANSLATION_FUN = nullptr; -UrlDecodeFn* const URL_DECODE = nullptr; static void SetupWalletToolArgs(ArgsManager& argsman) { diff --git a/src/bitcoind.cpp b/src/bitcoind.cpp index b364c7e5439a..b01859aaa30c 100644 --- a/src/bitcoind.cpp +++ b/src/bitcoind.cpp @@ -10,7 +10,6 @@ #include #include -#include #include #include #include @@ -34,7 +33,6 @@ using node::NodeContext; const std::function G_TRANSLATION_FUN = nullptr; -UrlDecodeFn* const URL_DECODE = urlDecode; #if HAVE_DECL_FORK diff --git a/src/common/run_command.cpp b/src/common/run_command.cpp index e5356490ef7f..347b4860955f 100644 --- a/src/common/run_command.cpp +++ b/src/common/run_command.cpp @@ -12,7 +12,7 @@ #include #ifdef ENABLE_EXTERNAL_SIGNER -#include +#include #endif // ENABLE_EXTERNAL_SIGNER UniValue RunCommandParseJSON(const std::string& str_command, const std::string& str_std_in) diff --git a/src/common/url.cpp b/src/common/url.cpp index 5200d55096a7..283352a38e51 100644 --- a/src/common/url.cpp +++ b/src/common/url.cpp @@ -4,19 +4,36 @@ #include -#include - -#include +#include #include +#include +#include -std::string urlDecode(const std::string &urlEncoded) { +std::string UrlDecode(std::string_view url_encoded) +{ std::string res; - if (!urlEncoded.empty()) { - char *decoded = evhttp_uridecode(urlEncoded.c_str(), false, nullptr); - if (decoded) { - res = std::string(decoded); - free(decoded); + res.reserve(url_encoded.size()); + + for (size_t i = 0; i < url_encoded.size(); ++i) { + char c = url_encoded[i]; + // Special handling for percent which should be followed by two hex digits + // representing an octet values, see RFC 3986, Section 2.1 Percent-Encoding + if (c == '%' && i + 2 < url_encoded.size()) { + unsigned int decoded_value{0}; + auto [p, ec] = std::from_chars(url_encoded.data() + i + 1, url_encoded.data() + i + 3, decoded_value, 16); + + // Only if there is no error and the pointer is set to the end of + // the string, we can be sure both characters were valid hex + if (ec == std::errc{} && p == url_encoded.data() + i + 3) { + res += static_cast(decoded_value); + // Next two characters are part of the percent encoding + i += 2; + continue; + } + // In case of invalid percent encoding, add the '%' and continue } + res += c; } + return res; } diff --git a/src/common/url.h b/src/common/url.h index 7bbd8b60de3c..3213ef90f058 100644 --- a/src/common/url.h +++ b/src/common/url.h @@ -6,9 +6,12 @@ #define BITCOIN_COMMON_URL_H #include +#include -using UrlDecodeFn = std::string(const std::string& url_encoded); -UrlDecodeFn urlDecode; -extern UrlDecodeFn* const URL_DECODE; +/* Decode a URL. + * + * Notably this implementation does not decode a '+' to a ' '. + */ +std::string UrlDecode(std::string_view url_encoded); #endif // BITCOIN_COMMON_URL_H diff --git a/src/qt/main.cpp b/src/qt/main.cpp index 19a8b3685548..4ca0650dc40b 100644 --- a/src/qt/main.cpp +++ b/src/qt/main.cpp @@ -4,7 +4,6 @@ #include -#include #include #include @@ -17,7 +16,6 @@ extern const std::function G_TRANSLATION_FUN = [](const char* psz) { return QCoreApplication::translate("dash-core", psz).toStdString(); }; -UrlDecodeFn* const URL_DECODE = urlDecode; MAIN_FUNCTION { diff --git a/src/test/common_url_tests.cpp b/src/test/common_url_tests.cpp new file mode 100644 index 000000000000..cc893cbed724 --- /dev/null +++ b/src/test/common_url_tests.cpp @@ -0,0 +1,72 @@ +// Copyright (c) 2024-present The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or https://opensource.org/license/mit/. + +#include + +#include + +#include + +BOOST_AUTO_TEST_SUITE(common_url_tests) + +// These test vectors were ported from test/regress.c in the libevent library +// which used to be a dependency of the UrlDecode function. + +BOOST_AUTO_TEST_CASE(encode_decode_test) { + BOOST_CHECK_EQUAL(UrlDecode("Hello"), "Hello"); + BOOST_CHECK_EQUAL(UrlDecode("99"), "99"); + BOOST_CHECK_EQUAL(UrlDecode(""), ""); + BOOST_CHECK_EQUAL(UrlDecode("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ123456789-.~_"), + "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ123456789-.~_"); + BOOST_CHECK_EQUAL(UrlDecode("%20"), " "); + BOOST_CHECK_EQUAL(UrlDecode("%FF%F0%E0"), "\xff\xf0\xe0"); + BOOST_CHECK_EQUAL(UrlDecode("%01%19"), "\x01\x19"); + BOOST_CHECK_EQUAL(UrlDecode("http%3A%2F%2Fwww.ietf.org%2Frfc%2Frfc3986.txt"), + "http://www.ietf.org/rfc/rfc3986.txt"); + BOOST_CHECK_EQUAL(UrlDecode("1%2B2%3D3"), "1+2=3"); +} + +BOOST_AUTO_TEST_CASE(decode_malformed_test) { + BOOST_CHECK_EQUAL(UrlDecode("%%xhello th+ere \xff"), "%%xhello th+ere \xff"); + + BOOST_CHECK_EQUAL(UrlDecode("%"), "%"); + BOOST_CHECK_EQUAL(UrlDecode("%%"), "%%"); + BOOST_CHECK_EQUAL(UrlDecode("%%%"), "%%%"); + BOOST_CHECK_EQUAL(UrlDecode("%%%%"), "%%%%"); + + BOOST_CHECK_EQUAL(UrlDecode("+"), "+"); + BOOST_CHECK_EQUAL(UrlDecode("++"), "++"); + + BOOST_CHECK_EQUAL(UrlDecode("?"), "?"); + BOOST_CHECK_EQUAL(UrlDecode("??"), "??"); + + BOOST_CHECK_EQUAL(UrlDecode("%G1"), "%G1"); + BOOST_CHECK_EQUAL(UrlDecode("%2"), "%2"); + BOOST_CHECK_EQUAL(UrlDecode("%ZX"), "%ZX"); + + BOOST_CHECK_EQUAL(UrlDecode("valid%20string%G1"), "valid string%G1"); + BOOST_CHECK_EQUAL(UrlDecode("%20invalid%ZX"), " invalid%ZX"); + BOOST_CHECK_EQUAL(UrlDecode("%20%G1%ZX"), " %G1%ZX"); + + BOOST_CHECK_EQUAL(UrlDecode("%1 "), "%1 "); + BOOST_CHECK_EQUAL(UrlDecode("% 9"), "% 9"); + BOOST_CHECK_EQUAL(UrlDecode(" %Z "), " %Z "); + BOOST_CHECK_EQUAL(UrlDecode(" % X"), " % X"); + + BOOST_CHECK_EQUAL(UrlDecode("%-1"), "%-1"); + BOOST_CHECK_EQUAL(UrlDecode("%1-"), "%1-"); +} + +BOOST_AUTO_TEST_CASE(decode_lowercase_hex_test) { + BOOST_CHECK_EQUAL(UrlDecode("%f0%a0%b0"), "\xf0\xa0\xb0"); +} + +BOOST_AUTO_TEST_CASE(decode_internal_nulls_test) { + std::string result1{"\0\0x\0\0", 5}; + BOOST_CHECK_EQUAL(UrlDecode("%00%00x%00%00"), result1); + std::string result2{"abc\0\0", 5}; + BOOST_CHECK_EQUAL(UrlDecode("abc%00%00"), result2); +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/fuzz/poolresource.cpp b/src/test/fuzz/poolresource.cpp index ce64ef6472cf..f764d9f8dbe7 100644 --- a/src/test/fuzz/poolresource.cpp +++ b/src/test/fuzz/poolresource.cpp @@ -63,9 +63,9 @@ class PoolResourceFuzzer { if (m_total_allocated > 0x1000000) return; size_t alignment_bits = m_provider.ConsumeIntegralInRange(0, 7); - size_t alignment = 1 << alignment_bits; + size_t alignment = size_t{1} << alignment_bits; size_t size_bits = m_provider.ConsumeIntegralInRange(0, 16 - alignment_bits); - size_t size = m_provider.ConsumeIntegralInRange(1U << size_bits, (1U << (size_bits + 1)) - 1U) << alignment_bits; + size_t size = m_provider.ConsumeIntegralInRange(size_t{1} << size_bits, (size_t{1} << (size_bits + 1)) - 1U) << alignment_bits; Allocate(size, alignment); } diff --git a/src/test/fuzz/string.cpp b/src/test/fuzz/string.cpp index 33490b0c1f28..0d15ec776ea7 100644 --- a/src/test/fuzz/string.cpp +++ b/src/test/fuzz/string.cpp @@ -180,7 +180,7 @@ FUZZ_TARGET(string) (void)ToUpper(random_string_1); (void)TrimString(random_string_1); (void)TrimString(random_string_1, random_string_2); - (void)urlDecode(random_string_1); + (void)UrlDecode(random_string_1); (void)ContainsNoNUL(random_string_1); (void)_(random_string_1.c_str()); try { diff --git a/src/test/system_tests.cpp b/src/test/system_tests.cpp index ce7ed3912c79..3a11152bf071 100644 --- a/src/test/system_tests.cpp +++ b/src/test/system_tests.cpp @@ -7,7 +7,7 @@ #include #ifdef ENABLE_EXTERNAL_SIGNER -#include +#include #endif // ENABLE_EXTERNAL_SIGNER #include diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 2616b5ae1bfe..21b5b83cfb40 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -9,7 +9,6 @@ #include #include #include -#include #include #include #include @@ -92,7 +91,6 @@ using node::fPruneMode; using node::fReindex; const std::function G_TRANSLATION_FUN = nullptr; -UrlDecodeFn* const URL_DECODE = nullptr; FastRandomContext g_insecure_rand_ctx; /** Random context to get unique temp data dirs. Separate from g_insecure_rand_ctx, which can be seeded from a const env var */ diff --git a/src/test/validation_chainstate_tests.cpp b/src/test/validation_chainstate_tests.cpp index 73920bda6743..3d7d9e50e368 100644 --- a/src/test/validation_chainstate_tests.cpp +++ b/src/test/validation_chainstate_tests.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include @@ -96,14 +97,14 @@ BOOST_FIXTURE_TEST_CASE(chainstate_update_tip, TestChain100Setup) BOOST_CHECK_EQUAL(chainman.GetAll().size(), 2); - CChainState& background_cs{*[&] { + CChainState& background_cs{*Assert([&]() -> CChainState* { for (CChainState* cs : chainman.GetAll()) { if (cs != &chainman.ActiveChainstate()) { return cs; } } - assert(false); - }()}; + return nullptr; + }())}; // Create a block to append to the validation chain. std::vector noTxns; diff --git a/src/txorphanage.cpp b/src/txorphanage.cpp index d03924c2fe7d..50c0758f32f1 100644 --- a/src/txorphanage.cpp +++ b/src/txorphanage.cpp @@ -8,13 +8,14 @@ #include #include #include +#include #include -/** Expiration time for orphan transactions in seconds */ -static constexpr int64_t ORPHAN_TX_EXPIRE_TIME = 20 * 60; -/** Minimum time between orphan transactions expire time checks in seconds */ -static constexpr int64_t ORPHAN_TX_EXPIRE_INTERVAL = 5 * 60; +/** Expiration time for orphan transactions */ +static constexpr auto ORPHAN_TX_EXPIRE_TIME{20min}; +/** Minimum time between orphan transactions expire time checks */ +static constexpr auto ORPHAN_TX_EXPIRE_INTERVAL{5min}; bool TxOrphanage::AddTx(const CTransactionRef& tx, NodeId peer) @@ -39,7 +40,7 @@ bool TxOrphanage::AddTx(const CTransactionRef& tx, NodeId peer) return false; } - auto ret = m_orphans.emplace(hash, OrphanTx{tx, peer, GetTime() + ORPHAN_TX_EXPIRE_TIME, m_orphan_list.size(), sz}); + auto ret = m_orphans.emplace(hash, OrphanTx{tx, peer, Now() + ORPHAN_TX_EXPIRE_TIME, m_orphan_list.size(), sz}); assert(ret.second); m_orphan_list.push_back(ret.first); for (const CTxIn& txin : tx->vin) { @@ -121,12 +122,12 @@ void TxOrphanage::LimitOrphans(unsigned int max_orphans_size) LOCK(m_mutex); unsigned int nEvicted = 0; - static int64_t nNextSweep; - int64_t nNow = GetTime(); + static NodeSeconds nNextSweep; + auto nNow{Now()}; if (nNextSweep <= nNow) { // Sweep out expired orphan pool entries: int nErased = 0; - int64_t nMinExpTime = nNow + ORPHAN_TX_EXPIRE_TIME - ORPHAN_TX_EXPIRE_INTERVAL; + auto nMinExpTime = nNow + ORPHAN_TX_EXPIRE_TIME - ORPHAN_TX_EXPIRE_INTERVAL; std::map::iterator iter = m_orphans.begin(); while (iter != m_orphans.end()) { diff --git a/src/txorphanage.h b/src/txorphanage.h index bd68ea091627..0bd364d672f6 100644 --- a/src/txorphanage.h +++ b/src/txorphanage.h @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -70,7 +71,7 @@ class TxOrphanage { struct OrphanTx { CTransactionRef tx; NodeId fromPeer; - int64_t nTimeExpire; + NodeSeconds nTimeExpire; size_t list_pos; size_t nTxSize; }; diff --git a/src/util/subprocess.hpp b/src/util/subprocess.h similarity index 74% rename from src/util/subprocess.hpp rename to src/util/subprocess.h index dd53a8fbb4fb..25ba2c438f93 100644 --- a/src/util/subprocess.hpp +++ b/src/util/subprocess.h @@ -1,6 +1,8 @@ +// Based on the https://github.com/arun11299/cpp-subprocess project. + /*! -Documentation for C++ subprocessing libraray. +Documentation for C++ subprocessing library. @copyright The code is licensed under the [MIT License](http://opensource.org/licenses/MIT): @@ -31,8 +33,10 @@ Documentation for C++ subprocessing libraray. @version 1.0.0 */ -#ifndef SUBPROCESS_HPP -#define SUBPROCESS_HPP +#ifndef BITCOIN_UTIL_SUBPROCESS_H +#define BITCOIN_UTIL_SUBPROCESS_H + +#include #include #include @@ -106,7 +110,7 @@ namespace subprocess { // from pipe static const size_t SP_MAX_ERR_BUF_SIZ = 1024; -// Default buffer capcity for OutBuffer and ErrBuffer. +// Default buffer capacity for OutBuffer and ErrBuffer. // If the data exceeds this capacity, the buffer size is grown // by 1.5 times its previous capacity static const size_t DEFAULT_BUF_CAP_BYTES = 8192; @@ -148,32 +152,13 @@ class OSError: public std::runtime_error { public: OSError(const std::string& err_msg, int err_code): - std::runtime_error( err_msg + ": " + std::strerror(err_code) ) + std::runtime_error(err_msg + ": " + SysErrorString(err_code)) {} }; -//-------------------------------------------------------------------- - -//Environment Variable types -#ifndef _MSC_VER - using env_string_t = std::string; - using env_char_t = char; -#else - using env_string_t = std::wstring; - using env_char_t = wchar_t; -#endif -using env_map_t = std::map; -using env_vector_t = std::vector; - //-------------------------------------------------------------------- namespace util { - template - inline bool is_ready(std::shared_future const &f) - { - return f.wait_for(std::chrono::seconds(0)) == std::future_status::ready; - } - inline void quote_argument(const std::wstring &argument, std::wstring &command_line, bool force) { @@ -303,100 +288,6 @@ namespace util if (!SetHandleInformation(*child_handle, HANDLE_FLAG_INHERIT, 0)) throw OSError("SetHandleInformation", 0); } - - // env_map_t MapFromWindowsEnvironment() - // * Imports current Environment in a C-string table - // * Parses the strings by splitting on the first "=" per line - // * Creates a map of the variables - // * Returns the map - inline env_map_t MapFromWindowsEnvironment(){ - wchar_t *variable_strings_ptr; - wchar_t *environment_strings_ptr; - std::wstring delimeter(L"="); - int del_len = delimeter.length(); - env_map_t mapped_environment; - - // Get a pointer to the environment block. - environment_strings_ptr = GetEnvironmentStringsW(); - // If the returned pointer is NULL, exit. - if (environment_strings_ptr == NULL) - { - throw OSError("GetEnvironmentStringsW", 0); - } - - // Variable strings are separated by NULL byte, and the block is - // terminated by a NULL byte. - - variable_strings_ptr = (wchar_t *) environment_strings_ptr; - - //Since the environment map ends with a null, we can loop until we find it. - while (*variable_strings_ptr) - { - // Create a string from Variable String - env_string_t current_line(variable_strings_ptr); - // Find the first "equals" sign. - auto pos = current_line.find(delimeter); - // Assuming it's not missing ... - if(pos!=std::wstring::npos){ - // ... parse the key and value. - env_string_t key = current_line.substr(0, pos); - env_string_t value = current_line.substr(pos + del_len); - // Map the entry. - mapped_environment[key] = value; - } - // Jump to next line in the environment map. - variable_strings_ptr += std::wcslen(variable_strings_ptr) + 1; - } - // We're done with the old environment map buffer. - FreeEnvironmentStringsW(environment_strings_ptr); - - // Return the map. - return mapped_environment; - } - - // env_vector_t WindowsEnvironmentVectorFromMap(const env_map_t &source_map) - // * Creates a vector buffer for the new environment string table - // * Copies in the mapped variables - // * Returns the vector - inline env_vector_t WindowsEnvironmentVectorFromMap(const env_map_t &source_map) - { - // Make a new environment map buffer. - env_vector_t environment_map_buffer; - // Give it some space. - environment_map_buffer.reserve(4096); - - // And fill'er up. - for(auto kv: source_map){ - // Create the line - env_string_t current_line(kv.first); current_line += L"="; current_line += kv.second; - // Add the line to the buffer. - std::copy(current_line.begin(), current_line.end(), std::back_inserter(environment_map_buffer)); - // Append a null - environment_map_buffer.push_back(0); - } - // Append one last null because of how Windows does it's environment maps. - environment_map_buffer.push_back(0); - - return environment_map_buffer; - } - - // env_vector_t CreateUpdatedWindowsEnvironmentVector(const env_map_t &changes_map) - // * Merges host environment with new mapped variables - // * Creates and returns string vector based on map - inline env_vector_t CreateUpdatedWindowsEnvironmentVector(const env_map_t &changes_map){ - // Import the environment map - env_map_t environment_map = MapFromWindowsEnvironment(); - // Merge in the changes with overwrite - for(auto& it: changes_map) - { - environment_map[it.first] = it.second; - } - // Create a Windows-usable Environment Map Buffer - env_vector_t environment_map_strings_vector = WindowsEnvironmentVectorFromMap(environment_map); - - return environment_map_strings_vector; - } - #endif /*! @@ -429,26 +320,6 @@ namespace util } - /*! - * Function: join - * Parameters: - * [in] vec : Vector of strings which needs to be joined to form - * a single string with words seperated by a seperator char. - * [in] sep : String used to seperate 2 words in the joined string. - * Default constructed to ' ' (space). - * [out] string: Joined string. - */ - static inline - std::string join(const std::vector& vec, - const std::string& sep = " ") - { - std::string res; - for (auto& elem : vec) res.append(elem + sep); - res.erase(--res.end()); - return res; - } - - #ifndef __USING_WINDOWS__ /*! * Function: set_clo_on_exec @@ -649,56 +520,6 @@ namespace util * ------------------------------- */ -/*! - * The buffer size of the stdin/stdout/stderr - * streams of the child process. - * Default value is 0. - */ -struct bufsize { - explicit bufsize(int siz): bufsiz(siz) {} - int bufsiz = 0; -}; - -/*! - * Option to defer spawning of the child process - * till `Popen::start_process` API is called. - * Default value is false. - */ -struct defer_spawn { - explicit defer_spawn(bool d): defer(d) {} - bool defer = false; -}; - -/*! - * Option to close all file descriptors - * when the child process is spawned. - * The close fd list does not include - * input/output/error if they are explicitly - * set as part of the Popen arguments. - * - * Default value is false. - */ -struct close_fds { - explicit close_fds(bool c): close_all(c) {} - bool close_all = false; -}; - -/*! - * Option to make the child process as the - * session leader and thus the process - * group leader. - * Default value is false. - */ -struct session_leader { - explicit session_leader(bool sl): leader_(sl) {} - bool leader_ = false; -}; - -struct shell { - explicit shell(bool s): shell_(s) {} - bool shell_ = false; -}; - /*! * Base class for all arguments involving string value. */ @@ -711,7 +532,7 @@ struct string_arg }; /*! - * Option to specify the executable name seperately + * Option to specify the executable name separately * from the args sequence. * In this case the cmd args must only contain the * options required for this executable. @@ -724,34 +545,6 @@ struct executable: string_arg executable(T&& arg): string_arg(std::forward(arg)) {} }; -/*! - * Option to set the current working directory - * of the spawned process. - * - * Eg: cwd{"/som/path/x"} - */ -struct cwd: string_arg -{ - template - cwd(T&& arg): string_arg(std::forward(arg)) {} -}; - -/*! - * Option to specify environment variables required by - * the spawned process. - * - * Eg: environment{{ {"K1", "V1"}, {"K2", "V2"},... }} - */ -struct environment -{ - environment(env_map_t&& env): - env_(std::move(env)) {} - explicit environment(const env_map_t& env): - env_(env) {} - env_map_t env_; -}; - - /*! * Used for redirecting input/output/error */ @@ -852,7 +645,7 @@ struct error wr_ch_ = fd; } explicit error(IOTYPE typ) { - assert ((typ == PIPE || typ == STDOUT) && "STDERR not aloowed"); + assert ((typ == PIPE || typ == STDOUT) && "STDERR not allowed"); if (typ == PIPE) { #ifndef __USING_WINDOWS__ std::tie(rd_ch_, wr_ch_) = util::pipe_cloexec(); @@ -868,44 +661,6 @@ struct error int wr_ch_ = -1; }; -// Impoverished, meager, needy, truly needy -// version of type erasure to store function pointers -// needed to provide the functionality of preexec_func -// ATTN: Can be used only to execute functions with no -// arguments and returning void. -// Could have used more efficient methods, ofcourse, but -// that wont yield me the consistent syntax which I am -// aiming for. If you know, then please do let me know. - -class preexec_func -{ -public: - preexec_func() {} - - template - explicit preexec_func(Func f): holder_(new FuncHolder(std::move(f))) - {} - - void operator()() { - (*holder_)(); - } - -private: - struct HolderBase { - virtual void operator()() const = 0; - virtual ~HolderBase(){}; - }; - template - struct FuncHolder: HolderBase { - FuncHolder(T func): func_(std::move(func)) {} - void operator()() const override { func_(); } - // The function pointer/reference - T func_; - }; - - std::unique_ptr holder_ = nullptr; -}; - // ~~~~ End Popen Args ~~~~ @@ -915,8 +670,8 @@ class preexec_func * This is basically used to determine the length of the actual * data stored inside the dynamically resized vector. * - * This is what is returned as the output to communicate and check_output - * functions, so, users must know about this class. + * This is what is returned as the output to the communicate + * function, so, users must know about this class. * * OutBuffer and ErrBuffer are just different typedefs to this class. */ @@ -927,22 +682,6 @@ class Buffer explicit Buffer(size_t cap) { buf.resize(cap); } void add_cap(size_t cap) { buf.resize(cap); } -#if 0 - Buffer(const Buffer& other): - buf(other.buf), - length(other.length) - { - std::cout << "COPY" << std::endl; - } - - Buffer(Buffer&& other): - buf(std::move(other.buf)), - length(other.length) - { - std::cout << "MOVE" << std::endl; - } -#endif - public: std::vector buf; size_t length = 0; @@ -1005,17 +744,9 @@ struct ArgumentDeducer ArgumentDeducer(Popen* p): popen_(p) {} void set_option(executable&& exe); - void set_option(cwd&& cwdir); - void set_option(bufsize&& bsiz); - void set_option(environment&& env); - void set_option(defer_spawn&& defer); - void set_option(shell&& sh); void set_option(input&& inp); void set_option(output&& out); void set_option(error&& err); - void set_option(close_fds&& cfds); - void set_option(preexec_func&& prefunc); - void set_option(session_leader&& sleader); private: Popen* popen_ = nullptr; @@ -1166,9 +897,6 @@ class Streams HANDLE g_hChildStd_ERR_Wr = nullptr; #endif - // Buffer size for the IO streams - int bufsiz_ = 0; - // Pipes for communicating with child // Emulates stdin @@ -1198,26 +926,23 @@ class Streams * interface to the client. * * API's provided by the class: - * 1. Popen({"cmd"}, output{..}, error{..}, cwd{..}, ....) + * Popen({"cmd"}, output{..}, error{..}, ....) * Command provided as a sequence. - * 2. Popen("cmd arg1"m output{..}, error{..}, cwd{..}, ....) + * Popen("cmd arg1", output{..}, error{..}, ....) * Command provided in a single string. - * 3. wait() - Wait for the child to exit. - * 4. retcode() - The return code of the exited child. - * 5. pid() - PID of the spawned child. - * 6. poll() - Check the status of the running child. - * 7. kill(sig_num) - Kill the child. SIGTERM used by default. - * 8. send(...) - Send input to the input channel of the child. - * 9. communicate(...) - Get the output/error from the child and close the channels - * from the parent side. - *10. input() - Get the input channel/File pointer. Can be used for - * cutomizing the way of sending input to child. - *11. output() - Get the output channel/File pointer. Usually used - in case of redirection. See piping examples. - *12. error() - Get the error channel/File poiner. Usually used - in case of redirection. - *13. start_process() - Start the child process. Only to be used when - * `defer_spawn` option was provided in Popen constructor. + * wait() - Wait for the child to exit. + * retcode() - The return code of the exited child. + * pid() - PID of the spawned child. + * poll() - Check the status of the running child. + * send(...) - Send input to the input channel of the child. + * communicate(...) - Get the output/error from the child and close the channels + * from the parent side. + * input() - Get the input channel/File pointer. Can be used for + * customizing the way of sending input to child. + * output() - Get the output channel/File pointer. Usually used + in case of redirection. See piping examples. + * error() - Get the error channel/File pointer. Usually used + in case of redirection. */ class Popen { @@ -1235,7 +960,7 @@ class Popen // Setup the communication channels of the Popen class stream_.setup_comm_channels(); - if (!defer_process_start_) execute_process(); + execute_process(); } template @@ -1247,7 +972,7 @@ class Popen // Setup the communication channels of the Popen class stream_.setup_comm_channels(); - if (!defer_process_start_) execute_process(); + execute_process(); } template @@ -1258,19 +983,9 @@ class Popen // Setup the communication channels of the Popen class stream_.setup_comm_channels(); - if (!defer_process_start_) execute_process(); - } - -/* - ~Popen() - { -#ifdef __USING_WINDOWS__ - CloseHandle(this->process_handle_); -#endif + execute_process(); } -*/ - void start_process() noexcept(false); int pid() const noexcept { return child_pid_; } @@ -1280,10 +995,6 @@ class Popen int poll() noexcept(false); - // Does not fail, Caller is expected to recheck the - // status with a call to poll() - void kill(int sig_num = 9); - void set_out_buf_cap(size_t cap) { stream_.set_out_buf_cap(cap); } void set_err_buf_cap(size_t cap) { stream_.set_err_buf_cap(cap); } @@ -1345,20 +1056,11 @@ class Popen std::future cleanup_future_; #endif - bool defer_process_start_ = false; - bool close_fds_ = false; - bool has_preexec_fn_ = false; - bool shell_ = false; - bool session_leader_ = false; - std::string exe_name_; - std::string cwd_; - env_map_t env_; - preexec_func preexec_fn_; // Command in string format std::string args_; - // Comamnd provided as sequence + // Command provided as sequence std::vector vargs_; std::vector cargv_; @@ -1389,20 +1091,6 @@ inline void Popen::populate_c_argv() cargv_.push_back(nullptr); } -inline void Popen::start_process() noexcept(false) -{ - // The process was started/tried to be started - // in the constructor itself. - // For explicitly calling this API to start the - // process, 'defer_spawn' argument must be set to - // true in the constructor. - if (!defer_process_start_) { - assert (0); - return; - } - execute_process(); -} - inline int Popen::wait() noexcept(false) { #ifdef __USING_WINDOWS__ @@ -1474,33 +1162,9 @@ inline int Popen::poll() noexcept(false) #endif } -inline void Popen::kill(int sig_num) -{ -#ifdef __USING_WINDOWS__ - if (!TerminateProcess(this->process_handle_, (UINT)sig_num)) { - throw OSError("TerminateProcess", 0); - } -#else - if (session_leader_) killpg(child_pid_, sig_num); - else ::kill(child_pid_, sig_num); -#endif -} - - inline void Popen::execute_process() noexcept(false) { #ifdef __USING_WINDOWS__ - if (this->shell_) { - throw OSError("shell not currently supported on windows", 0); - } - - void* environment_string_table_ptr = nullptr; - env_vector_t environment_string_vector; - if(this->env_.size()){ - environment_string_vector = util::CreateUpdatedWindowsEnvironmentVector(env_); - environment_string_table_ptr = (void*)environment_string_vector.data(); - } - if (exe_name_.length()) { this->vargs_.insert(this->vargs_.begin(), this->exe_name_); this->populate_c_argv(); @@ -1546,8 +1210,8 @@ inline void Popen::execute_process() noexcept(false) NULL, // process security attributes NULL, // primary thread security attributes TRUE, // handles are inherited - creation_flags, // creation flags - environment_string_table_ptr, // use provided environment + creation_flags, // creation flags + NULL, // use parent's environment NULL, // use parent's current directory &siStartInfo, // STARTUPINFOW pointer &piProcInfo); // receives PROCESS_INFORMATION @@ -1586,14 +1250,6 @@ inline void Popen::execute_process() noexcept(false) int err_rd_pipe, err_wr_pipe; std::tie(err_rd_pipe, err_wr_pipe) = util::pipe_cloexec(); - if (shell_) { - auto new_cmd = util::join(vargs_); - vargs_.clear(); - vargs_.insert(vargs_.begin(), {"/bin/sh", "-c"}); - vargs_.push_back(new_cmd); - populate_c_argv(); - } - if (exe_name_.length()) { vargs_.insert(vargs_.begin(), exe_name_); populate_c_argv(); @@ -1660,30 +1316,6 @@ namespace detail { popen_->exe_name_ = std::move(exe.arg_value); } - inline void ArgumentDeducer::set_option(cwd&& cwdir) { - popen_->cwd_ = std::move(cwdir.arg_value); - } - - inline void ArgumentDeducer::set_option(bufsize&& bsiz) { - popen_->stream_.bufsiz_ = bsiz.bufsiz; - } - - inline void ArgumentDeducer::set_option(environment&& env) { - popen_->env_ = std::move(env.env_); - } - - inline void ArgumentDeducer::set_option(defer_spawn&& defer) { - popen_->defer_process_start_ = defer.defer; - } - - inline void ArgumentDeducer::set_option(shell&& sh) { - popen_->shell_ = sh.shell_; - } - - inline void ArgumentDeducer::set_option(session_leader&& sleader) { - popen_->session_leader_ = sleader.leader_; - } - inline void ArgumentDeducer::set_option(input&& inp) { if (inp.rd_ch_ != -1) popen_->stream_.read_from_parent_ = inp.rd_ch_; if (inp.wr_ch_ != -1) popen_->stream_.write_to_child_ = inp.wr_ch_; @@ -1706,15 +1338,6 @@ namespace detail { if (err.rd_ch_ != -1) popen_->stream_.err_read_ = err.rd_ch_; } - inline void ArgumentDeducer::set_option(close_fds&& cfds) { - popen_->close_fds_ = cfds.close_all; - } - - inline void ArgumentDeducer::set_option(preexec_func&& prefunc) { - popen_->preexec_fn_ = std::move(prefunc); - popen_->has_preexec_fn_ = true; - } - inline void Child::execute_child() { #ifndef __USING_WINDOWS__ @@ -1761,41 +1384,8 @@ namespace detail { if (stream.err_write_ != -1 && stream.err_write_ > 2) close(stream.err_write_); - // Close all the inherited fd's except the error write pipe - if (parent_->close_fds_) { - int max_fd = sysconf(_SC_OPEN_MAX); - if (max_fd == -1) throw OSError("sysconf failed", errno); - - for (int i = 3; i < max_fd; i++) { - if (i == err_wr_pipe_) continue; - close(i); - } - } - - // Change the working directory if provided - if (parent_->cwd_.length()) { - sys_ret = chdir(parent_->cwd_.c_str()); - if (sys_ret == -1) throw OSError("chdir failed", errno); - } - - if (parent_->has_preexec_fn_) { - parent_->preexec_fn_(); - } - - if (parent_->session_leader_) { - sys_ret = setsid(); - if (sys_ret == -1) throw OSError("setsid failed", errno); - } - // Replace the current image with the executable - if (parent_->env_.size()) { - for (auto& kv : parent_->env_) { - setenv(kv.first.c_str(), kv.second.c_str(), 1); - } - sys_ret = execvp(parent_->exe_name_.c_str(), parent_->cargv_.data()); - } else { - sys_ret = execvp(parent_->exe_name_.c_str(), parent_->cargv_.data()); - } + sys_ret = execvp(parent_->exe_name_.c_str(), parent_->cargv_.data()); if (sys_ret == -1) throw OSError("execve failed", errno); @@ -1838,16 +1428,7 @@ namespace detail { for (auto& h : handles) { if (h == nullptr) continue; - switch (bufsiz_) { - case 0: - setvbuf(h, nullptr, _IONBF, BUFSIZ); - break; - case 1: - setvbuf(h, nullptr, _IONBF, BUFSIZ); - break; - default: - setvbuf(h, nullptr, _IOFBF, bufsiz_); - }; + setvbuf(h, nullptr, _IONBF, BUFSIZ); } #endif } @@ -1983,4 +1564,4 @@ namespace detail { } -#endif // SUBPROCESS_HPP +#endif // BITCOIN_UTIL_SUBPROCESS_H diff --git a/src/wallet/rpc/util.cpp b/src/wallet/rpc/util.cpp index e67be931af6d..75b369b45f2a 100644 --- a/src/wallet/rpc/util.cpp +++ b/src/wallet/rpc/util.cpp @@ -10,6 +10,7 @@ #include #include +#include #include #include @@ -60,9 +61,9 @@ bool ParseIncludeWatchonly(const UniValue& include_watchonly, const CWallet& wal bool GetWalletNameFromJSONRPCRequest(const JSONRPCRequest& request, std::string& wallet_name) { - if (URL_DECODE && request.URI.substr(0, WALLET_ENDPOINT_BASE.size()) == WALLET_ENDPOINT_BASE) { + if (request.URI.starts_with(WALLET_ENDPOINT_BASE)) { // wallet endpoint was used - wallet_name = URL_DECODE(request.URI.substr(WALLET_ENDPOINT_BASE.size())); + wallet_name = UrlDecode(std::string_view{request.URI}.substr(WALLET_ENDPOINT_BASE.size())); return true; } return false; diff --git a/test/lint/lint-spelling.py b/test/lint/lint-spelling.py index fb4d2495c691..79c4533bdf55 100755 --- a/test/lint/lint-spelling.py +++ b/test/lint/lint-spelling.py @@ -12,7 +12,7 @@ from subprocess import check_output, STDOUT, CalledProcessError IGNORE_WORDS_FILE = 'test/lint/spelling.ignore-words.txt' -FILES_ARGS = ['git', 'ls-files', '--', ":(exclude)build-aux/m4/", ":(exclude)contrib/seeds/*.txt", ":(exclude)depends/", ":(exclude)doc/release-notes/", ":(exclude)src/dashbls/", ":(exclude)src/crc32c/", ":(exclude)src/crypto/", ":(exclude)src/ctpl_stl.h", ":(exclude)src/cxxtimer.hpp", ":(exclude)src/immer/", ":(exclude)src/leveldb/", ":(exclude)src/qt/locale/", ":(exclude)src/qt/*.qrc", ":(exclude)src/secp256k1/", ":(exclude)src/minisketch/", ":(exclude)contrib/builder-keys/", ":(exclude)contrib/guix/patches", ":(exclude)src/util/subprocess.hpp", ":(exclude)src/wallet/bip39_english.h"] +FILES_ARGS = ['git', 'ls-files', '--', ":(exclude)build-aux/m4/", ":(exclude)contrib/seeds/*.txt", ":(exclude)depends/", ":(exclude)doc/release-notes/", ":(exclude)src/dashbls/", ":(exclude)src/crc32c/", ":(exclude)src/crypto/", ":(exclude)src/ctpl_stl.h", ":(exclude)src/cxxtimer.hpp", ":(exclude)src/immer/", ":(exclude)src/leveldb/", ":(exclude)src/qt/locale/", ":(exclude)src/qt/*.qrc", ":(exclude)src/secp256k1/", ":(exclude)src/minisketch/", ":(exclude)contrib/builder-keys/", ":(exclude)contrib/guix/patches", ":(exclude)src/wallet/bip39_english.h"] def check_codespell_install():