Skip to content

Commit

Permalink
Merge #18270: util: Fail to parse whitespace-only strings in ParseMon…
Browse files Browse the repository at this point in the history
…ey(...) (instead of parsing as zero)

100213c util: Fail to parse space-only strings in ParseMoney(...) (instead of parsing as zero) (practicalswift)

Pull request description:

  Fail to parse whitespace-only strings in `ParseMoney(...)` (instead of parsing as `0`).

  This is a follow-up to #18225 ("util: Fail to parse empty string in `ParseMoney`") which made `ParseMoney("")` fail instead of parsing as `0`.

  Context: #18225 (comment)

  Current non-test call sites:

  ```
  $ git grep ParseMoney ":(exclude)src/test/"
  src/bitcoin-tx.cpp:    if (!ParseMoney(strValue, value))
  src/init.cpp:        if (!ParseMoney(gArgs.GetArg("-incrementalrelayfee", ""), n))
  src/init.cpp:        if (!ParseMoney(gArgs.GetArg("-minrelaytxfee", ""), n)) {
  src/init.cpp:        if (!ParseMoney(gArgs.GetArg("-blockmintxfee", ""), n))
  src/init.cpp:        if (!ParseMoney(gArgs.GetArg("-dustrelayfee", ""), n))
  src/miner.cpp:    if (gArgs.IsArgSet("-blockmintxfee") && ParseMoney(gArgs.GetArg("-blockmintxfee", ""), n)) {
  src/util/moneystr.cpp:bool ParseMoney(const std::string& str, CAmount& nRet)
  src/util/moneystr.h:NODISCARD bool ParseMoney(const std::string& str, CAmount& nRet);
  src/wallet/wallet.cpp:        if (!ParseMoney(gArgs.GetArg("-mintxfee", ""), n) || 0 == n) {
  src/wallet/wallet.cpp:        if (!ParseMoney(gArgs.GetArg("-fallbackfee", ""), nFeePerK)) {
  src/wallet/wallet.cpp:        if (!ParseMoney(gArgs.GetArg("-discardfee", ""), nFeePerK)) {
  src/wallet/wallet.cpp:        if (!ParseMoney(gArgs.GetArg("-paytxfee", ""), nFeePerK)) {
  src/wallet/wallet.cpp:        if (!ParseMoney(gArgs.GetArg("-maxtxfee", ""), nMaxFee)) {
  ```

ACKs for top commit:
  Empact:
    ACK 100213c
  sipa:
    ACK 100213c
  theStack:
    ACK 100213c

Tree-SHA512: cadfb1ac8276cf54736c3444705f2650e7a08023673aedc729fabe751ae80f6c490fc0945ee38dbfd02c95e4d9853d1e4c84f5d3c310f44eaf3585afec8a4c22
  • Loading branch information
MarcoFalke committed Mar 27, 2020
2 parents 7f9dedb + 100213c commit 0dc6218
Show file tree
Hide file tree
Showing 2 changed files with 27 additions and 9 deletions.
20 changes: 20 additions & 0 deletions src/test/util_tests.cpp
Expand Up @@ -1182,6 +1182,12 @@ BOOST_AUTO_TEST_CASE(util_ParseMoney)
BOOST_CHECK_EQUAL(ret, COIN);
BOOST_CHECK(ParseMoney("1", ret));
BOOST_CHECK_EQUAL(ret, COIN);
BOOST_CHECK(ParseMoney(" 1", ret));
BOOST_CHECK_EQUAL(ret, COIN);
BOOST_CHECK(ParseMoney("1 ", ret));
BOOST_CHECK_EQUAL(ret, COIN);
BOOST_CHECK(ParseMoney(" 1 ", ret));
BOOST_CHECK_EQUAL(ret, COIN);
BOOST_CHECK(ParseMoney("0.1", ret));
BOOST_CHECK_EQUAL(ret, COIN/10);
BOOST_CHECK(ParseMoney("0.01", ret));
Expand All @@ -1198,12 +1204,26 @@ BOOST_AUTO_TEST_CASE(util_ParseMoney)
BOOST_CHECK_EQUAL(ret, COIN/10000000);
BOOST_CHECK(ParseMoney("0.00000001", ret));
BOOST_CHECK_EQUAL(ret, COIN/100000000);
BOOST_CHECK(ParseMoney(" 0.00000001 ", ret));
BOOST_CHECK_EQUAL(ret, COIN/100000000);
BOOST_CHECK(ParseMoney("0.00000001 ", ret));
BOOST_CHECK_EQUAL(ret, COIN/100000000);
BOOST_CHECK(ParseMoney(" 0.00000001", ret));
BOOST_CHECK_EQUAL(ret, COIN/100000000);

// Parsing amount that can not be represented in ret should fail
BOOST_CHECK(!ParseMoney("0.000000001", ret));

// Parsing empty string should fail
BOOST_CHECK(!ParseMoney("", ret));
BOOST_CHECK(!ParseMoney(" ", ret));
BOOST_CHECK(!ParseMoney(" ", ret));

// Parsing two numbers should fail
BOOST_CHECK(!ParseMoney("1 2", ret));
BOOST_CHECK(!ParseMoney(" 1 2 ", ret));
BOOST_CHECK(!ParseMoney(" 1.2 3 ", ret));
BOOST_CHECK(!ParseMoney(" 1 2.3 ", ret));

// Attempted 63 bit overflow should fail
BOOST_CHECK(!ParseMoney("92233720368.54775808", ret));
Expand Down
16 changes: 7 additions & 9 deletions src/util/moneystr.cpp
Expand Up @@ -31,21 +31,19 @@ std::string FormatMoney(const CAmount& n)
}


bool ParseMoney(const std::string& str, CAmount& nRet)
bool ParseMoney(const std::string& money_string, CAmount& nRet)
{
if (!ValidAsCString(str)) {
if (!ValidAsCString(money_string)) {
return false;
}

const std::string str = TrimString(money_string);
if (str.empty()) {
return false;
}

std::string strWhole;
int64_t nUnits = 0;
const char* p = str.c_str();
while (IsSpace(*p))
p++;
for (; *p; p++)
{
if (*p == '.')
Expand All @@ -60,14 +58,14 @@ bool ParseMoney(const std::string& str, CAmount& nRet)
break;
}
if (IsSpace(*p))
break;
return false;
if (!IsDigit(*p))
return false;
strWhole.insert(strWhole.end(), *p);
}
for (; *p; p++)
if (!IsSpace(*p))
return false;
if (*p) {
return false;
}
if (strWhole.size() > 10) // guard against 63 bit overflow
return false;
if (nUnits < 0 || nUnits > COIN)
Expand Down

0 comments on commit 0dc6218

Please sign in to comment.