util: Check if specified config file cannot be opened#22622
Merged
maflcko merged 2 commits intobitcoin:masterfrom Aug 23, 2021
Merged
util: Check if specified config file cannot be opened#22622maflcko merged 2 commits intobitcoin:masterfrom
maflcko merged 2 commits intobitcoin:masterfrom
Conversation
n-thumann
force-pushed
the
check_conf_file
branch
from
e4a85aa to
127b460
Compare
Contributor
|
ACK 127b460 I agree that failing to read an explicitly specified config file should fail initialization. Tested that starting with a non-openable config file throws an error. |
n-thumann
changed the title
Contributor
|
Concept ACK |
Contributor
|
tACK 127b460 Tested on macOS v11.4 Patch works as described. I got the following error from testing both cases: config file doesn't exist and config file is inaccessible (due to permissions): $ bitcoind -datadir=/tmp/bitcoin -regtest -conf=/tmp/bitcoin/regtest/bitcoin.conf
Error: Error reading configuration file: specified config file "/tmp/bitcoin/regtest/bitcoin.conf" could not be opened. |
Contributor
|
Concept ACK |
NikhilBartwal
approved these changes
Aug 8, 2021
theStack
approved these changes
Aug 9, 2021
Contributor
theStack
left a comment
theStack
left a comment
There was a problem hiding this comment.
Tested ACK 127b460
Checked that starting src/bitcoind -conf=doesnt_exist leads to the newly introduced error message (running OpenBSD 6.9 here). Also tested that running the extended test in the second commit fails on master branch.
sidhujag
pushed a commit
to syscoin/syscoin
that referenced
this pull request
Aug 23, 2021
…pened 127b460 test: Check if specified config file cannot be opened (nthumann) 6bb5470 util: Check if specified config file cannot be opened (nthumann) Pull request description: Fixes bitcoin#22612. When running e.g. `./src/bitcoind -datadir=/tmp/bitcoin -regtest -conf=/tmp/bitcoin/regtest/bitcoin.conf` and the specified config cannot be opened (doesn't exist, permission denied, ...), the initialization silently uses the default config. As voidburn already noted: > I can't think of a situation in which a config file is specified explicitly (in the startup options, as per service unit linked above), but inaccessible, where the fail condition should be to keep booting using defaults instead. With this patch applied, the initialization will fail immediately, if the specified config file cannot be opened. If no config file is explicitly specified, the behavior is unchanged. This not only affects `bitcoind`, but also `bitcoin-cli` and `bitcoin-qt`. In the example below the datadir is accessible, but the config file is not due to insufficient permissions: ``` $ ./src/bitcoind -datadir=/tmp/bitcoin -regtest --debug=1 -conf=/tmp/bitcoin/regtest/bitcoin.conf Error: Error reading configuration file: specified config file "/tmp/bitcoin/regtest/bitcoin.conf" could not be opened. ``` ACKs for top commit: 0xB10C: ACK 127b460 Zero-1729: tACK 127b460 theStack: Tested ACK 127b460 Tree-SHA512: 4fe487921485426f1d1da8d256c388af517b984b639d776aec7b159b3e23b669824093d3bdd31139d9415ed5f5de405b3e6a51b110c8ab471f12b9c99ac67cc1
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #22612.
When running e.g.
./src/bitcoind -datadir=/tmp/bitcoin -regtest -conf=/tmp/bitcoin/regtest/bitcoin.confand the specified config cannot be opened (doesn't exist, permission denied, ...), the initialization silently uses the default config.As voidburn already noted:
With this patch applied, the initialization will fail immediately, if the specified config file cannot be opened. If no config file is explicitly specified, the behavior is unchanged. This not only affects
bitcoind, but alsobitcoin-cliandbitcoin-qt.In the example below the datadir is accessible, but the config file is not due to insufficient permissions: