<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<p>Hello, thanks for your patch!</p>
<p> </p>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+ for i = 1, select('#', ...) do
+ if type(results[i] == 'cdata') then
+ results[i] = msgpack.decode(msgpack.encode(results[i]))
</pre>
</blockquote>
I suppose that it should be "type(results[i]) == 'cdata'". Now this
condition works always.<br>
<p> </p>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+ if not ... then
+ return ...
+ end</pre>
</blockquote>
<p>What do you want to achieve with it?</p>
<p>This condition can't distinguish what it's a function without
arguments or has nil as first argument.</p>
<p>I think this check should be dropped or replaced to something as
"select('#', ...) == 0".<br>
</p>
<p> </p>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">+ local results = {...}</pre>
</blockquote>
<p>This transformation drops trailing nils that should be evaluated
to box.NULL.<br>
</p>
<p>Please, add tests to check that all nils you passed will be
converted to box.NULLs after transformation.<br>
</p>
<p><br>
</p>
<p><span style="color: rgb(0, 0, 0); font-family: Helvetica, Arial,
Tahoma, Verdana, sans-serif; font-size: 15px; font-style:
normal; font-variant-ligatures: normal; font-variant-caps:
normal; font-weight: 400; letter-spacing: normal; orphans: 2;
text-align: start; text-indent: 0px; text-transform: none;
white-space: normal; widows: 2; word-spacing: 0px;
-webkit-text-stroke-width: 0px; background-color: rgb(255, 255,
255); text-decoration-style: initial; text-decoration-color:
initial; display: inline !important; float: none;">--</span><br
style="color: rgb(0, 0, 0); font-family: Helvetica, Arial,
Tahoma, Verdana, sans-serif; font-size: 15px; font-style:
normal; font-variant-ligatures: normal; font-variant-caps:
normal; font-weight: 400; letter-spacing: normal; orphans: 2;
text-align: start; text-indent: 0px; text-transform: none;
white-space: normal; widows: 2; word-spacing: 0px;
-webkit-text-stroke-width: 0px; background-color: rgb(255, 255,
255); text-decoration-style: initial; text-decoration-color:
initial;">
<span style="color: rgb(0, 0, 0); font-family: Helvetica, Arial,
Tahoma, Verdana, sans-serif; font-size: 15px; font-style:
normal; font-variant-ligatures: normal; font-variant-caps:
normal; font-weight: 400; letter-spacing: normal; orphans: 2;
text-align: start; text-indent: 0px; text-transform: none;
white-space: normal; widows: 2; word-spacing: 0px;
-webkit-text-stroke-width: 0px; background-color: rgb(255, 255,
255); text-decoration-style: initial; text-decoration-color:
initial; display: inline !important; float: none;">Oleg Babin</span></p>
<p><br>
</p>
<div class="moz-cite-prefix">On 13/12/2019 00:28, Maria wrote:<br>
</div>
<blockquote type="cite"
cite="mid:20191212212838.37772-1-maria.khaydich@tarantool.org">
<pre class="moz-quote-pre" wrap="">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:
<a class="moz-txt-link-freetext" href="https://github.com/tarantool/tarantool/issues/4513">https://github.com/tarantool/tarantool/issues/4513</a>
<a class="moz-txt-link-freetext" href="https://github.com/tarantool/tarantool/issues/4602">https://github.com/tarantool/tarantool/issues/4602</a>
Branch:
<a class="moz-txt-link-freetext" href="https://github.com/tarantool/tarantool/compare/eljashm/gh-4513-netbox.self-convert-tuples-to-table-type">https://github.com/tarantool/tarantool/compare/eljashm/gh-4513-netbox.self-convert-tuples-to-table-type</a>
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
</pre>
</blockquote>
</body>
</html>