diff --git a/qa/rpc-tests/walletbackup.py b/qa/rpc-tests/walletbackup.py index e71065781..5f3eb6fcc 100755 --- a/qa/rpc-tests/walletbackup.py +++ b/qa/rpc-tests/walletbackup.py @@ -47,8 +47,13 @@ class WalletBackupTest(BitcoinTestFramework): # This mirrors how the network was setup in the bash test def setup_network(self, split=False): + # -exportdir option means we must provide a valid path to the destination folder for wallet backups + ed0 = "-exportdir=" + self.options.tmpdir + "/node0" + ed1 = "-exportdir=" + self.options.tmpdir + "/node1" + ed2 = "-exportdir=" + self.options.tmpdir + "/node2" + # nodes 1, 2,3 are spenders, let's give them a keypool=100 - extra_args = [["-keypool=100"], ["-keypool=100"], ["-keypool=100"], []] + extra_args = [["-keypool=100", ed0], ["-keypool=100", ed1], ["-keypool=100", ed2], []] self.nodes = start_nodes(4, self.options.tmpdir, extra_args) connect_nodes(self.nodes[0], 3) connect_nodes(self.nodes[1], 3) @@ -122,12 +127,12 @@ class WalletBackupTest(BitcoinTestFramework): logging.info("Backing up") tmpdir = self.options.tmpdir - self.nodes[0].backupwallet(tmpdir + "/node0/wallet.bak") - self.nodes[0].dumpwallet(tmpdir + "/node0/wallet.dump") - self.nodes[1].backupwallet(tmpdir + "/node1/wallet.bak") - self.nodes[1].dumpwallet(tmpdir + "/node1/wallet.dump") - self.nodes[2].backupwallet(tmpdir + "/node2/wallet.bak") - self.nodes[2].dumpwallet(tmpdir + "/node2/wallet.dump") + self.nodes[0].backupwallet("walletbak") + self.nodes[0].dumpwallet("walletdump") + self.nodes[1].backupwallet("walletbak") + self.nodes[1].dumpwallet("walletdump") + self.nodes[2].backupwallet("walletbak") + self.nodes[2].dumpwallet("walletdump") logging.info("More transactions") for i in range(5): @@ -159,9 +164,9 @@ class WalletBackupTest(BitcoinTestFramework): shutil.rmtree(self.options.tmpdir + "/node2/regtest/chainstate") # Restore wallets from backup - shutil.copyfile(tmpdir + "/node0/wallet.bak", tmpdir + "/node0/regtest/wallet.dat") - shutil.copyfile(tmpdir + "/node1/wallet.bak", tmpdir + "/node1/regtest/wallet.dat") - shutil.copyfile(tmpdir + "/node2/wallet.bak", tmpdir + "/node2/regtest/wallet.dat") + shutil.copyfile(tmpdir + "/node0/walletbak", tmpdir + "/node0/regtest/wallet.dat") + shutil.copyfile(tmpdir + "/node1/walletbak", tmpdir + "/node1/regtest/wallet.dat") + shutil.copyfile(tmpdir + "/node2/walletbak", tmpdir + "/node2/regtest/wallet.dat") logging.info("Re-starting nodes") self.start_three() @@ -185,9 +190,9 @@ class WalletBackupTest(BitcoinTestFramework): assert_equal(self.nodes[1].getbalance(), 0) assert_equal(self.nodes[2].getbalance(), 0) - self.nodes[0].importwallet(tmpdir + "/node0/wallet.dump") - self.nodes[1].importwallet(tmpdir + "/node1/wallet.dump") - self.nodes[2].importwallet(tmpdir + "/node2/wallet.dump") + self.nodes[0].importwallet(tmpdir + "/node0/walletdump") + self.nodes[1].importwallet(tmpdir + "/node1/walletdump") + self.nodes[2].importwallet(tmpdir + "/node2/walletdump") sync_blocks(self.nodes) diff --git a/src/init.cpp b/src/init.cpp index a048b9def..3521801f9 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -289,6 +289,7 @@ std::string HelpMessage(HelpMessageMode mode) #endif } strUsage += HelpMessageOpt("-datadir=", _("Specify data directory")); + strUsage += HelpMessageOpt("-exportdir=", _("Specify directory to be used when exporting data")); strUsage += HelpMessageOpt("-dbcache=", strprintf(_("Set database cache size in megabytes (%d to %d, default: %d)"), nMinDbCache, nMaxDbCache, nDefaultDbCache)); strUsage += HelpMessageOpt("-loadblock=", _("Imports blocks from external blk000??.dat file") + " " + _("on startup")); strUsage += HelpMessageOpt("-maxorphantx=", strprintf(_("Keep at most unconnectable transactions in memory (default: %u)"), DEFAULT_MAX_ORPHAN_TRANSACTIONS)); diff --git a/src/test/rpc_wallet_tests.cpp b/src/test/rpc_wallet_tests.cpp index 8471ebebc..d0ee27aea 100644 --- a/src/test/rpc_wallet_tests.cpp +++ b/src/test/rpc_wallet_tests.cpp @@ -358,16 +358,26 @@ BOOST_AUTO_TEST_CASE(rpc_wallet_z_exportwallet) pwalletMain->GetPaymentAddresses(addrs); BOOST_CHECK(addrs.size()==1); + // Set up paths + boost::filesystem::path tmppath = boost::filesystem::temp_directory_path(); + boost::filesystem::path tmpfilename = boost::filesystem::unique_path("%%%%%%%%"); + boost::filesystem::path exportfilepath = tmppath / tmpfilename; + + // export will fail since exportdir is not set + BOOST_CHECK_THROW(CallRPC(string("z_exportwallet ") + tmpfilename.string()), runtime_error); + + // set exportdir + mapArgs["-exportdir"] = tmppath.native(); + + // run some tests BOOST_CHECK_THROW(CallRPC("z_exportwallet"), runtime_error); BOOST_CHECK_THROW(CallRPC("z_exportwallet toomany args"), runtime_error); - - boost::filesystem::path temp = boost::filesystem::temp_directory_path() / - boost::filesystem::unique_path(); - const std::string path = temp.native(); + BOOST_CHECK_THROW(CallRPC(string("z_exportwallet invalid!*/_chars.txt")), runtime_error); + + BOOST_CHECK_NO_THROW(CallRPC(string("z_exportwallet ") + tmpfilename.string())); - BOOST_CHECK_NO_THROW(CallRPC(string("z_exportwallet ") + path)); auto addr = paymentAddress.Get(); libzcash::SpendingKey key; @@ -382,7 +392,7 @@ BOOST_AUTO_TEST_CASE(rpc_wallet_z_exportwallet) EnsureWalletIsUnlocked(); ifstream file; - file.open(path.c_str(), std::ios::in | std::ios::ate); + file.open(exportfilepath.string().c_str(), std::ios::in | std::ios::ate); BOOST_CHECK(file.is_open()); bool fVerified = false; int64_t nFilesize = std::max((int64_t)1, (int64_t)file.tellg()); diff --git a/src/util.cpp b/src/util.cpp index 8fbdfc65b..01e9562ac 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -484,6 +484,26 @@ const boost::filesystem::path &ZC_GetParamsDir() return path; } +// Return the user specified export directory. Create directory if it doesn't exist. +// If user did not set option, return an empty path. +// If there is a filesystem problem, throw an exception. +const boost::filesystem::path GetExportDir() +{ + namespace fs = boost::filesystem; + fs::path path; + if (mapArgs.count("-exportdir")) { + path = fs::system_complete(mapArgs["-exportdir"]); + if (fs::exists(path) && !fs::is_directory(path)) { + throw std::runtime_error(strprintf("The -exportdir '%s' already exists and is not a directory", path.string())); + } + if (!fs::exists(path) && !fs::create_directories(path)) { + throw std::runtime_error(strprintf("Failed to create directory at -exportdir '%s'", path.string())); + } + } + return path; +} + + const boost::filesystem::path &GetDataDir(bool fNetSpecific) { namespace fs = boost::filesystem; diff --git a/src/util.h b/src/util.h index b7d255e4d..5ca64c4bb 100644 --- a/src/util.h +++ b/src/util.h @@ -138,6 +138,7 @@ boost::filesystem::path GetSpecialFolderPath(int nFolder, bool fCreate = true); boost::filesystem::path GetTempPath(); void ShrinkDebugFile(); void runCommand(std::string strCommand); +const boost::filesystem::path GetExportDir(); inline bool IsSwitchChar(char c) { diff --git a/src/utilstrencodings.cpp b/src/utilstrencodings.cpp index c15bddc6f..646536f1e 100644 --- a/src/utilstrencodings.cpp +++ b/src/utilstrencodings.cpp @@ -30,6 +30,22 @@ string SanitizeString(const string& str) return strResult; } +string SanitizeFilename(const string& str) +{ + /** + * safeChars chosen to restrict filename, keeping it simple to avoid cross-platform issues. + * http://stackoverflow.com/a/2306003 + */ + static string safeChars("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ01234567890"); + string strResult; + for (std::string::size_type i = 0; i < str.size(); i++) + { + if (safeChars.find(str[i]) != std::string::npos) + strResult.push_back(str[i]); + } + return strResult; +} + const signed char p_util_hexdigit[256] = { -1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1, -1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1,-1, diff --git a/src/utilstrencodings.h b/src/utilstrencodings.h index b0edd8b54..e08de362b 100644 --- a/src/utilstrencodings.h +++ b/src/utilstrencodings.h @@ -22,6 +22,7 @@ /** This is needed because the foreach macro can't get over the comma in pair */ #define PAIRTYPE(t1, t2) std::pair +std::string SanitizeFilename(const std::string& str); std::string SanitizeString(const std::string& str); std::vector ParseHex(const char* psz); std::vector ParseHex(const std::string& str); diff --git a/src/wallet/rpcdump.cpp b/src/wallet/rpcdump.cpp index 789bd4537..07d81bce5 100644 --- a/src/wallet/rpcdump.cpp +++ b/src/wallet/rpcdump.cpp @@ -430,7 +430,9 @@ Value z_exportwallet(const Array& params, bool fHelp) "z_exportwallet \"filename\"\n" "\nExports all wallet keys, for taddr and zaddr, in a human-readable format.\n" "\nArguments:\n" - "1. \"filename\" (string, required) The filename\n" + "1. \"filename\" (string, required) The filename, saved in folder set by zcashd -exportdir option\n" + "\nResult:\n" + "\"path\" (string) The full path of the destination file\n" "\nExamples:\n" + HelpExampleCli("z_exportwallet", "\"test\"") + HelpExampleRpc("z_exportwallet", "\"test\"") @@ -449,7 +451,9 @@ Value dumpwallet(const Array& params, bool fHelp) "dumpwallet \"filename\"\n" "\nDumps taddr wallet keys in a human-readable format.\n" "\nArguments:\n" - "1. \"filename\" (string, required) The filename\n" + "1. \"filename\" (string, required) The filename, saved in folder set by zcashd -exportdir option\n" + "\nResult:\n" + "\"path\" (string) The full path of the destination file\n" "\nExamples:\n" + HelpExampleCli("dumpwallet", "\"test\"") + HelpExampleRpc("dumpwallet", "\"test\"") @@ -464,8 +468,24 @@ Value dumpwallet_impl(const Array& params, bool fHelp, bool fDumpZKeys) EnsureWalletIsUnlocked(); + boost::filesystem::path exportdir; + try { + exportdir = GetExportDir(); + } catch (const std::runtime_error& e) { + throw JSONRPCError(RPC_INTERNAL_ERROR, e.what()); + } + if (exportdir.empty()) { + throw JSONRPCError(RPC_WALLET_ERROR, "Cannot export wallet until the zcashd -exportdir option has been set"); + } + std::string unclean = params[0].get_str(); + std::string clean = SanitizeFilename(unclean); + if (clean.compare(unclean) != 0) { + throw JSONRPCError(RPC_WALLET_ERROR, strprintf("Filename is invalid as only alphanumeric characters are allowed. Try '%s' instead.", clean)); + } + boost::filesystem::path exportfilepath = exportdir / clean; + ofstream file; - file.open(params[0].get_str().c_str()); + file.open(exportfilepath.string().c_str()); if (!file.is_open()) throw JSONRPCError(RPC_INVALID_PARAMETER, "Cannot open wallet dump file"); @@ -523,7 +543,8 @@ Value dumpwallet_impl(const Array& params, bool fHelp, bool fDumpZKeys) file << "# End of dump\n"; file.close(); - return Value::null; + + return exportfilepath.string(); } diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp index 8a4ff1c1c..4b7120b37 100644 --- a/src/wallet/rpcwallet.cpp +++ b/src/wallet/rpcwallet.cpp @@ -1769,21 +1769,38 @@ Value backupwallet(const Array& params, bool fHelp) if (fHelp || params.size() != 1) throw runtime_error( "backupwallet \"destination\"\n" - "\nSafely copies wallet.dat to destination, which can be a directory or a path with filename.\n" + "\nSafely copies wallet.dat to destination filename\n" "\nArguments:\n" - "1. \"destination\" (string) The destination directory or file\n" + "1. \"destination\" (string, required) The destination filename, saved in the directory set by -exportdir option.\n" + "\nResult:\n" + "\"path\" (string) The full path of the destination file\n" "\nExamples:\n" - + HelpExampleCli("backupwallet", "\"backup.dat\"") - + HelpExampleRpc("backupwallet", "\"backup.dat\"") + + HelpExampleCli("backupwallet", "\"backupdata\"") + + HelpExampleRpc("backupwallet", "\"backupdata\"") ); LOCK2(cs_main, pwalletMain->cs_wallet); - string strDest = params[0].get_str(); - if (!BackupWallet(*pwalletMain, strDest)) + boost::filesystem::path exportdir; + try { + exportdir = GetExportDir(); + } catch (const std::runtime_error& e) { + throw JSONRPCError(RPC_INTERNAL_ERROR, e.what()); + } + if (exportdir.empty()) { + throw JSONRPCError(RPC_WALLET_ERROR, "Cannot backup wallet until the -exportdir option has been set"); + } + std::string unclean = params[0].get_str(); + std::string clean = SanitizeFilename(unclean); + if (clean.compare(unclean) != 0) { + throw JSONRPCError(RPC_WALLET_ERROR, strprintf("Filename is invalid as only alphanumeric characters are allowed. Try '%s' instead.", clean)); + } + boost::filesystem::path exportfilepath = exportdir / clean; + + if (!BackupWallet(*pwalletMain, exportfilepath.string())) throw JSONRPCError(RPC_WALLET_ERROR, "Error: Wallet backup failed!"); - return Value::null; + return exportfilepath.string(); }