Rapidjson: Parse<kParseNumbersAsStringsFlag>() raises an error on big integer

[lelit]

The following slightly modified simplereader.cpp:

#include "rapidjson/reader.h"
#include "rapidjson/error/en.h"
#include <iostream>

using namespace rapidjson;
using namespace std;

struct MyHandler {
    bool Null() { cout << "Null()" << endl; return true; }
    bool Bool(bool b) { cout << "Bool(" << boolalpha << b << ")" << endl; return true; }
    bool Int(int i) { cout << "Int(" << i << ")" << endl; return true; }
    bool Uint(unsigned u) { cout << "Uint(" << u << ")" << endl; return true; }
    bool Int64(int64_t i) { cout << "Int64(" << i << ")" << endl; return true; }
    bool Uint64(uint64_t u) { cout << "Uint64(" << u << ")" << endl; return true; }
    bool Double(double d) { cout << "Double(" << d << ")" << endl; return true; }
    bool RawNumber(const char* str, SizeType length, bool copy) { 
        cout << "Number(" << str << ", " << length << ", " << boolalpha << copy << ")" << endl;
        return true;
    }
    bool String(const char* str, SizeType length, bool copy) { 
        cout << "String(" << str << ", " << length << ", " << boolalpha << copy << ")" << endl;
        return true;
    }
    bool StartObject() { cout << "StartObject()" << endl; return true; }
    bool Key(const char* str, SizeType length, bool copy) {
        cout << "Key(" << str << ", " << length << ", " << boolalpha << copy << ")" << endl;
        return true;
    }
    bool EndObject(SizeType memberCount) { cout << "EndObject(" << memberCount << ")" << endl; return true; }
    bool StartArray() { cout << "StartArray()" << endl; return true; }
    bool EndArray(SizeType elementCount) { cout << "EndArray(" << elementCount << ")" << endl; return true; }
};

int main() {
    const char json[] = " { \"hello\" : \"world\", \"t\" : true , \"f\" : false, \"n\": null, \"i\":123, \"pi\": 3.1416, \"a\":[1, 2, 3, 4], \"bigint\":100000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000 } ";

    MyHandler handler;
    Reader reader;
    StringStream ss(json);
    if (!reader.Parse<kParseNumbersAsStringsFlag>(ss, handler)) {
        fprintf(stderr, "\nError(%u): %s\n", static_cast<unsigned>(reader.GetErrorOffset()), GetParseError_En(reader.GetParseErrorCode()));
        return 1;
    }
    return 0;
}

produces the following output:

StartObject()
Key(hello, 5, true)
String(world, 5, true)
Key(t, 1, true)
Bool(true)
Key(f, 1, true)
Bool(false)
Key(n, 1, true)
Null()
Key(i, 1, true)
Number(123, 3, true)
Key(pi, 2, true)
Number(3.1416, 6, true)
Key(a, 1, true)
StartArray()
Number(1, 1, true)
Number(2, 1, true)
Number(3, 1, true)
Number(4, 1, true)
EndArray(4)
Key(bigint, 6, true)

Error(109): Number too big to be stored in double.

Why does it try to convert the bigint to a C double, even with the kParseNumbersAsStringsFlag flag?

Am I missing something or is this an issue?

[miloyip]

Yes, I think when kParseNumbersAsStringsFlag is used, it should ignore the computation of mantissa and exponent and merely validate its syntax.
Do you want to submit a PR on this?

[lelit]

Further study seems to suggest that I was wrong: current master does not exhibit the problem, and by any chance I was tricked by a _system_ include when I compiled the modified simplereader.cpp above (my system's has v1.1.0 rapidjson-dev installed in /usr/include/rapidjson).

I installed the following patch in current master:

diff --git a/test/unittest/readertest.cpp b/test/unittest/readertest.cpp
index 2deadb79..e3d51481 100644
--- a/test/unittest/readertest.cpp
+++ b/test/unittest/readertest.cpp
@@ -1975,6 +1975,17 @@ TEST(Reader, NumbersAsStrings) {
         Reader reader;
         EXPECT_TRUE(reader.Parse<kParseNumbersAsStringsFlag>(s, h));
     }
+    {
+        char n1e319[321];   // '1' followed by 319 '0'
+        n1e319[0] = '1';
+        for (int i = 1; i < 320; i++)
+            n1e319[i] = '0';
+        n1e319[320] = '\0';
+        StringStream s(n1e319);
+        NumbersAsStringsHandler h(n1e319);
+        Reader reader;
+        EXPECT_TRUE(reader.Parse<kParseNumbersAsStringsFlag>(s, h));
+    }
 }

and it successfully passes, while it fails with v1.1.0.

So I tried to bisecting the problem, and narrowed the _fixup_ between commit 01c71740 (that fails) and commit 6cc3910a (correct). Could not figure out how to spot the exact commit because I was not able to compile most of changes in that range (that is 80dba56a..7101911d) due to syntax error:

../../include/rapidjson/internal/strtod.h:231:32: error: ‘INT_MAX’ was not declared in this scope
     RAPIDJSON_ASSERT(length <= INT_MAX);

If my analysis is correct, this issue can be closed.

Do you think the test above brings any benefit to the coverage? If so, I could submit a PR just for that.

[abolz]

The fix for this is in https://github.com/Tencent/rapidjson/pull/1290/commits/f5e5d47fac0f654749c4d6267015005b74643dff, which skips the overflow test in the kParseNumbersAsStringsFlag path. Your test would definitely bring some benefit to the coverage!!

[lelit]

Great, thank you, I was not able to spot the _fixing commit_... I'll try to propose a PR then!

[johndog]

Looks like this issue can be closed?