netmail-open / wjelement

advanced, flexible JSON manipulation in C
GNU Lesser General Public License v3.0
108 stars 56 forks source link

Value truncated #112

Closed reister-kenneth closed 1 year ago

reister-kenneth commented 1 year ago

The following code is broken. For the time being I've modified the following lines of code to prevent the truncation (2^32 - 1)

    INT64 testvalue = 0xFFFFFFFFFFFF;
    char* json_string = "{ \"i64\": 281474976710655}";

    WJReader reader = WJROpenMemDocument(json_string, NULL, 0);
    WJElement jsonClone = WJEOpenDocument(reader, NULL, NULL, NULL);
    WJRCloseDocument(reader);

    INT64 iVal = WJEInt64(jsonClone, "i64", WJE_GET, 0);
    if (iVal != testvalue)
    {
        //We're broke!
    }
    //This test is to detect if we upgrade libraray WJElement and our parsing of INT64 values then breaks
    WJECloseDocument(jsonClone);

wjreader.c

~line 1323
            case WJR_TYPE_MIXED:
                ((WJRMixedNumber *) value)->hasDecimalPoint = FALSE;
                ((WJRMixedNumber *) value)->i = strtoll(doc->read, &end, 0);      //    <------ swapped strtoul for strtoll

~line 1364
                case WJR_TYPE_MIXED:
                    ((WJRMixedNumber *) value)->hasDecimalPoint = FALSE;
                    ((WJRMixedNumber *) value)->i = strtoll(doc->read, &end, 0);      //    <------ swapped strtoul for strtoll
penduin commented 1 year ago

Interesting. What architecture and compiler are you using? If I run your sample code (and use "int64" instead of "INT64") iVal and testvalue match, with or without your wjreader.c changes.

reister-kenneth commented 1 year ago

Windows 32 bit, but that doesn't matter. On Windows, long is 4 bytes no matter if your binary is 32 or 64 bit. Reference for strtoul https://learn.microsoft.com/en-us/cpp/c-runtime-library/reference/strtoul-strtoul-l-wcstoul-wcstoul-l?view=msvc-170

reister-kenneth commented 1 year ago

My call stack in case it's helpful. Maybe something else is not working right? Load thinks the data is of type WJR_TYPE_NUMBER.

WJRNumber(void * value, WJRNumberType type, WJReaderPublic * indoc) Line 1325   C
WJRIntOrDouble(WJReaderPublic * doc, unsigned __int64 * i, double * d) Line 1452    C
_WJELoad(_WJElement * parent, WJReaderPublic * reader, char * where, int(*)(WJElementPublic *, char *, void *, const char *, const int) loadcb, void * data, const char * file, const int line) Line 284    C
_WJELoad(_WJElement * parent, WJReaderPublic * reader, char * where, int(*)(WJElementPublic *, char *, void *, const char *, const int) loadcb, void * data, const char * file, const int line) Line 255    C
_WJEOpenDocument(WJReaderPublic * reader, char * where, int(*)(WJElementPublic *, char *, void *, const char *, const int) loadcb, void * data, const char * file, const int line) Line 318 C
penduin commented 1 year ago

your change looks good, and shouldn't introduce any side-effects that i can see. merged!
thanks @reister-kenneth