* [Tarantool-patches] [PATCH v2 0/2] Improve json error mesages. @ 2019-12-15 14:42 Roman Khabibov 2019-12-15 14:42 ` [Tarantool-patches] [PATCH v2 1/2] json: make error messages more readable Roman Khabibov 2019-12-15 14:42 ` [Tarantool-patches] [PATCH v2 2/2] json: print context in error mesages Roman Khabibov 0 siblings, 2 replies; 6+ messages in thread From: Roman Khabibov @ 2019-12-15 14:42 UTC (permalink / raw) To: tarantool-patches; +Cc: v.shpilevoy The first patch fixes the token naming. The second one adds the context. Roman Khabibov (2): json: make error messages more readable json: print context in error mesages test/app-tap/json.test.lua | 50 ++++++++++++++- third_party/lua-cjson/lua_cjson.c | 100 +++++++++++++++++++++++------- 2 files changed, 125 insertions(+), 25 deletions(-) Branch: https://github.com/tarantool/tarantool/tree/romanhabibov/gh-4339-json-err Issue: https://github.com/tarantool/tarantool/issues/4339 -- 2.21.0 (Apple Git-122) ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Tarantool-patches] [PATCH v2 1/2] json: make error messages more readable 2019-12-15 14:42 [Tarantool-patches] [PATCH v2 0/2] Improve json error mesages Roman Khabibov @ 2019-12-15 14:42 ` Roman Khabibov 2019-12-21 14:56 ` Roman Khabibov 2019-12-15 14:42 ` [Tarantool-patches] [PATCH v2 2/2] json: print context in error mesages Roman Khabibov 1 sibling, 1 reply; 6+ messages in thread From: Roman Khabibov @ 2019-12-15 14:42 UTC (permalink / raw) To: tarantool-patches; +Cc: v.shpilevoy Print tokens themselves instead of token names "T_*" in the error messages. Part of #4339 --- test/app-tap/json.test.lua | 32 +++++++++++++++++++++++++- third_party/lua-cjson/lua_cjson.c | 38 +++++++++++++++---------------- 2 files changed, 50 insertions(+), 20 deletions(-) diff --git a/test/app-tap/json.test.lua b/test/app-tap/json.test.lua index fadfc74ec..6d511e686 100755 --- a/test/app-tap/json.test.lua +++ b/test/app-tap/json.test.lua @@ -22,7 +22,7 @@ end tap.test("json", function(test) local serializer = require('json') - test:plan(40) + test:plan(51) test:test("unsigned", common.test_unsigned, serializer) test:test("signed", common.test_signed, serializer) @@ -154,4 +154,34 @@ tap.test("json", function(test) _, err_msg = pcall(serializer.decode, '{"hello": "world",\n 100: 200}') test:ok(string.find(err_msg, 'line 2 at character 2') ~= nil, 'mistake on second line') + + -- + -- gh-4339: Make sure that tokens 'T_*' are absent in error + -- messages and a context is printed. + -- + _, err_msg = pcall(serializer.decode, '{{: "world"}') + test:ok(string.find(err_msg, '\'{\'') ~= nil, '"{" instead of T_OBJ_BEGIN') + _, err_msg = pcall(serializer.decode, '{"a": "world"}}') + test:ok(string.find(err_msg, '\'}\'') ~= nil, '"}" instead of T_OBJ_END') + _, err_msg = pcall(serializer.decode, '{[: "world"}') + test:ok(string.find(err_msg, '\'[\'', 1, true) ~= nil, + '"[" instead of T_ARR_BEGIN') + _, err_msg = pcall(serializer.decode, '{]: "world"}') + test:ok(string.find(err_msg, '\']\'', 1, true) ~= nil, + '"]" instead of T_ARR_END') + _, err_msg = pcall(serializer.decode, '{1: "world"}') + test:ok(string.find(err_msg, 'int') ~= nil, 'int instead of T_INT') + _, err_msg = pcall(serializer.decode, '{1.0: "world"}') + test:ok(string.find(err_msg, 'number') ~= nil, 'number instead of T_NUMBER') + _, err_msg = pcall(serializer.decode, '{true: "world"}') + test:ok(string.find(err_msg, 'boolean') ~= nil, + 'boolean instead of T_BOOLEAN') + _, err_msg = pcall(serializer.decode, '{null: "world"}') + test:ok(string.find(err_msg, 'null') ~= nil, 'null instead of T_NULL') + _, err_msg = pcall(serializer.decode, '{:: "world"}') + test:ok(string.find(err_msg, 'colon') ~= nil, 'colon instead of T_COLON') + _, err_msg = pcall(serializer.decode, '{,: "world"}') + test:ok(string.find(err_msg, 'comma') ~= nil, 'comma instead of T_COMMA') + _, err_msg = pcall(serializer.decode, '{') + test:ok(string.find(err_msg, 'end') ~= nil, 'end instead of T_END') end) diff --git a/third_party/lua-cjson/lua_cjson.c b/third_party/lua-cjson/lua_cjson.c index 3d25814f3..e68b52847 100644 --- a/third_party/lua-cjson/lua_cjson.c +++ b/third_party/lua-cjson/lua_cjson.c @@ -72,23 +72,23 @@ typedef enum { } json_token_type_t; static const char *json_token_type_name[] = { - "T_OBJ_BEGIN", - "T_OBJ_END", - "T_ARR_BEGIN", - "T_ARR_END", - "T_STRING", - "T_UINT", - "T_INT", - "T_NUMBER", - "T_BOOLEAN", - "T_NULL", - "T_COLON", - "T_COMMA", - "T_END", - "T_WHITESPACE", - "T_LINEFEED", - "T_ERROR", - "T_UNKNOWN", + "'{'", + "'}'", + "'['", + "']'", + "string", + "unsigned int", + "int", + "number", + "boolean", + "null", + "colon", + "comma", + "end", + "whitespace", + "line feed", + "error", + "unknown symbol", NULL }; @@ -914,7 +914,7 @@ static void json_parse_object_context(lua_State *l, json_parse_t *json) } if (token.type != T_COMMA) - json_throw_parse_error(l, json, "comma or object end", &token); + json_throw_parse_error(l, json, "comma or '}'", &token); json_next_token(json, &token); } @@ -954,7 +954,7 @@ static void json_parse_array_context(lua_State *l, json_parse_t *json) } if (token.type != T_COMMA) - json_throw_parse_error(l, json, "comma or array end", &token); + json_throw_parse_error(l, json, "comma or ']'", &token); json_next_token(json, &token); } -- 2.21.0 (Apple Git-122) ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Tarantool-patches] [PATCH v2 1/2] json: make error messages more readable 2019-12-15 14:42 ` [Tarantool-patches] [PATCH v2 1/2] json: make error messages more readable Roman Khabibov @ 2019-12-21 14:56 ` Roman Khabibov 0 siblings, 0 replies; 6+ messages in thread From: Roman Khabibov @ 2019-12-21 14:56 UTC (permalink / raw) To: Alexander Turenko; +Cc: tarantool-patches, Vladislav Shpilevoy commit a427872128d30bd769e63a1254af774c1a24aa28 Author: Roman Khabibov <roman.habibov@tarantool.org> Date: Wed Oct 9 17:07:41 2019 +0300 json: make error messages more readable Print tokens themselves instead of token names "T_*" in the error messages. Part of #4339 diff --git a/test/app-tap/json.test.lua b/test/app-tap/json.test.lua index fadfc74ec..6d511e686 100755 --- a/test/app-tap/json.test.lua +++ b/test/app-tap/json.test.lua @@ -22,7 +22,7 @@ end tap.test("json", function(test) local serializer = require('json') - test:plan(40) + test:plan(51) test:test("unsigned", common.test_unsigned, serializer) test:test("signed", common.test_signed, serializer) @@ -154,4 +154,34 @@ tap.test("json", function(test) _, err_msg = pcall(serializer.decode, '{"hello": "world",\n 100: 200}') test:ok(string.find(err_msg, 'line 2 at character 2') ~= nil, 'mistake on second line') + + -- + -- gh-4339: Make sure that tokens 'T_*' are absent in error + -- messages and a context is printed. + -- + _, err_msg = pcall(serializer.decode, '{{: "world"}') + test:ok(string.find(err_msg, '\'{\'') ~= nil, '"{" instead of T_OBJ_BEGIN') + _, err_msg = pcall(serializer.decode, '{"a": "world"}}') + test:ok(string.find(err_msg, '\'}\'') ~= nil, '"}" instead of T_OBJ_END') + _, err_msg = pcall(serializer.decode, '{[: "world"}') + test:ok(string.find(err_msg, '\'[\'', 1, true) ~= nil, + '"[" instead of T_ARR_BEGIN') + _, err_msg = pcall(serializer.decode, '{]: "world"}') + test:ok(string.find(err_msg, '\']\'', 1, true) ~= nil, + '"]" instead of T_ARR_END') + _, err_msg = pcall(serializer.decode, '{1: "world"}') + test:ok(string.find(err_msg, 'int') ~= nil, 'int instead of T_INT') + _, err_msg = pcall(serializer.decode, '{1.0: "world"}') + test:ok(string.find(err_msg, 'number') ~= nil, 'number instead of T_NUMBER') + _, err_msg = pcall(serializer.decode, '{true: "world"}') + test:ok(string.find(err_msg, 'boolean') ~= nil, + 'boolean instead of T_BOOLEAN') + _, err_msg = pcall(serializer.decode, '{null: "world"}') + test:ok(string.find(err_msg, 'null') ~= nil, 'null instead of T_NULL') + _, err_msg = pcall(serializer.decode, '{:: "world"}') + test:ok(string.find(err_msg, 'colon') ~= nil, 'colon instead of T_COLON') + _, err_msg = pcall(serializer.decode, '{,: "world"}') + test:ok(string.find(err_msg, 'comma') ~= nil, 'comma instead of T_COMMA') + _, err_msg = pcall(serializer.decode, '{') + test:ok(string.find(err_msg, 'end') ~= nil, 'end instead of T_END') end) diff --git a/third_party/lua-cjson/lua_cjson.c b/third_party/lua-cjson/lua_cjson.c index 3d25814f3..e68b52847 100644 --- a/third_party/lua-cjson/lua_cjson.c +++ b/third_party/lua-cjson/lua_cjson.c @@ -72,23 +72,23 @@ typedef enum { } json_token_type_t; static const char *json_token_type_name[] = { - "T_OBJ_BEGIN", - "T_OBJ_END", - "T_ARR_BEGIN", - "T_ARR_END", - "T_STRING", - "T_UINT", - "T_INT", - "T_NUMBER", - "T_BOOLEAN", - "T_NULL", - "T_COLON", - "T_COMMA", - "T_END", - "T_WHITESPACE", - "T_LINEFEED", - "T_ERROR", - "T_UNKNOWN", + "'{'", + "'}'", + "'['", + "']'", + "string", + "unsigned int", + "int", + "number", + "boolean", + "null", + "colon", + "comma", + "end", + "whitespace", + "line feed", + "error", + "unknown symbol", NULL }; @@ -914,7 +914,7 @@ static void json_parse_object_context(lua_State *l, json_parse_t *json) } if (token.type != T_COMMA) - json_throw_parse_error(l, json, "comma or object end", &token); + json_throw_parse_error(l, json, "comma or '}'", &token); json_next_token(json, &token); } @@ -954,7 +954,7 @@ static void json_parse_array_context(lua_State *l, json_parse_t *json) } if (token.type != T_COMMA) - json_throw_parse_error(l, json, "comma or array end", &token); + json_throw_parse_error(l, json, "comma or ']'", &token); json_next_token(json, &token); } ^ permalink raw reply [flat|nested] 6+ messages in thread
* [Tarantool-patches] [PATCH v2 2/2] json: print context in error mesages 2019-12-15 14:42 [Tarantool-patches] [PATCH v2 0/2] Improve json error mesages Roman Khabibov 2019-12-15 14:42 ` [Tarantool-patches] [PATCH v2 1/2] json: make error messages more readable Roman Khabibov @ 2019-12-15 14:42 ` Roman Khabibov 2019-12-19 23:19 ` Vladislav Shpilevoy 1 sibling, 1 reply; 6+ messages in thread From: Roman Khabibov @ 2019-12-15 14:42 UTC (permalink / raw) To: tarantool-patches; +Cc: v.shpilevoy 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. Closes #4339 --- test/app-tap/json.test.lua | 20 +++++++++- third_party/lua-cjson/lua_cjson.c | 62 ++++++++++++++++++++++++++++--- 2 files changed, 76 insertions(+), 6 deletions(-) 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 tap.test("json", function(test) local serializer = require('json') - test:plan(51) + test:plan(57) 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') ~= nil, 'comma instead of T_COMMA') _, err_msg = pcall(serializer.decode, '{') test:ok(string.find(err_msg, 'end') ~= nil, 'end instead of T_END') + + -- + -- gh-4339: Make sure that context is printed. + -- + _, err_msg = pcall(serializer.decode, '{{: "world"}') + test:ok(string.find(err_msg, '{ >> {: "worl') ~= nil, 'context #1') + _, err_msg = pcall(serializer.decode, '{"a": "world"}}') + test:ok(string.find(err_msg, '"world"} >> }') ~= nil, 'context #2') + _, err_msg = pcall(serializer.decode, '{1: "world"}') + test:ok(string.find(err_msg, '{ >> 1: "worl') ~= nil, 'context #3') + _, err_msg = pcall(serializer.decode, '{') + test:ok(string.find(err_msg, '{ >> ') ~= nil, 'context #4') + _, err_msg = pcall(serializer.decode, '}') + test:ok(string.find(err_msg, ' >> }') ~= nil, 'context #5') + serializer.cfg{decode_max_depth = 1} + _, err_msg = pcall(serializer.decode, '{"a": {a = {}}}') + test:ok(string.find(err_msg, '{"a": >> {a = {}}') ~= nil, 'context #6') + end) diff --git a/third_party/lua-cjson/lua_cjson.c b/third_party/lua-cjson/lua_cjson.c index e68b52847..655d6550e 100644 --- a/third_party/lua-cjson/lua_cjson.c +++ b/third_party/lua-cjson/lua_cjson.c @@ -825,6 +825,50 @@ static void json_next_token(json_parse_t *json, json_token_t *token) json_set_token_error(token, json, "invalid token"); } +enum context_length { + CONTEXT_ARROW_LENGTH = 4, + CONTEXT_MAX_LENGTH_BEFORE = 8, + CONTEXT_MAX_LENGTH_AFTER = 8, + CONTEXT_MAX_LENGTH = 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 >= 0); + int length_before = column_index < CONTEXT_MAX_LENGTH_BEFORE ? + column_index : CONTEXT_MAX_LENGTH_BEFORE; + const char *src = json->cur_line_ptr + column_index - length_before; + int i = 0; + /* Fill context before the arrow. */ + for (; i < length_before; i++) + context[i] = src[i]; + + /* Make the arrow. */ + context[i] = ' '; + memset(context + i + 1, '>', CONTEXT_ARROW_LENGTH - 2); + context[i + CONTEXT_ARROW_LENGTH - 1] = ' '; + + /* Fill context after the arrow. */ + for (int n = 0; n < CONTEXT_MAX_LENGTH_AFTER && src[i] != '\0' && + src[i] != '\n'; i++) { + context[i + CONTEXT_ARROW_LENGTH] = src[i]; + n++; + } + assert(i + CONTEXT_ARROW_LENGTH <= CONTEXT_MAX_LENGTH); + context[i + CONTEXT_ARROW_LENGTH] = '\0'; +} + /* This function does not return. * DO NOT CALL WITH DYNAMIC MEMORY ALLOCATED. * The only supported exception is the temporary parser string @@ -843,9 +887,14 @@ static void json_throw_parse_error(lua_State *l, json_parse_t *json, else found = json_token_type_name[token->type]; + int column_index = 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, token->column_index + 1, + context); } static inline void json_decode_ascend(json_parse_t *json) @@ -862,10 +911,13 @@ static void json_decode_descend(lua_State *l, json_parse_t *json, int slots) return; } + 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); } static void json_parse_object_context(lua_State *l, json_parse_t *json) -- 2.21.0 (Apple Git-122) ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Tarantool-patches] [PATCH v2 2/2] json: print context in error mesages 2019-12-15 14:42 ` [Tarantool-patches] [PATCH v2 2/2] json: print context in error mesages Roman Khabibov @ 2019-12-19 23:19 ` Vladislav Shpilevoy 2019-12-21 14:56 ` Roman Khabibov 0 siblings, 1 reply; 6+ messages in thread From: Vladislav Shpilevoy @ 2019-12-19 23:19 UTC (permalink / raw) To: Roman Khabibov, tarantool-patches Hi! Thanks for the patch! See my review fixes here and on top of the branch. Please, review them, and if you are ok, then squash. In that case it will be LGTM. ============================================================================== 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 >= 0); int length_before = column_index < CONTEXT_MAX_LENGTH_BEFORE ? - column_index : CONTEXT_MAX_LENGTH_BEFORE; + column_index : CONTEXT_MAX_LENGTH_BEFORE; const char *src = json->cur_line_ptr + column_index - length_before; - int i = 0; /* Fill context before the arrow. */ - for (; i < length_before; i++) - context[i] = src[i]; + memcpy(context, src, length_before); + context += length_before; + src += length_before; /* Make the arrow. */ - context[i] = ' '; - memset(context + i + 1, '>', CONTEXT_ARROW_LENGTH - 2); - context[i + CONTEXT_ARROW_LENGTH - 1] = ' '; + *(context++) = ' '; + memset(context, '>', CONTEXT_ARROW_LENGTH - 2); + context += CONTEXT_ARROW_LENGTH - 2; + *(context++) = ' '; /* Fill context after the arrow. */ - for (int n = 0; n < CONTEXT_MAX_LENGTH_AFTER && src[i] != '\0' && - src[i] != '\n'; i++) { - context[i + CONTEXT_ARROW_LENGTH] = src[i]; - n++; - } - assert(i + CONTEXT_ARROW_LENGTH <= CONTEXT_MAX_LENGTH); - context[i + CONTEXT_ARROW_LENGTH] = '\0'; + const char *end = context + CONTEXT_MAX_LENGTH_AFTER; + for (; context < end && *src != '\0' && *src != '\n'; ++src, ++context) + *context = *src; + *context = '\0'; } /* This function does not return. @@ -893,8 +891,7 @@ static void json_throw_parse_error(lua_State *l, json_parse_t *json, /* 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); } static inline void json_decode_ascend(json_parse_t *json) ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [Tarantool-patches] [PATCH v2 2/2] json: print context in error mesages 2019-12-19 23:19 ` Vladislav Shpilevoy @ 2019-12-21 14:56 ` Roman Khabibov 0 siblings, 0 replies; 6+ messages in thread From: Roman Khabibov @ 2019-12-21 14:56 UTC (permalink / raw) To: Alexander Turenko; +Cc: tarantool-patches, Vladislav Shpilevoy Hi Thanks for the review. Vlad, I’m 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 <v.shpilevoy@tarantool.org> wrote: > > Hi! Thanks for the patch! > > See my review fixes here and on top of the branch. > > Please, review them, and if you are ok, then squash. > In that case it will be LGTM. > > ============================================================================== > > 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 >= 0); > int length_before = column_index < CONTEXT_MAX_LENGTH_BEFORE ? > - column_index : CONTEXT_MAX_LENGTH_BEFORE; > + column_index : CONTEXT_MAX_LENGTH_BEFORE; > const char *src = json->cur_line_ptr + column_index - length_before; > - int i = 0; > /* Fill context before the arrow. */ > - for (; i < length_before; i++) > - context[i] = src[i]; > + memcpy(context, src, length_before); > + context += length_before; > + src += length_before; > > /* Make the arrow. */ > - context[i] = ' '; > - memset(context + i + 1, '>', CONTEXT_ARROW_LENGTH - 2); > - context[i + CONTEXT_ARROW_LENGTH - 1] = ' '; > + *(context++) = ' '; > + memset(context, '>', CONTEXT_ARROW_LENGTH - 2); > + context += CONTEXT_ARROW_LENGTH - 2; > + *(context++) = ' '; > > /* Fill context after the arrow. */ > - for (int n = 0; n < CONTEXT_MAX_LENGTH_AFTER && src[i] != '\0' && > - src[i] != '\n'; i++) { > - context[i + CONTEXT_ARROW_LENGTH] = src[i]; > - n++; > - } > - assert(i + CONTEXT_ARROW_LENGTH <= CONTEXT_MAX_LENGTH); > - context[i + CONTEXT_ARROW_LENGTH] = '\0'; > + const char *end = context + CONTEXT_MAX_LENGTH_AFTER; > + for (; context < end && *src != '\0' && *src != '\n'; ++src, ++context) > + *context = *src; > + *context = '\0'; > } > > /* This function does not return. > @@ -893,8 +891,7 @@ static void json_throw_parse_error(lua_State *l, json_parse_t *json, > > /* 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); > } > > static inline void json_decode_ascend(json_parse_t *json) commit f990fa9fa9e25efc50229ca40418fd427f6e8261 Author: Roman Khabibov <roman.habibov@tarantool.org> Date: Thu Dec 12 16:55:33 2019 +0300 json: print context in error mesages 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. 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 tap.test("json", function(test) local serializer = require('json') - test:plan(51) + test:plan(57) 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') ~= nil, 'comma instead of T_COMMA') _, err_msg = pcall(serializer.decode, '{') test:ok(string.find(err_msg, 'end') ~= nil, 'end instead of T_END') + + -- + -- gh-4339: Make sure that context is printed. + -- + _, err_msg = pcall(serializer.decode, '{{: "world"}') + test:ok(string.find(err_msg, '{ >> {: "worl') ~= nil, 'context #1') + _, err_msg = pcall(serializer.decode, '{"a": "world"}}') + test:ok(string.find(err_msg, '"world"} >> }') ~= nil, 'context #2') + _, err_msg = pcall(serializer.decode, '{1: "world"}') + test:ok(string.find(err_msg, '{ >> 1: "worl') ~= nil, 'context #3') + _, err_msg = pcall(serializer.decode, '{') + test:ok(string.find(err_msg, '{ >> ') ~= nil, 'context #4') + _, err_msg = pcall(serializer.decode, '}') + test:ok(string.find(err_msg, ' >> }') ~= nil, 'context #5') + serializer.cfg{decode_max_depth = 1} + _, err_msg = pcall(serializer.decode, '{"a": {a = {}}}') + test:ok(string.find(err_msg, '{"a": >> {a = {}}') ~= 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"); } +enum context_length { + CONTEXT_ARROW_LENGTH = 4, + CONTEXT_MAX_LENGTH_BEFORE = 8, + CONTEXT_MAX_LENGTH_AFTER = 8, + CONTEXT_MAX_LENGTH = 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 >= 0); + int length_before = column_index < CONTEXT_MAX_LENGTH_BEFORE ? + column_index : CONTEXT_MAX_LENGTH_BEFORE; + const char *src = json->cur_line_ptr + column_index - length_before; + /* Fill context before the arrow. */ + memcpy(context, src, length_before); + context += length_before; + src += length_before; + + /* Make the arrow. */ + *(context++) = ' '; + memset(context, '>', CONTEXT_ARROW_LENGTH - 2); + context += CONTEXT_ARROW_LENGTH - 2; + *(context++) = ' '; + + /* Fill context after the arrow. */ + const char *end = context + CONTEXT_MAX_LENGTH_AFTER; + for (; context < end && *src != '\0' && *src != '\n'; ++src, ++context) + *context = *src; + *context = '\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 = json_token_type_name[token->type]; + int column_index = 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); } 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; } + 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); } static void json_parse_object_context(lua_State *l, json_parse_t *json) ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2019-12-21 14:56 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-12-15 14:42 [Tarantool-patches] [PATCH v2 0/2] Improve json error mesages Roman Khabibov 2019-12-15 14:42 ` [Tarantool-patches] [PATCH v2 1/2] json: make error messages more readable Roman Khabibov 2019-12-21 14:56 ` Roman Khabibov 2019-12-15 14:42 ` [Tarantool-patches] [PATCH v2 2/2] json: print context in error mesages Roman Khabibov 2019-12-19 23:19 ` Vladislav Shpilevoy 2019-12-21 14:56 ` Roman Khabibov
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox