util: Report parse errors in configuration file by laanwj · Pull Request #14105 · bitcoin/bitcoin

laanwj · 2018-08-30T11:07:40Z

Report errors while parsing the configuration file, instead of silently ignoring them.

$ src/bitcoind -regtest
Error reading configuration file: parse error on line 22: nodebuglogfile, if you intended to specify a negated option, use nodebuglogfile=1 instead
$ src/bitcoind -regtest
Error reading configuration file: parse error on line 22: sdafsdfafs
$ src/bitcoind -regtest
Error reading configuration file: parse error on line 24: -nodebuglogfile=1, options in the configuration file must be specified without leading -

(inspired by #14100 (comment))

laanwj · 2018-08-30T11:08:50Z

src/util.cpp

we could also change this logic here to interpret it as noX=1, but I'm not sure what kind of syntax we want to support (bare "statements" are not ini-like) so I've left it as a suggestion

practicalswift · 2018-08-30T11:32:51Z

Concept ACK

Nice usability improvement!

Report errors while parsing the configuration file, instead of silently ignoring them. $ src/bitcoind -regtest Error reading configuration file: parse error on line 22: nodebuglogfile, if you intended to specify a negated option, use nodebuglogfile=1 instead $ src/bitcoind -regtest Error reading configuration file: parse error on line 22: sdafsdfafs $ src/bitcoind -regtest Error reading configuration file: parse error on line 24: -nodebuglogfile=1, options in the configuration file must be specified without leading -

laanwj · 2018-08-30T12:43:48Z

updated to also check for leading - in config file

ken2812221 · 2018-09-04T14:28:12Z

utACK a66c0f7

maflcko · 2018-09-05T18:32:52Z

Could add some tests, otherwise looks good:

diff --git a/test/functional/feature_config_args.py b/test/functional/feature_config_args.py
index 62091048f9..9be59b32b4 100755
--- a/test/functional/feature_config_args.py
+++ b/test/functional/feature_config_args.py
@@ -14,8 +14,29 @@ class ConfArgsTest(BitcoinTestFramework):
         self.setup_clean_chain = True
         self.num_nodes = 1
 
+    def test_config_file_parser(self):
+        # Assume node is stopped
+
+        inc_conf_file_path = os.path.join(self.nodes[0].datadir, 'include.conf')
+        with open(os.path.join(self.nodes[0].datadir, 'bitcoin.conf'), 'a', encoding='utf-8') as conf:
+            conf.write('includeconf={}\n'.format(inc_conf_file_path))
+
+        with open(inc_conf_file_path, 'w', encoding='utf-8') as conf:
+            conf.write('-dash=1\n')
+        self.nodes[0].assert_start_raises_init_error(expected_msg='Error reading configuration file: parse error on line 1: -dash=1, options in configuration file must be specified without leading -')
+
+        with open(inc_conf_file_path, 'w', encoding='utf-8') as conf:
+            conf.write('nono\n')
+        self.nodes[0].assert_start_raises_init_error(expected_msg='Error reading configuration file: parse error on line 1: nono, if you intended to specify a negated option, use nono=1 instead')
+
+        with open(inc_conf_file_path, 'w', encoding='utf-8') as conf:
+            conf.write('')  # clear
+
     def run_test(self):
         self.stop_node(0)
+
+        self.test_config_file_parser()
+
         # Remove the -datadir argument so it doesn't override the config file
         self.nodes[0].args = [arg for arg in self.nodes[0].args if not arg.startswith("-datadir")]

laanwj · 2018-09-06T09:30:57Z

@MarcoFalke thanks, that's a neat test, added commit

ed2332a test: Add test for config file parsing errors (MarcoFalke) a66c0f7 util: Report parse errors in configuration file (Wladimir J. van der Laan) Pull request description: Report errors while parsing the configuration file, instead of silently ignoring them. $ src/bitcoind -regtest Error reading configuration file: parse error on line 22: nodebuglogfile, if you intended to specify a negated option, use nodebuglogfile=1 instead $ src/bitcoind -regtest Error reading configuration file: parse error on line 22: sdafsdfafs $ src/bitcoind -regtest Error reading configuration file: parse error on line 24: -nodebuglogfile=1, options in the configuration file must be specified without leading - (inspired by #14100 (comment)) Tree-SHA512: d516342b65db2969edf200390994bbbda23654c648f85dcc99f9f2d217d3d59a72e0f58227be7b4746529dcfa54ba26d8188ba9f14a57c9ab00015d7283fade2

Report errors while parsing the configuration file, instead of silently ignoring them. $ src/bitcoind -regtest Error reading configuration file: parse error on line 22: nodebuglogfile, if you intended to specify a negated option, use nodebuglogfile=1 instead $ src/bitcoind -regtest Error reading configuration file: parse error on line 22: sdafsdfafs $ src/bitcoind -regtest Error reading configuration file: parse error on line 24: -nodebuglogfile=1, options in the configuration file must be specified without leading - Github-Pull: #14105 Rebased-From: a66c0f7 Tree-SHA512: 2b6be1ab643623e6ef9b53354820147a6c5d2baae3795ffe428fc60d8563ec00a68a379aee4029380f80f892abe23763afb1c75c32b60a13bffe7b82496bf2bb

Github-Pull: #14105 Rebased-From: ed2332a Tree-SHA512: 17fa88a2848f1c9c9c8a127b5ea4c45761ce8e06a609dd40f8e90bb9117d88c9d2c81e752c9c0f1a44ecadbb5bedd2973bc4548da2a6d463c789797191e85ab1

maflcko · 2018-09-06T14:30:53Z

Backported in eb202ea and 83aafd5

Report errors while parsing the configuration file, instead of silently ignoring them. $ src/bitcoind -regtest Error reading configuration file: parse error on line 22: nodebuglogfile, if you intended to specify a negated option, use nodebuglogfile=1 instead $ src/bitcoind -regtest Error reading configuration file: parse error on line 22: sdafsdfafs $ src/bitcoind -regtest Error reading configuration file: parse error on line 24: -nodebuglogfile=1, options in the configuration file must be specified without leading - Github-Pull: bitcoin#14105 Rebased-From: a66c0f7 Tree-SHA512: 2b6be1ab643623e6ef9b53354820147a6c5d2baae3795ffe428fc60d8563ec00a68a379aee4029380f80f892abe23763afb1c75c32b60a13bffe7b82496bf2bb

Github-Pull: bitcoin#14105 Rebased-From: ed2332a Tree-SHA512: 17fa88a2848f1c9c9c8a127b5ea4c45761ce8e06a609dd40f8e90bb9117d88c9d2c81e752c9c0f1a44ecadbb5bedd2973bc4548da2a6d463c789797191e85ab1

Report errors while parsing the configuration file, instead of silently ignoring them. $ src/bitcoind -regtest Error reading configuration file: parse error on line 22: nodebuglogfile, if you intended to specify a negated option, use nodebuglogfile=1 instead $ src/bitcoind -regtest Error reading configuration file: parse error on line 22: sdafsdfafs $ src/bitcoind -regtest Error reading configuration file: parse error on line 24: -nodebuglogfile=1, options in the configuration file must be specified without leading - Github-Pull: bitcoin#14105 Rebased-From: a66c0f7 Tree-SHA512: 2b6be1ab643623e6ef9b53354820147a6c5d2baae3795ffe428fc60d8563ec00a68a379aee4029380f80f892abe23763afb1c75c32b60a13bffe7b82496bf2bb

Github-Pull: bitcoin#14105 Rebased-From: ed2332a Tree-SHA512: 17fa88a2848f1c9c9c8a127b5ea4c45761ce8e06a609dd40f8e90bb9117d88c9d2c81e752c9c0f1a44ecadbb5bedd2973bc4548da2a6d463c789797191e85ab1

ed2332a test: Add test for config file parsing errors (MarcoFalke) a66c0f7 util: Report parse errors in configuration file (Wladimir J. van der Laan) Pull request description: Report errors while parsing the configuration file, instead of silently ignoring them. $ src/bitcoind -regtest Error reading configuration file: parse error on line 22: nodebuglogfile, if you intended to specify a negated option, use nodebuglogfile=1 instead $ src/bitcoind -regtest Error reading configuration file: parse error on line 22: sdafsdfafs $ src/bitcoind -regtest Error reading configuration file: parse error on line 24: -nodebuglogfile=1, options in the configuration file must be specified without leading - (inspired by bitcoin#14100 (comment)) Tree-SHA512: d516342b65db2969edf200390994bbbda23654c648f85dcc99f9f2d217d3d59a72e0f58227be7b4746529dcfa54ba26d8188ba9f14a57c9ab00015d7283fade2

laanwj added Utils/log/libs Needs backport labels Aug 30, 2018

laanwj added this to the 0.17.0 milestone Aug 30, 2018

laanwj commented Aug 30, 2018

View reviewed changes

laanwj force-pushed the 2018_08_parse_error_reporting branch from e4924ae to a66c0f7 Compare August 30, 2018 12:43

laanwj mentioned this pull request Aug 31, 2018

doc: Change documentation for =0 for non-boolean options #14100

Merged

test: Add test for config file parsing errors

ed2332a

laanwj merged commit ed2332a into bitcoin:master Sep 6, 2018

maflcko removed the Needs backport label Sep 6, 2018

HashUnlimited mentioned this pull request Sep 14, 2018

util: Report parse errors in configuration file chaincoin/chaincoin#668

Merged

HashUnlimited mentioned this pull request Sep 14, 2018

test: Add test for config file parsing errors chaincoin/chaincoin#669

Merged

Bushstar mentioned this pull request Oct 11, 2018

Updates from bitcoin/master FeatherCoin/Feathercoin#366

Merged

bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

util: Report parse errors in configuration file#14105

util: Report parse errors in configuration file#14105
laanwj merged 2 commits intobitcoin:masterfrom
laanwj:2018_08_parse_error_reporting

laanwj commented Aug 30, 2018 •

edited

Loading

Uh oh!

laanwj Aug 30, 2018 •

edited

Loading

Uh oh!

practicalswift commented Aug 30, 2018

Uh oh!

laanwj commented Aug 30, 2018 •

edited

Loading

Uh oh!

ken2812221 commented Sep 4, 2018

Uh oh!

maflcko commented Sep 5, 2018

Uh oh!

laanwj commented Sep 6, 2018

Uh oh!

maflcko commented Sep 6, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

laanwj commented Aug 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

laanwj Aug 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

practicalswift commented Aug 30, 2018

Uh oh!

laanwj commented Aug 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ken2812221 commented Sep 4, 2018

Uh oh!

maflcko commented Sep 5, 2018

Uh oh!

laanwj commented Sep 6, 2018

Uh oh!

maflcko commented Sep 6, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

laanwj commented Aug 30, 2018 •

edited

Loading

laanwj Aug 30, 2018 •

edited

Loading

laanwj commented Aug 30, 2018 •

edited

Loading