diff options
author | Russell Yanofsky <russ@yanofsky.org> | 2019-12-19 16:27:15 -0500 |
---|---|---|
committer | Russell Yanofsky <russ@yanofsky.org> | 2019-12-19 16:27:15 -0500 |
commit | 900d8f6f70859f528e84c5c38d0332f81d19df55 (patch) | |
tree | 77a9236d14f792e881aee790e4cebd7425f1a7cf | |
parent | 6677be64f69b3d6f60f5a675ff9746def27a2de8 (diff) | |
download | bitcoin-900d8f6f70859f528e84c5c38d0332f81d19df55.tar.xz |
util: Disallow network-qualified command line options
Previously these were allowed but ignored.
-rw-r--r-- | doc/release-notes.md | 7 | ||||
-rw-r--r-- | src/test/util_tests.cpp | 26 | ||||
-rw-r--r-- | src/util/system.cpp | 23 | ||||
-rwxr-xr-x | test/functional/feature_config_args.py | 2 |
4 files changed, 42 insertions, 16 deletions
diff --git a/doc/release-notes.md b/doc/release-notes.md index eec1ef9c46..99ca53c597 100644 --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -104,6 +104,13 @@ Wallet Low-level changes ================= +Command line +------------ + +Command line options prefixed with main/test/regtest network names like +`-main.port=8333` `-test.server=1` previously were allowed but ignored. Now +they trigger "Invalid parameter" errors on startup. + Tests ----- diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 86ea56ff36..7b024ba75f 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -340,6 +340,27 @@ BOOST_AUTO_TEST_CASE(util_ParseParameters) BOOST_CHECK(testArgs.GetArgs("-ccc").size() == 2); } +BOOST_AUTO_TEST_CASE(util_ParseInvalidParameters) +{ + TestArgsManager test; + test.SetupArgs({{"-registered", ArgsManager::ALLOW_ANY}}); + + const char* argv[] = {"ignored", "-registered"}; + std::string error; + BOOST_CHECK(test.ParseParameters(2, (char**)argv, error)); + BOOST_CHECK_EQUAL(error, ""); + + argv[1] = "-unregistered"; + BOOST_CHECK(!test.ParseParameters(2, (char**)argv, error)); + BOOST_CHECK_EQUAL(error, "Invalid parameter -unregistered"); + + // Make sure registered parameters prefixed with a chain name trigger errors. + // (Previously, they were accepted and ignored.) + argv[1] = "-test.registered"; + BOOST_CHECK(!test.ParseParameters(2, (char**)argv, error)); + BOOST_CHECK_EQUAL(error, "Invalid parameter -test.registered"); +} + static void TestParse(const std::string& str, bool expected_bool, int64_t expected_int) { TestArgsManager test; @@ -835,7 +856,8 @@ struct ArgsMergeTestingSetup : public BasicTestingSetup { void ForEachMergeSetup(Fn&& fn) { ActionList arg_actions = {}; - ForEachNoDup(arg_actions, SET, SECTION_NEGATE, [&] { + // command_line_options do not have sections. Only iterate over SET and NEGATE + ForEachNoDup(arg_actions, SET, NEGATE, [&] { ActionList conf_actions = {}; ForEachNoDup(conf_actions, SET, SECTION_NEGATE, [&] { for (bool soft_set : {false, true}) { @@ -995,7 +1017,7 @@ BOOST_FIXTURE_TEST_CASE(util_ArgsMerge, ArgsMergeTestingSetup) // Results file is formatted like: // // <input> || <IsArgSet/IsArgNegated/GetArg output> | <GetArgs output> | <GetUnsuitable output> - BOOST_CHECK_EQUAL(out_sha_hex, "b835eef5977d69114eb039a976201f8c7121f34fe2b7ea2b73cafb516e5c9dc8"); + BOOST_CHECK_EQUAL(out_sha_hex, "8fd4877bb8bf337badca950ede6c917441901962f160e52514e06a60dea46cde"); } // Similar test as above, but for ArgsManager::GetChainName function. diff --git a/src/util/system.cpp b/src/util/system.cpp index d99a87a9f2..5587764c58 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -311,21 +311,18 @@ bool ArgsManager::ParseParameters(int argc, const char* const argv[], std::strin std::string section; util::SettingsValue value = InterpretOption(section, key, val); Optional<unsigned int> flags = GetArgFlags('-' + key); - if (flags) { - if (!CheckValid(key, value, *flags, error)) { - return false; - } - // Weird behavior preserved for backwards compatibility: command - // line options with section prefixes are allowed but ignored. It - // would be better if these options triggered the Invalid parameter - // error below. - if (section.empty()) { - m_settings.command_line_options[key].push_back(value); - } - } else { - error = strprintf("Invalid parameter -%s", key); + + // Unknown command line options and command line options with dot + // characters (which are returned from InterpretOption with nonempty + // section strings) are not valid. + if (!flags || !section.empty()) { + error = strprintf("Invalid parameter %s", argv[i]); return false; } + + if (!CheckValid(key, value, *flags, error)) return false; + + m_settings.command_line_options[key].push_back(value); } // we do not allow -includeconf from command line diff --git a/test/functional/feature_config_args.py b/test/functional/feature_config_args.py index 801209b1c0..8397eb9f59 100755 --- a/test/functional/feature_config_args.py +++ b/test/functional/feature_config_args.py @@ -23,7 +23,7 @@ class ConfArgsTest(BitcoinTestFramework): conf.write('includeconf={}\n'.format(inc_conf_file_path)) self.nodes[0].assert_start_raises_init_error( - expected_msg='Error: Error parsing command line arguments: Invalid parameter -dash_cli', + expected_msg='Error: Error parsing command line arguments: Invalid parameter -dash_cli=1', extra_args=['-dash_cli=1'], ) with open(inc_conf_file_path, 'w', encoding='utf-8') as conf: |