Tarantool development patches archive
 help / color / mirror / Atom feed
From: Alexander Turenko <alexander.turenko@tarantool.org>
To: Vladimir Davydov <vdavydov.dev@gmail.com>
Cc: Alexander Turenko <alexander.turenko@tarantool.org>,
	tarantool-patches@freelists.org
Subject: [PATCH v2 5/6] net.box: add helpers to decode msgpack headers
Date: Wed,  9 Jan 2019 23:20:13 +0300	[thread overview]
Message-ID: <1f8e93d96735a92500b0ae40d51b19107399a550.1547064388.git.alexander.turenko@tarantool.org> (raw)
In-Reply-To: <cover.1547064388.git.alexander.turenko@tarantool.org>

Needed for #3276.

@TarantoolBot document
Title: net.box: helpers to decode msgpack headers

They allow to skip iproto packet and msgpack array headers and pass raw
msgpack data to some other function, say, merger.

Contracts:

```
net_box.check_iproto_data(buf.rpos, buf.wpos - buf.rpos)
    -> new_rpos
    -> nil, err_msg
msgpack.check_array(buf.rpos, buf.wpos - buf.rpos, [, arr_len])
    -> new_rpos, arr_len
    -> nil, err_msg
```

Below the example with msgpack.decode() as the function that need raw
msgpack data. It is just to illustrate the approach, there is no sense
to skip iproto/array headers manually in Lua and then decode the rest in
Lua. But it worth when the raw msgpack data is subject to process in a C
module.

```lua
local function single_select(space, ...)
    return box.space[space]:select(...)
end

local function batch_select(spaces, ...)
    local res = {}
    for _, space in ipairs(spaces) do
        table.insert(res, box.space[space]:select(...))
    end
    return res
end

_G.single_select = single_select
_G.batch_select = batch_select

local res

local buf = buffer.ibuf()
conn.space.s:select(nil, {buffer = buf})
-- check and skip iproto_data header
buf.rpos = assert(net_box.check_iproto_data(buf.rpos, buf.wpos - buf.rpos))
-- check that we really got data from :select() as result
res, buf.rpos = msgpack.decode(buf.rpos, buf.wpos - buf.rpos)
-- check that the buffer ends
assert(buf.rpos == buf.wpos)

buf:recycle()
conn:call('single_select', {'s'}, {buffer = buf})
-- check and skip the iproto_data header
buf.rpos = assert(net_box.check_iproto_data(buf.rpos, buf.wpos - buf.rpos))
-- check and skip the array around return values
buf.rpos = assert(msgpack.check_array(buf.rpos, buf.wpos - buf.rpos, 1))
-- check that we really got data from :select() as result
res, buf.rpos = msgpack.decode(buf.rpos, buf.wpos - buf.rpos)
-- check that the buffer ends
assert(buf.rpos == buf.wpos)

buf:recycle()
local spaces = {'s', 't'}
conn:call('batch_select', {spaces}, {buffer = buf})
-- check and skip the iproto_data header
buf.rpos = assert(net_box.check_iproto_data(buf.rpos, buf.wpos - buf.rpos))
-- check and skip the array around return values
buf.rpos = assert(msgpack.check_array(buf.rpos, buf.wpos - buf.rpos, 1))
-- check and skip the array header before the first select result
buf.rpos = assert(msgpack.check_array(buf.rpos, buf.wpos - buf.rpos, #spaces))
-- check that we really got data from s:select() as result
res, buf.rpos = msgpack.decode(buf.rpos, buf.wpos - buf.rpos)
-- t:select() data
res, buf.rpos = msgpack.decode(buf.rpos, buf.wpos - buf.rpos)
-- check that the buffer ends
assert(buf.rpos == buf.wpos)
```
---
 src/box/lua/net_box.c         |  49 +++++++++++
 src/box/lua/net_box.lua       |   1 +
 src/lua/msgpack.c             |  66 ++++++++++++++
 test/app-tap/msgpack.test.lua | 157 +++++++++++++++++++++++++++++++++-
 test/box/net.box.result       |  58 +++++++++++++
 test/box/net.box.test.lua     |  26 ++++++
 6 files changed, 356 insertions(+), 1 deletion(-)

