highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-11-11 06:28:31 +01:00
Code Issues Packages Projects Releases Wiki Activity

Merge #19502: Bugfix: Wallet: Soft-fail exceptions within ListWalletDir file checks

Browse Source 
24d2d3341d QA: wallet_multiwallet: Check that recursive symlink directory and wallet.dat loops are ignored (Luke Dashjr)
69f59af54d Bugfix: Wallet: Soft-fail exceptions within ListWalletDir file checks (Luke Dashjr)

Pull request description:

  Previously, an exception would be thrown, which could kill the node in some circumstances.

  Includes test changes to cause failure.

  Review with `?w=1`

ACKs for top commit:
  hebasto:
    re-ACK 24d2d3341d, rebased only since my [previous](https://github.com/bitcoin/bitcoin/pull/19502#pullrequestreview-520552944) review.
  promag:
    Tested ACK 24d2d3341d, test change fails on master.
  meshcollider:
    utACK 24d2d3341d

Tree-SHA512: f701f81b3aa3d3e15cee52ac9e7c31a73c0d8166e56bf077235294507cbcee099829fedc432a1c4b6d8780885f4e37897b44b980b08125771de3c849c000499e
This commit is contained in:
Samuel Dobson
2020-11-12 12:51:17 +13:00
parent d9f5132736 24d2d3341d
commit c2d8ba6265
2 changed files with 38 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
src/wallet/walletutil.cpp
View File

@@ -49,6 +49,7 @@ std::vector<fs::path> ListWalletDir()
continue; continue;
} }
try {
// Get wallet path relative to walletdir by removing walletdir from the wallet path. // Get wallet path relative to walletdir by removing walletdir from the wallet path.
// This can be replaced by boost::filesystem::lexically_relative once boost is bumped to 1.60. // This can be replaced by boost::filesystem::lexically_relative once boost is bumped to 1.60.
const fs::path path = it->path().string().substr(offset); const fs::path path = it->path().string().substr(offset);
@@ -70,6 +71,10 @@ std::vector<fs::path> ListWalletDir()
paths.emplace_back(path); paths.emplace_back(path);
} }
} }
} catch (const std::exception& e) {
LogPrintf("%s: Error scanning %s: %s\n", __func__, it->path().string(), e.what());
it.no_push();
}
} }
return paths; return paths;

17
test/functional/wallet_multiwallet.py
View File

@@ -10,6 +10,7 @@ from decimal import Decimal
from threading import Thread from threading import Thread
import os import os
import shutil import shutil
import stat
import time import time
from test_framework.authproxy import JSONRPCException from test_framework.authproxy import JSONRPCException
@@ -78,6 +79,11 @@ class MultiWalletTest(BitcoinTestFramework):
os.mkdir(wallet_dir('w7')) os.mkdir(wallet_dir('w7'))
os.symlink('w7', wallet_dir('w7_symlink')) os.symlink('w7', wallet_dir('w7_symlink'))
os.symlink('..', wallet_dir('recursive_dir_symlink'))
os.mkdir(wallet_dir('self_walletdat_symlink'))
os.symlink('wallet.dat', wallet_dir('self_walletdat_symlink/wallet.dat'))
# rename wallet.dat to make sure plain wallet file paths (as opposed to # rename wallet.dat to make sure plain wallet file paths (as opposed to
# directory paths) can be loaded # directory paths) can be loaded
# create another dummy wallet for use in testing backups later # create another dummy wallet for use in testing backups later
@@ -117,7 +123,16 @@ class MultiWalletTest(BitcoinTestFramework):
self.nodes[0].createwallet(wallet_name) self.nodes[0].createwallet(wallet_name)
for wallet_name in to_load: for wallet_name in to_load:
self.nodes[0].loadwallet(wallet_name) self.nodes[0].loadwallet(wallet_name)
assert_equal(sorted(map(lambda w: w['name'], self.nodes[0].listwalletdir()['wallets'])), sorted(in_wallet_dir))
os.mkdir(wallet_dir('no_access'))
os.chmod(wallet_dir('no_access'), 0)
try:
with self.nodes[0].assert_debug_log(expected_msgs=['Too many levels of symbolic links', 'Error scanning']):
walletlist = self.nodes[0].listwalletdir()['wallets']
finally:
# Need to ensure access is restored for cleanup
os.chmod(wallet_dir('no_access'), stat.S_IRUSR | stat.S_IWUSR | stat.S_IXUSR)
assert_equal(sorted(map(lambda w: w['name'], walletlist)), sorted(in_wallet_dir))
assert_equal(set(node.listwallets()), set(wallet_names)) assert_equal(set(node.listwallets()), set(wallet_names))