[Tarantool-patches] [PATCH] serializer: serialize recursive structures
Sergey Bronnikov
sergeyb at tarantool.org
Wed Apr 21 15:34:36 MSK 2021
Hello,
seems patch series below is a bit outdated, so I'll review and quote
actual version in a branch.
> @TarantoolBot
> __serialize parameter
> If __serialize parameter is initialized as a function then this
> function should be pure. A pure function is a function which
> returns identical values for identical arguments (no variation
> with local static variables, non-local variables, mutable
> reference arguments or input streams). Otherwise, the behavior
> is undefined.
1. __serialize is a method, not parameter
--- /dev/null
+++ b/changelogs/unreleased/serializer-bug.md
@@ -0,0 +1,4 @@
+## bugfix/core
+
+* Recursive structures appeared after __serialize call don't lead
+* to segfault anymore in lua and yaml serializers (gh-5392).
2. seems issue number is wrong, proper one is #3228
3. I have reviewed a code coverage and propose to cover uncovered
branches in lua_field_try_serialize(), see [1]
I believe it would be enough to add more examples to proposed tests to
enhance coverage.
1. https://coveralls.io/builds/38864750/source?filename=src%2Flua%2Futils.c
On 11.03.2021 08:49, Roman Khabibov via Tarantool-patches wrote:
> Fix bug with bus error during serializing of recursive
> structures.
>
> Closes #3228
> ---
>
> Branch: https://github.com/tarantool/tarantool/tree/romanhabibov/gh-3228-visited-set
> Issue: https://github.com/tarantool/tarantool/issues/3228
>
> src/box/lua/call.c | 6 +-
> src/box/lua/execute.c | 2 +-
> src/box/lua/serialize_lua.c | 96 ++------
> src/box/lua/tuple.c | 2 +-
> src/box/sql/func.c | 2 +-
> src/lua/msgpack.c | 10 +-
> src/lua/pickle.c | 2 +-
> src/lua/utils.c | 226 ++++++++++++++++--
> src/lua/utils.h | 43 +++-
> ...-3228-serializer-look-for-recursion.result | 67 ++++++
> ...228-serializer-look-for-recursion.test.lua | 17 ++
> test/swim/swim.result | 18 +-
> third_party/lua-cjson/lua_cjson.c | 4 +-
> third_party/lua-yaml/lyaml.cc | 88 +++----
> 14 files changed, 390 insertions(+), 193 deletions(-)
> create mode 100644 test/app/gh-3228-serializer-look-for-recursion.result
> create mode 100644 test/app/gh-3228-serializer-look-for-recursion.test.lua
>
> diff --git a/src/box/lua/call.c b/src/box/lua/call.c
> index 0315e720c..e4907a3f7 100644
> --- a/src/box/lua/call.c
> +++ b/src/box/lua/call.c
> @@ -193,7 +193,7 @@ luamp_encode_call_16(lua_State *L, struct luaL_serializer *cfg,
> */
> for (int i = 1; i <= nrets; ++i) {
> struct luaL_field field;
> - if (luaL_tofield(L, cfg, NULL, i, &field) < 0)
> + if (luaL_tofield(L, cfg, 0, NULL, i, &field) < 0)
> return luaT_error(L);
> struct tuple *tuple;
> if (field.type == MP_EXT &&
> @@ -222,7 +222,7 @@ luamp_encode_call_16(lua_State *L, struct luaL_serializer *cfg,
> * Inspect the first result
> */
> struct luaL_field root;
> - if (luaL_tofield(L, cfg, NULL, 1, &root) < 0)
> + if (luaL_tofield(L, cfg, 0, NULL, 1, &root) < 0)
> return luaT_error(L);
> struct tuple *tuple;
> if (root.type == MP_EXT && (tuple = luaT_istuple(L, 1)) != NULL) {
> @@ -252,7 +252,7 @@ luamp_encode_call_16(lua_State *L, struct luaL_serializer *cfg,
> for (uint32_t t = 1; t <= root.size; t++) {
> lua_rawgeti(L, 1, t);
> struct luaL_field field;
> - if (luaL_tofield(L, cfg, NULL, -1, &field) < 0)
> + if (luaL_tofield(L, cfg, 0, NULL, -1, &field) < 0)
> return luaT_error(L);
> if (field.type == MP_EXT && (tuple = luaT_istuple(L, -1))) {
> tuple_to_mpstream(tuple, stream);
> diff --git a/src/box/lua/execute.c b/src/box/lua/execute.c
> index 926a0a61c..76d271f5c 100644
> --- a/src/box/lua/execute.c
> +++ b/src/box/lua/execute.c
> @@ -328,7 +328,7 @@ lua_sql_bind_decode(struct lua_State *L, struct sql_bind *bind, int idx, int i)
> bind->name = NULL;
> bind->name_len = 0;
> }
> - if (luaL_tofield(L, luaL_msgpack_default, NULL, -1, &field) < 0)
> + if (luaL_tofield(L, luaL_msgpack_default, 0, NULL, -1, &field) < 0)
> return -1;
> switch (field.type) {
> case MP_UINT:
> diff --git a/src/box/lua/serialize_lua.c b/src/box/lua/serialize_lua.c
> index caa08a60f..ab612dfc2 100644
> --- a/src/box/lua/serialize_lua.c
> +++ b/src/box/lua/serialize_lua.c
> @@ -215,7 +215,7 @@ trace_node(struct lua_dumper *d)
> struct luaL_field field;
>
> memset(&field, 0, sizeof(field));
> - luaL_checkfield(d->L, d->cfg, lua_gettop(d->L), &field);
> + luaL_checkfield(d->L, d->cfg, 0, lua_gettop(d->L), &field);
>
> if (field.type < lengthof(mp_type_names)) {
> if (field.type == MP_EXT) {
> @@ -234,7 +234,7 @@ trace_node(struct lua_dumper *d)
>
> memset(&field, 0, sizeof(field));
>
> - luaL_checkfield(d->L, d->cfg, top, &field);
> + luaL_checkfield(d->L, d->cfg, 0, top, &field);
> say_info("serializer-trace: node :\tfield type %s (%d)",
> type_str, field.type);
> }
> @@ -339,41 +339,16 @@ emit_node(struct lua_dumper *d, struct node *nd, int indent,
> static const char *
> get_lua_anchor(struct lua_dumper *d)
> {
> - const char *s = "";
> -
> - lua_pushvalue(d->L, -1);
> - lua_rawget(d->L, d->anchortable_index);
> - if (!lua_toboolean(d->L, -1)) {
> - lua_pop(d->L, 1);
> - return NULL;
> - }
> -
> - if (lua_isboolean(d->L, -1)) {
> - /*
> - * This element is referenced more
> - * than once but has not been named.
> - */
> - char buf[32];
> - snprintf(buf, sizeof(buf), "%u", d->anchor_number++);
> - lua_pop(d->L, 1);
> - lua_pushvalue(d->L, -1);
> - lua_pushstring(d->L, buf);
> - s = lua_tostring(d->L, -1);
> - lua_rawset(d->L, d->anchortable_index);
> - trace_anchor(s, false);
> - } else {
> - /*
> - * An aliased element.
> - *
> - * FIXME: Need an example to use.
> - *
> - * const char *str = lua_tostring(d->L, -1);
> - */
> - const char *str = lua_tostring(d->L, -1);
> - trace_anchor(str, true);
> - lua_pop(d->L, 1);
> + const char *anchor = NULL;
> + int code = get_anchor(d->L, d->anchortable_index, &d->anchor_number,
> + &anchor);
> + if (code == 1) {
> + trace_anchor(anchor, false);
> + } else if (code == 2) {
> + trace_anchor(anchor, true);
> + return "";
> }
> - return s;
> + return anchor;
> }
>
> static void
> @@ -783,7 +758,7 @@ dump_node(struct lua_dumper *d, struct node *nd, int indent)
> return -1;
>
> memset(field, 0, sizeof(*field));
> - luaL_checkfield(d->L, d->cfg, lua_gettop(d->L), field);
> + luaL_checkfield(d->L, d->cfg, 0, lua_gettop(d->L), field);
>
> switch (field->type) {
> case MP_NIL:
> @@ -881,49 +856,6 @@ dump_node(struct lua_dumper *d, struct node *nd, int indent)
> return emit_node(d, nd, indent, str, len);
> }
>
> -/**
> - * Find references to tables, we use it
> - * to find self references in tables.
> - */
> -static void
> -find_references(struct lua_dumper *d)
> -{
> - int newval;
> -
> - if (lua_type(d->L, -1) != LUA_TTABLE)
> - return;
> -
> - /* Copy of a table for self refs */
> - lua_pushvalue(d->L, -1);
> - lua_rawget(d->L, d->anchortable_index);
> - if (lua_isnil(d->L, -1))
> - newval = 0;
> - else if (!lua_toboolean(d->L, -1))
> - newval = 1;
> - else
> - newval = -1;
> - lua_pop(d->L, 1);
> -
> - if (newval != -1) {
> - lua_pushvalue(d->L, -1);
> - lua_pushboolean(d->L, newval);
> - lua_rawset(d->L, d->anchortable_index);
> - }
> -
> - if (newval != 0)
> - return;
> -
> - /*
> - * Other values and keys in the table
> - */
> - lua_pushnil(d->L);
> - while (lua_next(d->L, -2) != 0) {
> - find_references(d);
> - lua_pop(d->L, 1);
> - find_references(d);
> - }
> -}
> -
> /**
> * Dump recursively from the root node.
> */
> @@ -935,7 +867,7 @@ dump_root(struct lua_dumper *d)
> };
> int ret;
>
> - luaL_checkfield(d->L, d->cfg, lua_gettop(d->L), &nd.field);
> + luaL_checkfield(d->L, d->cfg, 0, lua_gettop(d->L), &nd.field);
>
> if (nd.field.type != MP_ARRAY || nd.field.size != 1) {
> d->err = EINVAL;
> @@ -984,7 +916,7 @@ lua_encode(lua_State *L, struct luaL_serializer *serializer,
>
> /* Push copy of arg we're processing */
> lua_pushvalue(L, 1);
> - find_references(&dumper);
> + find_references(dumper.L, dumper.anchortable_index, "before");
>
> if (dump_root(&dumper) != 0)
> goto out;
> diff --git a/src/box/lua/tuple.c b/src/box/lua/tuple.c
> index 3e6f043b4..4d417ec34 100644
> --- a/src/box/lua/tuple.c
> +++ b/src/box/lua/tuple.c
> @@ -411,7 +411,7 @@ luamp_convert_key(struct lua_State *L, struct luaL_serializer *cfg,
> return tuple_to_mpstream(tuple, stream);
>
> struct luaL_field field;
> - if (luaL_tofield(L, cfg, NULL, index, &field) < 0)
> + if (luaL_tofield(L, cfg, 0, NULL, index, &field) < 0)
> luaT_error(L);
> if (field.type == MP_ARRAY) {
> lua_pushvalue(L, index);
> diff --git a/src/box/sql/func.c b/src/box/sql/func.c
> index f15d27051..d4a7064f0 100644
> --- a/src/box/sql/func.c
> +++ b/src/box/sql/func.c
> @@ -262,7 +262,7 @@ port_lua_get_vdbemem(struct port *base, uint32_t *size)
> return NULL;
> for (int i = 0; i < argc; i++) {
> struct luaL_field field;
> - if (luaL_tofield(L, luaL_msgpack_default,
> + if (luaL_tofield(L, luaL_msgpack_default, 0,
> NULL, -1 - i, &field) < 0) {
> goto error;
> }
> diff --git a/src/lua/msgpack.c b/src/lua/msgpack.c
> index 24b0d2ccd..412736572 100644
> --- a/src/lua/msgpack.c
> +++ b/src/lua/msgpack.c
> @@ -156,11 +156,11 @@ restart: /* used by MP_EXT of unidentified subtype */
> lua_pushnil(L); /* first key */
> while (lua_next(L, top) != 0) {
> lua_pushvalue(L, -2); /* push a copy of key to top */
> - if (luaL_tofield(L, cfg, opts, lua_gettop(L), field) < 0)
> + if (luaL_tofield(L, cfg, 0, opts, lua_gettop(L), field) < 0)
> return luaT_error(L);
> luamp_encode_r(L, cfg, opts, stream, field, level + 1);
> lua_pop(L, 1); /* pop a copy of key */
> - if (luaL_tofield(L, cfg, opts, lua_gettop(L), field) < 0)
> + if (luaL_tofield(L, cfg, 0, opts, lua_gettop(L), field) < 0)
> return luaT_error(L);
> luamp_encode_r(L, cfg, opts, stream, field, level + 1);
> lua_pop(L, 1); /* pop value */
> @@ -181,7 +181,7 @@ restart: /* used by MP_EXT of unidentified subtype */
> mpstream_encode_array(stream, size);
> for (uint32_t i = 0; i < size; i++) {
> lua_rawgeti(L, top, i + 1);
> - if (luaL_tofield(L, cfg, opts, top + 1, field) < 0)
> + if (luaL_tofield(L, cfg, 0, opts, top + 1, field) < 0)
> return luaT_error(L);
> luamp_encode_r(L, cfg, opts, stream, field, level + 1);
> lua_pop(L, 1);
> @@ -204,7 +204,7 @@ restart: /* used by MP_EXT of unidentified subtype */
> if (type != MP_EXT)
> return type; /* Value has been packed by the trigger */
> /* Try to convert value to serializable type */
> - luaL_convertfield(L, cfg, top, field);
> + luaL_convertfield(L, cfg, 0, top, field);
> /* handled by luaL_convertfield */
> assert(field->type != MP_EXT);
> assert(lua_gettop(L) == top);
> @@ -229,7 +229,7 @@ luamp_encode(struct lua_State *L, struct luaL_serializer *cfg,
> }
>
> struct luaL_field field;
> - if (luaL_tofield(L, cfg, opts, lua_gettop(L), &field) < 0)
> + if (luaL_tofield(L, cfg, 0, opts, lua_gettop(L), &field) < 0)
> return luaT_error(L);
> enum mp_type top_type = luamp_encode_r(L, cfg, opts, stream, &field, 0);
>
> diff --git a/src/lua/pickle.c b/src/lua/pickle.c
> index 65208b5b3..86f0f77e8 100644
> --- a/src/lua/pickle.c
> +++ b/src/lua/pickle.c
> @@ -80,7 +80,7 @@ lbox_pack(struct lua_State *L)
> if (i > nargs)
> luaL_error(L, "pickle.pack: argument count does not match "
> "the format");
> - luaL_checkfield(L, luaL_msgpack_default, i, &field);
> + luaL_checkfield(L, luaL_msgpack_default, 0, i, &field);
> switch (*format) {
> case 'B':
> case 'b':
> diff --git a/src/lua/utils.c b/src/lua/utils.c
> index b5a6ca5b7..64404e3fc 100644
> --- a/src/lua/utils.c
> +++ b/src/lua/utils.c
> @@ -439,9 +439,54 @@ lua_gettable_wrapper(lua_State *L)
> return 1;
> }
>
> +/**
> + * Check if node with @a idx serialized.
> + */
> +static int
> +is_serialized(struct lua_State *L, int anchortable_index, int idx)
> +{
> + if (anchortable_index == 0)
> + return 0;
> + lua_pushvalue(L, idx);
> + lua_rawget(L, anchortable_index);
> + int is_serialized = 0;
> + if (lua_isnil(L, -1) == 0) {
> + lua_pushstring(L, "serialized");
> + lua_rawget(L, -2);
> + is_serialized = lua_toboolean(L, -1);
> + lua_pop(L, 1);
> + }
> + lua_pop(L, 1);
> + return is_serialized;
> +}
> +
> +/**
> + * Mark node with @a idx as serialized.
> + */
> +static void
> +mark_as_serialized(struct lua_State *L, int anchortable_index, int idx)
> +{
> + if (anchortable_index == 0)
> + return;
> + /* Push copy of table. */
> + lua_pushvalue(L, idx);
> + lua_pushvalue(L, idx);
> + lua_rawget(L, anchortable_index);
> + if (lua_isnil(L, -1) == 1) {
> + lua_pop(L, 1);
> + lua_newtable(L);
> + }
> + int flags_index = lua_gettop(L);
> + lua_pushstring(L, "serialized");
> + lua_pushboolean(L, true);
> + lua_rawset(L, flags_index);
> + lua_rawset(L, anchortable_index);
> +}
> +
> static void
> lua_field_inspect_ucdata(struct lua_State *L, struct luaL_serializer *cfg,
> - int idx, struct luaL_field *field)
> + int anchortable_index, int idx,
> + struct luaL_field *field)
> {
> if (!cfg->encode_load_metatables)
> return;
> @@ -463,12 +508,148 @@ lua_field_inspect_ucdata(struct lua_State *L, struct luaL_serializer *cfg,
> lua_pcall(L, 1, 1, 0);
> /* replace obj with the unpacked value */
> lua_replace(L, idx);
> - if (luaL_tofield(L, cfg, NULL, idx, field) < 0)
> + if (luaL_tofield(L, cfg, anchortable_index, NULL, idx,
> + field) < 0)
> luaT_error(L);
> } /* else ignore lua_gettable exceptions */
> lua_settop(L, top); /* remove temporary objects */
> }
>
> +void
> +find_references(struct lua_State *L, int anchortable_index, const char *mode)
> +{
> + int type = lua_type(L, -1);
> + if (type != LUA_TTABLE || anchortable_index == 0)
> + return;
> +
> + int node_index = lua_gettop(L);
> + lua_pushvalue(L, node_index);
> +
> + /* Get value with flags from anchor table. */
> + lua_pushvalue(L, node_index);
> + lua_rawget(L, anchortable_index);
> + if (lua_isnil(L, -1) == 1) {
> + lua_pop(L, 1);
> + lua_newtable(L);
> + }
> + int flags_index = lua_gettop(L);
> +
> + /* Get and increment counter. */
> + lua_pushstring(L, mode);
> + lua_rawget(L, flags_index);
> +
> + int new_count = 0;
> + if (lua_isnil(L, -1) == 1) {
> + new_count = 1;
> + lua_pop(L, 1);
> + } else {
> + lua_Integer number = lua_tointeger(L, -1);
> + lua_pop(L, 1);
> + if (number < 2)
> + new_count = ++number;
> + else
> + new_count = -1;
> + }
> +
> + /* Push new count to value with flags. */
> + if (new_count == -1) {
> + lua_pop(L, 2);
> + assert(node_index == lua_gettop(L));
> + return;
> + }
> + lua_pushstring(L, mode);
> + lua_pushinteger(L, new_count);
> + lua_rawset(L, flags_index);
> +
> + /* Check if node is already serialized. */
> + bool already_serialized = false;
> + if (strcmp(mode, "after") == 0) {
> + lua_pushstring(L, "serialized");
> + lua_rawget(L, flags_index);
> + already_serialized = lua_toboolean(L, -1);
> + lua_pop(L, 1);
> + }
> + lua_rawset(L, anchortable_index);
> + assert(node_index == lua_gettop(L));
> + if (already_serialized == true)
> + return;
> +
> + /* Recursively process other table values. */
> + lua_pushnil(L);
> + while (lua_next(L, -2) != 0) {
> + /* Find references on value. */
> + find_references(L, anchortable_index, mode);
> + lua_pop(L, 1);
> + /* Find references on key. */
> + find_references(L, anchortable_index, mode);
> + }
> +}
> +
> +int
> +get_anchor(struct lua_State *L, int anchortable_index,
> + unsigned int *anchor_number, const char **anchor)
> +{
> + lua_pushvalue(L, -1);
> + lua_rawget(L, anchortable_index);
> + if (lua_isnil(L, -1) == 1) {
> + lua_pop(L, 1);
> + *anchor = NULL;
> + return 0;
> + }
> +
> + lua_pushstring(L, "before");
> + lua_rawget(L, -2);
> + const char *str = NULL;
> + if (lua_type(L, -1) == LUA_TSTRING)
> + str = lua_tostring(L, -1);
> + lua_Integer number_before = lua_tointeger(L, -1);
> + lua_pop(L, 1);
> + lua_Integer number_after = 0;
> + if (str == NULL) {
> + lua_pushstring(L, "after");
> + lua_rawget(L, -2);
> + if (lua_type(L, -1) == LUA_TSTRING)
> + str = lua_tostring(L, -1);
> + number_after = lua_tointeger(L, -1);
> + lua_pop(L, 1);
> + }
> +
> + *anchor = "";
> + int ret = 0;
> + if ((number_before > 1 || number_after > 1) && str == NULL) {
> + /*
> + * This element is referenced more than once but
> + * has not been named.
> + */
> + char buf[32];
> + snprintf(buf, sizeof(buf), "%u", (*anchor_number)++);
> + lua_pop(L, 1);
> + lua_pushvalue(L, -1);
> +
> + lua_pushvalue(L, -1);
> + lua_rawget(L, anchortable_index);
> + if (number_before > 1)
> + lua_pushstring(L, "before");
> + else
> + lua_pushstring(L, "after");
> + lua_pushstring(L, buf);
> + *anchor = lua_tostring(L, -1);
> + lua_rawset(L, -3);
> + lua_rawset(L, anchortable_index);
> + ret = 1;
> + } else if (str != NULL) {
> + /* This is an aliased element. */
> + lua_pop(L, 1);
> + *anchor = str;
> + ret = 2;
> + } else {
> + lua_pop(L, 1);
> + *anchor = NULL;
> + ret = 0;
> + }
> + return ret;
> +}
> +
> /**
> * Call __serialize method of a table object by index
> * if the former exists.
> @@ -496,11 +677,13 @@ lua_field_inspect_ucdata(struct lua_State *L, struct luaL_serializer *cfg,
> * proceed with default table encoding.
> */
> static int
> -lua_field_try_serialize(struct lua_State *L, struct luaL_serializer *cfg,
> - int idx, struct luaL_field *field)
> +lua_field_try_serialize(struct lua_State *L, int anchortable_index,
> + struct luaL_serializer *cfg, int idx,
> + struct luaL_field *field)
> {
> if (luaL_getmetafield(L, idx, LUAL_SERIALIZE) == 0)
> return 1;
> + mark_as_serialized(L, anchortable_index, idx);
> if (lua_isfunction(L, -1)) {
> /* copy object itself */
> lua_pushvalue(L, idx);
> @@ -508,7 +691,9 @@ lua_field_try_serialize(struct lua_State *L, struct luaL_serializer *cfg,
> diag_set(LuajitError, lua_tostring(L, -1));
> return -1;
> }
> - if (luaL_tofield(L, cfg, NULL, -1, field) != 0)
> + find_references(L, anchortable_index, "after");
> + if (luaL_tofield(L, cfg, anchortable_index, NULL, -1,
> + field) != 0)
> return -1;
> lua_replace(L, idx);
> return 0;
> @@ -541,16 +726,19 @@ lua_field_try_serialize(struct lua_State *L, struct luaL_serializer *cfg,
> }
>
> static int
> -lua_field_inspect_table(struct lua_State *L, struct luaL_serializer *cfg,
> - int idx, struct luaL_field *field)
> +lua_field_inspect_table(struct lua_State *L, int anchortable_index,
> + struct luaL_serializer *cfg, int idx,
> + struct luaL_field *field)
> {
> assert(lua_type(L, idx) == LUA_TTABLE);
> uint32_t size = 0;
> uint32_t max = 0;
>
> - if (cfg->encode_load_metatables) {
> + if (cfg->encode_load_metatables &&
> + is_serialized(L, anchortable_index, idx) != 1) {
> int top = lua_gettop(L);
> - int res = lua_field_try_serialize(L, cfg, idx, field);
> + int res = lua_field_try_serialize(L, anchortable_index, cfg,
> + idx, field);
> if (res == -1)
> return -1;
> assert(lua_gettop(L) == top);
> @@ -612,14 +800,14 @@ lua_field_tostring(struct lua_State *L, struct luaL_serializer *cfg, int idx,
> lua_call(L, 1, 1);
> lua_replace(L, idx);
> lua_settop(L, top);
> - if (luaL_tofield(L, cfg, NULL, idx, field) < 0)
> + if (luaL_tofield(L, cfg, 0, NULL, idx, field) < 0)
> luaT_error(L);
> }
>
> int
> luaL_tofield(struct lua_State *L, struct luaL_serializer *cfg,
> - const struct serializer_opts *opts, int index,
> - struct luaL_field *field)
> + int anchortable_index, const struct serializer_opts *opts,
> + int index, struct luaL_field *field)
> {
> if (index < 0)
> index = lua_gettop(L) + index + 1;
> @@ -753,7 +941,8 @@ luaL_tofield(struct lua_State *L, struct luaL_serializer *cfg,
> case LUA_TTABLE:
> {
> field->compact = false;
> - return lua_field_inspect_table(L, cfg, index, field);
> + return lua_field_inspect_table(L, anchortable_index, cfg,
> + index, field);
> }
> case LUA_TLIGHTUSERDATA:
> case LUA_TUSERDATA:
> @@ -773,8 +962,8 @@ luaL_tofield(struct lua_State *L, struct luaL_serializer *cfg,
> }
>
> void
> -luaL_convertfield(struct lua_State *L, struct luaL_serializer *cfg, int idx,
> - struct luaL_field *field)
> +luaL_convertfield(struct lua_State *L, struct luaL_serializer *cfg,
> + int anchortable_index, int idx, struct luaL_field *field)
> {
> if (idx < 0)
> idx = lua_gettop(L) + idx + 1;
> @@ -789,9 +978,12 @@ luaL_convertfield(struct lua_State *L, struct luaL_serializer *cfg, int idx,
> */
> GCcdata *cd = cdataV(L->base + idx - 1);
> if (cd->ctypeid > CTID_CTYPEID)
> - lua_field_inspect_ucdata(L, cfg, idx, field);
> + lua_field_inspect_ucdata(L, cfg,
> + anchortable_index, idx,
> + field);
> } else if (type == LUA_TUSERDATA) {
> - lua_field_inspect_ucdata(L, cfg, idx, field);
> + lua_field_inspect_ucdata(L, cfg, anchortable_index, idx,
> + field);
> }
> }
>
> diff --git a/src/lua/utils.h b/src/lua/utils.h
> index 37531676d..ef9c70dfb 100644
> --- a/src/lua/utils.h
> +++ b/src/lua/utils.h
> @@ -354,6 +354,29 @@ struct luaL_field {
> bool compact; /* a flag used by YAML serializer */
> };
>
> +/**
> + * Find references to tables to look up self references in tables.
> + *
> + * @param L Lua stack.
> + * @param anchortable_index Index of anchor table on stack.
> + * @param mode When the function is called before or after dump
> + function.
> + */
> +void
> +find_references(struct lua_State *L, int anchortable_index, const char *mode);
> +
> +/**
> + * Generate aliases and anchor numbers for self references.
> + *
> + * @param L Lua stack.
> + * @param anchortable_index Index of anchor table on stack.
> + * @param[out] anchor_number Ptr to the number anchors.
> + * @param[out] anchor Ptr to anchor string.
> + */
> +int
> +get_anchor(struct lua_State *L, int anchortable_index,
> + unsigned int *anchor_number, const char **anchor);
> +
> /**
> * @brief Convert a value from the Lua stack to a lua_field structure.
> * This function is designed for use with Lua bindings and data
> @@ -388,6 +411,7 @@ struct luaL_field {
> *
> * @param L stack
> * @param cfg configuration
> + * @param anchortable_index Index of anchor table on stack.
> * @param opts the Lua serializer additional options.
> * @param index stack index
> * @param field conversion result
> @@ -397,8 +421,8 @@ struct luaL_field {
> */
> int
> luaL_tofield(struct lua_State *L, struct luaL_serializer *cfg,
> - const struct serializer_opts *opts, int index,
> - struct luaL_field *field);
> + int anchortable_index, const struct serializer_opts *opts,
> + int index, struct luaL_field *field);
>
> /**
> * @brief Try to convert userdata/cdata values using defined conversion logic.
> @@ -406,18 +430,21 @@ luaL_tofield(struct lua_State *L, struct luaL_serializer *cfg,
> *
> * @param L stack
> * @param cfg configuration
> + * @param anchortable_index Index of anchor table on stack.
> * @param idx stack index
> * @param field conversion result
> */
> void
> -luaL_convertfield(struct lua_State *L, struct luaL_serializer *cfg, int idx,
> - struct luaL_field *field);
> +luaL_convertfield(struct lua_State *L, struct luaL_serializer *cfg,
> + int anchortable_index, int idx, struct luaL_field *field);
>
> /**
> * @brief A wrapper for luaL_tofield() and luaL_convertfield() that
> * tries to convert value or raise an error.
> * @param L stack
> * @param cfg configuration
> + * @param anchortable_index Stack index of table with node visit
> + and serialization info.
> * @param idx stack index
> * @param field conversion result
> * @sa lua_tofield()
> @@ -433,14 +460,14 @@ luaL_convertfield(struct lua_State *L, struct luaL_serializer *cfg, int idx,
> * (tostring) -> (nil) -> exception
> */
> static inline void
> -luaL_checkfield(struct lua_State *L, struct luaL_serializer *cfg, int idx,
> - struct luaL_field *field)
> +luaL_checkfield(struct lua_State *L, struct luaL_serializer *cfg,
> + int anchortable_index, int idx, struct luaL_field *field)
> {
> - if (luaL_tofield(L, cfg, NULL, idx, field) < 0)
> + if (luaL_tofield(L, cfg, anchortable_index, NULL, idx, field) < 0)
> luaT_error(L);
> if (field->type != MP_EXT || field->ext_type != MP_UNKNOWN_EXTENSION)
> return;
> - luaL_convertfield(L, cfg, idx, field);
> + luaL_convertfield(L, cfg, anchortable_index, idx, field);
> }
>
> void
> diff --git a/test/app/gh-3228-serializer-look-for-recursion.result b/test/app/gh-3228-serializer-look-for-recursion.result
> new file mode 100644
> index 000000000..773470c8e
> --- /dev/null
> +++ b/test/app/gh-3228-serializer-look-for-recursion.result
> @@ -0,0 +1,67 @@
> +-- test-run result file version 2
> +test_run = require('test_run').new()
> + | ---
> + | ...
> +
> +--
> +-- gh-3228: Check the error message in the case of a __serialize
> +-- function generating infinite recursion.
> +--
> +setmetatable({}, {__serialize = function(a) return a end})
> + | ---
> + | - []
> + | ...
> +setmetatable({}, {__serialize = function(a) return {a} end})
> + | ---
> + | - - []
> + | ...
> +setmetatable({}, {__serialize = function(a) return {a, a} end})
> + | ---
> + | - - &0 []
> + | - *0
> + | ...
> +setmetatable({}, {__serialize = function(a) return {a, a, a} end})
> + | ---
> + | - - &0 []
> + | - *0
> + | - *0
> + | ...
> +setmetatable({}, {__serialize = function(a) return {{a, a}, a} end})
> + | ---
> + | - - - &0 []
> + | - *0
> + | - *0
> + | ...
> +setmetatable({}, {__serialize = function(a) return {a, 1} end})
> + | ---
> + | - - []
> + | - 1
> + | ...
> +setmetatable({}, {__serialize = function(a) return {{{{a}}}} end})
> + | ---
> + | - - - - - []
> + | ...
> +setmetatable({}, {__serialize = function(a) return {{{{1}}}} end})
> + | ---
> + | - - - - - 1
> + | ...
> +b = {}
> + | ---
> + | ...
> +setmetatable({b}, {__serialize = function(a) return {a_1 = a, a_2 = a, b_1 = b, b_2 = b} end})
> + | ---
> + | - b_2: &0 []
> + | a_2: &1
> + | - *0
> + | a_1: *1
> + | b_1: *0
> + | ...
> +setmetatable({b}, {__serialize = function(a) return {a_1 = a, a_2 = {a, b}, b = b} end})
> + | ---
> + | - b: &0 []
> + | a_2:
> + | - &1
> + | - *0
> + | - *0
> + | a_1: *1
> + | ...
> diff --git a/test/app/gh-3228-serializer-look-for-recursion.test.lua b/test/app/gh-3228-serializer-look-for-recursion.test.lua
> new file mode 100644
> index 000000000..1c9b0375f
> --- /dev/null
> +++ b/test/app/gh-3228-serializer-look-for-recursion.test.lua
> @@ -0,0 +1,17 @@
> +test_run = require('test_run').new()
> +
> +--
> +-- gh-3228: Check the error message in the case of a __serialize
> +-- function generating infinite recursion.
> +--
> +setmetatable({}, {__serialize = function(a) return a end})
> +setmetatable({}, {__serialize = function(a) return {a} end})
> +setmetatable({}, {__serialize = function(a) return {a, a} end})
> +setmetatable({}, {__serialize = function(a) return {a, a, a} end})
> +setmetatable({}, {__serialize = function(a) return {{a, a}, a} end})
> +setmetatable({}, {__serialize = function(a) return {a, 1} end})
> +setmetatable({}, {__serialize = function(a) return {{{{a}}}} end})
> +setmetatable({}, {__serialize = function(a) return {{{{1}}}} end})
> +b = {}
> +setmetatable({b}, {__serialize = function(a) return {a_1 = a, a_2 = a, b_1 = b, b_2 = b} end})
> +setmetatable({b}, {__serialize = function(a) return {a_1 = a, a_2 = {a, b}, b = b} end})
> diff --git a/test/swim/swim.result b/test/swim/swim.result
> index bfc83c143..539131677 100644
> --- a/test/swim/swim.result
> +++ b/test/swim/swim.result
> @@ -1322,16 +1322,13 @@ m_list
> incarnation: cdata {generation = 0ULL, version = 1ULL}
> uuid: 00000000-0000-1000-8000-000000000002
> payload_size: 0
> - - uri: 127.0.0.1:<port>
> - status: alive
> - incarnation: cdata {generation = 0ULL, version = 2ULL}
> - uuid: 00000000-0000-1000-8000-000000000001
> - payload_size: 8
> - - uri: 127.0.0.1:<port>
> + - &0
> + uri: 127.0.0.1:<port>
> status: alive
> incarnation: cdata {generation = 0ULL, version = 2ULL}
> uuid: 00000000-0000-1000-8000-000000000001
> payload_size: 8
> + - *0
> ...
> e_list
> ---
> @@ -1374,16 +1371,13 @@ fiber.sleep(0)
> -- Two events - status update to 'left', and 'drop'.
> m_list
> ---
> -- - uri: 127.0.0.1:<port>
> - status: left
> - incarnation: cdata {generation = 0ULL, version = 1ULL}
> - uuid: 00000000-0000-1000-8000-000000000002
> - payload_size: 0
> - - uri: 127.0.0.1:<port>
> +- - &0
> + uri: 127.0.0.1:<port>
> status: left
> incarnation: cdata {generation = 0ULL, version = 1ULL}
> uuid: 00000000-0000-1000-8000-000000000002
> payload_size: 0
> + - *0
> ...
> e_list
> ---
> diff --git a/third_party/lua-cjson/lua_cjson.c b/third_party/lua-cjson/lua_cjson.c
> index 38e999870..67a9762bc 100644
> --- a/third_party/lua-cjson/lua_cjson.c
> +++ b/third_party/lua-cjson/lua_cjson.c
> @@ -354,7 +354,7 @@ static void json_append_object(lua_State *l, struct luaL_serializer *cfg,
> comma = 1;
>
> struct luaL_field field;
> - luaL_checkfield(l, cfg, -2, &field);
> + luaL_checkfield(l, cfg, 0, -2, &field);
> if (field.type == MP_UINT) {
> strbuf_append_char(json, '"');
> json_append_uint(cfg, json, field.ival);
> @@ -384,7 +384,7 @@ static void json_append_data(lua_State *l, struct luaL_serializer *cfg,
> int current_depth, strbuf_t *json)
> {
> struct luaL_field field;
> - luaL_checkfield(l, cfg, -1, &field);
> + luaL_checkfield(l, cfg, 0, -1, &field);
> switch (field.type) {
> case MP_UINT:
> return json_append_uint(cfg, json, field.ival);
> diff --git a/third_party/lua-yaml/lyaml.cc b/third_party/lua-yaml/lyaml.cc
> index 9c3a4a646..72f269e6c 100644
> --- a/third_party/lua-yaml/lyaml.cc
> +++ b/third_party/lua-yaml/lyaml.cc
> @@ -500,38 +500,23 @@ usage_error:
> static int dump_node(struct lua_yaml_dumper *dumper);
>
> static yaml_char_t *get_yaml_anchor(struct lua_yaml_dumper *dumper) {
> - const char *s = "";
> - lua_pushvalue(dumper->L, -1);
> - lua_rawget(dumper->L, dumper->anchortable_index);
> - if (!lua_toboolean(dumper->L, -1)) {
> - lua_pop(dumper->L, 1);
> - return NULL;
> - }
> -
> - if (lua_isboolean(dumper->L, -1)) {
> - /* this element is referenced more than once but has not been named */
> - char buf[32];
> - snprintf(buf, sizeof(buf), "%u", dumper->anchor_number++);
> - lua_pop(dumper->L, 1);
> - lua_pushvalue(dumper->L, -1);
> - lua_pushstring(dumper->L, buf);
> - s = lua_tostring(dumper->L, -1);
> - lua_rawset(dumper->L, dumper->anchortable_index);
> - } else {
> - /* this is an aliased element */
> + const char *anchor = NULL;
> + if (get_anchor(dumper->L, dumper->anchortable_index, &dumper->anchor_number,
> + &anchor) == 2) {
> yaml_event_t ev;
> - const char *str = lua_tostring(dumper->L, -1);
> - if (!yaml_alias_event_initialize(&ev, (yaml_char_t *) str) ||
> + if (!yaml_alias_event_initialize(&ev, (yaml_char_t *) anchor) ||
> !yaml_emitter_emit(&dumper->emitter, &ev))
> luaL_error(dumper->L, OOM_ERRMSG);
> - lua_pop(dumper->L, 1);
> + return (yaml_char_t *)"";
> }
> - return (yaml_char_t *)s;
> + return (yaml_char_t *) anchor;
> }
>
> -static int dump_table(struct lua_yaml_dumper *dumper, struct luaL_field *field){
> +static int dump_table(struct lua_yaml_dumper *dumper, struct luaL_field *field,
> + yaml_char_t *anchor) {
> yaml_event_t ev;
> - yaml_char_t *anchor = get_yaml_anchor(dumper);
> + if (anchor == NULL)
> + anchor = get_yaml_anchor(dumper);
>
> if (anchor && !*anchor) return 1;
>
> @@ -556,10 +541,12 @@ static int dump_table(struct lua_yaml_dumper *dumper, struct luaL_field *field){
> yaml_emitter_emit(&dumper->emitter, &ev) != 0 ? 1 : 0;
> }
>
> -static int dump_array(struct lua_yaml_dumper *dumper, struct luaL_field *field){
> +static int dump_array(struct lua_yaml_dumper *dumper, struct luaL_field *field,
> + yaml_char_t *anchor) {
> unsigned i;
> yaml_event_t ev;
> - yaml_char_t *anchor = get_yaml_anchor(dumper);
> + if (anchor == NULL)
> + anchor = get_yaml_anchor(dumper);
>
> if (anchor && !*anchor)
> return 1;
> @@ -621,8 +608,18 @@ static int dump_node(struct lua_yaml_dumper *dumper)
> bool unused;
> (void) unused;
>
> + yaml_char_t *anchor = NULL;
> +
> + /*
> + * Keep anchor to print it if luaL_checkfield() push new value
> + * on stack after serialization.
> + */
> + if (lua_istable(dumper->L, -1) == 1)
> + anchor = get_yaml_anchor(dumper);
> +
> int top = lua_gettop(dumper->L);
> - luaL_checkfield(dumper->L, dumper->cfg, top, &field);
> + luaL_checkfield(dumper->L, dumper->cfg, dumper->anchortable_index, top,
> + &field);
> switch(field.type) {
> case MP_UINT:
> snprintf(buf, sizeof(buf) - 1, "%" PRIu64, field.ival);
> @@ -647,9 +644,9 @@ static int dump_node(struct lua_yaml_dumper *dumper)
> len = strlen(buf);
> break;
> case MP_ARRAY:
> - return dump_array(dumper, &field);
> + return dump_array(dumper, &field, anchor);
> case MP_MAP:
> - return dump_table(dumper, &field);
> + return dump_table(dumper, &field, anchor);
> case MP_STR:
> str = lua_tolstring(dumper->L, -1, &len);
> if (yaml_is_null(str, len) || yaml_is_bool(str, len, &unused) ||
> @@ -745,35 +742,6 @@ static int append_output(void *arg, unsigned char *buf, size_t len) {
> return 1;
> }
>
> -static void find_references(struct lua_yaml_dumper *dumper) {
> - int newval = -1, type = lua_type(dumper->L, -1);
> - if (type != LUA_TTABLE)
> - return;
> -
> - lua_pushvalue(dumper->L, -1); /* push copy of table */
> - lua_rawget(dumper->L, dumper->anchortable_index);
> - if (lua_isnil(dumper->L, -1))
> - newval = 0;
> - else if (!lua_toboolean(dumper->L, -1))
> - newval = 1;
> - lua_pop(dumper->L, 1);
> - if (newval != -1) {
> - lua_pushvalue(dumper->L, -1);
> - lua_pushboolean(dumper->L, newval);
> - lua_rawset(dumper->L, dumper->anchortable_index);
> - }
> - if (newval)
> - return;
> -
> - /* recursively process other table values */
> - lua_pushnil(dumper->L);
> - while (lua_next(dumper->L, -2) != 0) {
> - find_references(dumper); /* find references on value */
> - lua_pop(dumper->L, 1);
> - find_references(dumper); /* find references on key */
> - }
> -}
> -
> int
> lua_yaml_encode(lua_State *L, struct luaL_serializer *serializer,
> const char *tag_handle, const char *tag_prefix)
> @@ -819,7 +787,7 @@ lua_yaml_encode(lua_State *L, struct luaL_serializer *serializer,
> dumper.anchortable_index = lua_gettop(L);
> dumper.anchor_number = 0;
> lua_pushvalue(L, 1); /* push copy of arg we're processing */
> - find_references(&dumper);
> + find_references(L, dumper.anchortable_index, "before");
> dump_document(&dumper);
> if (dumper.error)
> goto error;
More information about the Tarantool-patches
mailing list