[tarantool-patches] [PATCH] socket: better args handling for connect()/bind()

[tarantool-patches] [PATCH] socket: better args handling for connect()/bind()

[Roman Khabibov]

Check args types in socket.connect() and socket.bind() functions.
'host' should be a string and 'port' should be a string or a
number.

Part of #4138
---
Branch: https://github.com/tarantool/tarantool/tree/romanhabibov/gh-4138-getaddrinfo
Issue: https://github.com/tarantool/tarantool/issues/4138

 src/lua/socket.lua       |  6 ++++--
 test/app/socket.result   | 25 +++++++++++++++++++++++++
 test/app/socket.test.lua |  8 ++++++++
 3 files changed, 37 insertions(+), 2 deletions(-)

diff --git a/src/lua/socket.lua b/src/lua/socket.lua
index e4815adbb..c4a61074e 100644
--- a/src/lua/socket.lua
+++ b/src/lua/socket.lua
@@ -1554,7 +1554,8 @@ local function lsocket_tcp()
 end
 
 local function lsocket_connect(host, port)
-    if host == nil or port == nil then
+    if type(host) ~= 'string' or (type(port) ~= 'string' and
+       type(port) ~= 'number') then
         error("Usage: luasocket.connect(host, port)")
     end
     local s, err = tcp_connect(host, port)
@@ -1569,7 +1570,8 @@ local function lsocket_connect(host, port)
 end
 
 local function lsocket_bind(host, port, backlog)
-    if host == nil or port == nil then
+    if type(host) ~= 'string' or (type(port) ~= 'string' and
+        type(port) ~= 'number') then
         error("Usage: luasocket.bind(host, port [, backlog])")
     end
     local function prepare(s) return backlog end
diff --git a/test/app/socket.result b/test/app/socket.result
index 87b1d0387..20507bafb 100644
--- a/test/app/socket.result
+++ b/test/app/socket.result
@@ -2857,6 +2857,31 @@ check_err(err)
 ---
 - true
 ...
+-- gh-4138 Check the usage error in case of wrong args types.
+socket.connect('127.0.0.1', {3301})
+---
+- error: 'builtin/socket.lua: Usage: luasocket.connect(host, port)'
+...
+socket.bind('127.0.0.1', {3301})
+---
+- error: 'builtin/socket.lua: Usage: luasocket.bind(host, port [, backlog])'
+...
+socket.connect(127.0.0.1, 3301)
+---
+- error: '[string "socket.connect(127.0.0.1, 3301) "]:1: malformed number near ''127.0.0.1'''
+...
+socket.bind(127.0.0.1, 3301)
+---
+- error: '[string "socket.bind(127.0.0.1, 3301) "]:1: malformed number near ''127.0.0.1'''
+...
+socket.connect({'127.0.0.1'}, 3301)
+---
+- error: 'builtin/socket.lua: Usage: luasocket.connect(host, port)'
+...
+socket.bind({'127.0.0.1'}, 3301)
+---
+- error: 'builtin/socket.lua: Usage: luasocket.bind(host, port [, backlog])'
+...
 test_run:cmd("clear filter")
 ---
 - true
diff --git a/test/app/socket.test.lua b/test/app/socket.test.lua
index 08cb7fc9e..1974e8cea 100644
--- a/test/app/socket.test.lua
+++ b/test/app/socket.test.lua
@@ -985,6 +985,14 @@ check_err(err)
 s, err = socket.connect('non_exists_hostname', 3301)
 check_err(err)
 
+-- gh-4138 Check the usage error in case of wrong args types.
+socket.connect('127.0.0.1', {3301})
+socket.bind('127.0.0.1', {3301})
+socket.connect(127.0.0.1, 3301)
+socket.bind(127.0.0.1, 3301)
+socket.connect({'127.0.0.1'}, 3301)
+socket.bind({'127.0.0.1'}, 3301)
+
 test_run:cmd("clear filter")
 
 -- case: sicket receive inconsistent behavior
-- 
2.20.1 (Apple Git-117)

Re: [Tarantool-patches] [PATCH] socket: better args handling for connect()/bind()

[Alexander Turenko]

Please, rebase your commits on recent master and apply comments below.

> socket: better args handling for connect()/bind()

