diff --git a/src/common/system.cpp b/src/common/system.cpp index d7dd357f8c..ca27dad806 100644 --- a/src/common/system.cpp +++ b/src/common/system.cpp @@ -37,14 +37,12 @@ using util::ReplaceAll; // Application startup time (used for uptime calculation) const int64_t nStartupTime = GetTime(); -#ifndef WIN32 std::string ShellEscape(const std::string& arg) { std::string escaped = arg; - ReplaceAll(escaped, "'", "'\"'\"'"); + ReplaceAll(escaped, "'", "'\\''"); return "'" + escaped + "'"; } -#endif #if HAVE_SYSTEM void runCommand(const std::string& strCommand) diff --git a/src/common/system.h b/src/common/system.h index f572d716df..1cb3729da3 100644 --- a/src/common/system.h +++ b/src/common/system.h @@ -16,9 +16,7 @@ int64_t GetStartupTime(); void SetupEnvironment(); [[nodiscard]] bool SetupNetworking(); -#ifndef WIN32 std::string ShellEscape(const std::string& arg); -#endif #if HAVE_SYSTEM void runCommand(const std::string& strCommand); #endif diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp index 14d22bb54e..326bb2653b 100644 --- a/src/wallet/init.cpp +++ b/src/wallet/init.cpp @@ -77,7 +77,7 @@ void WalletInit::AddWalletOptions(ArgsManager& argsman) const argsman.AddArg("-walletbroadcast", strprintf("Make the wallet broadcast transactions (default: %u)", DEFAULT_WALLETBROADCAST), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); argsman.AddArg("-walletdir=", "Specify directory to hold wallets (default: /wallets if it exists, otherwise )", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::WALLET); #if HAVE_SYSTEM - argsman.AddArg("-walletnotify=", "Execute command when a wallet transaction changes. %s in cmd is replaced by TxID, %w is replaced by wallet name, %b is replaced by the hash of the block including the transaction (set to 'unconfirmed' if the transaction is not included) and %h is replaced by the block height (-1 if not included). %w is not currently implemented on windows. On systems where %w is supported, it should NOT be quoted because this would break shell escaping used to invoke the command.", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); + argsman.AddArg("-walletnotify=", "Execute command when a wallet transaction changes. %s in cmd is replaced by TxID, %w is replaced by wallet name, %b is replaced by the hash of the block including the transaction (set to 'unconfirmed' if the transaction is not included) and %h is replaced by the block height (-1 if not included). %w should NOT be quoted because this would break shell escaping used to invoke the command.", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); #endif argsman.AddArg("-walletrbf", strprintf("Send transactions with full-RBF opt-in enabled (RPC only, default: %u)", DEFAULT_WALLET_RBF), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 4cc9d072ac..56774d63bb 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -1232,16 +1232,7 @@ CWalletTx* CWallet::AddToWallet(CTransactionRef tx, const TxState& state, const #if HAVE_SYSTEM // notify an external script when a wallet transaction comes in or is updated if (!m_notify_tx_changed_scripts.empty()) { -#ifdef WIN32 - // Substituting the wallet name isn't currently supported on windows - // because windows shell escaping has not been implemented yet: - // https://github.com/bitcoin/bitcoin/pull/13339#issuecomment-537384875 - // A few ways it could be implemented in the future are described in: - // https://github.com/bitcoin/bitcoin/pull/13339#issuecomment-461288094 - const std::string walletname_escaped = "wallet_name_substitution_is_not_available_on_Windows"; -#else const std::string walletname_escaped = ShellEscape(GetName()); -#endif const std::string txid_hex = hash.GetHex(); std::string blockhash_hex, blockheight_str; if (auto* conf = wtx.state()) { diff --git a/test/functional/feature_notifications.py b/test/functional/feature_notifications.py index 3edee4be1a..da6e1edc8a 100755 --- a/test/functional/feature_notifications.py +++ b/test/functional/feature_notifications.py @@ -15,13 +15,13 @@ from test_framework.util import ( # Linux allow all characters other than \x00 # Windows disallow control characters (0-31) and /\?%:|"<> -FILE_CHAR_START = 32 if platform.system() == 'Windows' else 1 +FILE_CHAR_START = 43 if platform.system() == 'Windows' else 1 FILE_CHAR_END = 128 -FILE_CHARS_DISALLOWED = '/\\?%*:|"<>' if platform.system() == 'Windows' else '/' +FILE_CHARS_DISALLOWED = '/:;<>?@[\\]^`{|}' if platform.system() == 'Windows' else '/' UNCONFIRMED_HASH_STRING = 'unconfirmed' def notify_outputname(walletname, txid): - return txid if platform.system() == 'Windows' else f'{walletname}_{txid}' + return f'{walletname}_{txid}' class NotificationsTest(BitcoinTestFramework): @@ -33,7 +33,7 @@ class NotificationsTest(BitcoinTestFramework): self.setup_clean_chain = True def setup_network(self): - self.wallet = ''.join(chr(i) for i in range(FILE_CHAR_START, FILE_CHAR_END) if chr(i) not in FILE_CHARS_DISALLOWED) + self.wallet = 'wallet' + ''.join(chr(i) for i in range(FILE_CHAR_START, FILE_CHAR_END) if chr(i) not in FILE_CHARS_DISALLOWED) self.alertnotify_dir = os.path.join(self.options.tmpdir, "alertnotify") self.blocknotify_dir = os.path.join(self.options.tmpdir, "blocknotify") self.walletnotify_dir = os.path.join(self.options.tmpdir, "walletnotify")