From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id DE74846970E for ; Sat, 21 Dec 2019 17:56:12 +0300 (MSK) Content-Type: text/plain; charset=utf-8 Mime-Version: 1.0 (Mac OS X Mail 13.0 \(3594.4.19\)) From: Roman Khabibov In-Reply-To: <7e6ac7e1-8d82-7a64-3d05-0de305917528@tarantool.org> Date: Sat, 21 Dec 2019 17:56:10 +0300 Content-Transfer-Encoding: quoted-printable Message-Id: References: <7e6ac7e1-8d82-7a64-3d05-0de305917528@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v2 2/2] json: print context in error mesages List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Alexander Turenko Cc: tarantool-patches@dev.tarantool.org, Vladislav Shpilevoy Hi Thanks for the review. Vlad, I=E2=80=99m ok with it. Alexander, can = you, please, do a second review? = https://github.com/tarantool/tarantool/tree/romanhabibov/gh-4339-json-err > On Dec 20, 2019, at 02:19, Vladislav Shpilevoy = wrote: >=20 > Hi! Thanks for the patch! >=20 > See my review fixes here and on top of the branch. >=20 > Please, review them, and if you are ok, then squash. > In that case it will be LGTM. >=20 > = =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D >=20 > diff --git a/third_party/lua-cjson/lua_cjson.c = b/third_party/lua-cjson/lua_cjson.c > index 655d6550e..79b0253f0 100644 > --- a/third_party/lua-cjson/lua_cjson.c > +++ b/third_party/lua-cjson/lua_cjson.c > @@ -847,26 +847,24 @@ static void fill_context(char *context, = json_parse_t *json, int column_index) > { > assert(column_index >=3D 0); > int length_before =3D column_index < CONTEXT_MAX_LENGTH_BEFORE ? > - column_index : CONTEXT_MAX_LENGTH_BEFORE; > + column_index : CONTEXT_MAX_LENGTH_BEFORE; > const char *src =3D json->cur_line_ptr + column_index - = length_before; > - int i =3D 0; > /* Fill context before the arrow. */ > - for (; i < length_before; i++) > - context[i] =3D src[i]; > + memcpy(context, src, length_before); > + context +=3D length_before; > + src +=3D length_before; >=20 > /* Make the arrow. */ > - context[i] =3D ' '; > - memset(context + i + 1, '>', CONTEXT_ARROW_LENGTH - 2); > - context[i + CONTEXT_ARROW_LENGTH - 1] =3D ' '; > + *(context++) =3D ' '; > + memset(context, '>', CONTEXT_ARROW_LENGTH - 2); > + context +=3D CONTEXT_ARROW_LENGTH - 2; > + *(context++) =3D ' '; >=20 > /* Fill context after the arrow. */ > - for (int n =3D 0; n < CONTEXT_MAX_LENGTH_AFTER && src[i] !=3D = '\0' && > - src[i] !=3D '\n'; i++) { > - context[i + CONTEXT_ARROW_LENGTH] =3D src[i]; > - n++; > - } > - assert(i + CONTEXT_ARROW_LENGTH <=3D CONTEXT_MAX_LENGTH); > - context[i + CONTEXT_ARROW_LENGTH] =3D '\0'; > + const char *end =3D context + CONTEXT_MAX_LENGTH_AFTER; > + for (; context < end && *src !=3D '\0' && *src !=3D '\n'; ++src, = ++context) > + *context =3D *src; > + *context =3D '\0'; > } >=20 > /* This function does not return. > @@ -893,8 +891,7 @@ static void json_throw_parse_error(lua_State *l, = json_parse_t *json, >=20 > /* Note: token->column_index is 0 based, display starting from 1 = */ > luaL_error(l, "Expected %s but found %s on line %d at character %d = here " > - "'%s'", exp, found, json->line_count, = token->column_index + 1, > - context); > + "'%s'", exp, found, json->line_count, column_index + = 1, context); > } >=20 > static inline void json_decode_ascend(json_parse_t *json) commit f990fa9fa9e25efc50229ca40418fd427f6e8261 Author: Roman Khabibov Date: Thu Dec 12 16:55:33 2019 +0300 json: print context in error mesages =20 Context is just a string with a few characters before and after wrong token, wrong token itself and a symbolic arrow pointing to this token. =20 Closes #4339 diff --git a/test/app-tap/json.test.lua b/test/app-tap/json.test.lua index 6d511e686..70e9f6cf7 100755 --- a/test/app-tap/json.test.lua +++ b/test/app-tap/json.test.lua @@ -22,7 +22,7 @@ end =20 tap.test("json", function(test) local serializer =3D require('json') - test:plan(51) + test:plan(57) =20 test:test("unsigned", common.test_unsigned, serializer) test:test("signed", common.test_signed, serializer) @@ -184,4 +184,22 @@ tap.test("json", function(test) test:ok(string.find(err_msg, 'comma') ~=3D nil, 'comma instead of = T_COMMA') _, err_msg =3D pcall(serializer.decode, '{') test:ok(string.find(err_msg, 'end') ~=3D nil, 'end instead of = T_END') + + -- + -- gh-4339: Make sure that context is printed. + -- + _, err_msg =3D pcall(serializer.decode, '{{: "world"}') + test:ok(string.find(err_msg, '{ >> {: "worl') ~=3D nil, 'context = #1') + _, err_msg =3D pcall(serializer.decode, '{"a": "world"}}') + test:ok(string.find(err_msg, '"world"} >> }') ~=3D nil, 'context = #2') + _, err_msg =3D pcall(serializer.decode, '{1: "world"}') + test:ok(string.find(err_msg, '{ >> 1: "worl') ~=3D nil, 'context = #3') + _, err_msg =3D pcall(serializer.decode, '{') + test:ok(string.find(err_msg, '{ >> ') ~=3D nil, 'context #4') + _, err_msg =3D pcall(serializer.decode, '}') + test:ok(string.find(err_msg, ' >> }') ~=3D nil, 'context #5') + serializer.cfg{decode_max_depth =3D 1} + _, err_msg =3D pcall(serializer.decode, '{"a": {a =3D {}}}') + test:ok(string.find(err_msg, '{"a": >> {a =3D {}}') ~=3D nil, = 'context #6') + end) diff --git a/third_party/lua-cjson/lua_cjson.c = b/third_party/lua-cjson/lua_cjson.c index e68b52847..79b0253f0 100644 --- a/third_party/lua-cjson/lua_cjson.c +++ b/third_party/lua-cjson/lua_cjson.c @@ -825,6 +825,48 @@ static void json_next_token(json_parse_t *json, = json_token_t *token) json_set_token_error(token, json, "invalid token"); } =20 +enum context_length { + CONTEXT_ARROW_LENGTH =3D 4, + CONTEXT_MAX_LENGTH_BEFORE =3D 8, + CONTEXT_MAX_LENGTH_AFTER =3D 8, + CONTEXT_MAX_LENGTH =3D CONTEXT_MAX_LENGTH_BEFORE + = CONTEXT_MAX_LENGTH_AFTER + + CONTEXT_ARROW_LENGTH, +}; + +/** + * Copy characters near wrong token with the position @a + * column_index to a static string buffer @a context and lay out + * arrow " >> " before this token. + * + * @param context String static buffer to fill. + * @param json Structure with pointers to parsing string. + * @param column_index Position of wrong token in the current + * line. + */ +static void fill_context(char *context, json_parse_t *json, int = column_index) +{ + assert(column_index >=3D 0); + int length_before =3D column_index < CONTEXT_MAX_LENGTH_BEFORE ? + column_index : CONTEXT_MAX_LENGTH_BEFORE; + const char *src =3D json->cur_line_ptr + column_index - = length_before; + /* Fill context before the arrow. */ + memcpy(context, src, length_before); + context +=3D length_before; + src +=3D length_before; + + /* Make the arrow. */ + *(context++) =3D ' '; + memset(context, '>', CONTEXT_ARROW_LENGTH - 2); + context +=3D CONTEXT_ARROW_LENGTH - 2; + *(context++) =3D ' '; + + /* Fill context after the arrow. */ + const char *end =3D context + CONTEXT_MAX_LENGTH_AFTER; + for (; context < end && *src !=3D '\0' && *src !=3D '\n'; ++src, = ++context) + *context =3D *src; + *context =3D '\0'; +} + /* This function does not return. * DO NOT CALL WITH DYNAMIC MEMORY ALLOCATED. * The only supported exception is the temporary parser string @@ -843,9 +885,13 @@ static void json_throw_parse_error(lua_State *l, = json_parse_t *json, else found =3D json_token_type_name[token->type]; =20 + int column_index =3D token->column_index; + char context[CONTEXT_MAX_LENGTH + 1]; + fill_context(context, json, column_index); + /* Note: token->column_index is 0 based, display starting from 1 */ - luaL_error(l, "Expected %s but found %s on line %d at character = %d", exp, - found, json->line_count, token->column_index + 1); + luaL_error(l, "Expected %s but found %s on line %d at character %d = here " + "'%s'", exp, found, json->line_count, column_index + 1, = context); } =20 static inline void json_decode_ascend(json_parse_t *json) @@ -862,10 +908,13 @@ static void json_decode_descend(lua_State *l, = json_parse_t *json, int slots) return; } =20 + char context[CONTEXT_MAX_LENGTH + 1]; + fill_context(context, json, json->ptr - json->cur_line_ptr - 1); + strbuf_free(json->tmp); - luaL_error(l, "Found too many nested data structures (%d) on line = %d at " - "character %d", json->current_depth, json->line_count, - json->ptr - json->cur_line_ptr); + luaL_error(l, "Found too many nested data structures (%d) on line = %d at cha" + "racter %d here '%s'", json->current_depth, = json->line_count, + json->ptr - json->cur_line_ptr, context); } =20 static void json_parse_object_context(lua_State *l, json_parse_t *json)