I would say about validation rather then handling.

>
> Check args types in socket.connect() and socket.bind() functions.
> 'host' should be a string and 'port' should be a string or a
> number.
> 
> Part of #4138

It is better to place this commit before one that closes the issue.

> Branch: https://github.com/tarantool/tarantool/tree/romanhabibov/gh-4138-getaddrinfo
> Issue: https://github.com/tarantool/tarantool/issues/4138

> @@ -1569,7 +1570,8 @@ local function lsocket_connect(host, port)
>  end
>  
>  local function lsocket_bind(host, port, backlog)
> -    if host == nil or port == nil then
> +    if type(host) ~= 'string' or (type(port) ~= 'string' and
> +        type(port) ~= 'number') then

I would check backport against 'number' too (if it present).

> +socket.connect(127.0.0.1, 3301)
> +---
> +- error: '[string "socket.connect(127.0.0.1, 3301) "]:1: malformed number near ''127.0.0.1'''

Did you read this error? It is the parsing error from Lua interpreter.

Re: [Tarantool-patches] [PATCH] socket: better args handling for connect()/bind()

[Roman Khabibov]

> On Nov 1, 2019, at 15:32, Alexander Turenko <alexander.turenko@tarantool.org> wrote:
> 
> Please, rebase your commits on recent master and apply comments below.
> 
>> socket: better args handling for connect()/bind()
> 
> I would say about validation rather then handling.
    socket: better args validation for connect()/bind()

>> 
>> Check args types in socket.connect() and socket.bind() functions.
>> 'host' should be a string and 'port' should be a string or a
>> number.
>> 
>> Part of #4138
> 
> It is better to place this commit before one that closes the issue.
> 
>> Branch: https://github.com/tarantool/tarantool/tree/romanhabibov/gh-4138-getaddrinfo
>> Issue: https://github.com/tarantool/tarantool/issues/4138
> 
>> @@ -1569,7 +1570,8 @@ local function lsocket_connect(host, port)
>> end
>> 
>> local function lsocket_bind(host, port, backlog)
>> -    if host == nil or port == nil then
>> +    if type(host) ~= 'string' or (type(port) ~= 'string' and
>> +        type(port) ~= 'number') then
> 
> I would check backport against 'number' too (if it present).
Backlog?

@@ -1544,7 +1544,8 @@ local function lsocket_tcp()
 end
 
 local function lsocket_connect(host, port)
-    if host == nil or port == nil then
+    if type(host) ~= 'string' or (type(port) ~= 'string' and
+       type(port) ~= 'number') then
         error("Usage: luasocket.connect(host, port)")
     end
     local s = tcp_connect(host, port)
@@ -1556,7 +1557,8 @@ local function lsocket_connect(host, port)
 end
 
 local function lsocket_bind(host, port, backlog)
-    if host == nil or port == nil then
+    if type(host) ~= 'string' or (type(port) ~= 'string' and type(port) ~=
+       'number') or (backlog ~= nil and type(backlog) ~= 'number') then
         error("Usage: luasocket.bind(host, port [, backlog])")
     end
     local function prepare(s) return backlog end

>> +socket.connect(127.0.0.1, 3301)
>> +---
>> +- error: '[string "socket.connect(127.0.0.1, 3301) "]:1: malformed number near ''127.0.0.1'''
> 
> Did you read this error? It is the parsing error from Lua interpreter.
diff --git a/test/app/socket.result b/test/app/socket.result
index fd299424c..6a72fb3cf 100644
--- a/test/app/socket.result
+++ b/test/app/socket.result
@@ -2812,6 +2812,31 @@ server:close()
 ---
 - true
 ...
