util: Report parse errors in configuration file

laanwj commented at 11:07 am on August 30, 2018: member

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 added the label Utils/log/libs on Aug 30, 2018

laanwj added the label Needs backport on Aug 30, 2018

laanwj added this to the milestone 0.17.0 on Aug 30, 2018

in src/util.cpp:839 in e4924aeb14 outdated

829@@ -830,17 +830,27 @@ static std::vector<std::pair<std::string, std::string>> GetConfigOptions(std::is
830                 std::string name = prefix + TrimString(str.substr(0, pos), pattern);
831                 std::string value = TrimString(str.substr(pos + 1), pattern);
832                 options.emplace_back(name, value);
833+            } else {
834+                error = strprintf("parse error on line %i: %s", linenr, str);
835+                if (str.size() >= 2 && str.substr(0, 2) == "no") {
836+                    error += strprintf(", if you intended to specify a negated option, use %s=1 instead", str);

laanwj commented at 11:08 am on August 30, 2018:

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 commented at 11:32 am on August 30, 2018: contributor

Concept ACK

Nice usability improvement!

util: Report parse errors in configuration file

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 -

a66c0f78a9

laanwj force-pushed on Aug 30, 2018

laanwj commented at 12:43 pm on August 30, 2018: member

updated to also check for leading - in config file

fanquake added this to the "For backport" column in a project

fanquake removed this from the "For backport" column in a project

ken2812221 commented at 2:28 pm on September 4, 2018: contributor

utACK a66c0f78a941968340f030911765a84219908c4d

MarcoFalke commented at 6:32 pm on September 5, 2018: member

Could add some tests, otherwise looks good:

 0diff --git a/test/functional/feature_config_args.py b/test/functional/feature_config_args.py
 1index 62091048f9..9be59b32b4 100755
 2--- a/test/functional/feature_config_args.py
 3+++ b/test/functional/feature_config_args.py
 4@@ -14,8 +14,29 @@ class ConfArgsTest(BitcoinTestFramework):
 5         self.setup_clean_chain = True
 6         self.num_nodes = 1
 7 
 8+    def test_config_file_parser(self):
 9+        # Assume node is stopped
10+
11+        inc_conf_file_path = os.path.join(self.nodes[0].datadir, 'include.conf')
12+        with open(os.path.join(self.nodes[0].datadir, 'bitcoin.conf'), 'a', encoding='utf-8') as conf:
13+            conf.write('includeconf={}\n'.format(inc_conf_file_path))
14+
15+        with open(inc_conf_file_path, 'w', encoding='utf-8') as conf:
16+            conf.write('-dash=1\n')
17+        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 -')
18+
19+        with open(inc_conf_file_path, 'w', encoding='utf-8') as conf:
20+            conf.write('nono\n')
21+        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')
22+
23+        with open(inc_conf_file_path, 'w', encoding='utf-8') as conf:
24+            conf.write('')  # clear
25+
26     def run_test(self):
27         self.stop_node(0)
28+
29+        self.test_config_file_parser()
30+
31         # Remove the -datadir argument so it doesn't override the config file
32         self.nodes[0].args = [arg for arg in self.nodes[0].args if not arg.startswith("-datadir")]
33

test: Add test for config file parsing errors ed2332aeff

laanwj commented at 9:30 am on September 6, 2018: member

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

laanwj merged this on Sep 6, 2018

laanwj closed this on Sep 6, 2018

laanwj referenced this in commit a6aca8dc2f on Sep 6, 2018

laanwj referenced this in commit 83aafd5b32 on Sep 6, 2018

laanwj referenced this in commit eb202ea21d on Sep 6, 2018

MarcoFalke commented at 2:30 pm on September 6, 2018: member

Backported in eb202ea21de1fbcc32fb3db63fcce4135b7a2849 and 83aafd5

MarcoFalke removed the label Needs backport on Sep 6, 2018

HashUnlimited referenced this in commit d81297d9ea on Sep 14, 2018

HashUnlimited referenced this in commit 319e9513f0 on Sep 14, 2018

uhliksk referenced this in commit 642b4b03c0 on Oct 18, 2018

uhliksk referenced this in commit 4d7b1ec2ea on Oct 18, 2018

PastaPastaPasta referenced this in commit 6575bcdf9e on Jul 19, 2021

PastaPastaPasta referenced this in commit 23c024eca9 on Jul 19, 2021

PastaPastaPasta referenced this in commit 5b0f4b4265 on Jul 19, 2021

PastaPastaPasta referenced this in commit a1a1a0766b on Jul 19, 2021

MarcoFalke locked this on Sep 8, 2021

util: Report parse errors in configuration file #14105