[tarantool-patches] Re: [PATCH v1 1/1] box: fix custom delimiter for telnet connection

[Alexander Turenko]

We discussed it a bit more voicely and Kirill fixed some comments. I'll
put them below for the record.

LGTM then.

Vlad, can you, please, look at the patch?

On Tue, Mar 05, 2019 at 05:41:10PM +0300, Kirill Shcherbatov wrote:
> On 05.03.2019 16:43, Alexander Turenko wrote:
> > This patch looks good for me, except several minor things.
> Hi! Thank you for review. Fixed everything.
> 
> ============================================
> 
> In order to give a user ability to use a delimiter symbol within a
> code the real delimiter is user-provided 'delim' plus "\n".
> Since telnet sends "\r\n" on line break, the updated expression
> delim + "\n" could not be found in a sequence data+delim+"\r\n",
> so delimiter feature did not work at all.
> Added delim + "\r" check along with delim + "\n", that solves the
> described problem and does not violate backward compatibility.
> 
> Closes #2027
> ---
>  src/box/lua/console.lua       | 12 +++++++++---
>  test/app-tap/console.test.lua | 14 +++++++++++++-
>  2 files changed, 22 insertions(+), 4 deletions(-)
> 
> diff --git a/src/box/lua/console.lua b/src/box/lua/console.lua
> index 028001127..74e9addad 100644
> --- a/src/box/lua/console.lua
> +++ b/src/box/lua/console.lua
> @@ -344,8 +344,14 @@ end
>  -- Read command from connected client console.listen()
>  --
>  local function client_read(self)
> -    local delim = self.delimiter .. "\n"
> -    local buf = self.client:read(delim)
> +    --
> +    -- Byte sequences that come over the network and come from
> +    -- the local client console may have a different terminal
> +    -- character. We support bouth possible options: CR and LF.
> +    --

Typo: bouth -> both.

CR and LF -> LF and CRLF.

> +    local delim_cr = self.delimiter .. "\r"
> +    local delim_lf = self.delimiter .. "\n"
> +    local buf = self.client:read({delimiter = {delim_lf, delim_cr}})
>      if buf == nil then
>          return nil
>      elseif buf == "" then
> @@ -355,7 +361,7 @@ local function client_read(self)
>          return nil
>      end
>      -- remove trailing delimiter
> -    return buf:sub(1, -#delim-1)
> +    return buf:sub(1, -#self.delimiter-2)
>  end
>  
>  --
> diff --git a/test/app-tap/console.test.lua b/test/app-tap/console.test.lua
> index 4f915afd7..ccd33a2d0 100755
> --- a/test/app-tap/console.test.lua
> +++ b/test/app-tap/console.test.lua
> @@ -21,7 +21,7 @@ local EOL = "\n...\n"
>  
>  test = tap.test("console")
>  
> -test:plan(59)
> +test:plan(61)
>  
>  -- Start console and connect to it
>  local server = console.listen(CONSOLE_SOCKET)
> @@ -255,6 +255,18 @@ box.session.on_connect(nil, console_on_connect)
>  box.session.on_disconnect(nil, console_on_disconnect)
>  box.session.on_auth(nil, console_on_auth)
>  
> +
> +--
> +-- gh-2027: Fix custom delimiter for console connection.

console -> telnet

> +--
> +client = socket.tcp_connect("unix/", CONSOLE_SOCKET)
> +_ = client:read(128)
> +client:write("console = require('console'); console.delimiter('#');\n")
> +test:is(yaml.decode(client:read(EOL))[1], nil, "session type")
> +client:write("box.NULL#\r\n")
> +test:is(yaml.decode(client:read(EOL))[1], box.NULL, "test new delimiter")
> +client:close()
> +
>  --
>  -- gh-2642 "box.session.type()"
>  --
> -- 
> 2.21.0