+-- gh-4138 Check the usage error in case of wrong args types.
+socket.connect('127.0.0.1', {3301})
+---
+- error: 'builtin/socket.lua: Usage: luasocket.connect(host, port)'
+...
+socket.bind('127.0.0.1', {3301})
+---
+- error: 'builtin/socket.lua: Usage: luasocket.bind(host, port [, backlog])'
+...
+socket.connect({'127.0.0.1'}, 3301)
+---
+- error: 'builtin/socket.lua: Usage: luasocket.connect(host, port)'
+...
+socket.bind({'127.0.0.1'}, 3301)
+---
+- error: 'builtin/socket.lua: Usage: luasocket.bind(host, port [, backlog])'
+...
+socket.bind('127.0.0.1', 3301, {1})
+---
+- error: 'builtin/socket.lua: Usage: luasocket.bind(host, port [, backlog])'
+...
+socket.bind('127.0.0.1', 3301, '1')
+---
+- error: 'builtin/socket.lua: Usage: luasocket.bind(host, port [, backlog])'
+...
 test_run:cmd("clear filter")
 ---
 - true

commit 5a0c462bb03a860034a0c23ef6d720b4aa2858dd
Author: Roman Khabibov <roman.habibov@tarantool.org>
Date:   Mon Sep 9 10:34:03 2019 +0300

    socket: better args validation for connect()/bind()
    
    Check args types in socket.connect() and socket.bind() functions.
    'host' should be a string and 'port' should be a string or a
    number.
    
    Part of #4138

diff --git a/src/lua/socket.lua b/src/lua/socket.lua
index a334ad45b..2a4c69f45 100644
--- a/src/lua/socket.lua
+++ b/src/lua/socket.lua
@@ -1544,7 +1544,8 @@ local function lsocket_tcp()
 end
 
 local function lsocket_connect(host, port)
-    if host == nil or port == nil then
+    if type(host) ~= 'string' or (type(port) ~= 'string' and
+       type(port) ~= 'number') then
         error("Usage: luasocket.connect(host, port)")
     end
     local s = tcp_connect(host, port)
@@ -1556,7 +1557,8 @@ local function lsocket_connect(host, port)
 end
 
 local function lsocket_bind(host, port, backlog)
-    if host == nil or port == nil then
+    if type(host) ~= 'string' or (type(port) ~= 'string' and type(port) ~=
+       'number') or (backlog ~= nil and type(backlog) ~= 'number') then
         error("Usage: luasocket.bind(host, port [, backlog])")
     end
     local function prepare(s) return backlog end
diff --git a/test/app/socket.result b/test/app/socket.result
index fd299424c..6a72fb3cf 100644
--- a/test/app/socket.result
+++ b/test/app/socket.result
@@ -2812,6 +2812,31 @@ server:close()
 ---
 - true
 ...
+-- gh-4138 Check the usage error in case of wrong args types.
+socket.connect('127.0.0.1', {3301})
+---
+- error: 'builtin/socket.lua: Usage: luasocket.connect(host, port)'
+...
+socket.bind('127.0.0.1', {3301})
+---
+- error: 'builtin/socket.lua: Usage: luasocket.bind(host, port [, backlog])'
+...
+socket.connect({'127.0.0.1'}, 3301)
+---
+- error: 'builtin/socket.lua: Usage: luasocket.connect(host, port)'
+...
+socket.bind({'127.0.0.1'}, 3301)
+---
+- error: 'builtin/socket.lua: Usage: luasocket.bind(host, port [, backlog])'
+...
+socket.bind('127.0.0.1', 3301, {1})
+---
+- error: 'builtin/socket.lua: Usage: luasocket.bind(host, port [, backlog])'
+...
+socket.bind('127.0.0.1', 3301, '1')
+---
+- error: 'builtin/socket.lua: Usage: luasocket.bind(host, port [, backlog])'
+...
 test_run:cmd("clear filter")
 ---
 - true
diff --git a/test/app/socket.test.lua b/test/app/socket.test.lua
index c72d41763..022cd4f40 100644
--- a/test/app/socket.test.lua
+++ b/test/app/socket.test.lua
@@ -958,6 +958,14 @@ fiber.cancel(echo_fiber)
 client:read(1, 5) == ''
 server:close()
 
+-- gh-4138 Check the usage error in case of wrong args types.
+socket.connect('127.0.0.1', {3301})
+socket.bind('127.0.0.1', {3301})
+socket.connect({'127.0.0.1'}, 3301)
+socket.bind({'127.0.0.1'}, 3301)
+socket.bind('127.0.0.1', 3301, {1})
+socket.bind('127.0.0.1', 3301, '1')
+
 test_run:cmd("clear filter")
 
 -- case: sicket receive inconsistent behavior