From fa3cd2853530c86c261ac7266ffe4f1726fe9ce6 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Fri, 1 Oct 2021 17:33:35 +0200 Subject: [PATCH 1/2] refactor: Remove unused ParsePrechecks from ParseIntegral Also: * Remove redundant {} from return statement * Add missing failing c-string test case and "-" and "+" strings * Add missing failing test cases for non-int32_t integral types --- src/test/util_tests.cpp | 58 +++++++++++++++++++++++++-------------- src/util/strencodings.cpp | 5 ---- src/util/strencodings.h | 6 ++-- 3 files changed, 41 insertions(+), 28 deletions(-) diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index a13700d733b..de78b1bb509 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -1474,6 +1474,35 @@ BOOST_AUTO_TEST_CASE(test_ParseInt32) BOOST_CHECK(!ParseInt32("32482348723847471234", nullptr)); } +template +static void RunToIntegralTests() +{ + BOOST_CHECK(!ToIntegral(STRING_WITH_EMBEDDED_NULL_CHAR)); + BOOST_CHECK(!ToIntegral(" 1")); + BOOST_CHECK(!ToIntegral("1 ")); + BOOST_CHECK(!ToIntegral("1a")); + BOOST_CHECK(!ToIntegral("1.1")); + BOOST_CHECK(!ToIntegral("1.9")); + BOOST_CHECK(!ToIntegral("+01.9")); + BOOST_CHECK(!ToIntegral("-")); + BOOST_CHECK(!ToIntegral("+")); + BOOST_CHECK(!ToIntegral(" -1")); + BOOST_CHECK(!ToIntegral("-1 ")); + BOOST_CHECK(!ToIntegral(" -1 ")); + BOOST_CHECK(!ToIntegral("+1")); + BOOST_CHECK(!ToIntegral(" +1")); + BOOST_CHECK(!ToIntegral(" +1 ")); + BOOST_CHECK(!ToIntegral("+-1")); + BOOST_CHECK(!ToIntegral("-+1")); + BOOST_CHECK(!ToIntegral("++1")); + BOOST_CHECK(!ToIntegral("--1")); + BOOST_CHECK(!ToIntegral("")); + BOOST_CHECK(!ToIntegral("aap")); + BOOST_CHECK(!ToIntegral("0x1")); + BOOST_CHECK(!ToIntegral("-32482348723847471234")); + BOOST_CHECK(!ToIntegral("32482348723847471234")); +} + BOOST_AUTO_TEST_CASE(test_ToIntegral) { BOOST_CHECK_EQUAL(ToIntegral("1234").value(), 1'234); @@ -1486,27 +1515,14 @@ BOOST_AUTO_TEST_CASE(test_ToIntegral) BOOST_CHECK_EQUAL(ToIntegral("-1234").value(), -1'234); BOOST_CHECK_EQUAL(ToIntegral("-1").value(), -1); - BOOST_CHECK(!ToIntegral(" 1")); - BOOST_CHECK(!ToIntegral("1 ")); - BOOST_CHECK(!ToIntegral("1a")); - BOOST_CHECK(!ToIntegral("1.1")); - BOOST_CHECK(!ToIntegral("1.9")); - BOOST_CHECK(!ToIntegral("+01.9")); - BOOST_CHECK(!ToIntegral(" -1")); - BOOST_CHECK(!ToIntegral("-1 ")); - BOOST_CHECK(!ToIntegral(" -1 ")); - BOOST_CHECK(!ToIntegral("+1")); - BOOST_CHECK(!ToIntegral(" +1")); - BOOST_CHECK(!ToIntegral(" +1 ")); - BOOST_CHECK(!ToIntegral("+-1")); - BOOST_CHECK(!ToIntegral("-+1")); - BOOST_CHECK(!ToIntegral("++1")); - BOOST_CHECK(!ToIntegral("--1")); - BOOST_CHECK(!ToIntegral("")); - BOOST_CHECK(!ToIntegral("aap")); - BOOST_CHECK(!ToIntegral("0x1")); - BOOST_CHECK(!ToIntegral("-32482348723847471234")); - BOOST_CHECK(!ToIntegral("32482348723847471234")); + RunToIntegralTests(); + RunToIntegralTests(); + RunToIntegralTests(); + RunToIntegralTests(); + RunToIntegralTests(); + RunToIntegralTests(); + RunToIntegralTests(); + RunToIntegralTests(); BOOST_CHECK(!ToIntegral("-9223372036854775809")); BOOST_CHECK_EQUAL(ToIntegral("-9223372036854775808").value(), -9'223'372'036'854'775'807LL - 1LL); diff --git a/src/util/strencodings.cpp b/src/util/strencodings.cpp index 0aa80ea0aed..90bf39f0100 100644 --- a/src/util/strencodings.cpp +++ b/src/util/strencodings.cpp @@ -281,16 +281,11 @@ std::string DecodeBase32(const std::string& str, bool* pf_invalid) return std::string((const char*)vchRet.data(), vchRet.size()); } -[[nodiscard]] static bool ParsePrechecks(const std::string&); - namespace { template bool ParseIntegral(const std::string& str, T* out) { static_assert(std::is_integral::value); - if (!ParsePrechecks(str)) { - return false; - } // Replicate the exact behavior of strtol/strtoll/strtoul/strtoull when // handling leading +/- for backwards compatibility. if (str.length() >= 2 && str[0] == '+' && str[1] == '-') { diff --git a/src/util/strencodings.h b/src/util/strencodings.h index 1217572c451..07e19668902 100644 --- a/src/util/strencodings.h +++ b/src/util/strencodings.h @@ -97,7 +97,9 @@ constexpr inline bool IsSpace(char c) noexcept { } /** - * Convert string to integral type T. + * Convert string to integral type T. Leading whitespace, a leading +, or any + * trailing character fail the parsing. The required format expressed as regex + * is `-?[0-9]+`. * * @returns std::nullopt if the entire string could not be parsed, or if the * parsed value is not in the range representable by the type T. @@ -111,7 +113,7 @@ std::optional ToIntegral(const std::string& str) if (first_nonmatching != str.data() + str.size() || error_condition != std::errc{}) { return std::nullopt; } - return {result}; + return result; } /** From fa9d72a7947d2cff541794e21e0040c3c1d43b32 Mon Sep 17 00:00:00 2001 From: MarcoFalke Date: Mon, 4 Oct 2021 09:55:05 +0200 Subject: [PATCH 2/2] Remove unused ParseDouble and ParsePrechecks --- src/test/fuzz/parse_numbers.cpp | 3 --- src/test/util_tests.cpp | 26 -------------------------- src/util/strencodings.cpp | 25 ------------------------- src/util/strencodings.h | 7 ------- 4 files changed, 61 deletions(-) diff --git a/src/test/fuzz/parse_numbers.cpp b/src/test/fuzz/parse_numbers.cpp index 69e58c3f63c..33b50042908 100644 --- a/src/test/fuzz/parse_numbers.cpp +++ b/src/test/fuzz/parse_numbers.cpp @@ -14,9 +14,6 @@ FUZZ_TARGET(parse_numbers) (void)ParseMoney(random_string); - double d; - (void)ParseDouble(random_string, &d); - uint8_t u8; (void)ParseUInt8(random_string, &u8); diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index de78b1bb509..dbd94eedfa4 100644 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -1730,32 +1730,6 @@ BOOST_AUTO_TEST_CASE(test_ParseUInt64) BOOST_CHECK(!ParseUInt64("-1234", &n)); } -BOOST_AUTO_TEST_CASE(test_ParseDouble) -{ - double n; - // Valid values - BOOST_CHECK(ParseDouble("1234", nullptr)); - BOOST_CHECK(ParseDouble("0", &n) && n == 0.0); - BOOST_CHECK(ParseDouble("1234", &n) && n == 1234.0); - BOOST_CHECK(ParseDouble("01234", &n) && n == 1234.0); // no octal - BOOST_CHECK(ParseDouble("2147483647", &n) && n == 2147483647.0); - BOOST_CHECK(ParseDouble("-2147483648", &n) && n == -2147483648.0); - BOOST_CHECK(ParseDouble("-1234", &n) && n == -1234.0); - BOOST_CHECK(ParseDouble("1e6", &n) && n == 1e6); - BOOST_CHECK(ParseDouble("-1e6", &n) && n == -1e6); - // Invalid values - BOOST_CHECK(!ParseDouble("", &n)); - BOOST_CHECK(!ParseDouble(" 1", &n)); // no padding inside - BOOST_CHECK(!ParseDouble("1 ", &n)); - BOOST_CHECK(!ParseDouble("1a", &n)); - BOOST_CHECK(!ParseDouble("aap", &n)); - BOOST_CHECK(!ParseDouble("0x1", &n)); // no hex - BOOST_CHECK(!ParseDouble(STRING_WITH_EMBEDDED_NULL_CHAR, &n)); - // Overflow and underflow - BOOST_CHECK(!ParseDouble("-1e10000", nullptr)); - BOOST_CHECK(!ParseDouble("1e10000", nullptr)); -} - BOOST_AUTO_TEST_CASE(test_FormatParagraph) { BOOST_CHECK_EQUAL(FormatParagraph("", 79, 0), ""); diff --git a/src/util/strencodings.cpp b/src/util/strencodings.cpp index 90bf39f0100..53989a8d28e 100644 --- a/src/util/strencodings.cpp +++ b/src/util/strencodings.cpp @@ -302,17 +302,6 @@ bool ParseIntegral(const std::string& str, T* out) } }; // namespace -[[nodiscard]] static bool ParsePrechecks(const std::string& str) -{ - if (str.empty()) // No empty string allowed - return false; - if (str.size() >= 1 && (IsSpace(str[0]) || IsSpace(str[str.size()-1]))) // No padding allowed - return false; - if (!ValidAsCString(str)) // No embedded NUL characters allowed - return false; - return true; -} - bool ParseInt32(const std::string& str, int32_t* out) { return ParseIntegral(str, out); @@ -343,20 +332,6 @@ bool ParseUInt64(const std::string& str, uint64_t* out) return ParseIntegral(str, out); } -bool ParseDouble(const std::string& str, double *out) -{ - if (!ParsePrechecks(str)) - return false; - if (str.size() >= 2 && str[0] == '0' && str[1] == 'x') // No hexadecimal floats allowed - return false; - std::istringstream text(str); - text.imbue(std::locale::classic()); - double result; - text >> result; - if(out) *out = result; - return text.eof() && !text.fail(); -} - std::string FormatParagraph(const std::string& in, size_t width, size_t indent) { std::stringstream out; diff --git a/src/util/strencodings.h b/src/util/strencodings.h index 07e19668902..688707d1884 100644 --- a/src/util/strencodings.h +++ b/src/util/strencodings.h @@ -158,13 +158,6 @@ std::optional ToIntegral(const std::string& str) */ [[nodiscard]] bool ParseUInt64(const std::string& str, uint64_t *out); -/** - * Convert string to double with strict parse error feedback. - * @returns true if the entire string could be parsed as valid double, - * false if not the entire string could be parsed or when overflow or underflow occurred. - */ -[[nodiscard]] bool ParseDouble(const std::string& str, double *out); - /** * Convert a span of bytes to a lower-case hexadecimal string. */