[Tarantool-patches] [PATCH v2 2/2] json: print context in error mesages
Roman Khabibov
roman.habibov at tarantool.org
Sat Dec 21 17:56:10 MSK 2019
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 at 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 at 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)
More information about the Tarantool-patches
mailing list