From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 3A63D4696C3 for ; Thu, 12 Mar 2020 13:24:39 +0300 (MSK) From: Roman Khabibov Date: Thu, 12 Mar 2020 13:24:34 +0300 Message-Id: <20200312102434.97300-3-roman.habibov@tarantool.org> In-Reply-To: <20200312102434.97300-1-roman.habibov@tarantool.org> References: <20200312102434.97300-1-roman.habibov@tarantool.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH v2 2/2] lua: return getaddrinfo() errors List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: tarantool-patches@dev.tarantool.org Add getaddrinfo() errors into the several fuctions of socket. Now getaddrinfo() can return a pair of values (nil and error message) in case of error. Closes #4138 @TarantoolBot document Title: socket API changes * socket.getaddrinfo() Can return error message as second return value in case of internal getaddrinfo() error. Example: tarantool> socket.getaddrinfo('non_exists_hostname', 3301) --- - null - 'getaddrinfo: nodename nor servname provided, or not known' ... * socket.tcp_connect() Can return socket.getaddrinfo() error message as second return value. Example: tarantool> socket.tcp_connect('non_exists_hostname', 3301) --- - null - 'getaddrinfo: nodename nor servname provided, or not known' ... --- src/box/lua/net_box.lua | 7 +++-- src/lua/socket.c | 16 +++++++++- src/lua/socket.lua | 22 ++++++++++---- test/app/socket.result | 63 ++++++++++++++++++++++++++++++++++++--- test/app/socket.test.lua | 35 ++++++++++++++++++++-- test/box/net.box.result | 31 +++++++++++++++++++ test/box/net.box.test.lua | 20 +++++++++++++ 7 files changed, 179 insertions(+), 15 deletions(-) diff --git a/src/box/lua/net_box.lua b/src/box/lua/net_box.lua index 3f611c027..8068a8637 100644 --- a/src/box/lua/net_box.lua +++ b/src/box/lua/net_box.lua @@ -154,9 +154,12 @@ local function next_id(id) return band(id + 1, 0x7FFFFFFF) end local function establish_connection(host, port, timeout) local timeout = timeout or DEFAULT_CONNECT_TIMEOUT local begin = fiber.clock() - local s = socket.tcp_connect(host, port, timeout) + local s, err = socket.tcp_connect(host, port, timeout) if not s then - return nil, errno.strerror(errno()) + if not err then + return nil, errno.strerror(errno()) + end + return nil, err end local msg = s:read({chunk = IPROTO_GREETING_SIZE}, timeout - (fiber.clock() - begin)) diff --git a/src/lua/socket.c b/src/lua/socket.c index e75a8802e..ba683ad3a 100644 --- a/src/lua/socket.c +++ b/src/lua/socket.c @@ -775,6 +775,18 @@ lbox_getaddrinfo_result_wrapper(struct lua_State *L) return 1; } +/** + * Wrap coio_getaddrinfo() and call it. Push returned values onto + * @a L Lua stack. + * + * @param L Lua stack. + * + * @retval 1 Number of returned values by Lua function if + * coio_getaddrinfo() success. + * @retval 2 Number of returned values by Lua function if + * coio_getaddrinfo() is failed (first is nil, second is error + * message). + */ static int lbox_socket_getaddrinfo(struct lua_State *L) { @@ -817,7 +829,9 @@ lbox_socket_getaddrinfo(struct lua_State *L) if (dns_res != 0) { lua_pushnil(L); - return 1; + struct error *err = diag_get()->last; + lua_pushstring(L, err->errmsg); + return 2; } /* no results */ diff --git a/src/lua/socket.lua b/src/lua/socket.lua index a334ad45b..ce6dab301 100644 --- a/src/lua/socket.lua +++ b/src/lua/socket.lua @@ -1041,9 +1041,12 @@ local function tcp_connect(host, port, timeout) end local timeout = timeout or TIMEOUT_INFINITY local stop = fiber.clock() + timeout - local dns = getaddrinfo(host, port, timeout, { type = 'SOCK_STREAM', + local dns, err = getaddrinfo(host, port, timeout, { type = 'SOCK_STREAM', protocol = 'tcp' }) - if dns == nil or #dns == 0 then + if dns == nil then + return nil, err + end + if #dns == 0 then boxerrno(boxerrno.EINVAL) return nil end @@ -1360,8 +1363,12 @@ local function lsocket_tcp_connect(self, host, port) -- This function is broken by design local ga_opts = { family = 'AF_INET', type = 'SOCK_STREAM' } local timeout = deadline - fiber.clock() - local dns = getaddrinfo(host, port, timeout, ga_opts) - if dns == nil or #dns == 0 then + local dns, err = getaddrinfo(host, port, timeout, ga_opts) + if dns == nil then + self._errno = boxerrno.EINVAL + return nil, err + end + if #dns == 0 then self._errno = boxerrno.EINVAL return nil, socket_error(self) end @@ -1547,9 +1554,12 @@ local function lsocket_connect(host, port) if host == nil or port == nil then error("Usage: luasocket.connect(host, port)") end - local s = tcp_connect(host, port) + local s, err = tcp_connect(host, port) if not s then - return nil, boxerrno.strerror() + if not err then + return nil, boxerrno.strerror() + end + return nil, err end setmetatable(s, lsocket_tcp_client_mt) return s diff --git a/test/app/socket.result b/test/app/socket.result index 9f0ea0a5d..3f26bf2b0 100644 --- a/test/app/socket.result +++ b/test/app/socket.result @@ -941,10 +941,20 @@ sc:close() - true ... -- tcp_connect --- test timeout -socket.tcp_connect('127.0.0.1', 80, 0.00000000001) +-- Test timeout. In this test, tcp_connect can return the second +-- output value from internal.getaddrinfo (usually on Mac OS, but +-- theoretically it can happen on Linux too). Sometimes +-- getaddrinfo() is timed out, sometimes connect. On Linux however +-- getaddrinfo is fast enough to never give timeout error in +-- the case. So, there are two sources of timeout errors that are +-- reported differently. This difference has appeared after +-- gh-4138 patch. +s, err = socket.tcp_connect('127.0.0.1', 80, 0.00000000001) --- -- null +... +s == nil +--- +- true ... -- AF_INET s = socket('AF_INET', 'SOCK_STREAM', 'tcp') @@ -1595,7 +1605,7 @@ fio.stat(path) == nil { socket.tcp_connect('abrakadabra#123') == nil, errno.strerror() } --- - - true - - Invalid argument + - Input/output error ... -- wrong options for getaddrinfo socket.getaddrinfo('host', 'port', { type = 'WRONG' }) == nil and errno() == errno.EINVAL @@ -2914,3 +2924,48 @@ srv:close() --- - true ... +-- gh-4138 Check getaddrinfo() error from socket:connect() only. +-- Error code and error message returned by getaddrinfo() depends +-- on system's gai_strerror(). +test_run:cmd("setopt delimiter ';'") +--- +- true +... +function check_err(err) + if err == 'getaddrinfo: nodename nor servname provided, or not known' or + err == 'getaddrinfo: Servname not supported for ai_socktype' or + err == 'getaddrinfo: Name or service not known' or + err == 'getaddrinfo: hostname nor servname provided, or not known' or + err == 'getaddrinfo: Temporary failure in name resolution' or + err == 'getaddrinfo: Name could not be resolved at this time' then + return true + end + return false +end; +--- +... +test_run:cmd("setopt delimiter ''"); +--- +- true +... +s, err = socket.getaddrinfo('non_exists_hostname', 3301) +--- +... +check_err(err) +--- +- true +... +s, err = socket.connect('non_exists_hostname', 3301) +--- +... +check_err(err) +--- +- true +... +s, err = socket.tcp_connect('non_exists_hostname', 3301) +--- +... +check_err(err) +--- +- true +... diff --git a/test/app/socket.test.lua b/test/app/socket.test.lua index d1fe7ada6..c455d3ddb 100644 --- a/test/app/socket.test.lua +++ b/test/app/socket.test.lua @@ -300,8 +300,16 @@ sc:close() -- tcp_connect --- test timeout -socket.tcp_connect('127.0.0.1', 80, 0.00000000001) +-- Test timeout. In this test, tcp_connect can return the second +-- output value from internal.getaddrinfo (usually on Mac OS, but +-- theoretically it can happen on Linux too). Sometimes +-- getaddrinfo() is timed out, sometimes connect. On Linux however +-- getaddrinfo is fast enough to never give timeout error in +-- the case. So, there are two sources of timeout errors that are +-- reported differently. This difference has appeared after +-- gh-4138 patch. +s, err = socket.tcp_connect('127.0.0.1', 80, 0.00000000001) +s == nil -- AF_INET s = socket('AF_INET', 'SOCK_STREAM', 'tcp') @@ -988,3 +996,26 @@ s:receive('*a') s:close() srv:close() +-- gh-4138 Check getaddrinfo() error from socket:connect() only. +-- Error code and error message returned by getaddrinfo() depends +-- on system's gai_strerror(). +test_run:cmd("setopt delimiter ';'") +function check_err(err) + if err == 'getaddrinfo: nodename nor servname provided, or not known' or + err == 'getaddrinfo: Servname not supported for ai_socktype' or + err == 'getaddrinfo: Name or service not known' or + err == 'getaddrinfo: hostname nor servname provided, or not known' or + err == 'getaddrinfo: Temporary failure in name resolution' or + err == 'getaddrinfo: Name could not be resolved at this time' then + return true + end + return false +end; +test_run:cmd("setopt delimiter ''"); + +s, err = socket.getaddrinfo('non_exists_hostname', 3301) +check_err(err) +s, err = socket.connect('non_exists_hostname', 3301) +check_err(err) +s, err = socket.tcp_connect('non_exists_hostname', 3301) +check_err(err) diff --git a/test/box/net.box.result b/test/box/net.box.result index e3dabf7d9..3e1ce6b11 100644 --- a/test/box/net.box.result +++ b/test/box/net.box.result @@ -3927,6 +3927,37 @@ test_run:grep_log('default', '00000040:.*') --- - null ... +-- gh-4138 Check getaddrinfo() error from connect() only. Error +-- code and error message returned by getaddrinfo() depends on +-- system's gai_strerror(). +test_run:cmd("setopt delimiter ';'") +--- +- true +... +function check_err(err) + if err == 'getaddrinfo: nodename nor servname provided, or not known' or + err == 'getaddrinfo: Servname not supported for ai_socktype' or + err == 'getaddrinfo: Name or service not known' or + err == 'getaddrinfo: hostname nor servname provided, or not known' or + err == 'getaddrinfo: Temporary failure in name resolution' or + err == 'getaddrinfo: Name could not be resolved at this time' then + return true + end + return false +end; +--- +... +test_run:cmd("setopt delimiter ''"); +--- +- true +... +s = remote.connect('non_exists_hostname:3301') +--- +... +check_err(s['error']) +--- +- true +... box.cfg{log_level=log_level} --- ... diff --git a/test/box/net.box.test.lua b/test/box/net.box.test.lua index 8e65ff470..cad8754c3 100644 --- a/test/box/net.box.test.lua +++ b/test/box/net.box.test.lua @@ -1582,4 +1582,24 @@ test_run:wait_log('default', '00000030:.*', nil, 10) -- we expect nothing below, so don't wait test_run:grep_log('default', '00000040:.*') +-- gh-4138 Check getaddrinfo() error from connect() only. Error +-- code and error message returned by getaddrinfo() depends on +-- system's gai_strerror(). +test_run:cmd("setopt delimiter ';'") +function check_err(err) + if err == 'getaddrinfo: nodename nor servname provided, or not known' or + err == 'getaddrinfo: Servname not supported for ai_socktype' or + err == 'getaddrinfo: Name or service not known' or + err == 'getaddrinfo: hostname nor servname provided, or not known' or + err == 'getaddrinfo: Temporary failure in name resolution' or + err == 'getaddrinfo: Name could not be resolved at this time' then + return true + end + return false +end; +test_run:cmd("setopt delimiter ''"); + +s = remote.connect('non_exists_hostname:3301') +check_err(s['error']) + box.cfg{log_level=log_level} -- 2.21.0 (Apple Git-122)