Hello, thanks for your patch! > + for i = 1, select('#', ...) do > + if type(results[i] == 'cdata') then > + results[i] = msgpack.decode(msgpack.encode(results[i])) I suppose that it should be  "type(results[i]) == 'cdata'". Now this condition works always. > + if not ... then > + return ... > + end What do you want to achieve with it? This condition can't distinguish what it's a function without arguments or has nil as first argument. I think this check should be dropped or replaced to something as "select('#', ...) == 0". > + local results = {...} This transformation drops trailing nils that should be evaluated to box.NULL. Please, add tests to check that all nils you passed will be converted to box.NULLs after transformation. -- Oleg Babin On 13/12/2019 00:28, Maria wrote: > Despite what was stated in the documentation, netbox.connect was not always > equivalent to netbox.self. In particular, they converted tuple to different > types - table and cdata respectively. > Netbox.self also allowed to modify source objects after transfer, which was > not an example of expected behavior either. > > The patch fixes both those issues and covers all cases where netbox.self and > connect perform conversion of types - e.g., for box.error. > > Closes #4513, #4602 > --- > Issues: > https://github.com/tarantool/tarantool/issues/4513 > https://github.com/tarantool/tarantool/issues/4602 > Branch: > https://github.com/tarantool/tarantool/compare/eljashm/gh-4513-netbox.self-convert-tuples-to-table-type > > src/box/lua/net_box.lua | 11 +++- > test/box/net.box.result | 4 +- > test/box/net_connect_self.result | 81 ++++++++++++++++++++++++++++++ > test/box/net_connect_self.test.lua | 33 ++++++++++++ > 4 files changed, 125 insertions(+), 4 deletions(-) > create mode 100644 test/box/net_connect_self.result > create mode 100644 test/box/net_connect_self.test.lua > > diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua > index c2e1bb9c4..aa36ba7d1 100644 > --- a/src/box/lua/net_box.lua > +++ b/src/box/lua/net_box.lua > @@ -1523,7 +1523,16 @@ local function handle_eval_result(status, ...) > rollback() > return box.error(E_PROC_LUA, (...)) > end > - return ... > + if not ... then > + return ... > + end > + local results = {...} > + for i = 1, select('#', ...) do > + if type(results[i] == 'cdata') then > + results[i] = msgpack.decode(msgpack.encode(results[i])) > + end > + end > + return unpack(results) > end > > this_module.self = { > diff --git a/test/box/net.box.result b/test/box/net.box.result > index e3dabf7d9..836a4fe8c 100644 > --- a/test/box/net.box.result > +++ b/test/box/net.box.result > @@ -626,9 +626,7 @@ box.schema.func.drop('pause') > -- call > remote.self:call('test_foo', {'a', 'b', 'c'}) > --- > -- - - a: 1 > - - - b: 2 > - - c > +- [[{'a': 1}], [{'b': 2}], 'c'] > ... > cn:call('test_foo', {'a', 'b', 'c'}) > --- > diff --git a/test/box/net_connect_self.result b/test/box/net_connect_self.result > new file mode 100644 > index 000000000..b6ec38fb8 > --- /dev/null > +++ b/test/box/net_connect_self.result > @@ -0,0 +1,81 @@ > +-- test-run result file version 2 > +remote = require('net.box') > + | --- > + | ... > +-- > +-- gh-4513 netbox.connect and netbox.self should be interchangeable > +-- > +space = box.schema.space.create('gh4513') > + | --- > + | ... > +box.schema.user.grant('guest','read, write, execute','universe') > + | --- > + | ... > +idx = box.space.gh4513:create_index('primary') > + | --- > + | ... > +box.space.gh4513:insert({1}) > + | --- > + | - [1] > + | ... > + > +type(remote.connect(box.cfg.listen):eval("return box.space.gh4513:get({1})")) > + | --- > + | - table > + | ... > +type(remote.self:eval("return box.space.gh4513:get{1}")) > + | --- > + | - table > + | ... > + > +type(remote.connect(box.cfg.listen):eval('return box.error.new(1, "test error")')) > + | --- > + | - string > + | ... > +type(remote.self:eval('return box.error.new(1, "test error")')) > + | --- > + | - string > + | ... > + > +type(remote.self:eval("return box.NULL")) > + | --- > + | - cdata > + | ... > +type(remote.connect(box.cfg.listen):eval("return box.NULL")) > + | --- > + | - cdata > + | ... > + > +-- > +-- gh-4602 net.box:self allows to modify source object after transfer > +-- > +x = {} > + | --- > + | ... > +function test() return x end > + | --- > + | ... > +box.schema.func.create('test') > + | --- > + | ... > +box.schema.user.grant('guest', 'execute', 'function', 'test') > + | --- > + | ... > + > +y = remote.connect(box.cfg.listen):call('test') > + | --- > + | ... > +-- should be false > +y == x > + | --- > + | - false > + | ... > + > +z = remote.self:call('test') > + | --- > + | ... > +-- should be false as well > +z == x > + | --- > + | - false > + | ... > diff --git a/test/box/net_connect_self.test.lua b/test/box/net_connect_self.test.lua > new file mode 100644 > index 000000000..76b16704f > --- /dev/null > +++ b/test/box/net_connect_self.test.lua > @@ -0,0 +1,33 @@ > +remote = require('net.box') > +-- > +-- gh-4513 netbox.connect and netbox.self should be interchangeable > +-- > +space = box.schema.space.create('gh4513') > +box.schema.user.grant('guest','read, write, execute','universe') > +idx = box.space.gh4513:create_index('primary') > +box.space.gh4513:insert({1}) > + > +type(remote.connect(box.cfg.listen):eval("return box.space.gh4513:get({1})")) > +type(remote.self:eval("return box.space.gh4513:get{1}")) > + > +type(remote.connect(box.cfg.listen):eval('return box.error.new(1, "test error")')) > +type(remote.self:eval('return box.error.new(1, "test error")')) > + > +type(remote.self:eval("return box.NULL")) > +type(remote.connect(box.cfg.listen):eval("return box.NULL")) > + > +-- > +-- gh-4602 net.box:self allows to modify source object after transfer > +-- > +x = {} > +function test() return x end > +box.schema.func.create('test') > +box.schema.user.grant('guest', 'execute', 'function', 'test') > + > +y = remote.connect(box.cfg.listen):call('test') > +-- should be false > +y == x > + > +z = remote.self:call('test') > +-- should be false as well > +z == x