Fix parsing negative int64 of length 19 when double_fallback is enabled

Summary:
There was a bug in the logic where negative 18-digit numbers would be incorrectly assessed as not fitting int64_t even though they do.
The issue was that `integral.size() < maxIntLen` condition didn't work correctly for some negative numbers. In particular, for 18-digit negative numbers, `integral.size()` would be 19 (minus sign + 18 digits) which is not smaller than `maxIntLen` which is also 19. The subsequent check `(negative && integral.size() == minIntLen && integral <= minInt)` would also fail because `minIntLen` is 20.

Reviewed By: yfeldblum

Differential Revision: D33732185

fbshipit-source-id: 0ff70387e3b450acf45efc950a9114a13e752386

folly/json.cpp

@@ -546,19 +546,19 @@ dynamic parseNumber(Input& in) {
 
   auto const wasE = *in == 'e' || *in == 'E';
 
-  constexpr const char* maxInt = "9223372036854775807";
-  constexpr const char* minInt = "-9223372036854775808";
-  constexpr auto maxIntLen = constexpr_strlen(maxInt);
-  constexpr auto minIntLen = constexpr_strlen(minInt);
-
   if (*in != '.' && !wasE && in.getOpts().parse_numbers_as_strings) {
     return integral;
   }
 
+  constexpr const char* maxIntStr = "9223372036854775807";
+  constexpr const char* minIntStr = "-9223372036854775808";
+  constexpr auto maxIntLen = constexpr_strlen(maxIntStr);
+  constexpr auto minIntLen = constexpr_strlen(minIntStr);
+  auto extremaLen = negative ? minIntLen : maxIntLen;
+  auto extremaStr = negative ? minIntStr : maxIntStr;
   if (*in != '.' && !wasE) {
-    if (LIKELY(!in.getOpts().double_fallback || integral.size() < maxIntLen) ||
-        (!negative && integral.size() == maxIntLen && integral <= maxInt) ||
-        (negative && integral.size() == minIntLen && integral <= minInt)) {
+    if (LIKELY(!in.getOpts().double_fallback || integral.size() < extremaLen) ||
+        (integral.size() == extremaLen && integral <= extremaStr)) {
       auto val = to<int64_t>(integral);
       in.skipWhitespace();
       return val;

folly/test/JsonTest.cpp

@@ -16,6 +16,7 @@
 
 #include <folly/json.h>
 
+#include <cstdint>
 #include <iterator>
 #include <limits>
 
@@ -695,7 +696,7 @@ TEST(Json, ParseDoubleFallback) {
   EXPECT_EQ(
       std::numeric_limits<int64_t>::max(),
       parseJson("{\"a\":9223372036854775807}").items().begin()->second.asInt());
-  // with double_fallback
+  // with double_fallback numbers outside int64_t range are parsed as double
   folly::json::serialization_opts opts;
   opts.double_fallback = true;
   EXPECT_EQ(
@@ -722,6 +723,38 @@ TEST(Json, ParseDoubleFallback) {
           .items()
           .begin()
           ->second.asDouble());
+  // show that some precision gets lost
+  EXPECT_EQ(
+      847605071342477612345678900000.0,
+      parseJson("{\"a\":847605071342477612345678912345}", opts)
+          .items()
+          .begin()
+          ->second.asDouble());
+  EXPECT_DOUBLE_EQ(
+      -9223372036854775809.0,
+      parseJson("{\"a\":-9223372036854775809}", opts) // first smaller than min
+          .items()
+          .begin()
+          ->second.asDouble());
+  EXPECT_DOUBLE_EQ(
+      9223372036854775808.0,
+      parseJson("{\"a\":9223372036854775808}", opts) // first larger than max
+          .items()
+          .begin()
+          ->second.asDouble());
+  EXPECT_DOUBLE_EQ(
+      -10000000000000000000.0,
+      parseJson("{\"a\":-10000000000000000000}", opts) // minus + 20 digits
+          .items()
+          .begin()
+          ->second.asDouble());
+  EXPECT_DOUBLE_EQ(
+      10000000000000000000.0,
+      parseJson("{\"a\":10000000000000000000}", opts) // 20 digits
+          .items()
+          .begin()
+          ->second.asDouble());
+  // numbers within int64_t range are deserialized as int64_t, no loss
   EXPECT_EQ(
       std::numeric_limits<int64_t>::min(),
       parseJson("{\"a\":-9223372036854775808}", opts)
@@ -734,16 +767,48 @@ TEST(Json, ParseDoubleFallback) {
           .items()
           .begin()
           ->second.asInt());
-  // show that some precision gets lost
   EXPECT_EQ(
-      847605071342477612345678900000.0,
-      parseJson("{\"a\":847605071342477612345678912345}", opts)
+      INT64_C(-1234567890123456789),
+      parseJson("{\"a\":-1234567890123456789}", opts) // minus + 19 digits
           .items()
           .begin()
-          ->second.asDouble());
+          ->second.asInt());
+  EXPECT_EQ(
+      INT64_C(1234567890123456789),
+      parseJson("{\"a\":1234567890123456789}", opts) // 19 digits
+          .items()
+          .begin()
+          ->second.asInt());
+  EXPECT_EQ(
+      INT64_C(-123456789012345678),
+      parseJson("{\"a\":-123456789012345678}", opts) // minus + 18 digits
+          .items()
+          .begin()
+          ->second.asInt());
+  EXPECT_EQ(
+      INT64_C(123456789012345678),
+      parseJson("{\"a\":123456789012345678}", opts) // 18 digits
+          .items()
+          .begin()
+          ->second.asInt());
   EXPECT_EQ(
       toJson(parseJson(R"({"a":-9223372036854775808})", opts)),
       R"({"a":-9223372036854775808})");
+  EXPECT_EQ(
+      toJson(parseJson(R"({"a":9223372036854775807})", opts)),
+      R"({"a":9223372036854775807})");
+  EXPECT_EQ(
+      toJson(parseJson(R"({"a":-1234567890123456789})", opts)),
+      R"({"a":-1234567890123456789})");
+  EXPECT_EQ(
+      toJson(parseJson(R"({"a":1234567890123456789})", opts)),
+      R"({"a":1234567890123456789})");
+  EXPECT_EQ(
+      toJson(parseJson(R"({"a":-123456789012345678})", opts)),
+      R"({"a":-123456789012345678})");
+  EXPECT_EQ(
+      toJson(parseJson(R"({"a":123456789012345678})", opts)),
+      R"({"a":123456789012345678})");
 }
 
 TEST(Json, ParseNumbersAsStrings) {