[PATCH 2/3] socket: don't truncate a datagram in recv/recvfrom

[Alexander Turenko <alexander.turenko@tarantool.org>]

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