-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
BUG: json.decode fails for nums larger than sys.maxsize #34984
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 18 commits
d22da4d
46d05a3
cce506c
a0a6846
c60cabc
3c91879
e4eb0af
f1a3e0a
f3894e5
bfe6595
c18f685
4027704
e002cc4
aa74e5a
60cccc8
6fd68ef
74980f4
618f97f
eeea214
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -43,6 +43,7 @@ Numeric decoder derived from from TCL library | |||
#include <limits.h> | ||||
#include <locale.h> | ||||
#include <math.h> | ||||
#include <stdio.h> | ||||
#include <stdlib.h> | ||||
#include <string.h> | ||||
#include <wchar.h> | ||||
|
@@ -64,6 +65,7 @@ struct DecoderState { | |||
int escHeap; | ||||
int lastType; | ||||
JSUINT32 objDepth; | ||||
char *cStr; // storage for BigNum | ||||
void *prv; | ||||
JSONObjectDecoder *dec; | ||||
}; | ||||
|
@@ -118,6 +120,7 @@ FASTCALL_ATTR JSOBJ FASTCALL_MSVC decode_numeric(struct DecoderState *ds) { | |||
int intNeg = 1; | ||||
int mantSize = 0; | ||||
JSUINT64 intValue; | ||||
JSLONG newDigit; | ||||
int chr; | ||||
int decimalCount = 0; | ||||
double frcValue = 0.0; | ||||
|
@@ -160,12 +163,35 @@ FASTCALL_ATTR JSOBJ FASTCALL_MSVC decode_numeric(struct DecoderState *ds) { | |||
// FIXME: Check for arithmetic overflow here | ||||
// PERF: Don't do 64-bit arithmetic here unless we know we have | ||||
// to | ||||
intValue = intValue * 10ULL + (JSLONG)(chr - 48); | ||||
newDigit = (JSLONG)(chr - 48); | ||||
|
||||
// TO DO: need to fix overflow catching | ||||
if (intValue*10ULL + newDigit > overflowLimit) { | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this works generally. If you want a reference for overflow detection try looking at the python implementation of PyLong_AsLongLongAndOverflow There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel like this might be ok because overflowLimit is adjusted earlier on depending on whether the number is positive or negative (in line 140
But I'm down to rewrite this part to follow the PyLong_AsLongLongAndOverflow implementation:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The problem is if |
||||
|
||||
// convert current inValue into string | ||||
int length = snprintf( NULL, 0, "%lu", intValue); | ||||
char* intValue_asStr = malloc( length + 1 ); | ||||
snprintf(intValue_asStr, length + 1, "%lu", intValue); | ||||
|
||||
if (strlen(ds->cStr)== 0) { // first overflow | ||||
ds->cStr = (char*)realloc(ds->cStr, strlen(intValue_asStr)+1); | ||||
strcpy(ds->cStr, intValue_asStr); | ||||
} else { // has overflown before | ||||
char* cStr_prev = malloc(strlen(ds->cStr)); | ||||
memcpy(cStr_prev, ds->cStr, strlen(ds->cStr)); | ||||
|
||||
size_t new_size = strlen(ds->cStr) + strlen(intValue_asStr); | ||||
ds->cStr = (char*)realloc(ds->cStr, new_size); | ||||
|
||||
strcpy(ds->cStr, cStr_prev); | ||||
strcat(ds->cStr, intValue_asStr); | ||||
} | ||||
|
||||
if (intValue > overflowLimit) { | ||||
return SetError(ds, -1, overflowLimit == LLONG_MAX | ||||
? "Value is too big" | ||||
: "Value is too small"); | ||||
// then reset intValue | ||||
intValue = (newDigit==0) ? 10 : newDigit; | ||||
} | ||||
else { | ||||
intValue = intValue * 10ULL + newDigit; | ||||
} | ||||
|
||||
offset++; | ||||
|
@@ -196,7 +222,25 @@ FASTCALL_ATTR JSOBJ FASTCALL_MSVC decode_numeric(struct DecoderState *ds) { | |||
ds->lastType = JT_INT; | ||||
ds->start = offset; | ||||
|
||||
if ((intValue >> 31)) { | ||||
// check if ds->cStr has been written to | ||||
if (strlen(ds->cStr)>0){ | ||||
|
||||
// covert intValue to cString | ||||
int length = snprintf( NULL, 0, "%lu", intValue); | ||||
char* intValue_asStr = malloc( length + 1 ); | ||||
snprintf(intValue_asStr, length + 1, "%lu", intValue); | ||||
|
||||
char* cStr_prev = malloc(strlen(ds->cStr)); | ||||
memcpy(cStr_prev, ds->cStr, strlen(ds->cStr)); | ||||
|
||||
size_t new_size = strlen(ds->cStr) + strlen(intValue_asStr); | ||||
ds->cStr = (char*)realloc(ds->cStr, new_size); | ||||
strcpy(ds->cStr, cStr_prev); | ||||
strcat(ds->cStr, intValue_asStr); | ||||
|
||||
return ds->dec->newBigNum(ds->prv, ds->cStr); | ||||
} | ||||
else if ((intValue >> 31)) { | ||||
return ds->dec->newLong(ds->prv, (JSINT64)(intValue * (JSINT64)intNeg)); | ||||
} else { | ||||
return ds->dec->newInt(ds->prv, (JSINT32)(intValue * intNeg)); | ||||
|
@@ -1170,6 +1214,9 @@ JSOBJ JSON_DecodeObject(JSONObjectDecoder *dec, const char *buffer, | |||
ds.dec->errorStr = NULL; | ||||
ds.dec->errorOffset = NULL; | ||||
ds.objDepth = 0; | ||||
|
||||
ds.cStr = malloc(sizeof("\0")); | ||||
strcpy(ds.cStr, "\0"); | ||||
|
||||
ds.dec = dec; | ||||
|
||||
|
@@ -1198,5 +1245,7 @@ JSOBJ JSON_DecodeObject(JSONObjectDecoder *dec, const char *buffer, | |||
return SetError(&ds, -1, "Trailing data"); | ||||
} | ||||
|
||||
free(ds.cStr); | ||||
|
||||
return ret; | ||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to change the struct here; just look at the existing implementation of
decode_string
and mirror that