SERVER-23702 Fix server crashing on invalid decimal toString invariant check

3 files changed, 45 insertions, 28 deletions

diff --git a/src/mongo/platform/decimal128.cpp b/src/mongo/platform/decimal128.cpp
index 990b18e1569..def39a899e9 100644
--- a/src/mongo/platform/decimal128.cpp
+++ b/src/mongo/platform/decimal128.cpp
@@ -371,6 +371,17 @@ double Decimal128::toDouble(std::uint32_t* signalingFlags, RoundingMode roundMod
 }
 
 std::string Decimal128::toString() const {
+    // If the decimal is a variant of NaN (i.e. sNaN, -NaN, +NaN, etc...) or a variant of
+    // Inf (i.e. +Inf, Inf, -Inf), return either NaN, Infinity, or -Infinity
+    if (!isFinite()) {
+        if (this->isEqual(kPositiveInfinity)) {
+            return "Infinity";
+        } else if (this->isEqual(kNegativeInfinity)) {
+            return "-Infinity";
+        }
+        invariant(isNaN());
+        return "NaN";
+    }
     BID_UINT128 dec128 = decimal128ToLibraryType(_value);
     char decimalCharRepresentation[1 /* mantissa sign */ + 34 /* mantissa */ +
                                    1 /* scientific E */ + 1 /* exponent sign */ + 4 /* exponent */ +
@@ -389,16 +400,7 @@ std::string Decimal128::toString() const {
 
     std::string dec128String(decimalCharRepresentation);
 
-    // If the string is NaN or Infinity, return either NaN, +Inf, or -Inf
     std::string::size_type ePos = dec128String.find("E");
-    if (ePos == std::string::npos) {
-        if (dec128String == "-NaN" || dec128String == "+NaN")
-            return "NaN";
-        if (dec128String[0] == '+')
-            return "Inf";
-        invariant(dec128String == "-Inf");
-        return dec128String;
-    }
 
     // Calculate the precision and exponent of the number and output it in a readable manner
     int precision = 0;
@@ -483,6 +485,10 @@ bool Decimal128::isInfinite() const {
     return bid128_isInf(decimal128ToLibraryType(_value));
 }
 
+bool Decimal128::isFinite() const {
+    return bid128_isFinite(decimal128ToLibraryType(_value));
+}
+
 bool Decimal128::isNegative() const {
     return bid128_isSigned(decimal128ToLibraryType(_value));
 }
diff --git a/src/mongo/platform/decimal128.h b/src/mongo/platform/decimal128.h
index 4ef6c8da60b..f347ec42f70 100644
--- a/src/mongo/platform/decimal128.h
+++ b/src/mongo/platform/decimal128.h
@@ -304,6 +304,11 @@ public:
     bool isNegative() const;
 
     /**
+     * Return true if and only if a Decimal128 is Zero, Normal, or Subnormal (not Inf or NaN)
+     */
+    bool isFinite() const;
+
+    /**
      * This set of mathematical operation functions implement the corresponding
      * IEEE 754-2008 operations on self and other.
      * The operations are commutative, so a.add(b) is equivalent to b.add(a).
diff --git a/src/mongo/platform/decimal128_test.cpp b/src/mongo/platform/decimal128_test.cpp
index 7846ef58bf0..0632de053e6 100644
--- a/src/mongo/platform/decimal128_test.cpp
+++ b/src/mongo/platform/decimal128_test.cpp
@@ -661,13 +661,6 @@ TEST(Decimal128Test, TestDecimal128ToStringNeg) {
     ASSERT_EQUALS(result, "-2.087015E-278");
 }
 
-TEST(Decimal128Test, TestDecimal128ToStringPosNaN) {
-    std::string s = "+NaN";
-    Decimal128 d(s);
-    std::string result = d.toString();
-    ASSERT_EQUALS(result, "NaN");
-}
-
 TEST(Decimal128Test, TestDecimal128ToStringInRangeZero1) {
     std::string s = "0";
     Decimal128 d(s);
@@ -823,25 +816,38 @@ TEST(Decimal128Test, TestDecimal128ToStringOutRangePos3) {
     ASSERT_EQUALS(result, "1.234567890123456789012345678901234E+33");
 }
 
-TEST(Decimal128Test, TestDecimal128ToStringNegNaN) {
-    std::string s = "-NaN";
+TEST(Decimal128Test, TestDecimal128ToStringInvalidToNaN) {
+    std::string s = "Some garbage string";
     Decimal128 d(s);
-    std::string result = d.toString();
-    ASSERT_EQUALS(result, "NaN");
+    ASSERT_EQUALS(d.toString(), "NaN");
+}
+
+TEST(Decimal128Test, TestDecimal128ToStringNaN) {
+    std::string s[3] = {"-NaN", "+NaN", "NaN"};
+    for (auto& item : s) {
+        Decimal128 d(item);
+        ASSERT_EQUALS(d.toString(), "NaN");
+    }
+
+    // Testing a NaN with a payload
+    Decimal128 payloadNaN(Decimal128::Value({/*payload*/ 0x1, 0x7cull << 56}));
+    ASSERT_EQUALS(payloadNaN.toString(), "NaN");
 }
 
 TEST(Decimal128Test, TestDecimal128ToStringPosInf) {
-    std::string s = "+Infinity";
-    Decimal128 d(s);
-    std::string result = d.toString();
-    ASSERT_EQUALS(result, "Inf");
+    std::string s[3] = {"Inf", "Infinity", "+Inf"};
+    for (auto& item : s) {
+        Decimal128 d(item);
+        ASSERT_EQUALS(d.toString(), "Infinity");
+    }
 }
 
 TEST(Decimal128Test, TestDecimal128ToStringNegInf) {
-    std::string s = "-Infinity";
-    Decimal128 d(s);
-    std::string result = d.toString();
-    ASSERT_EQUALS(result, "-Inf");
+    std::string s[2] = {"-Infinity", "-Inf"};
+    for (auto& item : s) {
+        Decimal128 d(item);
+        ASSERT_EQUALS(d.toString(), "-Infinity");
+    }
 }
 
 // Tests for Decimal128 operations that use a signaling flag