diff options
author | fanquake <fanquake@gmail.com> | 2022-01-11 12:09:58 +0800 |
---|---|---|
committer | fanquake <fanquake@gmail.com> | 2022-01-11 13:54:51 +0800 |
commit | fa74718414d7adb36095d277967d70c538a6ffdb (patch) | |
tree | a5de01b22d3f8d75a262ad01d13fd4ffdc7e9afe | |
parent | 80ceede7a0bf42c0966bd89bddbbfb3e62a5a2d9 (diff) | |
parent | 3a45dc36a663ea67f13e7d5c413518b9385ec594 (diff) | |
download | bitcoin-fa74718414d7adb36095d277967d70c538a6ffdb.tar.xz |
Merge bitcoin/bitcoin#24026: Block unsafe std::string fs::path conversion copy_file calls
3a45dc36a663ea67f13e7d5c413518b9385ec594 Change type of `backup_file` parameter in RestoreWallet/restoreWallet (Hennadii Stepanov)
213172c7348091794b173c2edf5f7f3bbe010912 refactor: Block unsafe std::string fs::path conversion copy_file calls (Hennadii Stepanov)
Pull request description:
This PR is an optional prerequisite for bitcoin/bitcoin#20744 "Use std::filesystem. Remove Boost Filesystem & System" which:
- makes further code changes safer
- prevents [some](https://cirrus-ci.com/task/6525835388649472) test failures on native Windows
ACKs for top commit:
ryanofsky:
Code review ACK 3a45dc36a663ea67f13e7d5c413518b9385ec594. Looks great! Thanks for debugging and fixing this and making #20744 smaller!
Tree-SHA512: c6dfaef6b45b9c268bc9ee9b943b9d679152c9d565ca4f86da8d33f8eb9b3cdbe9ba6df7b7578eacc0d00db6551048beff97419f86eb4b1d3182c43e2b4eb9a5
-rw-r--r-- | src/fs.h | 7 | ||||
-rw-r--r-- | src/interfaces/wallet.h | 3 | ||||
-rw-r--r-- | src/wallet/interfaces.cpp | 2 | ||||
-rw-r--r-- | src/wallet/rpc/backup.cpp | 2 | ||||
-rw-r--r-- | src/wallet/wallet.cpp | 4 | ||||
-rw-r--r-- | src/wallet/wallet.h | 3 |
6 files changed, 15 insertions, 6 deletions
@@ -92,6 +92,13 @@ static inline path operator+(path p1, path p2) return p1; } +// Disallow implicit std::string conversion for copy_file +// to avoid locale-dependent encoding on Windows. +static inline void copy_file(const path& from, const path& to, copy_option options) +{ + boost::filesystem::copy_file(from, to, options); +} + /** * Convert path object to byte string. On POSIX, paths natively are byte * strings, so this is trivial. On Windows, paths natively are Unicode, so an diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index c81ec30227..a83f0727a1 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -6,6 +6,7 @@ #define BITCOIN_INTERFACES_WALLET_H #include <consensus/amount.h> +#include <fs.h> #include <interfaces/chain.h> // For ChainClient #include <pubkey.h> // For CKeyID and CScriptID (definitions needed in CTxDestination instantiation) #include <script/standard.h> // For CTxDestination @@ -326,7 +327,7 @@ public: virtual std::string getWalletDir() = 0; //! Restore backup wallet - virtual std::unique_ptr<Wallet> restoreWallet(const std::string& backup_file, const std::string& wallet_name, bilingual_str& error, std::vector<bilingual_str>& warnings) = 0; + virtual std::unique_ptr<Wallet> restoreWallet(const fs::path& backup_file, const std::string& wallet_name, bilingual_str& error, std::vector<bilingual_str>& warnings) = 0; //! Return available wallets in wallet directory. virtual std::vector<std::string> listWalletDir() = 0; diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index 64775a5ddb..0154225e00 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -557,7 +557,7 @@ public: options.require_existing = true; return MakeWallet(m_context, LoadWallet(m_context, name, true /* load_on_start */, options, status, error, warnings)); } - std::unique_ptr<Wallet> restoreWallet(const std::string& backup_file, const std::string& wallet_name, bilingual_str& error, std::vector<bilingual_str>& warnings) override + std::unique_ptr<Wallet> restoreWallet(const fs::path& backup_file, const std::string& wallet_name, bilingual_str& error, std::vector<bilingual_str>& warnings) override { DatabaseStatus status; diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp index 0029d1b09c..b77d3af3c6 100644 --- a/src/wallet/rpc/backup.cpp +++ b/src/wallet/rpc/backup.cpp @@ -1887,7 +1887,7 @@ RPCHelpMan restorewallet() bilingual_str error; std::vector<bilingual_str> warnings; - const std::shared_ptr<CWallet> wallet = RestoreWallet(context, fs::PathToString(backup_file), wallet_name, load_on_start, status, error, warnings); + const std::shared_ptr<CWallet> wallet = RestoreWallet(context, backup_file, wallet_name, load_on_start, status, error, warnings); HandleWalletError(wallet, status, error); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 267710e8c7..4e40871073 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -357,12 +357,12 @@ std::shared_ptr<CWallet> CreateWallet(WalletContext& context, const std::string& return wallet; } -std::shared_ptr<CWallet> RestoreWallet(WalletContext& context, const std::string& backup_file, const std::string& wallet_name, std::optional<bool> load_on_start, DatabaseStatus& status, bilingual_str& error, std::vector<bilingual_str>& warnings) +std::shared_ptr<CWallet> RestoreWallet(WalletContext& context, const fs::path& backup_file, const std::string& wallet_name, std::optional<bool> load_on_start, DatabaseStatus& status, bilingual_str& error, std::vector<bilingual_str>& warnings) { DatabaseOptions options; options.require_existing = true; - if (!fs::exists(fs::u8path(backup_file))) { + if (!fs::exists(backup_file)) { error = Untranslated("Backup file does not exist"); status = DatabaseStatus::FAILED_INVALID_BACKUP_FILE; return nullptr; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index cbcdcaf3b8..c2abcfeea7 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -7,6 +7,7 @@ #define BITCOIN_WALLET_WALLET_H #include <consensus/amount.h> +#include <fs.h> #include <interfaces/chain.h> #include <interfaces/handler.h> #include <outputtype.h> @@ -60,7 +61,7 @@ std::vector<std::shared_ptr<CWallet>> GetWallets(WalletContext& context); std::shared_ptr<CWallet> GetWallet(WalletContext& context, const std::string& name); std::shared_ptr<CWallet> LoadWallet(WalletContext& context, const std::string& name, std::optional<bool> load_on_start, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error, std::vector<bilingual_str>& warnings); std::shared_ptr<CWallet> CreateWallet(WalletContext& context, const std::string& name, std::optional<bool> load_on_start, DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error, std::vector<bilingual_str>& warnings); -std::shared_ptr<CWallet> RestoreWallet(WalletContext& context, const std::string& backup_file, const std::string& wallet_name, std::optional<bool> load_on_start, DatabaseStatus& status, bilingual_str& error, std::vector<bilingual_str>& warnings); +std::shared_ptr<CWallet> RestoreWallet(WalletContext& context, const fs::path& backup_file, const std::string& wallet_name, std::optional<bool> load_on_start, DatabaseStatus& status, bilingual_str& error, std::vector<bilingual_str>& warnings); std::unique_ptr<interfaces::Handler> HandleLoadWallet(WalletContext& context, LoadWalletFn load_wallet); std::unique_ptr<WalletDatabase> MakeWalletDatabase(const std::string& name, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error); |