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

[Roman Khabibov <roman.habibov@tarantool.org>]

> 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