diff --git a/src/box/lua/net_box.c b/src/box/lua/net_box.c
index c7063d9c8..d71f33768 100644
--- a/src/box/lua/net_box.c
+++ b/src/box/lua/net_box.c
@@ -51,6 +51,9 @@
 
 #define cfg luaL_msgpack_default
 
+static uint32_t CTID_CHAR_PTR;
+static uint32_t CTID_CONST_CHAR_PTR;
+
 static inline size_t
 netbox_prepare_request(lua_State *L, struct mpstream *stream, uint32_t r_type)
 {
@@ -745,9 +748,54 @@ netbox_decode_execute(struct lua_State *L)
 	return 2;
 }
 
+/**
+ * net_box.check_iproto_data(buf.rpos, buf.wpos - buf.rpos)
+ *     -> new_rpos
+ *     -> nil, err_msg
+ */
+int
+netbox_check_iproto_data(struct lua_State *L)
+{
+	uint32_t ctypeid;
+	const char *data = *(const char **) luaL_checkcdata(L, 1, &ctypeid);
+	if (ctypeid != CTID_CHAR_PTR && ctypeid != CTID_CONST_CHAR_PTR)
+		return luaL_error(L,
+			"net_box.check_iproto_data: 'char *' or "
+			"'const char *' expected");
+
+	if (!lua_isnumber(L, 2))
+		return luaL_error(L, "net_box.check_iproto_data: number "
+				  "expected as 2nd argument");
+	const char *end = data + lua_tointeger(L, 2);
+
+	int ok = data < end &&
+		mp_typeof(*data) == MP_MAP &&
+		mp_check_map(data, end) <= 0 &&
+		mp_decode_map(&data) == 1 &&
+		data < end &&
+		mp_typeof(*data) == MP_UINT &&
+		mp_check_uint(data, end) <= 0 &&
+		mp_decode_uint(&data) == IPROTO_DATA;
+
+	if (!ok) {
+		lua_pushnil(L);
+		lua_pushstring(L,
+			"net_box.check_iproto_data: wrong iproto data packet");
+		return 2;
+	}
+
+	*(const char **) luaL_pushcdata(L, ctypeid) = data;
+	return 1;
+}
+
 int
 luaopen_net_box(struct lua_State *L)
 {
+	CTID_CHAR_PTR = luaL_ctypeid(L, "char *");
+	assert(CTID_CHAR_PTR != 0);
+	CTID_CONST_CHAR_PTR = luaL_ctypeid(L, "const char *");
+	assert(CTID_CONST_CHAR_PTR != 0);
+
 	static const luaL_Reg net_box_lib[] = {
 		{ "encode_ping",    netbox_encode_ping },
 		{ "encode_call_16", netbox_encode_call_16 },
@@ -765,6 +813,7 @@ luaopen_net_box(struct lua_State *L)
 		{ "communicate",    netbox_communicate },
 		{ "decode_select",  netbox_decode_select },
 		{ "decode_execute", netbox_decode_execute },
+		{ "check_iproto_data", netbox_check_iproto_data },
 		{ NULL, NULL}
 	};
 	/* luaL_register_module polutes _G */
diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
index 2bf772aa8..0a38efa5a 100644
--- a/src/box/lua/net_box.lua
+++ b/src/box/lua/net_box.lua
@@ -1424,6 +1424,7 @@ local this_module = {
     new = connect, -- Tarantool < 1.7.1 compatibility,
     wrap = wrap,
     establish_connection = establish_connection,
+    check_iproto_data = internal.check_iproto_data,
 }
 
 function this_module.timeout(timeout, ...)
diff --git a/src/lua/msgpack.c b/src/lua/msgpack.c
index b47006038..fca440660 100644
--- a/src/lua/msgpack.c
+++ b/src/lua/msgpack.c
@@ -51,6 +51,7 @@ luamp_error(void *error_ctx)
 }
 
 static uint32_t CTID_CHAR_PTR;
+static uint32_t CTID_CONST_CHAR_PTR;
 static uint32_t CTID_STRUCT_IBUF;
 
 struct luaL_serializer *luaL_msgpack_default = NULL;
@@ -418,6 +419,68 @@ lua_ibuf_msgpack_decode(lua_State *L)
 	return 2;
 }
 
+/**
+ * msgpack.check_array(buf.rpos, buf.wpos - buf.rpos, [, arr_len])
+ *     -> new_rpos, arr_len
+ *     -> nil, err_msg
+ */
+static int
+lua_check_array(lua_State *L)
+{
+	uint32_t ctypeid;
+	const char *data = *(const char **) luaL_checkcdata(L, 1, &ctypeid);
+	if (ctypeid != CTID_CHAR_PTR && ctypeid != CTID_CONST_CHAR_PTR)
+		return luaL_error(L, "msgpack.check_array: 'char *' or "
+				  "'const char *' expected");
+
+	if (!lua_isnumber(L, 2))
+		return luaL_error(L, "msgpack.check_array: number expected as "
+				  "2nd argument");
+	const char *end = data + lua_tointeger(L, 2);
+
+	if (!lua_isnoneornil(L, 3) && !lua_isnumber(L, 3))
+		return luaL_error(L, "msgpack.check_array: number or nil "
+				  "expected as 3rd argument");
+
+	static const char *end_of_buffer_msg = "msgpack.check_array: "
+		"unexpected end of buffer";
+
+	if (data >= end) {
+		lua_pushnil(L);
+		lua_pushstring(L, end_of_buffer_msg);
+		return 2;
+	}
+
+	if (mp_typeof(*data) != MP_ARRAY) {
+		lua_pushnil(L);
+		lua_pushstring(L, "msgpack.check_array: wrong array header");
+		return 2;
+	}
+
+	if (mp_check_array(data, end) > 0) {
+		lua_pushnil(L);
+		lua_pushstring(L, end_of_buffer_msg);
+		return 2;
+	}
+
+	uint32_t len = mp_decode_array(&data);
+
+	if (!lua_isnoneornil(L, 3)) {
+		uint32_t exp_len = (uint32_t) lua_tointeger(L, 3);
+		if (len != exp_len) {
+			lua_pushnil(L);
+			lua_pushfstring(L, "msgpack.check_array: expected "
+					"array of length %d, got length %d",
+					len, exp_len);
+			return 2;
+		}
+	}
+
+	*(const char **) luaL_pushcdata(L, ctypeid) = data;
+	lua_pushinteger(L, len);
+	return 2;
+}
+
 static int
 lua_msgpack_new(lua_State *L);
 
@@ -426,6 +489,7 @@ static const luaL_Reg msgpacklib[] = {
 	{ "decode", lua_msgpack_decode },
 	{ "decode_unchecked", lua_msgpack_decode_unchecked },
 	{ "ibuf_decode", lua_ibuf_msgpack_decode },
+	{ "check_array", lua_check_array },
 	{ "new", lua_msgpack_new },
 	{ NULL, NULL }
 };
@@ -447,6 +511,8 @@ luaopen_msgpack(lua_State *L)
 	assert(CTID_STRUCT_IBUF != 0);
 	CTID_CHAR_PTR = luaL_ctypeid(L, "char *");
 	assert(CTID_CHAR_PTR != 0);
+	CTID_CONST_CHAR_PTR = luaL_ctypeid(L, "const char *");
+	assert(CTID_CONST_CHAR_PTR != 0);
 	luaL_msgpack_default = luaL_newserializer(L, "msgpack", msgpacklib);
 	return 1;
 }
