From a66c0f78a941968340f030911765a84219908c4d Mon Sep 17 00:00:00 2001 From: "Wladimir J. van der Laan" Date: Thu, 30 Aug 2018 13:04:19 +0200 Subject: [PATCH 1/2] 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 - --- src/util.cpp | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/src/util.cpp b/src/util.cpp index 55b09dcff8e..b832fd1f404 100644 --- a/src/util.cpp +++ b/src/util.cpp @@ -812,11 +812,11 @@ static std::string TrimString(const std::string& str, const std::string& pattern return str.substr(front, end - front + 1); } -static std::vector> GetConfigOptions(std::istream& stream) +static bool GetConfigOptions(std::istream& stream, std::string& error, std::vector> &options) { - std::vector> 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); @@ -826,21 +826,34 @@ static std::vector> 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& option : GetConfigOptions(stream)) { + std::vector> options; + if (!GetConfigOptions(stream, error, options)) { + return false; + } + for (const std::pair& option : options) { std::string strKey = std::string("-") + option.first; std::string strValue = option.second; From ed2332aeffb071a3404be9cff8f9fb8a81a9fbfb Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Thu, 6 Sep 2018 11:29:40 +0200 Subject: [PATCH 2/2] test: Add test for config file parsing errors --- test/functional/feature_config_args.py | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/test/functional/feature_config_args.py b/test/functional/feature_config_args.py index 62091048f97..9be59b32b42 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")]