Skip to content
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

Cherrypick: Fixed non-conformance in JSON parsing for empty strings in numeric fields #19259

Merged
merged 2 commits into from
Nov 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 14 additions & 6 deletions php/ext/google/protobuf/message.c
Original file line number Diff line number Diff line change
Expand Up @@ -722,12 +722,20 @@ PHP_METHOD(Message, mergeFromJsonString) {
}

upb_Status_Clear(&status);
if (!upb_JsonDecode(data, data_len, intern->msg, intern->desc->msgdef,
DescriptorPool_GetSymbolTable(), options, arena,
&status)) {
zend_throw_exception_ex(NULL, 0, "Error occurred during parsing: %s",
upb_Status_ErrorMessage(&status));
return;
int result = upb_JsonDecodeDetectingNonconformance(
data, data_len, intern->msg, intern->desc->msgdef,
DescriptorPool_GetSymbolTable(), options, arena, &status);

switch (result) {
case kUpb_JsonDecodeResult_Ok:
break;
case kUpb_JsonDecodeResult_OkWithEmptyStringNumerics:
zend_error(E_USER_WARNING, "%s", upb_Status_ErrorMessage(&status));
return;
case kUpb_JsonDecodeResult_Error:
zend_throw_exception_ex(NULL, 0, "Error occurred during parsing: %s",
upb_Status_ErrorMessage(&status));
return;
}
}

Expand Down
49 changes: 39 additions & 10 deletions php/ext/google/protobuf/php-upb.c
Original file line number Diff line number Diff line change
Expand Up @@ -523,6 +523,7 @@ typedef struct {
upb_Arena* arena; /* TODO: should we have a tmp arena for tmp data? */
const upb_DefPool* symtab;
int depth;
int result;
upb_Status* status;
jmp_buf err;
int line;
Expand Down Expand Up @@ -1152,6 +1153,16 @@ static int64_t jsondec_strtoint64(jsondec* d, upb_StringView str) {
return ret;
}

static void jsondec_checkempty(jsondec* d, upb_StringView str,
const upb_FieldDef* f) {
if (str.size != 0) return;
d->result = kUpb_JsonDecodeResult_OkWithEmptyStringNumerics;
upb_Status_SetErrorFormat(d->status,
"Empty string is not a valid number (field: %s). "
"This will be an error in a future version.",
upb_FieldDef_FullName(f));
}

/* Primitive value types ******************************************************/

/* Parse INT32 or INT64 value. */
Expand All @@ -1173,6 +1184,7 @@ static upb_MessageValue jsondec_int(jsondec* d, const upb_FieldDef* f) {
}
case JD_STRING: {
upb_StringView str = jsondec_string(d);
jsondec_checkempty(d, str, f);
val.int64_val = jsondec_strtoint64(d, str);
break;
}
Expand Down Expand Up @@ -1210,6 +1222,7 @@ static upb_MessageValue jsondec_uint(jsondec* d, const upb_FieldDef* f) {
}
case JD_STRING: {
upb_StringView str = jsondec_string(d);
jsondec_checkempty(d, str, f);
val.uint64_val = jsondec_strtouint64(d, str);
break;
}
Expand Down Expand Up @@ -1238,14 +1251,26 @@ static upb_MessageValue jsondec_double(jsondec* d, const upb_FieldDef* f) {
break;
case JD_STRING:
str = jsondec_string(d);
if (jsondec_streql(str, "NaN")) {
if (str.size == 0) {
jsondec_checkempty(d, str, f);
val.double_val = 0.0;
} else if (jsondec_streql(str, "NaN")) {
val.double_val = NAN;
} else if (jsondec_streql(str, "Infinity")) {
val.double_val = INFINITY;
} else if (jsondec_streql(str, "-Infinity")) {
val.double_val = -INFINITY;
} else {
val.double_val = strtod(str.data, NULL);
char* end;
val.double_val = strtod(str.data, &end);
if (end != str.data + str.size) {
d->result = kUpb_JsonDecodeResult_OkWithEmptyStringNumerics;
upb_Status_SetErrorFormat(
d->status,
"Non-number characters in quoted number (field: %s). "
"This will be an error in a future version.",
upb_FieldDef_FullName(f));
}
}
break;
default:
Expand Down Expand Up @@ -1987,10 +2012,10 @@ static void jsondec_wellknown(jsondec* d, upb_Message* msg,
}
}

static bool upb_JsonDecoder_Decode(jsondec* const d, upb_Message* const msg,
const upb_MessageDef* const m) {
static int upb_JsonDecoder_Decode(jsondec* const d, upb_Message* const msg,
const upb_MessageDef* const m) {
UPB_ASSERT(!upb_Message_IsFrozen(msg));
if (UPB_SETJMP(d->err)) return false;
if (UPB_SETJMP(d->err)) return kUpb_JsonDecodeResult_Error;

jsondec_tomsg(d, msg, m);

Expand All @@ -1999,16 +2024,19 @@ static bool upb_JsonDecoder_Decode(jsondec* const d, upb_Message* const msg,
jsondec_consumews(d);

if (d->ptr == d->end) {
return true;
return d->result;
} else {
jsondec_seterrmsg(d, "unexpected trailing characters");
return false;
return kUpb_JsonDecodeResult_Error;
}
}

bool upb_JsonDecode(const char* buf, size_t size, upb_Message* msg,
const upb_MessageDef* m, const upb_DefPool* symtab,
int options, upb_Arena* arena, upb_Status* status) {
int upb_JsonDecodeDetectingNonconformance(const char* buf, size_t size,
upb_Message* msg,
const upb_MessageDef* m,
const upb_DefPool* symtab,
int options, upb_Arena* arena,
upb_Status* status) {
UPB_ASSERT(!upb_Message_IsFrozen(msg));
jsondec d;

Expand All @@ -2021,6 +2049,7 @@ bool upb_JsonDecode(const char* buf, size_t size, upb_Message* msg,
d.status = status;
d.options = options;
d.depth = 64;
d.result = kUpb_JsonDecodeResult_Ok;
d.line = 1;
d.line_begin = d.ptr;
d.debug_field = NULL;
Expand Down
Loading
Loading