Merge bitcoin#24498: qt: Avoid crash on startup if int specified in s…

…ettings.json

5b1aae1 qt: Avoid crash on startup if int specified in settings.json (Ryan Ofsky)
84b0973 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 bitcoin#24457 (comment). 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 bitcoin#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 5b1aae1
  achow101:
    ACK 5b1aae1
  jonatack:
    Code review ACK 5b1aae1

Tree-SHA512: 958991b4bead9b82a3879fdca0f8d6405e2a212b7c46cf356f078843a4f156e27fd75fc46e2013aa5159582ead06d343c1ed248d678b3e5bbd312f247e37894c

# Conflicts:
#	build_msvc/test_bitcoin-qt/test_bitcoin-qt.vcxproj

src/test/util_tests.cpp

@@ -18,13 +18,15 @@
 #include <util/moneystr.h>
 #include <util/overflow.h>
 #include <util/ranges_set.h>
+#include <util/readwritefile.h>
 #include <util/spanparsing.h>
 #include <util/strencodings.h>
 #include <util/string.h>
 #include <util/time.h>
 #include <util/vector.h>
 
 #include <array>
+#include <fstream>
 #include <deque>
 #include <optional>
 #include <random>
@@ -2747,6 +2749,52 @@ BOOST_AUTO_TEST_CASE(util_ParseByteUnits)
     BOOST_CHECK(!ParseByteUnits("1x", noop));
 }
 
+BOOST_AUTO_TEST_CASE(util_ReadBinaryFile)
+{
+    fs::path tmpfolder = m_args.GetDataDirBase();
+    fs::path tmpfile = tmpfolder / "read_binary.dat";
+    std::string expected_text;
+    for (int i = 0; i < 30; i++) {
+        expected_text += "0123456789";
+    }
+    {
+        std::ofstream file{tmpfile};
+        file << expected_text;
+    }
+    {
+        // read all contents in file
+        auto [valid, text] = ReadBinaryFile(tmpfile);
+        BOOST_CHECK(valid);
+        BOOST_CHECK_EQUAL(text, expected_text);
+    }
+    {
+        // read half contents in file
+        auto [valid, text] = ReadBinaryFile(tmpfile, expected_text.size() / 2);
+        BOOST_CHECK(valid);
+        BOOST_CHECK_EQUAL(text, expected_text.substr(0, expected_text.size() / 2));
+    }
+    {
+        // read from non-existent file
+        fs::path invalid_file = tmpfolder / "invalid_binary.dat";
+        auto [valid, text] = ReadBinaryFile(invalid_file);
+        BOOST_CHECK(!valid);
+        BOOST_CHECK(text.empty());
+    }
+}
+
+BOOST_AUTO_TEST_CASE(util_WriteBinaryFile)
+{
+    fs::path tmpfolder = m_args.GetDataDirBase();
+    fs::path tmpfile = tmpfolder / "write_binary.dat";
+    std::string expected_text = "bitcoin";
+    auto valid = WriteBinaryFile(tmpfile, expected_text);
+    std::string actual_text;
+    std::ifstream file{tmpfile};
+    file >> actual_text;
+    BOOST_CHECK(valid);
+    BOOST_CHECK_EQUAL(actual_text, expected_text);
+}
+
 BOOST_AUTO_TEST_CASE(clearshrink_test)
 {
     {

src/util/readwritefile.cpp

@@ -20,15 +20,15 @@ std::pair<bool,std::string> ReadBinaryFile(const fs::path &filename, size_t maxs
     std::string retval;
     char buffer[128];
     do {
-        const size_t n = fread(buffer, 1, sizeof(buffer), f);
+        const size_t n = fread(buffer, 1, std::min(sizeof(buffer), maxsize - retval.size()), f);
         // Check for reading errors so we don't return any data if we couldn't
         // read the entire file (or up to maxsize)
         if (ferror(f)) {
             fclose(f);
             return std::make_pair(false,"");
         }
         retval.append(buffer, buffer+n);
-    } while (!feof(f) && retval.size() <= maxsize);
+    } while (!feof(f) && retval.size() < maxsize);
     fclose(f);
     return std::make_pair(true,retval);
 }