From 762f85c436e0cad6626e219e6fa7f6175bcdb899 Mon Sep 17 00:00:00 2001 From: barton26 Date: Tue, 22 Mar 2022 15:19:56 -0400 Subject: [PATCH 1/3] Fix crash when parsing command line with -noincludeconf=0 --- src/util/system.cpp | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/util/system.cpp b/src/util/system.cpp index b8cfec289f..78c4f77321 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -273,14 +273,12 @@ bool ArgsManager::ParseParameters(int argc, const char* const argv[], std::strin } // we do not allow -includeconf from command line - bool success = true; if (auto* includes = util::FindKey(m_settings.command_line_options, "includeconf")) { - for (const auto& include : util::SettingsSpan(*includes)) { - error += "-includeconf cannot be used from commandline; -includeconf=" + include.get_str() + "\n"; - success = false; - } + const auto& include{*util::SettingsSpan(*includes).begin()}; // pick first value as example + error = "-includeconf cannot be used from commandline; -includeconf=" + include.write(); + return false; } - return success; + return true; } std::optional ArgsManager::GetArgFlags(const std::string& name) const From e9e876ad0065269dcc3b204751fb4653107b584f Mon Sep 17 00:00:00 2001 From: barton26 Date: Tue, 22 Mar 2022 15:33:08 -0400 Subject: [PATCH 2/3] util: Properly handle -noincludeconf on command line --- src/util/system.cpp | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/src/util/system.cpp b/src/util/system.cpp index 78c4f77321..dda6e3e158 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -272,11 +272,14 @@ bool ArgsManager::ParseParameters(int argc, const char* const argv[], std::strin m_settings.command_line_options[key].push_back(value); } - // we do not allow -includeconf from command line + // we do not allow -includeconf from command line, only -noincludeconf if (auto* includes = util::FindKey(m_settings.command_line_options, "includeconf")) { - const auto& include{*util::SettingsSpan(*includes).begin()}; // pick first value as example - error = "-includeconf cannot be used from commandline; -includeconf=" + include.write(); - return false; + const util::SettingsSpan values{*includes}; + // Range may be empty if -noincludeconf was passed + if (!values.empty()) { + error = "-includeconf cannot be used from commandline; -includeconf=" + values.begin()->write(); + return false; // pick first value as example + } } return true; } From 21b5506e213a5e4fb1b882eacde7bef24af44ee5 Mon Sep 17 00:00:00 2001 From: "James C. Owens" Date: Sat, 26 Mar 2022 23:21:38 -0400 Subject: [PATCH 3/3] Implement TestArgsManager struct to support newer ArgsManager tests --- src/test/util_tests.cpp | 162 ++++++++++++++++++++++++++++++++++++++++ src/util/system.cpp | 5 ++ src/util/system.h | 9 +++ 3 files changed, 176 insertions(+) diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 60c847d43f..b7b1c5b478 100755 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -4,10 +4,13 @@ #include #include +#include "univalue/include/univalue.h" #include #include #include +//#include +#include #include @@ -145,6 +148,165 @@ BOOST_AUTO_TEST_CASE(util_DateTimeStrFormat) */ } +struct TestArgsManager : public ArgsManager +{ + TestArgsManager() { m_network_only_args.clear(); } + void ReadConfigString(const std::string str_config) + { + std::istringstream streamConfig(str_config); + { + LOCK(cs_args); + m_settings.ro_config.clear(); + m_config_sections.clear(); + } + std::string error; + BOOST_REQUIRE(ReadConfigStream(streamConfig, "", error)); + } + void SetNetworkOnlyArg(const std::string arg) + { + LOCK(cs_args); + m_network_only_args.insert(arg); + } + void SetupArgs(const std::vector>& args) + { + for (const auto& arg : args) { + AddArg(arg.first, "", arg.second, OptionsCategory::OPTIONS); + } + } + using ArgsManager::GetSetting; + using ArgsManager::GetSettingsList; + using ArgsManager::ReadConfigStream; + using ArgsManager::cs_args; + using ArgsManager::m_network; + using ArgsManager::m_settings; +}; + +/* This is reserved when we port over the TestChain 100 block setup class from Bitcoin. +//! Test GetSetting and GetArg type coercion, negation, and default value handling. +class CheckValueTest : public TestChain100Setup +{ +public: + struct Expect { + util::SettingsValue setting; + bool default_string = false; + bool default_int = false; + bool default_bool = false; + const char* string_value = nullptr; + std::optional int_value; + std::optional bool_value; + std::optional> list_value; + const char* error = nullptr; + + explicit Expect(util::SettingsValue s) : setting(std::move(s)) {} + Expect& DefaultString() { default_string = true; return *this; } + Expect& DefaultInt() { default_int = true; return *this; } + Expect& DefaultBool() { default_bool = true; return *this; } + Expect& String(const char* s) { string_value = s; return *this; } + Expect& Int(int64_t i) { int_value = i; return *this; } + Expect& Bool(bool b) { bool_value = b; return *this; } + Expect& List(std::vector m) { list_value = std::move(m); return *this; } + Expect& Error(const char* e) { error = e; return *this; } + }; + + void CheckValue(unsigned int flags, const char* arg, const Expect& expect) + { + TestArgsManager test; + test.SetupArgs({{"-value", flags}}); + const char* argv[] = {"ignored", arg}; + std::string error; + bool success = test.ParseParameters(arg ? 2 : 1, (char**)argv, error); + + BOOST_CHECK_EQUAL(test.GetSetting("-value").write(), expect.setting.write()); + auto settings_list = test.GetSettingsList("-value"); + if (expect.setting.isNull() || expect.setting.isFalse()) { + BOOST_CHECK_EQUAL(settings_list.size(), 0U); + } else { + BOOST_CHECK_EQUAL(settings_list.size(), 1U); + BOOST_CHECK_EQUAL(settings_list[0].write(), expect.setting.write()); + } + + if (expect.error) { + BOOST_CHECK(!success); + BOOST_CHECK_NE(error.find(expect.error), std::string::npos); + } else { + BOOST_CHECK(success); + BOOST_CHECK_EQUAL(error, ""); + } + + if (expect.default_string) { + BOOST_CHECK_EQUAL(test.GetArg("-value", "zzzzz"), "zzzzz"); + } else if (expect.string_value) { + BOOST_CHECK_EQUAL(test.GetArg("-value", "zzzzz"), expect.string_value); + } else { + BOOST_CHECK(!success); + } + + if (expect.default_int) { + BOOST_CHECK_EQUAL(test.GetIntArg("-value", 99999), 99999); + } else if (expect.int_value) { + BOOST_CHECK_EQUAL(test.GetIntArg("-value", 99999), *expect.int_value); + } else { + BOOST_CHECK(!success); + } + + if (expect.default_bool) { + BOOST_CHECK_EQUAL(test.GetBoolArg("-value", false), false); + BOOST_CHECK_EQUAL(test.GetBoolArg("-value", true), true); + } else if (expect.bool_value) { + BOOST_CHECK_EQUAL(test.GetBoolArg("-value", false), *expect.bool_value); + BOOST_CHECK_EQUAL(test.GetBoolArg("-value", true), *expect.bool_value); + } else { + BOOST_CHECK(!success); + } + + if (expect.list_value) { + auto l = test.GetArgs("-value"); + BOOST_CHECK_EQUAL_COLLECTIONS(l.begin(), l.end(), expect.list_value->begin(), expect.list_value->end()); + } else { + BOOST_CHECK(!success); + } + } +}; + +BOOST_FIXTURE_TEST_CASE(util_CheckValue, CheckValueTest) +{ + using M = ArgsManager; + + CheckValue(M::ALLOW_ANY, nullptr, Expect{{}}.DefaultString().DefaultInt().DefaultBool().List({})); + CheckValue(M::ALLOW_ANY, "-novalue", Expect{false}.String("0").Int(0).Bool(false).List({})); + CheckValue(M::ALLOW_ANY, "-novalue=", Expect{false}.String("0").Int(0).Bool(false).List({})); + CheckValue(M::ALLOW_ANY, "-novalue=0", Expect{true}.String("1").Int(1).Bool(true).List({"1"})); + CheckValue(M::ALLOW_ANY, "-novalue=1", Expect{false}.String("0").Int(0).Bool(false).List({})); + CheckValue(M::ALLOW_ANY, "-novalue=2", Expect{false}.String("0").Int(0).Bool(false).List({})); + CheckValue(M::ALLOW_ANY, "-novalue=abc", Expect{true}.String("1").Int(1).Bool(true).List({"1"})); + CheckValue(M::ALLOW_ANY, "-value", Expect{""}.String("").Int(0).Bool(true).List({""})); + CheckValue(M::ALLOW_ANY, "-value=", Expect{""}.String("").Int(0).Bool(true).List({""})); + CheckValue(M::ALLOW_ANY, "-value=0", Expect{"0"}.String("0").Int(0).Bool(false).List({"0"})); + CheckValue(M::ALLOW_ANY, "-value=1", Expect{"1"}.String("1").Int(1).Bool(true).List({"1"})); + CheckValue(M::ALLOW_ANY, "-value=2", Expect{"2"}.String("2").Int(2).Bool(true).List({"2"})); + CheckValue(M::ALLOW_ANY, "-value=abc", Expect{"abc"}.String("abc").Int(0).Bool(false).List({"abc"})); +} +*/ + +struct NoIncludeConfTest { + std::string Parse(const char* arg) + { + TestArgsManager test; + test.SetupArgs({{"-includeconf", ArgsManager::ALLOW_ANY}}); + std::array argv{"ignored", arg}; + std::string error; + (void)test.ParseParameters(argv.size(), argv.data(), error); + return error; + } +}; + +BOOST_FIXTURE_TEST_CASE(util_NoIncludeConf, NoIncludeConfTest) +{ + BOOST_CHECK_EQUAL(Parse("-noincludeconf"), ""); + BOOST_CHECK_EQUAL(Parse("-includeconf"), "-includeconf cannot be used from commandline; -includeconf=\"\""); + BOOST_CHECK_EQUAL(Parse("-includeconf=file"), "-includeconf cannot be used from commandline; -includeconf=\"file\""); +} + BOOST_AUTO_TEST_CASE(util_ParseParameters) { const char *argv_test[] = {"-ignored", "-a", "-b", "-ccc=argument", "-ccc=multiple", "f", "-d=e"}; diff --git a/src/util/system.cpp b/src/util/system.cpp index dda6e3e158..40632f3f2d 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -494,6 +494,11 @@ std::string ArgsManager::GetArg(const std::string& strArg, const std::string& st return value.isNull() ? strDefault : value.isFalse() ? "0" : value.isTrue() ? "1" : value.get_str(); } +int64_t ArgsManager::GetIntArg(const std::string& strArg, int64_t nDefault) const +{ + return GetArg(strArg, nDefault); +} + int64_t ArgsManager::GetArg(const std::string& strArg, int64_t nDefault) const { const util::SettingsValue value = GetSetting(strArg); diff --git a/src/util/system.h b/src/util/system.h index 659474496f..c2a1a5a8cc 100644 --- a/src/util/system.h +++ b/src/util/system.h @@ -269,6 +269,15 @@ class ArgsManager */ int64_t GetArg(const std::string& strArg, int64_t nDefault) const; + /** + * Return integer argument or default value (alias for GetArg used in newer Bitcoin code) + * + * @param strArg Argument to get (e.g. "-foo") + * @param nDefault (e.g. 1) + * @return command-line argument (0 if invalid number) or default value + */ + int64_t GetIntArg(const std::string& strArg, int64_t nDefault) const; + /** * Return boolean argument or default value *