Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
To: tarantool-patches@freelists.org
Cc: alexander.turenko@tarantool.org
Subject: [tarantool-patches] [PATCH v2 2/4] msgpack: make msgpackffi use encode_max_depth option
Date: Mon,  9 Sep 2019 21:00:08 +0200	[thread overview]
Message-ID: <945d59a2cc72f44f66e51c29e14cff5b4acae8c0.1568055477.git.v.shpilevoy@tarantool.org> (raw)
In-Reply-To: <cover.1568055477.git.v.shpilevoy@tarantool.org>

Msgpack Lua module is not a simple set of functions. It is a
global serializer object used by plenty of other Lua and C
modules. Msgpack as a serializer can be configured, and in theory
its configuration updates should affect all other modules. For
example, a user could change encode_max_depth:

    require('msgpack').cfg({encode_max_depth = <new_value>})

And that would make tuple:update() accept tables with <new_value>
depth without a crop.

But in fact msgpack configuration didn't affect some places, such
as this one. And all the others who use msgpackffi.

This patch fixes it, for encode_max_depth option. Other options
are still ignored.

Part of #4434
---
 src/lua/msgpackffi.lua           |  3 +--
 test/app-tap/msgpackffi.test.lua | 26 +++++++++++++++++++++++++-
 2 files changed, 26 insertions(+), 3 deletions(-)

diff --git a/src/lua/msgpackffi.lua b/src/lua/msgpackffi.lua
index f7ee44291..65c57aa6e 100644
--- a/src/lua/msgpackffi.lua
+++ b/src/lua/msgpackffi.lua
@@ -4,7 +4,6 @@ local ffi = require('ffi')
 local buffer = require('buffer')
 local builtin = ffi.C
 local msgpack = require('msgpack') -- .NULL, .array_mt, .map_mt, .cfg
-local MAXNESTING = 16
 local int8_ptr_t = ffi.typeof('int8_t *')
 local uint8_ptr_t = ffi.typeof('uint8_t *')
 local uint16_ptr_t = ffi.typeof('uint16_t *')
@@ -219,7 +218,7 @@ local function encode_r(buf, obj, level)
     elseif type(obj) == "string" then
         encode_str(buf, obj)
     elseif type(obj) == "table" then
-        if level >= MAXNESTING then -- Limit nested tables
+        if level >= msgpack.cfg.encode_max_depth then
             encode_nil(buf)
             return
         end
diff --git a/test/app-tap/msgpackffi.test.lua b/test/app-tap/msgpackffi.test.lua
index f2a8f254b..e26247625 100755
--- a/test/app-tap/msgpackffi.test.lua
+++ b/test/app-tap/msgpackffi.test.lua
@@ -36,7 +36,7 @@ local function test_offsets(test, s)
 end
 
 local function test_other(test, s)
-    test:plan(19)
+    test:plan(21)
     local buf = string.char(0x93, 0x6e, 0xcb, 0x42, 0x2b, 0xed, 0x30, 0x47,
         0x6f, 0xff, 0xff, 0xac, 0x77, 0x6b, 0x61, 0x71, 0x66, 0x7a, 0x73,
         0x7a, 0x75, 0x71, 0x71, 0x78)
@@ -68,6 +68,30 @@ local function test_other(test, s)
     test:is(#s.encode(-0x8001), 5, "len(encode(-0x8001))")
     test:is(#s.encode(-0x80000000), 5, "len(encode(-0x80000000))")
     test:is(#s.encode(-0x80000001), 9, "len(encode(-0x80000001))")
+
+    --
+    -- gh-4434: msgpackffi does not care about msgpack serializer
+    -- configuration, but it should.
+    --
+    local function check_depth(depth_to_try)
+        local t = nil
+        for i = 1, depth_to_try do t = {t} end
+        t = s.decode_unchecked(s.encode(t))
+        local level = 0
+        while t ~= nil do level = level + 1 t = t[1] end
+        return level
+    end
+    local msgpack = require('msgpack')
+    local max_depth = msgpack.cfg.encode_max_depth
+    local result_depth = check_depth(max_depth + 5)
+    test:is(result_depth, max_depth,
+            "msgpackffi uses msgpack.cfg.encode_max_depth")
+
+    msgpack.cfg({encode_max_depth = max_depth + 5})
+    result_depth = check_depth(max_depth + 5)
+    test:is(result_depth, max_depth + 5, "and uses it dynamically")
+
+    msgpack.cfg({encode_max_depth = max_depth})
 end
 
 tap.test("msgpackffi", function(test)
-- 
2.20.1 (Apple Git-117)

  parent reply	other threads:[~2019-09-09 18:56 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-09-10 20:24 [tarantool-patches] [PATCH v2 0/4] Serializer bugs Vladislav Shpilevoy
2019-09-09 19:00 ` [tarantool-patches] [PATCH v2 1/4] app: serializers update now is reflected in Lua Vladislav Shpilevoy
2019-09-12 23:22   ` [tarantool-patches] " Alexander Turenko
2019-09-13 22:32     ` Vladislav Shpilevoy
2019-09-09 19:00 ` Vladislav Shpilevoy [this message]
2019-09-12 23:24   ` [tarantool-patches] Re: [PATCH v2 2/4] msgpack: make msgpackffi use encode_max_depth option Alexander Turenko
2019-09-13 22:32     ` Vladislav Shpilevoy
2019-09-09 19:00 ` [tarantool-patches] [PATCH v2 3/4] tuple: use global msgpack serializer in Lua tuple Vladislav Shpilevoy
2019-09-12 23:27   ` [tarantool-patches] " Alexander Turenko
2019-09-13 22:32     ` Vladislav Shpilevoy
2019-09-09 19:00 ` [tarantool-patches] [PATCH v2 4/4] app: allow to raise an error on too nested tables Vladislav Shpilevoy
2019-09-12 23:32   ` [tarantool-patches] " Alexander Turenko
2019-09-13 22:32     ` Vladislav Shpilevoy
2019-09-10 20:25 ` [tarantool-patches] Re: [PATCH v2 0/4] Serializer bugs Vladislav Shpilevoy
2019-09-12 23:44 ` Alexander Turenko
2019-09-13 22:32   ` Vladislav Shpilevoy

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=945d59a2cc72f44f66e51c29e14cff5b4acae8c0.1568055477.git.v.shpilevoy@tarantool.org \
    --to=v.shpilevoy@tarantool.org \
    --cc=alexander.turenko@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --subject='Re: [tarantool-patches] [PATCH v2 2/4] msgpack: make msgpackffi use encode_max_depth option' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox