Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH] box: netbox.self and connect should work interchangeably
@ 2020-01-17 17:08 Maria
  2020-01-20 21:22 ` Vladislav Shpilevoy
  0 siblings, 1 reply; 10+ messages in thread
From: Maria @ 2020-01-17 17:08 UTC (permalink / raw)
  To: tarantool-patches, imun, v.shpilevoy

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.

The patch fixes the issue and covers all cases where netbox.self and connect
perform conversion of types - e.g., for box.error.

Closes #4513
---
Branch:
https://github.com/tarantool/tarantool/compare/eljashm/gh-4513-netbox.self-convert-tuples-to-table-type
Issue:
https://github.com/tarantool/tarantool/issues/4513

 src/box/lua/net_box.lua       | 10 ++++++++-
 test/app-tap/debug.result     |  8 +++----
 test/box-tap/net.box.test.lua | 39 ++++++++++++++++++++++++++++++++---
 3 files changed, 49 insertions(+), 8 deletions(-)

diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua
index b4811edfa..47ac72b80 100644
--- a/src/box/lua/net_box.lua
+++ b/src/box/lua/net_box.lua
@@ -1550,7 +1550,15 @@ local function handle_eval_result(status, ...)
         rollback()
         return box.error(E_PROC_LUA, (...))
     end
-    return ...
+
+    local results = {...}
+    for i = 0, select('#', results) 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/app-tap/debug.result b/test/app-tap/debug.result
index 72934b076..cdc990307 100644
--- a/test/app-tap/debug.result
+++ b/test/app-tap/debug.result
@@ -15,15 +15,15 @@ nil
 Exec: tarantool -e "print(debug.__dir__); os.exit(0)"
 .
 Exec: tarantool -e "print(require('net.box').self:call('debug.sourcefile')); os.exit(0)"
-nil
+
 Exec: tarantool -e "print(require('net.box').self:call('debug.sourcedir')); os.exit(0)"
 .
 Exec: tarantool -e "fn = function() return debug.__file__ end; print(require('net.box').self:call('fn')); os.exit(0)"
-nil
+
 Exec: tarantool -e "fn = function() return debug.__dir__ end; print(require('net.box').self:call('fn')); os.exit(0)"
 .
 Exec: tarantool -e "fn = function() local res = debug.sourcefile(); return res end; print(require('net.box').self:call('fn')); os.exit(0)"
-nil
+
 Exec: tarantool -e "fn = function() local res = debug.sourcedir(); return res end; print(require('net.box').self:call('fn')); os.exit(0)"
 .
 Exec: tarantool -e "print(loadstring('return debug.sourcefile()')()); os.exit(0)"
@@ -52,7 +52,7 @@ debug/test.lua
 Source: print(debug.__dir__)
 debug
 Source: print(require('net.box').self:call('debug.sourcefile'))
-nil
+
 Source: print(require('net.box').self:call('debug.sourcedir'))
 .
 Source: fn = function() return debug.__file__ end; print(require('net.box').self:call('fn'))
diff --git a/test/box-tap/net.box.test.lua b/test/box-tap/net.box.test.lua
index a46f28ad0..fefbe7ff7 100755
--- a/test/box-tap/net.box.test.lua
+++ b/test/box-tap/net.box.test.lua
@@ -7,7 +7,7 @@ local net_box = require('net.box')
 local test_run = require('test_run')
 local inspector = test_run.new()
 
-test:plan(5)
+test:plan(11)
 
 -- create tarantool instance
 test:is(
@@ -27,8 +27,41 @@ test:is(conn.space ~= nil, true, 'space exists')
 -- gh-1814: Segfault if using `net.box` before `box.cfg` start
 test:ok(not pcall(function() conn.space._vspace:insert() end), "error handling")
 
+--
+-- gh-4513 netbox.connect and netbox.self should be interchangeable
+--
+box.cfg{
+    listen = os.getenv('LISTEN')
+}
+
+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})
+
+local dst = box.cfg.listen
+local t1 = type(net_box.connect(dst):eval("return box.space.gh4513:get({1})"))
+local t2 = type(net_box.self:eval("return box.space.gh4513:get{1}"))
+
+-- connect and self should convert tuples to tables
+test:is(t1, "table", "connect converts to table")
+test:is(t2, "table", "self convert to table")
+
+t1 = type(net_box.connect(dst):eval("return box.error.new(1, 'test error')"))
+t2 = type(net_box.self:eval("return box.error.new(1, 'test error')"))
+
+-- check for converting strings
+test:is(t1, "string", "connect converts to strings")
+test:is(t2, "string", "self converts to strings")
+
+t1 = type(net_box.connect(dst):eval("return box.NULL"))
+t2 = type(net_box.self:eval("return box.NULL"))
+
+-- check for converting cdata
+test:is(t1, "cdata", "connect converts to cdata")
+test:is(t2, "cdata", "self converts to cdata")
+
 -- cleanup
 conn:close()
 inspector:cmd('stop server second with cleanup=1')
-test:check()
-os.exit(0)
+os.exit(test:check() and 0 or 1)
-- 
2.24.0

^ permalink raw reply	[flat|nested] 10+ messages in thread
* [Tarantool-patches] [PATCH] box: netbox.self and connect should work interchangeably
@ 2019-12-12 21:28 Maria
  2019-12-13 13:53 ` Oleg Babin
  0 siblings, 1 reply; 10+ messages in thread
From: Maria @ 2019-12-12 21:28 UTC (permalink / raw)
  To: tarantool-patches, georgy, alexander.turenko, v.shpilevoy

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)

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2020-03-08 22:57 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-01-17 17:08 [Tarantool-patches] [PATCH] box: netbox.self and connect should work interchangeably 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
  -- strict thread matches above, loose matches on Subject: below --
2019-12-12 21:28 Maria
2019-12-13 13:53 ` Oleg Babin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox