diff --git a/src/lua/msgpack.c b/src/lua/msgpack.c index ef315b692ab044ffbbb25a4c3d300b67cd9422e1..edbc15b72bdbf666e0fc43b7b2671860982db369 100644 --- a/src/lua/msgpack.c +++ b/src/lua/msgpack.c @@ -370,7 +370,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"); } @@ -386,7 +387,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; } @@ -468,7 +469,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) @@ -476,7 +478,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. */ @@ -486,6 +489,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; } @@ -499,8 +503,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); @@ -511,7 +516,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; } @@ -524,8 +529,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); @@ -536,7 +542,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 208b64eb115f3e2ca5dee3b0199f3664e0f147f2..de14c778c6dad685d4f2105de646b22a5eb72e11 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; uint32_t CTID_DECIMAL; @@ -1135,7 +1135,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; @@ -1144,6 +1145,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 d9fb0704fb24af64e0c0ed34fb3253c509b38a9c..0b36727695a4b2adf89b2ff5360823112075adad 100644 --- a/src/lua/utils.h +++ b/src/lua/utils.h @@ -71,9 +71,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 */ /** @@ -635,7 +632,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 03855975681390d1346c1e56e4e6d458c4f971fc..47edac6214a51c4c6233c51b2327d6a112010279 100644 --- a/test/app-tap/lua/serializer_test.lua +++ b/test/app-tap/lua/serializer_test.lua @@ -437,6 +437,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; @@ -448,4 +506,5 @@ return { test_ucdata = test_ucdata; test_decimal = test_decimal; 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 752f107a855c19c2de2ab9bf52e01b5bf9a50bd5..1df9d23724cfea11713b68baf33969e0dbddc42d 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 36ac26b7ef3a6d198e61e3a3b23574bb2b83692b..d20ed4ea7ab7fe89daf81bd39cba4d71eb90d001 100755 --- a/test/app-tap/msgpackffi.test.lua +++ b/test/app-tap/msgpackffi.test.lua @@ -130,4 +130,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)