From 615462e5a4f44fb800d554c3144b77a3d8fec340 Mon Sep 17 00:00:00 2001 From: fanquake Date: Mon, 7 Jun 2021 13:20:11 +0800 Subject: [PATCH] Merge bitcoin/bitcoin#22137: util: Properly handle -noincludeconf on command line (take 2) fa910b47656d0e69cccb1f31804f2b11aa45d053 util: Properly handle -noincludeconf on command line (MarcoFalke) Pull request description: Before: ``` $ ./src/qt/bitcoin-qt -noincludeconf (memory violation, can be observed with valgrind or similar) ``` After: ``` $ ./src/qt/bitcoin-qt -noincludeconf (passes startup) ``` Fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=34884 ACKs for top commit: practicalswift: cr ACK fa910b47656d0e69cccb1f31804f2b11aa45d053: patch looks correct ryanofsky: Code review ACK fa910b47656d0e69cccb1f31804f2b11aa45d053. Nice cleanups! Tree-SHA512: 5dfad82a78bca7a9a6bcc6aead2d7fbde166a09a5300a82f80dd1aee1de00e070bcb30b7472741a5396073b370898696e78c33038f94849219281d99358248ed --- src/test/util_tests.cpp | 19 +++++++++++++++++++ src/util/system.cpp | 11 +++++++---- 2 files changed, 26 insertions(+), 4 deletions(-) diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 3ec229db8f..3cbc730ecd 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -325,6 +325,25 @@ BOOST_FIXTURE_TEST_CASE(util_CheckValue, CheckValueTest) 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) { TestArgsManager testArgs; diff --git a/src/util/system.cpp b/src/util/system.cpp index 2685121761..078e53c593 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -352,11 +352,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; }