From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: From: Alexander Turenko Subject: [PATCH 2/3] socket: don't truncate a datagram in recv/recvfrom Date: Fri, 24 Aug 2018 05:47:38 +0300 Message-Id: In-Reply-To: References: In-Reply-To: References: To: Vladimir Davydov Cc: Alexander Turenko , tarantool-patches@freelists.org, Yaroslav Dynnikov List-ID: When the datagram length is larger then the buffer size we discard the datagram, return nil as the message, set errno to EMSGSIZE, but still return `from` table (in case of recvfrom). The commit changes behaviour of socket:recv and socket:recvfrom methods in case of a UDP socket. They still truncate the input in case of a TCP socket (because of stream nature of TCP). On Linux we set MSG_TRUNC for recv / recvfrom calls to receive the real length of the datagram. On Mac OS we call getsockopt(fd, SOL_SOCKET, SO_NREAD, &val, &len) before the recv / recvfrom call. These extra steps are avoided for TCP sockets, because MSG_TRUNC leads to discarding the input (Linux) and to avoid extra getsockopt syscall (Mac OS). Fixes #3619. --- src/lua/socket.c | 17 +++++ src/lua/socket.lua | 55 +++++++++++----- test/app/socket.result | 139 ++++++++++++++++++++++++++++++++++++++- test/app/socket.test.lua | 44 +++++++++++++ 4 files changed, 238 insertions(+), 17 deletions(-) diff --git a/src/lua/socket.c b/src/lua/socket.c index bf23eae1f..c716979f4 100644 --- a/src/lua/socket.c +++ b/src/lua/socket.c @@ -953,6 +953,13 @@ lbox_socket_recvfrom(struct lua_State *L) int fh = lua_tointeger(L, 1); int size = lua_tointeger(L, 2); int flags = lua_tointeger(L, 3); + int is_udp = lua_toboolean(L, 4); +#ifdef TARGET_OS_DARWIN + int dgram_length = lua_tointeger(L, 5); +#else + if (is_udp) + flags |= MSG_TRUNC; +#endif struct sockaddr_storage fa; socklen_t len = sizeof(fa); @@ -971,6 +978,16 @@ lbox_socket_recvfrom(struct lua_State *L) free(buf); lua_pushnil(L); return 1; +#ifdef TARGET_OS_DARWIN + } else if ((is_udp && dgram_length > size) || (!is_udp && res > size)) { +#else + } else if (res > size) { +#endif + free(buf); + lua_pushnil(L); + lbox_socket_push_addr(L, (struct sockaddr *)&fa, len); + errno = EMSGSIZE; + return 2; } lua_pushcfunction(L, lbox_socket_recvfrom_wrapper); diff --git a/src/lua/socket.lua b/src/lua/socket.lua index 946e11e9e..eed764f96 100644 --- a/src/lua/socket.lua +++ b/src/lua/socket.lua @@ -805,6 +805,26 @@ local function socket_dgram_length(self) return len end +-- @retval size (size or 512) for tcp, (size or dgram_length) for udp +-- @retval dgram_length or nil +local function socket_recv_buffer_size_internal(self, size, is_udp) + if not is_udp then + return size or 512 + end + + -- we obligated to ask for dgram_length on OS X, because we cannot use + -- MSG_TRUNC with the recv / recvfrom later + local dgram_length + if size == nil or jit.os == 'OSX' then + dgram_length = self:dgram_length() + if dgram_length == nil then + return nil + end + end + + return size or dgram_length, dgram_length +end + local function socket_recv(self, size, flags) local fd = check_socket(self) local iflags = get_iflags(internal.SEND_FLAGS, flags) @@ -813,13 +833,15 @@ local function socket_recv(self, size, flags) return nil end - if self:is_udp() then - size = size or self:dgram_length() - if size == nil then - return nil - end - else - size = size or 512 + local is_udp = self:is_udp() + local dgram_length + size, dgram_length = socket_recv_buffer_size_internal(self, size, is_udp) + if size == nil then + return nil + end + + if is_udp and jit.os ~= 'OSX' then + iflags = bit.bor(iflags, internal.SEND_FLAGS['MSG_TRUNC']) end self._errno = nil @@ -830,6 +852,9 @@ local function socket_recv(self, size, flags) if res == -1 then self._errno = boxerrno() return nil + elseif (dgram_length or res) > size then + self._errno = boxerrno.EMSGSIZE + return nil end return ffi.string(buf, res) end @@ -842,20 +867,18 @@ local function socket_recvfrom(self, size, flags) return nil end - if self:is_udp() then - size = size or self:dgram_length() - if size == nil then - return nil - end - else - size = size or 512 + local is_udp = self:is_udp() + local dgram_length + size, dgram_length = socket_recv_buffer_size_internal(self, size, is_udp) + if size == nil then + return nil end self._errno = nil - local res, from = internal.recvfrom(fd, size, iflags) + local res, from = internal.recvfrom(fd, size, iflags, is_udp, dgram_length) if res == nil then self._errno = boxerrno() - return nil + return nil, from end return res, from end diff --git a/test/app/socket.result b/test/app/socket.result index 07cd24c15..6c758cae6 100644 --- a/test/app/socket.result +++ b/test/app/socket.result @@ -884,7 +884,7 @@ data, from = s:recvfrom(10) ... data --- -- Hello, Wor +- null ... s:sendto(from.host, from.port, 'Hello, hello!') --- @@ -2622,6 +2622,60 @@ e --- - 0 ... +-- case: recv, datagram that is larger then the buffer of an explicit size +sending_socket:sendto('127.0.0.1', receiving_socket_port, message) +--- +- 1025 +... +received_message = receiving_socket:recv(512) +--- +... +e = receiving_socket:errno() +--- +... +received_message == nil -- expected true +--- +- true +... +received_message +--- +- null +... +e == errno.EMSGSIZE -- expected true +--- +- true +... +-- case: recvfrom, datagram that is larger then the buffer of an explicit size +sending_socket:sendto('127.0.0.1', receiving_socket_port, message) +--- +- 1025 +... +received_message, from = receiving_socket:recvfrom(512) +--- +... +e = receiving_socket:errno() +--- +... +received_message == nil -- expected true +--- +- true +... +received_message +--- +- null +... +from ~= nil -- expected true +--- +- true +... +from.host == '127.0.0.1' -- expected true +--- +- true +... +e == errno.EMSGSIZE -- expected true +--- +- true +... receiving_socket:close() --- - true @@ -2630,6 +2684,89 @@ sending_socket:close() --- - true ... +listening_socket = socket('AF_INET', 'SOCK_STREAM', 'tcp') +--- +... +listening_socket:setsockopt('SOL_SOCKET', 'SO_REUSEADDR', true) +--- +- true +... +listening_socket:bind('127.0.0.1', 0) +--- +- true +... +listening_socket:listen() +--- +- true +... +listening_socket_port = listening_socket:name().port +--- +... +sending_socket = socket('AF_INET', 'SOCK_STREAM', 'tcp') +--- +... +sending_socket:sysconnect('127.0.0.1', listening_socket_port) or errno() == errno.EINPROGRESS +--- +- true +... +receiving_socket = listening_socket:accept() +--- +... +-- case: recvfrom truncate the message with tcp +sending_socket:write(message) +--- +- 1025 +... +received_message, from = receiving_socket:recvfrom() +--- +... +e = receiving_socket:errno() +--- +... +received_message == message:sub(1, 512) -- expected true +--- +- true +... +received_message:len() == 512 -- expected true +--- +- true +... +received_message +--- +- xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx +... +received_message:len() +--- +- 512 +... +from == nil -- expected true +--- +- true +... +from +--- +- null +... +e == 0 -- expected true +--- +- true +... +e +--- +- 0 +... +receiving_socket:close() +--- +- true +... +sending_socket:close() +--- +- true +... +listening_socket:close() +--- +- true +... -- }}} test_run:cmd("clear filter") --- diff --git a/test/app/socket.test.lua b/test/app/socket.test.lua index c2dc7ea90..79927ccce 100644 --- a/test/app/socket.test.lua +++ b/test/app/socket.test.lua @@ -885,8 +885,52 @@ from.host == '127.0.0.1' -- expected true e == 0 -- expected true e +-- case: recv, datagram that is larger then the buffer of an explicit size +sending_socket:sendto('127.0.0.1', receiving_socket_port, message) +received_message = receiving_socket:recv(512) +e = receiving_socket:errno() +received_message == nil -- expected true +received_message +e == errno.EMSGSIZE -- expected true + +-- case: recvfrom, datagram that is larger then the buffer of an explicit size +sending_socket:sendto('127.0.0.1', receiving_socket_port, message) +received_message, from = receiving_socket:recvfrom(512) +e = receiving_socket:errno() +received_message == nil -- expected true +received_message +from ~= nil -- expected true +from.host == '127.0.0.1' -- expected true +e == errno.EMSGSIZE -- expected true + +receiving_socket:close() +sending_socket:close() + +listening_socket = socket('AF_INET', 'SOCK_STREAM', 'tcp') +listening_socket:setsockopt('SOL_SOCKET', 'SO_REUSEADDR', true) +listening_socket:bind('127.0.0.1', 0) +listening_socket:listen() +listening_socket_port = listening_socket:name().port +sending_socket = socket('AF_INET', 'SOCK_STREAM', 'tcp') +sending_socket:sysconnect('127.0.0.1', listening_socket_port) or errno() == errno.EINPROGRESS +receiving_socket = listening_socket:accept() + +-- case: recvfrom truncate the message with tcp +sending_socket:write(message) +received_message, from = receiving_socket:recvfrom() +e = receiving_socket:errno() +received_message == message:sub(1, 512) -- expected true +received_message:len() == 512 -- expected true +received_message +received_message:len() +from == nil -- expected true +from +e == 0 -- expected true +e + receiving_socket:close() sending_socket:close() +listening_socket:close() -- }}} -- 2.17.1