Tarantool development patches archive
 help / color / mirror / Atom feed
From: Maria <maria.khaydich@tarantool.org>
To: tarantool-patches@dev.tarantool.org, georgy@tarantool.org,
	alexander.turenko@tarantool.org, v.shpilevoy@tarantool.org
Subject: [Tarantool-patches] [PATCH] box: netbox.self and connect should work interchangeably
Date: Fri, 13 Dec 2019 00:28:38 +0300	[thread overview]
Message-ID: <20191212212838.37772-1-maria.khaydich@tarantool.org> (raw)

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
-- 
2.20.1 (Apple Git-117)

             reply	other threads:[~2019-12-12 21:28 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-12 21:28 Maria [this message]
2019-12-13 13:53 ` Oleg Babin
2020-01-17 17:08 Maria
2020-01-20 21:22 ` Vladislav Shpilevoy
2020-01-27 16:13   ` Maria Khaydich
2020-02-26 13:51     ` Igor Munkin
2020-03-03 18:05       ` Maria Khaydich
2020-03-03 22:49         ` Vladislav Shpilevoy
2020-03-06 12:19           ` Maria Khaydich
2020-03-08 22:57             ` 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=20191212212838.37772-1-maria.khaydich@tarantool.org \
    --to=maria.khaydich@tarantool.org \
    --cc=alexander.turenko@tarantool.org \
    --cc=georgy@tarantool.org \
    --cc=tarantool-patches@dev.tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH] box: netbox.self and connect should work interchangeably' \
    /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