Re: [Tarantool-patches] [PATCH] json: fix silent change of global json settings

[Nikita Pettik <korablev@tarantool.org>]

On 10 Feb 10:57, Olga Arkhangelskaia wrote:
> When json.decode is used with 2 arguments, 2nd argument seeps out to global
> json settings. Morover, 

Nit: Moreover.

> due to current serialier.cfg implementation it

-> serializer

> remains invisible while checking settings by json.cfg. To prevent sucj

-> such

> behaviour we stop writing to global serializer struct and use local one,
> to get one-time action.
> As was mention before json.cfg can not be trusted in this case, so to check that
> everything remained unchanged we call decode twice with and without 2nd
> argument.
> 
> Closes #4761

Note that there's no 'Closes #4761' label on your actual branch.
I guess you simply forgot to push updated branch.

> ---
> +    --
> +    -- gh-4761 json.decode silently changes global settings of json when called
> +    -- with 2d parameter
> +    --
> +    test:ok(pcall(serializer.decode,'{"1":{"b":{"c":1,"d":null}},"a":1}'))
>  
>      --
>      -- gh-3514: fix parsing integers with exponent in json
> diff --git a/third_party/lua-cjson/lua_cjson.c b/third_party/lua-cjson/lua_cjson.c
> index 3d25814f3..f855cbd80 100644
> --- a/third_party/lua-cjson/lua_cjson.c
> +++ b/third_party/lua-cjson/lua_cjson.c
> @@ -1004,13 +1004,13 @@ static int json_decode(lua_State *l)
>      luaL_argcheck(l, lua_gettop(l) == 2 || lua_gettop(l) == 1, 1,
>                    "expected 1 or 2 arguments");
>  
> +    struct luaL_serializer *cfg = luaL_checkserializer(l);

Nit: I'd add a brief comment here (to avoid any confusions concerning
copying object on the stack):

diff --git a/third_party/lua-cjson/lua_cjson.c b/third_party/lua-cjson/lua_cjson.c
index f855cbd80..c9c987c8c 100644
--- a/third_party/lua-cjson/lua_cjson.c
+++ b/third_party/lua-cjson/lua_cjson.c
@@ -1006,6 +1006,14 @@ static int json_decode(lua_State *l)
 
     struct luaL_serializer *cfg = luaL_checkserializer(l);
     struct luaL_serializer user_cfg = *cfg;
+    /*
+     * user_cfg is per-call local version of global cfg: it is
+     * used if user passes custom options to :decode() method
+     * as a separate arguments. In this case it is required
+     * to avoid modifying global parameters. Life span of
+     * user_cfg is restricted by the scope of :decode() so it
+     * is enough to allocate it on the stack.
+     */
     json.cfg = cfg;
     if (lua_gettop(l) == 2) {

> +    struct luaL_serializer user_cfg = *cfg;
> +    json.cfg = cfg;

What is more, you can avoid premature copying on the stack:

@@ -1005,9 +1005,10 @@ static int json_decode(lua_State *l)
                   "expected 1 or 2 arguments");
 
     struct luaL_serializer *cfg = luaL_checkserializer(l);
-    struct luaL_serializer user_cfg = *cfg;
+    struct luaL_serializer user_cfg;
     json.cfg = cfg;
     if (lua_gettop(l) == 2) {
+        user_cfg = *cfg;
         luaL_serializer_parse_options(l, &user_cfg);
         lua_pop(l, 1);
         json.cfg = &user_cfg;

>      if (lua_gettop(l) == 2) {
> -        struct luaL_serializer *user_cfg = luaL_checkserializer(l);
> -        luaL_serializer_parse_options(l, user_cfg);
> +        luaL_serializer_parse_options(l, &user_cfg);