diff --git a/src/libxrpl/protocol/STNumber.cpp b/src/libxrpl/protocol/STNumber.cpp index 3c5336405b..5944d4cd6b 100644 --- a/src/libxrpl/protocol/STNumber.cpp +++ b/src/libxrpl/protocol/STNumber.cpp @@ -263,36 +263,44 @@ numberFromJson(SField const& field, json::Value const& value) auto const negative = num.mantissa() < 0; auto mantissa = Number::externalToInternal(num.mantissa()); auto exponent = num.exponent(); - XRPL_ASSERT(negative == parts.negative, "numberFromJson : signs agree"); + + // Number zero has a special representation, so if the result is 0, nothing else needs to match. + // i.e. the Json string could be negative 0 ("-0") or a very "large" 0 ("0e99"), but they all + // represent 0. + bool const isZero = num == beast::kZero && parts.mantissa == 0; + XRPL_ASSERT(negative == parts.negative || isZero, "numberFromJson : signs agree"); static_assert( // NOLINTNEXTLINE(misc-redundant-expression) std::is_same_v); static_assert( // NOLINTNEXTLINE(misc-redundant-expression) std::is_same_v); - auto const minMantissa = Number::minMantissa(); - while (parts.exponent > exponent && parts.mantissa < minMantissa) + if (!isZero) { - parts.mantissa *= 10; - --parts.exponent; - } - while (parts.exponent < exponent && mantissa < minMantissa) - { - // This should only happen in the edge case where the internal mantissa > kMaxRep - mantissa *= 10; - --exponent; - } - while (parts.exponent < exponent && parts.mantissa % 10 == 0) - { - // LCOV_EXCL_START - // This will probably never be needed - parts.mantissa /= 10; - ++parts.exponent; - // LCOV_EXCL_STOP - } + auto constexpr minMantissa = Number::kMaxRep / 10; + while (parts.exponent > exponent && parts.mantissa < minMantissa) + { + parts.mantissa *= 10; + --parts.exponent; + } + while (parts.exponent < exponent && mantissa < minMantissa) + { + // This should only happen in the edge case where the internal mantissa > kMaxRep + mantissa *= 10; + --exponent; + } + while (parts.exponent < exponent && parts.mantissa % 10 == 0) + { + // LCOV_EXCL_START + // This will probably never be needed + parts.mantissa /= 10; + ++parts.exponent; + // LCOV_EXCL_STOP + } - if (negative != parts.negative || mantissa != parts.mantissa || exponent != parts.exponent) - Throw("number can not be represented"); + if (negative != parts.negative || mantissa != parts.mantissa || exponent != parts.exponent) + Throw("number can not be represented"); + } return STNumber{field, num}; } diff --git a/src/test/protocol/STNumber_test.cpp b/src/test/protocol/STNumber_test.cpp index 5c9c3fd83c..7339df7c04 100644 --- a/src/test/protocol/STNumber_test.cpp +++ b/src/test/protocol/STNumber_test.cpp @@ -108,23 +108,43 @@ struct STNumber_test : public beast::unit_test::Suite auto const minInt = std::to_string(std::numeric_limits::min()); if (Number::getMantissaScale() == MantissaRange::MantissaScale::Small) { - BEAST_EXPECT( - numberFromJson(sfNumber, maxInt) == - STNumber(sfNumber, Number{9'223'372'036'854'775, 3})); - BEAST_EXPECT( - numberFromJson(sfNumber, minInt) == - STNumber(sfNumber, Number{-9'223'372'036'854'775, 3})); + // min/maxInt can't be exactly represented with the small mantissa, so they + // don't parse, and are expected to throw. + try + { + auto const bad = numberFromJson(sfNumber, maxInt); + fail(); + } + catch (std::exception const& e) + { + BEAST_EXPECT(std::string(e.what()) == "number can not be represented"); + } + try + { + auto const bad = numberFromJson(sfNumber, minInt); + fail(); + } + catch (std::exception const& e) + { + BEAST_EXPECT(std::string(e.what()) == "number can not be represented"); + } } else { + // with large mantissas, maxint is fine BEAST_EXPECT( numberFromJson(sfNumber, maxInt) == STNumber(sfNumber, Number{9'223'372'036'854'775'807, 0})); - BEAST_EXPECT( - numberFromJson(sfNumber, minInt) == - STNumber( - sfNumber, - Number{true, 9'223'372'036'854'775'808ULL, 0, Number::Normalized{}})); + // but minint's mantissa is > kMaxRep, and so rounds, and thus can't be parsed + try + { + auto const bad = numberFromJson(sfNumber, minInt); + fail(); + } + catch (std::exception const& e) + { + BEAST_EXPECT(std::string(e.what()) == "number can not be represented"); + } } }