From eb3d5fa453bd04cc116e704c36a97250c93b7b4b Mon Sep 17 00:00:00 2001 From: warriorofwire <3454741+WarriorOfWire@users.noreply.github.com> Date: Sun, 10 May 2020 20:45:42 -0700 Subject: ujson: do not eat trailing whitespace Ujson should only worry about whitespace before JSON. This becomes apparent when you are using MP stream protocol to read directly from input buffers. When you attempt to read(1) on a UART (and possibly other protocols) you have to wait for either the byte or the timeout. Fixes: - Waiting for a timeout after you have completed reading a correct and complete JSON off the input. - Raising an OSError after reading a correct and complete JSON off the input. - Eating more data than semantically owned off the input buffer. - Blocking to start parsing JSON until the entire JSON body has been loaded into a potentially large, contiguous Python object. Code you would write before: ``` line = board_busio_uart_port.read_line() json_dict = json.loads(line) ``` or reaching for fixed buffers and swapping them around in Python. Code that did not work before that does now: ``` json_dict = json.load(board_busio_uart_port) ``` - This removes the need for intermediate copies of data when reading JSON from micropython stream protocol inputs. - It also increases total application speed by parsing JSON concurrently with receiving on boards that read from UART via DMA. - It simplifies code that users write while improving their apps. --- extmod/modujson.c | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) (limited to 'extmod') diff --git a/extmod/modujson.c b/extmod/modujson.c index 6b24bf578..daefe18ba 100644 --- a/extmod/modujson.c +++ b/extmod/modujson.c @@ -53,6 +53,10 @@ STATIC mp_obj_t mod_ujson_dumps(mp_obj_t obj) { } STATIC MP_DEFINE_CONST_FUN_OBJ_1(mod_ujson_dumps_obj, mod_ujson_dumps); +#define JSON_DEBUG(...) (void)0 +// #define JSON_DEBUG(...) mp_printf(&mp_plat_print __VA_OPT__(,) __VA_ARGS__) + + // The function below implements a simple non-recursive JSON parser. // // The JSON specification is at http://www.ietf.org/rfc/rfc4627.txt @@ -80,6 +84,7 @@ typedef struct _ujson_stream_t { STATIC byte ujson_stream_next(ujson_stream_t *s) { mp_uint_t ret = s->read(s->stream_obj, &s->cur, 1, &s->errcode); + JSON_DEBUG(" usjon_stream_next err:%2d cur: %c \n", s->errcode, s->cur); if (s->errcode != 0) { mp_raise_OSError(s->errcode); } @@ -92,6 +97,7 @@ STATIC byte ujson_stream_next(ujson_stream_t *s) { STATIC mp_obj_t mod_ujson_load(mp_obj_t stream_obj) { const mp_stream_p_t *stream_p = mp_get_stream_raise(stream_obj, MP_STREAM_OP_READ); ujson_stream_t s = {stream_obj, stream_p->read, 0, 0}; + JSON_DEBUG("got JSON stream\n"); vstr_t vstr; vstr_init(&vstr, 8); mp_obj_list_t stack; // we use a list as a simple stack for nested JSON @@ -101,6 +107,15 @@ STATIC mp_obj_t mod_ujson_load(mp_obj_t stream_obj) { mp_obj_type_t *stack_top_type = NULL; mp_obj_t stack_key = MP_OBJ_NULL; S_NEXT(s); + // Eat _leading_ whitespace. + // If we eat trailing whitespace we will block for timeout on streams like UART that + // wait for requested data. Furthermore, it is an OSError to read(1) and incur + // a timeout on those APIs. + // For these reasons, we must only eat _leading_ whitespace. + while (unichar_isspace(S_CUR(s))) { + JSON_DEBUG("Eating leading whitespace"); + S_NEXT(s); + } for (;;) { cont: if (S_END(s)) { @@ -262,14 +277,9 @@ STATIC mp_obj_t mod_ujson_load(mp_obj_t stream_obj) { } } success: - // eat trailing whitespace - while (unichar_isspace(S_CUR(s))) { - S_NEXT(s); - } - if (!S_END(s)) { - // unexpected chars - goto fail; - } + // It is legal for a stream to have contents before and after JSON. + // If this parser has consumed a full successful JSON and its parse + // stack is empty, the parse has succeeded. if (stack_top == MP_OBJ_NULL || stack.len != 0) { // not exactly 1 object goto fail; -- cgit v1.2.3 From d8491f31768b04322728bc436b2f8e6c5f2bf0d6 Mon Sep 17 00:00:00 2001 From: warriorofwire <3454741+WarriorOfWire@users.noreply.github.com> Date: Sun, 10 May 2020 21:56:01 -0700 Subject: ujson: back out overeager loads() change; only change load() --- extmod/modujson.c | 33 +++++++++++++++++++-------------- 1 file changed, 19 insertions(+), 14 deletions(-) (limited to 'extmod') diff --git a/extmod/modujson.c b/extmod/modujson.c index daefe18ba..0f93ccb11 100644 --- a/extmod/modujson.c +++ b/extmod/modujson.c @@ -94,7 +94,7 @@ STATIC byte ujson_stream_next(ujson_stream_t *s) { return s->cur; } -STATIC mp_obj_t mod_ujson_load(mp_obj_t stream_obj) { +STATIC mp_obj_t _mod_ujson_load(mp_obj_t stream_obj, bool return_first_json) { const mp_stream_p_t *stream_p = mp_get_stream_raise(stream_obj, MP_STREAM_OP_READ); ujson_stream_t s = {stream_obj, stream_p->read, 0, 0}; JSON_DEBUG("got JSON stream\n"); @@ -107,15 +107,6 @@ STATIC mp_obj_t mod_ujson_load(mp_obj_t stream_obj) { mp_obj_type_t *stack_top_type = NULL; mp_obj_t stack_key = MP_OBJ_NULL; S_NEXT(s); - // Eat _leading_ whitespace. - // If we eat trailing whitespace we will block for timeout on streams like UART that - // wait for requested data. Furthermore, it is an OSError to read(1) and incur - // a timeout on those APIs. - // For these reasons, we must only eat _leading_ whitespace. - while (unichar_isspace(S_CUR(s))) { - JSON_DEBUG("Eating leading whitespace"); - S_NEXT(s); - } for (;;) { cont: if (S_END(s)) { @@ -277,9 +268,19 @@ STATIC mp_obj_t mod_ujson_load(mp_obj_t stream_obj) { } } success: - // It is legal for a stream to have contents before and after JSON. - // If this parser has consumed a full successful JSON and its parse - // stack is empty, the parse has succeeded. + // It is legal for a stream to have contents after JSON. + // E.g., A UART is not closed after receiving an object; in load() we will + // return the first complete JSON object, while in loads() we will retain + // strict adherence to the buffer's complete semantic. + if (!return_first_json) { + while (unichar_isspace(S_CUR(s))) { + S_NEXT(s); + } + if (!S_END(s)) { + // unexpected chars + goto fail; + } + } if (stack_top == MP_OBJ_NULL || stack.len != 0) { // not exactly 1 object goto fail; @@ -290,6 +291,10 @@ STATIC mp_obj_t mod_ujson_load(mp_obj_t stream_obj) { fail: mp_raise_ValueError(translate("syntax error in JSON")); } + +STATIC mp_obj_t mod_ujson_load(mp_obj_t stream_obj) { + return _mod_ujson_load(stream_obj, true); +} STATIC MP_DEFINE_CONST_FUN_OBJ_1(mod_ujson_load_obj, mod_ujson_load); STATIC mp_obj_t mod_ujson_loads(mp_obj_t obj) { @@ -297,7 +302,7 @@ STATIC mp_obj_t mod_ujson_loads(mp_obj_t obj) { const char *buf = mp_obj_str_get_data(obj, &len); vstr_t vstr = {len, len, (char*)buf, true}; mp_obj_stringio_t sio = {{&mp_type_stringio}, &vstr, 0, MP_OBJ_NULL}; - return mod_ujson_load(MP_OBJ_FROM_PTR(&sio)); + return _mod_ujson_load(MP_OBJ_FROM_PTR(&sio), false); } STATIC MP_DEFINE_CONST_FUN_OBJ_1(mod_ujson_loads_obj, mod_ujson_loads); -- cgit v1.2.3