diff --git a/test/app-tap/msgpack.test.lua b/test/app-tap/msgpack.test.lua
index 0e1692ad9..d481d2da9 100755
--- a/test/app-tap/msgpack.test.lua
+++ b/test/app-tap/msgpack.test.lua
@@ -49,9 +49,163 @@ local function test_misc(test, s)
     test:ok(not st and e:match("null"), "null ibuf")
 end
 
+local function test_check_array(test, s)
+    local ffi = require('ffi')
+
+    local good_cases = {
+        {
+            'fixarray',
+            data = '\x94\x01\x02\x03\x04',
+            exp_len = 4,
+            exp_rewind = 1,
+        },
+        {
+            'array 16',
+            data = '\xdc\x00\x04\x01\x02\x03\x04',
+            exp_len = 4,
+            exp_rewind = 3,
+        },
+        {
+            'array 32',
+            data = '\xdd\x00\x00\x00\x04\x01\x02\x03\x04',
+            exp_len = 4,
+            exp_rewind = 5,
+        },
+    }
+
+    local bad_cases = {
+        {
+            'fixmap',
+            data = '\x80',
+            exp_err = 'msgpack.check_array: wrong array header',
+        },
+        {
+            'truncated array 16',
+            data = '\xdc\x00',
+            exp_err = 'msgpack.check_array: unexpected end of buffer',
+        },
+        {
+            'truncated array 32',
+            data = '\xdd\x00\x00\x00',
+            exp_err = 'msgpack.check_array: unexpected end of buffer',
+        },
+        {
+            'zero size buffer',
+            data = '\x90',
+            size = 0,
+            exp_err = 'msgpack.check_array: unexpected end of buffer',
+        },
+        {
+            'negative size buffer',
+            data = '\x90',
+            size = -1,
+            exp_err = 'msgpack.check_array: unexpected end of buffer',
+        },
+    }
+
+    local wrong_1_arg_not_cdata_err = 'expected cdata as 1 argument'
+    local wrong_1_arg_err = "msgpack.check_array: 'char *' or " ..
+        "'const char *' expected"
+    local wrong_2_arg_err = 'msgpack.check_array: number expected as 2nd ' ..
+        'argument'
+    local wrong_3_arg_err = 'msgpack.check_array: number or nil expected as ' ..
+        '3rd argument'
+
+    local bad_api_cases = {
+        {
+            '1st argument: nil',
+            args = {nil, 1},
+            exp_err = wrong_1_arg_not_cdata_err,
+        },
+        {
+            '1st argument: not cdata',
+            args = {1, 1},
+            exp_err = wrong_1_arg_not_cdata_err,
+        },
+        {
+            '1st argument: wrong cdata type',
+            args = {box.tuple.new(), 1},
+            exp_err = wrong_1_arg_err,
+        },
+        {
+            '2nd argument: nil',
+            args = {ffi.cast('char *', '\x90'), nil},
+            exp_err = wrong_2_arg_err,
+        },
+        {
+            '2nd argument: wrong type',
+            args = {ffi.cast('char *', '\x90'), 'eee'},
+            exp_err = wrong_2_arg_err,
+        },
+        {
+            '3rd argument: wrong type',
+            args = {ffi.cast('char *', '\x90'), 1, 'eee'},
+            exp_err = wrong_3_arg_err,
+        },
+    }
+
+    -- Add good cases with wrong expected length to the bad cases.
+    for _, case in ipairs(good_cases) do
+        table.insert(bad_cases, {
+            case[1],
+            data = case.data,
+            exp_len = case.exp_len + 1,
+            exp_err = 'msgpack.check_array: expected array of length'
+        })
+    end
+
+    test:plan(4 * #good_cases + 2 * #bad_cases + #bad_api_cases)
+
+    -- Good cases: don't pass 2nd argument.
+    for _, ctype in ipairs({'char *', 'const char *'}) do
+        for _, case in ipairs(good_cases) do
+            local buf = ffi.cast(ctype, case.data)
+            local size = case.size or #case.data
+            local new_buf, len = s.check_array(buf, size)
+            local rewind = new_buf - buf
+            local description = ('good; %s; %s; %s'):format(case[1], ctype,
+                'do not pass 2nd argument')
+            test:is_deeply({len, rewind}, {case.exp_len, case.exp_rewind},
+                description)
+        end
+    end
+
+    -- Good cases: pass right 2nd argument.
+    for _, ctype in ipairs({'char *', 'const char *'}) do
+        for _, case in ipairs(good_cases) do
+            local buf = ffi.cast(ctype, case.data)
+            local size = case.size or #case.data
+            local new_buf, len = s.check_array(buf, size, case.exp_len)
+            local rewind = new_buf - buf
+            local description = ('good; %s; %s; %s'):format(case[1], ctype,
+                'pass right 2nd argument')
+            test:is_deeply({len, rewind}, {case.exp_len, case.exp_rewind},
+                description)
+        end
+    end
+
+    -- Bad cases.
+    for _, ctype in ipairs({'char *', 'const char *'}) do
+        for _, case in ipairs(bad_cases) do
+            local buf = ffi.cast(ctype, case.data)
+            local size = case.size or #case.data
+            local n, err = s.check_array(buf, size, case.exp_len)
+            local description = ('bad; %s; %s'):format(case[1], ctype)
+            test:ok(n == nil and err:startswith(case.exp_err), description)
+        end
+    end
+
+    -- Bad API usage cases.
+    for _, case in ipairs(bad_api_cases) do
+        local ok, err = pcall(s.check_array, unpack(case.args))
+        local description = 'bad API usage; ' .. case[1]
+        test:is_deeply({ok, err}, {false, case.exp_err},  description)
+    end
+end
+
 tap.test("msgpack", function(test)
     local serializer = require('msgpack')
-    test:plan(10)
+    test:plan(11)
     test:test("unsigned", common.test_unsigned, serializer)
     test:test("signed", common.test_signed, serializer)
     test:test("double", common.test_double, serializer)
@@ -62,4 +216,5 @@ tap.test("msgpack", function(test)
     test:test("ucdata", common.test_ucdata, serializer)
     test:test("offsets", test_offsets, serializer)
     test:test("misc", test_misc, serializer)
+    test:test("check_array", test_check_array, serializer)
 end)
diff --git a/test/box/net.box.result b/test/box/net.box.result
index 2b5a84646..98ba9598e 100644
--- a/test/box/net.box.result
+++ b/test/box/net.box.result
@@ -3433,6 +3433,64 @@ c
 c:close()
 ---
 ...
+ffi = require('ffi')
+---
+...
+-- Case: valid iproto_data packet; char *.
+data = '\x81\x30\x90'
+---
+...
+rpos = ffi.cast('char *', data)
+---
+...
+net.check_iproto_data(rpos, #data) - rpos -- 2
+---
+- 2
+...
+-- Case: valid iproto_data packet; const char *.
+rpos = ffi.cast('const char *', data)
+---
+...
+net.check_iproto_data(rpos, #data) - rpos -- 2
+---
+- 2
+...
+-- Case: invalid iproto_data packet.
+data = '\x91\x01'
+---
+...
+rpos = ffi.cast('char *', data)
+---
+...
+net.check_iproto_data(rpos, #data) -- error
+---
+- null
+- 'net_box.check_iproto_data: wrong iproto data packet'
+...
+-- Case: truncated msgpack.
+data = '\x81'
+---
+...
+rpos = ffi.cast('char *', data)
+---
+...
+net.check_iproto_data(rpos, #data) -- error
+---
+- null
+- 'net_box.check_iproto_data: wrong iproto data packet'
+...
+-- Case: zero size buffer.
+data = ''
+---
+...
+rpos = ffi.cast('char *', data)
+---
+...
+net.check_iproto_data(rpos, #data) -- error
+---
+- null
+- 'net_box.check_iproto_data: wrong iproto data packet'
+...
 box.schema.func.drop('do_long')
 ---
 ...
diff --git a/test/box/net.box.test.lua b/test/box/net.box.test.lua
index 96d822820..f89cf7f4d 100644
--- a/test/box/net.box.test.lua
+++ b/test/box/net.box.test.lua
@@ -1388,6 +1388,32 @@ c = net.connect('8.8.8.8:123456', {wait_connected = false})
 c
 c:close()
 
+ffi = require('ffi')
+
+-- Case: valid iproto_data packet; char *.
+data = '\x81\x30\x90'
+rpos = ffi.cast('char *', data)
+net.check_iproto_data(rpos, #data) - rpos -- 2
+
+-- Case: valid iproto_data packet; const char *.
+rpos = ffi.cast('const char *', data)
+net.check_iproto_data(rpos, #data) - rpos -- 2
+
+-- Case: invalid iproto_data packet.
+data = '\x91\x01'
+rpos = ffi.cast('char *', data)
+net.check_iproto_data(rpos, #data) -- error
+
+-- Case: truncated msgpack.
+data = '\x81'
+rpos = ffi.cast('char *', data)
+net.check_iproto_data(rpos, #data) -- error
+
+-- Case: zero size buffer.
+data = ''
+rpos = ffi.cast('char *', data)
+net.check_iproto_data(rpos, #data) -- error
+
 box.schema.func.drop('do_long')
 box.schema.user.revoke('guest', 'write', 'space', '_schema')
 box.schema.user.revoke('guest', 'read,write', 'space', '_space')
-- 
2.20.1

  parent reply	other threads:[~2019-01-09 20:20 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-01-09 20:20 [PATCH v2 0/6] Merger Alexander Turenko
2019-01-09 20:20 ` [PATCH v2 1/6] Add luaL_iscallable with support of cdata metatype Alexander Turenko
2019-01-10 12:21   ` Vladimir Davydov
2019-01-09 20:20 ` [PATCH v2 2/6] Add functions to ease using Lua iterators from C Alexander Turenko
2019-01-10 12:29   ` Vladimir Davydov
2019-01-15 23:26     ` Alexander Turenko
2019-01-16  8:18       ` Vladimir Davydov
2019-01-16 11:40         ` Alexander Turenko
2019-01-16 12:20           ` Vladimir Davydov
2019-01-17  1:20             ` Alexander Turenko
2019-01-28 18:17         ` Alexander Turenko
2019-03-01  4:04           ` Alexander Turenko
2019-01-09 20:20 ` [PATCH v2 3/6] lua: add luaT_newtuple() Alexander Turenko
2019-01-10 12:44   ` Vladimir Davydov
2019-01-18 21:58     ` Alexander Turenko
2019-01-23 16:12       ` Vladimir Davydov
2019-01-28 16:51         ` Alexander Turenko
2019-03-01  4:08           ` Alexander Turenko
2019-01-09 20:20 ` [PATCH v2 4/6] lua: add luaT_new_key_def() Alexander Turenko
2019-01-10 13:07   ` Vladimir Davydov
2019-01-29 18:52     ` Alexander Turenko
2019-01-30 10:58       ` Alexander Turenko
2019-03-01  4:10         ` Alexander Turenko
2019-01-09 20:20 ` Alexander Turenko [this message]
2019-01-10 17:29   ` [PATCH v2 5/6] net.box: add helpers to decode msgpack headers Vladimir Davydov
2019-02-01 15:11     ` Alexander Turenko
2019-03-05 12:00       ` Alexander Turenko
2019-01-09 20:20 ` [PATCH v2 6/6] Add merger for tuple streams Alexander Turenko

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=1f8e93d96735a92500b0ae40d51b19107399a550.1547064388.git.alexander.turenko@tarantool.org \
    --to=alexander.turenko@tarantool.org \
    --cc=tarantool-patches@freelists.org \
    --cc=vdavydov.dev@gmail.com \
    --subject='Re: [PATCH v2 5/6] net.box: add helpers to decode msgpack headers' \
    /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