[Tarantool-patches] [PATCH 0/2] box/console: Fix hang on remote console enter

[Tarantool-patches] [PATCH 0/2] box/console: Fix hang on remote console enter

[Cyrill Gorcunov]

Alexander pointed that we might require more precise command handling
when setting up eos in remote unix connection. Plain number of
arguments in command strem is not enough. Otherwise one could
call '\set language lua' and hang a connection.

Sasha could you please test if it fixes the problem for you?
Actually I managed to repeat it without tarantoolctl but still.

p.s.: Sorry for resending the series seems i've had a wrong relay
settings in my mail sender.

---
The following changes since commit de9a7b1a37ec34054b062c513fa1bf9357699a8d:

  sql: use name instead of function pointer for UDF (2019-10-16 21:06:48 +0300)

are available in the Git repository at:

  https://github.com/tarantool/tarantool console-enter

for you to fetch changes up to 972d23c74fb4bff72889c7b6bd0455d39a4fa688:

  box/console: Test for exact command before setting eos (2019-10-16 23:45:51 +0300)

----------------------------------------------------------------
Cyrill Gorcunov (2):
      box/console: Move param handlers into file scope
      box/console: Test for exact command before setting eos

 src/box/lua/console.lua | 36 +++++++++++++++++++++---------------
 1 file changed, 21 insertions(+), 15 deletions(-)

[Tarantool-patches] [PATCH 1/2] box/console: Move param handlers into file scope

[Cyrill Gorcunov]

This will allow us to reuse this map in other routines
and same time optimize memory allocation a bit.

Part-of #4568

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 src/box/lua/console.lua | 25 +++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/src/box/lua/console.lua b/src/box/lua/console.lua
index 6a0133648..e1941f85d 100644
--- a/src/box/lua/console.lua
+++ b/src/box/lua/console.lua
@@ -256,24 +256,25 @@ local function set_output(storage, value)
     return true
 end
 
+local param_handlers = {
+    language = set_language,
+    lang = set_language,
+    l = set_language,
+    output = set_output,
+    o = set_output,
+    delimiter = set_delimiter,
+    delim = set_delimiter,
+    d = set_delimiter
+}
+
 local function set_param(storage, func, param, value)
-    local params = {
-        language = set_language,
-        lang = set_language,
-        l = set_language,
-        output = set_output,
-        o = set_output,
-        delimiter = set_delimiter,
-        delim = set_delimiter,
-        d = set_delimiter
-    }
     if param == nil then
         return format(false, 'Invalid set syntax, type \\help for help')
     end
-    if params[param] == nil then
+    if param_handlers[param] == nil then
         return format(false, 'Unknown parameter: ' .. tostring(param))
     end
-    return format(pcall(params[param], storage, value))
+    return format(pcall(param_handlers[param], storage, value))
 end
 
 local function help_wrapper(storage)
-- 
2.20.1

[Tarantool-patches] [PATCH 2/2] box/console: Test for exact command before setting eos

[Cyrill Gorcunov]

When we handle unix console connection we should setup
eos if only we meet "\set output" command in a stream.
For example there could be "\set language" which should
not affect eos settings.

Closes #4568

Signed-off-by: Cyrill Gorcunov <gorcunov@gmail.com>
---
 src/box/lua/console.lua | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/src/box/lua/console.lua b/src/box/lua/console.lua
index e1941f85d..f70ed830a 100644
--- a/src/box/lua/console.lua
+++ b/src/box/lua/console.lua
@@ -416,9 +416,14 @@ local text_connection_mt = {
             end
             local nr_items, items = parse_operators(command)
             if nr_items == 3 then
-                local err, fmt, opts = parse_output(items[3])
-                if not err then
-                    self.eos = output_eos[fmt]
+                --
+                -- Make sure it is exactly "\set output" command.
+                if operators[items[1]] == set_param and
+                    param_handlers[items[2]] == set_output then
+                    local err, fmt, opts = parse_output(items[3])
+                    if not err then
+                        self.eos = output_eos[fmt]
+                    end
                 end
             end
         end,
-- 
2.20.1

Re: [Tarantool-patches] [PATCH 0/2] box/console: Fix hang on remote console enter

[Cyrill Gorcunov]

[-- Attachment #1: Type: text/plain, Size: 290 bytes --]

On Thu, Oct 17, 2019 at 3:55 PM Alexander Turenko <
alexander.turenko@tarantool.org> wrote:

>
> BTW, I suggest to use `luacheck file.lua` to protect youself from some
> kind of mistakes. I use the following .luacheckrc file to reduce amount
> of false-positives:
>

Will do, thanks a lot!

[-- Attachment #2: Type: text/html, Size: 594 bytes --]

Re: [Tarantool-patches] [PATCH 0/2] box/console: Fix hang on remote console enter

[Alexander Turenko]

On Wed, Oct 16, 2019 at 11:51:55PM +0300, Cyrill Gorcunov wrote:
> Alexander pointed that we might require more precise command handling
> when setting up eos in remote unix connection. Plain number of
> arguments in command strem is not enough. Otherwise one could
> call '\set language lua' and hang a connection.
> 
> Sasha could you please test if it fixes the problem for you?
> Actually I managed to repeat it without tarantoolctl but still.

It fixes tarantoolctl enter and the patchset looks good for me.

Pushed to master and 2.2.

> 
> ---
> The following changes since commit de9a7b1a37ec34054b062c513fa1bf9357699a8d:
> 
>   sql: use name instead of function pointer for UDF (2019-10-16 21:06:48 +0300)
> 
> are available in the Git repository at:
> 
>   https://github.com/tarantool/tarantool console-enter
> 
> for you to fetch changes up to 972d23c74fb4bff72889c7b6bd0455d39a4fa688:
> 
>   box/console: Test for exact command before setting eos (2019-10-16 23:45:51 +0300)
> 
> ----------------------------------------------------------------
> Cyrill Gorcunov (2):
>       box/console: Move param handlers into file scope
>       box/console: Test for exact command before setting eos

We usually start a commit header from a small letter when it is next to
a prefix; I mean: "box/console: move param handlers into file scope".

Changed it before push.

> 
>  src/box/lua/console.lua | 36 +++++++++++++++++++++---------------
>  1 file changed, 21 insertions(+), 15 deletions(-)

BTW, I suggest to use `luacheck file.lua` to protect youself from some
kind of mistakes. I use the following .luacheckrc file to reduce amount
of false-positives:

 | std = {
 |     read_globals = {'box', 'require', 'debug', 'pcall', 'xpcall', 'tostring',
 |         'tonumber', 'type', 'assert', 'ipairs', 'math', 'error', 'string',
 |         'table', 'pairs', 'os', 'select', 'unpack', 'dofile', 'next',
 |         'getmetatable', 'setmetatable', 'rawget', 'print', 'shard_status',
 |         'loadstring', 'arg',
 |     },
 |     globals = {'package', '_G'}
 | }
 | redefined = False

It is not ideal however, just eliminates some false-positive warns I met
before.

WBR, Alexander Turenko.