[PATCH 2/3] socket: don't truncate a datagram in recv/recvfrom
Alexander Turenko
alexander.turenko at tarantool.org
Fri Aug 24 05:47:38 MSK 2018
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
More information about the Tarantool-patches
mailing list