Merge #14105: util: Report parse errors in configuration file

ed2332aeffb071a3404be9cff8f9fb8a81a9fbfb test: Add test for config file parsing errors (MarcoFalke)
a66c0f78a941968340f030911765a84219908c4d 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 https://github.com/bitcoin/bitcoin/pull/14100#issuecomment-417264823)

Tree-SHA512: d516342b65db2969edf200390994bbbda23654c648f85dcc99f9f2d217d3d59a72e0f58227be7b4746529dcfa54ba26d8188ba9f14a57c9ab00015d7283fade2

2 files changed, 39 insertions, 5 deletions

diff --git a/src/util.cpp b/src/util.cpp
index 3bb52e9b3d..84d8175389 100644
--- a/src/util.cpp
+++ b/src/util.cpp
@@ -818,11 +818,11 @@ static std::string TrimString(const std::string& str, const std::string& pattern
     return str.substr(front, end - front + 1);
 }
 
-static std::vector<std::pair<std::string, std::string>> GetConfigOptions(std::istream& stream)
+static bool GetConfigOptions(std::istream& stream, std::string& error, std::vector<std::pair<std::string, std::string>> &options)
 {
-    std::vector<std::pair<std::string, std::string>> options;
     std::string str, prefix;
     std::string::size_type pos;
+    int linenr = 1;
     while (std::getline(stream, str)) {
         if ((pos = str.find('#')) != std::string::npos) {
             str = str.substr(0, pos);
@@ -832,21 +832,34 @@ static std::vector<std::pair<std::string, std::string>> GetConfigOptions(std::is
         if (!str.empty()) {
             if (*str.begin() == '[' && *str.rbegin() == ']') {
                 prefix = str.substr(1, str.size() - 2) + '.';
+            } else if (*str.begin() == '-') {
+                error = strprintf("parse error on line %i: %s, options in configuration file must be specified without leading -", linenr, str);
+                return false;
             } else if ((pos = str.find('=')) != std::string::npos) {
                 std::string name = prefix + TrimString(str.substr(0, pos), pattern);
                 std::string value = TrimString(str.substr(pos + 1), pattern);
                 options.emplace_back(name, value);
+            } else {
+                error = strprintf("parse error on line %i: %s", linenr, str);
+                if (str.size() >= 2 && str.substr(0, 2) == "no") {
+                    error += strprintf(", if you intended to specify a negated option, use %s=1 instead", str);
+                }
+                return false;
             }
         }
+        ++linenr;
     }
-    return options;
+    return true;
 }
 
 bool ArgsManager::ReadConfigStream(std::istream& stream, std::string& error, bool ignore_invalid_keys)
 {
     LOCK(cs_args);
-
-    for (const std::pair<std::string, std::string>& option : GetConfigOptions(stream)) {
+    std::vector<std::pair<std::string, std::string>> options;
+    if (!GetConfigOptions(stream, error, options)) {
+        return false;
+    }
+    for (const std::pair<std::string, std::string>& option : options) {
         std::string strKey = std::string("-") + option.first;
         std::string strValue = option.second;
 
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")]