diff --git a/doc/release-notes.md b/doc/release-notes.md --- a/doc/release-notes.md +++ b/doc/release-notes.md @@ -19,3 +19,10 @@ and purpose key/value pairs has been deprecated in favor of an array of label names and will be removed in a future release. The previous behavior can be re-enabled in the interim by launching with `-deprecatedrpc=labelspurpose`. + +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. diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -436,6 +436,26 @@ } } +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; @@ -1002,7 +1022,9 @@ //! Enumerate all possible test configurations. template 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}) { @@ -1189,7 +1211,7 @@ // BOOST_CHECK_EQUAL( out_sha_hex, - "b835eef5977d69114eb039a976201f8c7121f34fe2b7ea2b73cafb516e5c9dc8"); + "8fd4877bb8bf337badca950ede6c917441901962f160e52514e06a60dea46cde"); } // Similar test as above, but for ArgsManager::GetChainName function. diff --git a/src/util/system.cpp b/src/util/system.cpp --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -336,21 +336,20 @@ std::string section; util::SettingsValue value = InterpretOption(section, key, val); std::optional 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 --- a/test/functional/feature_config_args.py +++ b/test/functional/feature_config_args.py @@ -23,7 +23,7 @@ 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: