mirrors/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-20 14:05:23 +01:00
Code Issues Projects Releases Wiki Activity

Merge bitcoin/bitcoin#24498: qt: Avoid crash on startup if int specified in settings.json

Browse source 
5b1aae12ca qt: Avoid crash on startup if int specified in settings.json (Ryan Ofsky)
84b0973e35 test: Add tests for GetArg methods / settings.json type coercion (Ryan Ofsky)

Pull request description:

  Should probably add this change to 23.x as suggested by Luke https://github.com/bitcoin/bitcoin/issues/24457#issuecomment-1059825678. If settings like `prune` are added to `settings.json` in the future, it would be preferable for 23.x releases to respect the setting instead of crash.

  ---

  Fix GUI startup crash reported by Rspigler in https://github.com/bitcoin/bitcoin/issues/24457 that happens if `settings.json` contains an integer value for any of the configuration options which GUI settings can currently clash with (-dbcache, -par, -spendzeroconfchange, -signer, -upnp, -natpmp, -listen, -server, -proxy, -proxy, -onion, -onion, -lang, and -prune).

  The fix is a one-line change in `ArgsManager::GetArg`. The rest of the PR just adds a regression test for the GUI and unit tests for ArgsManager::GetArg methods.

ACKs for top commit:
  laanwj:
    Code review ACK 5b1aae12ca
  achow101:
    ACK 5b1aae12ca
  jonatack:
    Code review ACK 5b1aae12ca

Tree-SHA512: 958991b4bead9b82a3879fdca0f8d6405e2a212b7c46cf356f078843a4f156e27fd75fc46e2013aa5159582ead06d343c1ed248d678b3e5bbd312f247e37894c
This commit is contained in:
Andrew Chow 2022-03-09 10:54:42 -05:00
commit 47bbd3ff4f
No known key found for this signature in database
GPG key ID: 17565732E08E5E41
7 changed files with 180 additions and 1 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
build_msvc/test_bitcoin-qt/test_bitcoin-qt.vcxproj
View file

@ -12,6 +12,7 @@
<ClCompile Include="..\..\src\test\util\setup_common.cpp" />
<ClCompile Include="..\..\src\qt\test\addressbooktests.cpp" />
<ClCompile Include="..\..\src\qt\test\apptests.cpp" />
<ClCompile Include="..\..\src\qt\test\optiontests.cpp" />
<ClCompile Include="..\..\src\qt\test\rpcnestedtests.cpp" />
<ClCompile Include="..\..\src\qt\test\test_main.cpp" />
<ClCompile Include="..\..\src\qt\test\uritests.cpp" />
@ -20,6 +21,7 @@
<ClCompile Include="..\..\src\wallet\test\wallet_test_fixture.cpp" />
<ClCompile Include="$(GeneratedFilesOutDir)\moc\moc_addressbooktests.cpp" />
<ClCompile Include="$(GeneratedFilesOutDir)\moc\moc_apptests.cpp" />
<ClCompile Include="$(GeneratedFilesOutDir)\moc\moc_optiontests.cpp" />
<ClCompile Include="$(GeneratedFilesOutDir)\moc\moc_rpcnestedtests.cpp" />
<ClCompile Include="$(GeneratedFilesOutDir)\moc\moc_uritests.cpp" />
<ClCompile Include="$(GeneratedFilesOutDir)\moc\moc_wallettests.cpp" />
@ -88,6 +90,7 @@
<ItemGroup>
<MocTestFiles Include="..\..\src\qt\test\addressbooktests.h" />
<MocTestFiles Include="..\..\src\qt\test\apptests.h" />
<MocTestFiles Include="..\..\src\qt\test\optiontests.h" />
<MocTestFiles Include="..\..\src\qt\test\rpcnestedtests.h" />
<MocTestFiles Include="..\..\src\qt\test\uritests.h" />
<MocTestFiles Include="..\..\src\qt\test\wallettests.h" />

3
src/Makefile.qttest.include
View file

@ -7,6 +7,7 @@ TESTS += qt/test/test_bitcoin-qt
TEST_QT_MOC_CPP = \
qt/test/moc_apptests.cpp \
qt/test/moc_optiontests.cpp \
qt/test/moc_rpcnestedtests.cpp \
qt/test/moc_uritests.cpp
@ -19,6 +20,7 @@ endif # ENABLE_WALLET
TEST_QT_H = \
qt/test/addressbooktests.h \
qt/test/apptests.h \
qt/test/optiontests.h \
qt/test/rpcnestedtests.h \
qt/test/uritests.h \
qt/test/util.h \
@ -30,6 +32,7 @@ qt_test_test_bitcoin_qt_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(BITCOIN_
qt_test_test_bitcoin_qt_SOURCES = \
init/bitcoin-qt.cpp \
qt/test/apptests.cpp \
qt/test/optiontests.cpp \
qt/test/rpcnestedtests.cpp \
qt/test/test_main.cpp \
qt/test/uritests.cpp \

31
src/qt/test/optiontests.cpp Normal file
View file

@ -0,0 +1,31 @@
// Copyright (c) 2018 The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
#include <qt/bitcoin.h>
#include <qt/test/optiontests.h>
#include <test/util/setup_common.h>
#include <util/system.h>
#include <QSettings>
#include <QTest>
#include <univalue.h>
//! Entry point for BitcoinApplication tests.
void OptionTests::optionTests()
{
// Test regression https://github.com/bitcoin/bitcoin/issues/24457. Ensure
// that setting integer prune value doesn't cause an exception to be thrown
// in the OptionsModel constructor
gArgs.LockSettings([&](util::Settings& settings) {
settings.forced_settings.erase("prune");
settings.rw_settings["prune"] = 3814;
});
gArgs.WriteSettingsFile();
OptionsModel{};
gArgs.LockSettings([&](util::Settings& settings) {
settings.rw_settings.erase("prune");
});
gArgs.WriteSettingsFile();
}

25
src/qt/test/optiontests.h Normal file
View file

@ -0,0 +1,25 @@
// Copyright (c) 2019 The Bitcoin Core developers
// Distributed under the MIT software license, see the accompanying
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
#ifndef BITCOIN_QT_TEST_OPTIONTESTS_H
#define BITCOIN_QT_TEST_OPTIONTESTS_H
#include <qt/optionsmodel.h>
#include <QObject>
class OptionTests : public QObject
{
Q_OBJECT
public:
explicit OptionTests(interfaces::Node& node) : m_node(node) {}
private Q_SLOTS:
void optionTests();
private:
interfaces::Node& m_node;
};
#endif // BITCOIN_QT_TEST_OPTIONTESTS_H

5
src/qt/test/test_main.cpp
View file

@ -10,6 +10,7 @@
#include <interfaces/node.h>
#include <qt/bitcoin.h>
#include <qt/test/apptests.h>
#include <qt/test/optiontests.h>
#include <qt/test/rpcnestedtests.h>
#include <qt/test/uritests.h>
#include <test/util/setup_common.h>
@ -89,6 +90,10 @@ int main(int argc, char* argv[])
if (QTest::qExec(&app_tests) != 0) {
fInvalid = true;
}
OptionTests options_tests(app.node());
if (QTest::qExec(&options_tests) != 0) {
fInvalid = true;
}
URITests test1;
if (QTest::qExec(&test1) != 0) {
fInvalid = true;

112
src/test/getarg_tests.cpp
View file

@ -3,6 +3,8 @@
// file COPYING or http://www.opensource.org/licenses/mit-license.php.
#include <test/util/setup_common.h>
#include <univalue.h>
#include <util/settings.h>
#include <util/strencodings.h>
#include <util/system.h>
@ -41,6 +43,116 @@ void SetupArgs(ArgsManager& local_args, const std::vector<std::pair<std::string,
}
}
// Test behavior of GetArg functions when string, integer, and boolean types
// are specified in the settings.json file. GetArg functions are convenience
// functions. The GetSetting method can always be used instead of GetArg
// methods to retrieve original values, and there's not always an objective
// answer to what GetArg behavior is best in every case. This test makes sure
// there's test coverage for whatever the current behavior is, so it's not
// broken or changed unintentionally.
BOOST_AUTO_TEST_CASE(setting_args)
{
ArgsManager args;
SetupArgs(args, {{"-foo", ArgsManager::ALLOW_ANY}});
auto set_foo = [&](const util::SettingsValue& value) {
args.LockSettings([&](util::Settings& settings) {
settings.rw_settings["foo"] = value;
});
};
set_foo("str");
BOOST_CHECK_EQUAL(args.GetSetting("foo").write(), "\"str\"");
BOOST_CHECK_EQUAL(args.GetArg("foo", "default"), "str");
BOOST_CHECK_EQUAL(args.GetIntArg("foo", 100), 0);
BOOST_CHECK_EQUAL(args.GetBoolArg("foo", true), false);
BOOST_CHECK_EQUAL(args.GetBoolArg("foo", false), false);
set_foo("99");
BOOST_CHECK_EQUAL(args.GetSetting("foo").write(), "\"99\"");
BOOST_CHECK_EQUAL(args.GetArg("foo", "default"), "99");
BOOST_CHECK_EQUAL(args.GetIntArg("foo", 100), 99);
BOOST_CHECK_EQUAL(args.GetBoolArg("foo", true), true);
BOOST_CHECK_EQUAL(args.GetBoolArg("foo", false), true);
set_foo("3.25");
BOOST_CHECK_EQUAL(args.GetSetting("foo").write(), "\"3.25\"");
BOOST_CHECK_EQUAL(args.GetArg("foo", "default"), "3.25");
BOOST_CHECK_EQUAL(args.GetIntArg("foo", 100), 3);
BOOST_CHECK_EQUAL(args.GetBoolArg("foo", true), true);
BOOST_CHECK_EQUAL(args.GetBoolArg("foo", false), true);
set_foo("0");
BOOST_CHECK_EQUAL(args.GetSetting("foo").write(), "\"0\"");
BOOST_CHECK_EQUAL(args.GetArg("foo", "default"), "0");
BOOST_CHECK_EQUAL(args.GetIntArg("foo", 100), 0);
BOOST_CHECK_EQUAL(args.GetBoolArg("foo", true), false);
BOOST_CHECK_EQUAL(args.GetBoolArg("foo", false), false);
set_foo("");
BOOST_CHECK_EQUAL(args.GetSetting("foo").write(), "\"\"");
BOOST_CHECK_EQUAL(args.GetArg("foo", "default"), "");
BOOST_CHECK_EQUAL(args.GetIntArg("foo", 100), 0);
BOOST_CHECK_EQUAL(args.GetBoolArg("foo", true), true);
BOOST_CHECK_EQUAL(args.GetBoolArg("foo", false), true);
set_foo(99);
BOOST_CHECK_EQUAL(args.GetSetting("foo").write(), "99");
BOOST_CHECK_EQUAL(args.GetArg("foo", "default"), "99");
BOOST_CHECK_EQUAL(args.GetIntArg("foo", 100), 99);
BOOST_CHECK_THROW(args.GetBoolArg("foo", true), std::runtime_error);
BOOST_CHECK_THROW(args.GetBoolArg("foo", false), std::runtime_error);
set_foo(3.25);
BOOST_CHECK_EQUAL(args.GetSetting("foo").write(), "3.25");
BOOST_CHECK_EQUAL(args.GetArg("foo", "default"), "3.25");
BOOST_CHECK_THROW(args.GetIntArg("foo", 100), std::runtime_error);
BOOST_CHECK_THROW(args.GetBoolArg("foo", true), std::runtime_error);
BOOST_CHECK_THROW(args.GetBoolArg("foo", false), std::runtime_error);
set_foo(0);
BOOST_CHECK_EQUAL(args.GetSetting("foo").write(), "0");
BOOST_CHECK_EQUAL(args.GetArg("foo", "default"), "0");
BOOST_CHECK_EQUAL(args.GetIntArg("foo", 100), 0);
BOOST_CHECK_THROW(args.GetBoolArg("foo", true), std::runtime_error);
BOOST_CHECK_THROW(args.GetBoolArg("foo", false), std::runtime_error);
set_foo(true);
BOOST_CHECK_EQUAL(args.GetSetting("foo").write(), "true");
BOOST_CHECK_EQUAL(args.GetArg("foo", "default"), "1");
BOOST_CHECK_EQUAL(args.GetIntArg("foo", 100), 1);
BOOST_CHECK_EQUAL(args.GetBoolArg("foo", true), true);
BOOST_CHECK_EQUAL(args.GetBoolArg("foo", false), true);
set_foo(false);
BOOST_CHECK_EQUAL(args.GetSetting("foo").write(), "false");
BOOST_CHECK_EQUAL(args.GetArg("foo", "default"), "0");
BOOST_CHECK_EQUAL(args.GetIntArg("foo", 100), 0);
BOOST_CHECK_EQUAL(args.GetBoolArg("foo", true), false);
BOOST_CHECK_EQUAL(args.GetBoolArg("foo", false), false);
set_foo(UniValue::VOBJ);
BOOST_CHECK_EQUAL(args.GetSetting("foo").write(), "{}");
BOOST_CHECK_THROW(args.GetArg("foo", "default"), std::runtime_error);
BOOST_CHECK_THROW(args.GetIntArg("foo", 100), std::runtime_error);
BOOST_CHECK_THROW(args.GetBoolArg("foo", true), std::runtime_error);
BOOST_CHECK_THROW(args.GetBoolArg("foo", false), std::runtime_error);
set_foo(UniValue::VARR);
BOOST_CHECK_EQUAL(args.GetSetting("foo").write(), "[]");
BOOST_CHECK_THROW(args.GetArg("foo", "default"), std::runtime_error);
BOOST_CHECK_THROW(args.GetIntArg("foo", 100), std::runtime_error);
BOOST_CHECK_THROW(args.GetBoolArg("foo", true), std::runtime_error);
BOOST_CHECK_THROW(args.GetBoolArg("foo", false), std::runtime_error);
set_foo(UniValue::VNULL);
BOOST_CHECK_EQUAL(args.GetSetting("foo").write(), "null");
BOOST_CHECK_EQUAL(args.GetArg("foo", "default"), "default");
BOOST_CHECK_EQUAL(args.GetIntArg("foo", 100), 100);
BOOST_CHECK_EQUAL(args.GetBoolArg("foo", true), true);
BOOST_CHECK_EQUAL(args.GetBoolArg("foo", false), false);
}
BOOST_AUTO_TEST_CASE(boolarg)
{
ArgsManager local_args;

2
src/util/system.cpp
View file

@ -591,7 +591,7 @@ bool ArgsManager::IsArgNegated(const std::string& strArg) const
std::string ArgsManager::GetArg(const std::string& strArg, const std::string& strDefault) const
{
const util::SettingsValue value = GetSetting(strArg);
return value.isNull() ? strDefault : value.isFalse() ? "0" : value.isTrue() ? "1" : value.get_str();
return value.isNull() ? strDefault : value.isFalse() ? "0" : value.isTrue() ? "1" : value.isNum() ? value.getValStr() : value.get_str();
}
int64_t ArgsManager::GetIntArg(const std::string& strArg, int64_t nDefault) const