From 23a5af3513f82ccdd134d0fe84db0af8548f866e Mon Sep 17 00:00:00 2001 From: Joshua Haberman Date: Tue, 25 Feb 2020 19:13:27 -0800 Subject: [PATCH] [json] fixed all remaining conformance bugs. --- upb/json_decode.c | 49 ++++++++++++++++++++++++++++++++++------------- upb/json_encode.c | 7 ++++--- upb/reflection.c | 12 ++++++++++++ upb/reflection.h | 3 +++ 4 files changed, 55 insertions(+), 16 deletions(-) diff --git a/upb/json_decode.c b/upb/json_decode.c index e879231f9f..54a55ad135 100644 --- a/upb/json_decode.c +++ b/upb/json_decode.c @@ -203,7 +203,7 @@ static bool jsondec_objnext(jsondec *d) { /* JSON number ****************************************************************/ -static void jsondec_skipdigits(jsondec *d) { +static bool jsondec_tryskipdigits(jsondec *d) { const char *start = d->ptr; while (d->ptr < d->end) { @@ -213,7 +213,11 @@ static void jsondec_skipdigits(jsondec *d) { d->ptr++; } - if (d->ptr == start) { + return d->ptr != start; +} + +static void jsondec_skipdigits(jsondec *d) { + if (!jsondec_tryskipdigits(d)) { jsondec_err(d, "Expected one or more digits"); } } @@ -225,9 +229,15 @@ static double jsondec_number(jsondec *d) { /* Skip over the syntax of a number, as specified by JSON. */ if (*d->ptr == '-') d->ptr++; - if (!jsondec_tryparsech(d, '0')) { + + if (jsondec_tryparsech(d, '0')) { + if (jsondec_tryskipdigits(d)) { + jsondec_err(d, "number cannot have leading zero"); + } + } else { jsondec_skipdigits(d); } + if (d->ptr == d->end) goto parse; if (jsondec_tryparsech(d, '.')) { jsondec_skipdigits(d); @@ -337,6 +347,8 @@ static size_t jsondec_unicode(jsondec *d, char* out) { cp = (high & 0x3ff) << 10; cp |= (low & 0x3ff); cp += 0x10000; + } else if (cp >= 0xdc00 && cp <= 0xdfff) { + jsondec_err(d, "Unpaired low surrogate"); } /* Write to UTF-8 */ @@ -846,6 +858,12 @@ static upb_msgval jsondec_msg(jsondec *d, const upb_fielddef *f) { return val; } +static bool jsondec_isvalue(const upb_fielddef *f) { + return upb_fielddef_type(f) == UPB_TYPE_MESSAGE && + upb_msgdef_wellknowntype(upb_fielddef_msgsubdef(f)) == + UPB_WELLKNOWN_VALUE; +} + static void jsondec_field(jsondec *d, upb_msg *msg, const upb_msgdef *m) { upb_strview name; const upb_fielddef *f; @@ -863,7 +881,12 @@ static void jsondec_field(jsondec *d, upb_msg *msg, const upb_msgdef *m) { return; } - if (jsondec_peek(d) == JD_NULL) { + if (upb_fielddef_containingoneof(f) && + upb_msg_hasoneof(msg, upb_fielddef_containingoneof(f))) { + jsondec_err(d, "More than one field for this oneof."); + } + + if (jsondec_peek(d) == JD_NULL && !jsondec_isvalue(f)) { /* JSON "null" indicates a default value, so no need to set anything. */ return jsondec_null(d); } @@ -1106,47 +1129,45 @@ static void jsondec_wellknownvalue(jsondec *d, upb_msg *msg, switch (jsondec_peek(d)) { case JD_NUMBER: /* double number_value = 2; */ - val.double_val = jsondec_number(d); f = upb_msgdef_itof(m, 2); + val.double_val = jsondec_number(d); break; case JD_STRING: /* string string_value = 3; */ - val.str_val = jsondec_string(d); f = upb_msgdef_itof(m, 3); + val.str_val = jsondec_string(d); break; case JD_FALSE: /* bool bool_value = 4; */ - val.bool_val = false; f = upb_msgdef_itof(m, 4); + val.bool_val = false; jsondec_false(d); break; case JD_TRUE: /* bool bool_value = 4; */ - val.bool_val = true; f = upb_msgdef_itof(m, 4); + val.bool_val = true; jsondec_true(d); break; case JD_NULL: /* NullValue null_value = 1; */ - val.int32_val = 0; f = upb_msgdef_itof(m, 1); + val.int32_val = 0; jsondec_null(d); break; /* Note: these cases return, because upb_msg_mutable() is enough. */ - case JD_OBJECT: { + case JD_OBJECT: /* Struct struct_value = 5; */ f = upb_msgdef_itof(m, 5); submsg = upb_msg_mutable(msg, f, d->arena).msg; jsondec_struct(d, submsg, upb_fielddef_msgsubdef(f)); return; - } - case JD_ARRAY: { + case JD_ARRAY: /* ListValue list_value = 6; */ f = upb_msgdef_itof(m, 6); submsg = upb_msg_mutable(msg, f, d->arena).msg; jsondec_listvalue(d, submsg, upb_fielddef_msgsubdef(f)); return; - } default: UPB_UNREACHABLE(); } @@ -1176,6 +1197,8 @@ static upb_strview jsondec_mask(jsondec *d, const char *buf, const char *end) { if (ch >= 'A' && ch <= 'Z') { *out++ = '_'; *out++ = ch + 32; + } else if (ch == '_') { + jsondec_err(d, "field mask may not contain '_'"); } else { *out++ = ch; } diff --git a/upb/json_encode.c b/upb/json_encode.c index caa0c8a234..285bcecea5 100644 --- a/upb/json_encode.c +++ b/upb/json_encode.c @@ -196,7 +196,6 @@ static void jsonenc_bytes(jsonenc *e, upb_strview str) { jsonenc_putbytes(e, buf, 4); break; case 1: - fprintf(stderr, "Base64 encode: %d\n", (int)ptr[0]); buf[0] = base64[ptr[0] >> 2]; buf[1] = base64[((ptr[0] & 0x3) << 4)]; buf[2] = '='; @@ -349,15 +348,17 @@ static void jsonenc_fieldpath(jsonenc *e, upb_strview path) { while (ptr < end) { char ch = *ptr; + if (ch >= 'A' && ch <= 'Z') { jsonenc_err(e, "Field mask element may not have upper-case letter."); } else if (ch == '_') { if (ptr == end - 1 || *(ptr + 1) < 'a' || *(ptr + 1) > 'z') { jsonenc_err(e, "Underscore must be followed by a lowercase letter."); } - } else { - jsonenc_putbytes(e, &ch, 1); + ch = *++ptr - 32; } + + jsonenc_putbytes(e, &ch, 1); ptr++; } } diff --git a/upb/reflection.c b/upb/reflection.c index b31a917469..481e3db943 100644 --- a/upb/reflection.c +++ b/upb/reflection.c @@ -91,6 +91,18 @@ bool upb_msg_has(const upb_msg *msg, const upb_fielddef *f) { } } +bool upb_msg_hasoneof(const upb_msg *msg, const upb_oneofdef *o) { + upb_oneof_iter i; + const upb_fielddef *f; + const upb_msglayout_field *field; + + upb_oneof_begin(&i, o); + if (upb_oneof_done(&i)) return false; + f = upb_oneof_iter_field(&i); + field = upb_fielddef_layout(f); + return *oneofcase(msg, field) != 0; +} + upb_msgval upb_msg_get(const upb_msg *msg, const upb_fielddef *f) { if (!upb_fielddef_haspresence(f) || upb_msg_has(msg, f)) { return _upb_msg_getraw(msg, f); diff --git a/upb/reflection.h b/upb/reflection.h index 95156b7d76..c55fe4a2e6 100644 --- a/upb/reflection.h +++ b/upb/reflection.h @@ -44,6 +44,9 @@ upb_mutmsgval upb_msg_mutable(upb_msg *msg, const upb_fielddef *f, upb_arena *a) /* May only be called for fields where upb_fielddef_haspresence(f) == true. */ bool upb_msg_has(const upb_msg *msg, const upb_fielddef *f); +/* Returns whether any field is set in the oneof. */ +bool upb_msg_hasoneof(const upb_msg *msg, const upb_oneofdef *o); + /* Sets the given field to the given value. For a msg/array/map/string, the * value must be in the same arena. */ void upb_msg_set(upb_msg *msg, const upb_fielddef *f, upb_msgval val,