From 7f7c673afb5b7af95d120b91b0a3e32ba5e03afa Mon Sep 17 00:00:00 2001
From: Alexander Turenko <alexander.turenko@tarantool.org>
Date: Wed, 6 Nov 2019 02:05:03 +0300
Subject: [PATCH] lua: don't modify pointer type in msgpack.decode*
msgpackffi.decode_unchecked([const] char *) returns two values: a
decoded result and a new pointer within passed buffer. After #3926 a
cdata type of the returned pointer follows a type of passed buffer.
This commit modifies behaviour of msgpack module in the same way. The
following functions now returns cdata<char *> or cdata<const char *>
depending of its argument:
* msgpack.decode(cdata<[const] char *>, number)
* msgpack.decode_unchecked(cdata<[const] char *>)
* msgpack.decode_array_header(cdata<[const] char *>, number)
* msgpack.decode_map_header(cdata<[const] char *>, number)
Follows up #3926.
(cherry picked from commit 2b9ef8d159a4492c1bdacb221dd20e14e67ec35d)
---
src/lua/msgpack.c | 22 +++++++----
src/lua/utils.c | 8 ++--
src/lua/utils.h | 6 +--
test/app-tap/lua/serializer_test.lua | 59 ++++++++++++++++++++++++++++
test/app-tap/msgpack.test.lua | 33 ++++++++++++++--
test/app-tap/msgpackffi.test.lua | 1 +
6 files changed, 111 insertions(+), 18 deletions(-)
diff --git a/src/lua/msgpack.c b/src/lua/msgpack.c
index 3229e01d4b..535400ff2f 100644
--- a/src/lua/msgpack.c
+++ b/src/lua/msgpack.c
@@ -339,7 +339,8 @@ static int
lua_msgpack_decode_cdata(lua_State *L, bool check)
{
const char *data;
- if (luaL_checkconstchar(L, 1, &data) != 0) {
+ uint32_t cdata_type;
+ if (luaL_checkconstchar(L, 1, &data, &cdata_type) != 0) {
return luaL_error(L, "msgpack.decode: "
"a Lua string or 'char *' expected");
}
@@ -355,7 +356,7 @@ lua_msgpack_decode_cdata(lua_State *L, bool check)
}
struct luaL_serializer *cfg = luaL_checkserializer(L);
luamp_decode(L, cfg, &data);
- *(const char **)luaL_pushcdata(L, CTID_CHAR_PTR) = data;
+ *(const char **)luaL_pushcdata(L, cdata_type) = data;
return 2;
}
@@ -437,7 +438,8 @@ lua_ibuf_msgpack_decode(lua_State *L)
*/
static int
verify_decode_header_args(lua_State *L, const char *func_name,
- const char **data_p, ptrdiff_t *size_p)
+ const char **data_p, uint32_t *cdata_type_p,
+ ptrdiff_t *size_p)
{
/* Verify arguments count. */
if (lua_gettop(L) != 2)
@@ -445,7 +447,8 @@ verify_decode_header_args(lua_State *L, const char *func_name,
/* Verify ptr type. */
const char *data;
- if (luaL_checkconstchar(L, 1, &data) != 0)
+ uint32_t cdata_type;
+ if (luaL_checkconstchar(L, 1, &data, &cdata_type) != 0)
return luaL_error(L, "%s: 'char *' expected", func_name);
/* Verify size type and value. */
@@ -455,6 +458,7 @@ verify_decode_header_args(lua_State *L, const char *func_name,
*data_p = data;
*size_p = size;
+ *cdata_type_p = cdata_type;
return 0;
}
@@ -468,8 +472,9 @@ lua_decode_array_header(lua_State *L)
{
const char *func_name = "msgpack.decode_array_header";
const char *data;
+ uint32_t cdata_type;
ptrdiff_t size;
- verify_decode_header_args(L, func_name, &data, &size);
+ verify_decode_header_args(L, func_name, &data, &cdata_type, &size);
if (mp_typeof(*data) != MP_ARRAY)
return luaL_error(L, "%s: unexpected msgpack type", func_name);
@@ -480,7 +485,7 @@ lua_decode_array_header(lua_State *L)
uint32_t len = mp_decode_array(&data);
lua_pushinteger(L, len);
- *(const char **) luaL_pushcdata(L, CTID_CHAR_PTR) = data;
+ *(const char **) luaL_pushcdata(L, cdata_type) = data;
return 2;
}
@@ -493,8 +498,9 @@ lua_decode_map_header(lua_State *L)
{
const char *func_name = "msgpack.decode_map_header";
const char *data;
+ uint32_t cdata_type;
ptrdiff_t size;
- verify_decode_header_args(L, func_name, &data, &size);
+ verify_decode_header_args(L, func_name, &data, &cdata_type, &size);
if (mp_typeof(*data) != MP_MAP)
return luaL_error(L, "%s: unexpected msgpack type", func_name);
@@ -505,7 +511,7 @@ lua_decode_map_header(lua_State *L)
uint32_t len = mp_decode_map(&data);
lua_pushinteger(L, len);
- *(const char **) luaL_pushcdata(L, CTID_CHAR_PTR) = data;
+ *(const char **) luaL_pushcdata(L, cdata_type) = data;
return 2;
}
diff --git a/src/lua/utils.c b/src/lua/utils.c
index 2f5b310f9f..1e8e55d1a3 100644
--- a/src/lua/utils.c
+++ b/src/lua/utils.c
@@ -43,8 +43,8 @@ int luaL_array_metatable_ref = LUA_REFNIL;
static uint32_t CTID_STRUCT_IBUF;
static uint32_t CTID_STRUCT_IBUF_PTR;
-uint32_t CTID_CHAR_PTR;
-uint32_t CTID_CONST_CHAR_PTR;
+static uint32_t CTID_CHAR_PTR;
+static uint32_t CTID_CONST_CHAR_PTR;
void *
luaL_pushcdata(struct lua_State *L, uint32_t ctypeid)
@@ -1125,7 +1125,8 @@ luaL_checkibuf(struct lua_State *L, int idx)
}
int
-luaL_checkconstchar(struct lua_State *L, int idx, const char **res)
+luaL_checkconstchar(struct lua_State *L, int idx, const char **res,
+ uint32_t *cdata_type_p)
{
if (lua_type(L, idx) != LUA_TCDATA)
return -1;
@@ -1134,6 +1135,7 @@ luaL_checkconstchar(struct lua_State *L, int idx, const char **res)
if (cdata_type != CTID_CHAR_PTR && cdata_type != CTID_CONST_CHAR_PTR)
return -1;
*res = cdata != NULL ? *(const char **) cdata : NULL;
+ *cdata_type_p = cdata_type;
return 0;
}
diff --git a/src/lua/utils.h b/src/lua/utils.h
index 6bbeb41ef1..7507e86852 100644
--- a/src/lua/utils.h
+++ b/src/lua/utils.h
@@ -68,9 +68,6 @@ struct ibuf;
extern struct lua_State *tarantool_L;
extern struct ibuf *tarantool_lua_ibuf;
-extern uint32_t CTID_CONST_CHAR_PTR;
-extern uint32_t CTID_CHAR_PTR;
-
/** \cond public */
/**
@@ -629,7 +626,8 @@ luaL_checkibuf(struct lua_State *L, int idx);
* char pointer.
*/
int
-luaL_checkconstchar(struct lua_State *L, int idx, const char **res);
+luaL_checkconstchar(struct lua_State *L, int idx, const char **res,
+ uint32_t *cdata_type_p);
/* {{{ Helper functions to interact with a Lua iterator from C */
diff --git a/test/app-tap/lua/serializer_test.lua b/test/app-tap/lua/serializer_test.lua
index cfb4d22737..5d377712fa 100644
--- a/test/app-tap/lua/serializer_test.lua
+++ b/test/app-tap/lua/serializer_test.lua
@@ -424,6 +424,64 @@ local function test_depth(test, s)
s.cfg({encode_deep_as_nil = deep_as_nil, encode_max_depth = max_depth})
end
+-- gh-3926: Ensure that a returned pointer has the same cdata type
+-- as passed argument.
+--
+-- The test case is applicable for msgpack and msgpackffi.
+local function test_decode_buffer(test, s)
+ local cases = {
+ {
+ 'decode(cdata<const char *>, number)',
+ func = s.decode,
+ args = {ffi.cast('const char *', '\x93\x01\x02\x03'), 4},
+ exp_res = {1, 2, 3},
+ exp_rewind = 4,
+ },
+ {
+ 'decode(cdata<char *>, number)',
+ func = s.decode,
+ args = {ffi.cast('char *', '\x93\x01\x02\x03'), 4},
+ exp_res = {1, 2, 3},
+ exp_rewind = 4,
+ },
+ {
+ 'decode_unchecked(cdata<const char *>)',
+ func = s.decode_unchecked,
+ args = {ffi.cast('const char *', '\x93\x01\x02\x03')},
+ exp_res = {1, 2, 3},
+ exp_rewind = 4,
+ },
+ {
+ 'decode_unchecked(cdata<char *>)',
+ func = s.decode_unchecked,
+ args = {ffi.cast('char *', '\x93\x01\x02\x03')},
+ exp_res = {1, 2, 3},
+ exp_rewind = 4,
+ },
+ }
+
+ test:plan(#cases)
+
+ for _, case in ipairs(cases) do
+ test:test(case[1], function(test)
+ test:plan(4)
+ local args_len = table.maxn(case.args)
+ local res, res_buf = case.func(unpack(case.args, 1, args_len))
+ test:is_deeply(res, case.exp_res, 'verify result')
+ local buf = case.args[1]
+ local rewind = res_buf - buf
+ test:is(rewind, case.exp_rewind, 'verify resulting buffer')
+ -- test:iscdata() is not sufficient here, because it
+ -- ignores 'const' qualifier (because of using
+ -- ffi.istype()).
+ test:is(type(res_buf), 'cdata', 'verify resulting buffer type')
+ local buf_ctype = tostring(ffi.typeof(buf))
+ local res_buf_ctype = tostring(ffi.typeof(res_buf))
+ test:is(res_buf_ctype, buf_ctype, 'verify resulting buffer ctype')
+ end)
+ end
+end
+
return {
test_unsigned = test_unsigned;
test_signed = test_signed;
@@ -434,4 +492,5 @@ return {
test_table = test_table;
test_ucdata = test_ucdata;
test_depth = test_depth;
+ test_decode_buffer = test_decode_buffer;
}
diff --git a/test/app-tap/msgpack.test.lua b/test/app-tap/msgpack.test.lua
index 752f107a85..1df9d23724 100755
--- a/test/app-tap/msgpack.test.lua
+++ b/test/app-tap/msgpack.test.lua
@@ -136,6 +136,27 @@ local function test_decode_array_map_header(test, s)
size = 4,
exp_err = end_of_buffer_err,
},
+ -- gh-3926: Ensure that a returned pointer has the same
+ -- cdata type as passed argument.
+ --
+ -- cdata<char *> arguments are passed in the cases above,
+ -- so only cdata<const char *> argument is checked here.
+ {
+ 'fixarray (const char *)',
+ func = s.decode_array_header,
+ data = ffi.cast('const char *', '\x94'),
+ size = 1,
+ exp_len = 4,
+ exp_rewind = 1,
+ },
+ {
+ 'fixmap (const char *)',
+ func = s.decode_map_header,
+ data = ffi.cast('const char *', '\x84'),
+ size = 1,
+ exp_len = 4,
+ exp_rewind = 1,
+ },
}
local bad_api_cases = {
@@ -206,9 +227,14 @@ local function test_decode_array_map_header(test, s)
else
local len, new_buf = case.func(case.data, case.size)
local rewind = new_buf - case.data
+
+ -- gh-3926: Verify cdata type of a returned buffer.
+ local data_ctype = tostring(ffi.typeof(case.data))
+ local new_buf_ctype = tostring(ffi.typeof(new_buf))
+
local description = ('good; %s'):format(case[1])
- test:is_deeply({len, rewind}, {case.exp_len, case.exp_rewind},
- description)
+ test:is_deeply({len, rewind, new_buf_ctype}, {case.exp_len,
+ case.exp_rewind, data_ctype}, description)
end
end
@@ -231,7 +257,7 @@ end
tap.test("msgpack", function(test)
local serializer = require('msgpack')
- test:plan(12)
+ test:plan(13)
test:test("unsigned", common.test_unsigned, serializer)
test:test("signed", common.test_signed, serializer)
test:test("double", common.test_double, serializer)
@@ -244,4 +270,5 @@ tap.test("msgpack", function(test)
test:test("offsets", test_offsets, serializer)
test:test("misc", test_misc, serializer)
test:test("decode_array_map", test_decode_array_map_header, serializer)
+ test:test("decode_buffer", common.test_decode_buffer, serializer)
end)
diff --git a/test/app-tap/msgpackffi.test.lua b/test/app-tap/msgpackffi.test.lua
index 1059449640..e82c7401ac 100755
--- a/test/app-tap/msgpackffi.test.lua
+++ b/test/app-tap/msgpackffi.test.lua
@@ -129,4 +129,5 @@ tap.test("msgpackffi", function(test)
--test:test("ucdata", common.test_ucdata, serializer)
test:test("offsets", test_offsets, serializer)
test:test("other", test_other, serializer)
+ test:test("decode_buffer", common.test_decode_buffer, serializer)
end)
--
GitLab