From: Alexander Turenko <alexander.turenko@tarantool.org> To: Vladimir Davydov <vdavydov.dev@gmail.com> Cc: Alexander Turenko <alexander.turenko@tarantool.org>, tarantool-patches@freelists.org, Yaroslav Dynnikov <yaroslav.dynnikov@tarantool.org> Subject: [PATCH v2] socket: evaluate buffer size in recv / recvfrom Date: Tue, 28 Aug 2018 00:05:10 +0300 [thread overview] Message-ID: <de71fd8383b9c6f6827850cf09c818a6f2970ec4.1535401788.git.alexander.turenko@tarantool.org> (raw) When size parameter is not passed to socket:recv() or socket:recvfrom() it will call a) or b) on the socket to evaluate size of the buffer to store the receiving datagram. Before this commit a datagram will be truncated to 512 bytes in the case. a) Linux: recv(fd, NULL, 0 , MSG_TRUNC | MSG_PEEK) b) Mac OS: getsockopt(fd, SOL_SOCKET, SO_NREAD, &val, &len) It is recommended to set 'size' parameter (size of the input buffer) explicitly based on known message format and known network conditions (say, set it less then MTU to prevent IP fragmentation, which can be inefficient) or pass it from a configuration option of a library / an application. The reason is that explicit buffer size provided allows to avoid extra syscall to evaluate necessary buffer size. When 'size' parameter is set explicitly for recv / recvfrom on a UDP socket and the next datagram length is larger then the size, the returned message will be truncated to the size provided and the rest of the datagram will be discarded. Of course, the tail will not be discarded in case of a TCP socket and will be available to read by the next recv / recvfrom call. Fixes #3619. --- branch: Totktonada/gh-3619-socket-recvfrom-detect-a-cropped-packet issue: https://github.com/tarantool/tarantool/issues/3619 travis-ci: https://travis-ci.org/tarantool/tarantool/builds/421272709 Changes since the previous version of the patch: - discarded the patch to prevent UDP datagram truncation as Yaroslav asks; - moved itype saving into the socket constructor; - removed is_udp and dgram_length from the public API (moved to socket.internal reflecting the msgpackffi approach); - adopted and added test cases for recv / recvfrom from the discarded patch to hold the current behaviour (truncate with explicit size). src/lua/socket.c | 3 + src/lua/socket.lua | 68 +++- test/app/socket.result | 672 ++++++++++++++++++++++++++++++++++++++- test/app/socket.test.lua | 249 ++++++++++++++- 4 files changed, 980 insertions(+), 12 deletions(-) diff --git a/src/lua/socket.c b/src/lua/socket.c index 1458d3ba4..84711b441 100644 --- a/src/lua/socket.c +++ b/src/lua/socket.c @@ -268,6 +268,9 @@ static const struct lbox_sockopt_reg so_opts[] = { #ifdef SO_PROTOCOL {"SO_PROTOCOL", SO_PROTOCOL, 1, 0, }, #endif +#ifdef SO_NREAD /* available on Darwin */ + {"SO_NREAD", SO_NREAD, 1, 0, }, +#endif {"SO_TYPE", SO_TYPE, 1, 0, }, diff --git a/src/lua/socket.lua b/src/lua/socket.lua index 06306eae2..ddf7ebd53 100644 --- a/src/lua/socket.lua +++ b/src/lua/socket.lua @@ -86,8 +86,12 @@ local function check_socket(socket) end end -local function make_socket(fd) - local socket = { _gc_socket = ffi.new(gc_socket_t, { fd = fd }) } +local function make_socket(fd, itype) + assert(itype ~= nil) + local socket = { + _gc_socket = ffi.new(gc_socket_t, { fd = fd }), + itype = itype, + } return setmetatable(socket, socket_mt) end @@ -590,7 +594,7 @@ local function socket_accept(self) self._errno = boxerrno() return nil end - local client = make_socket(client_fd) + local client = make_socket(client_fd, self.itype) if not client:nonblock(true) then client:close() return @@ -770,6 +774,38 @@ local function socket_send(self, octets, flags) return tonumber(res) end +local function socket_is_udp_internal(self) + assert(self.itype ~= nil) + return self.itype == internal.SO_TYPE['SOCK_DGRAM'] +end + +-- returns nil and sets EAGAIN when there are no datagrams in the receive queue +-- as well as when zero-length datagram is the next one +local function socket_dgram_length_internal(self) + local fd = check_socket(self) + if not socket_is_udp_internal(self) then + error("socket_dgram_length_internal() supports only a UDP socket") + end + + local len + if jit.os == 'OSX' then + self._errno = nil + len = self:getsockopt('SOL_SOCKET', 'SO_NREAD') + else + local iflags = get_iflags(internal.SEND_FLAGS, {'MSG_TRUNC', 'MSG_PEEK'}) + assert(iflags ~= nil) + self._errno = nil + len = tonumber(ffi.C.recv(fd, nil, 0, iflags)) + end + + if len == -1 or len == 0 then + self._errno = boxerrno() + return nil + end + + return len +end + local function socket_recv(self, size, flags) local fd = check_socket(self) local iflags = get_iflags(internal.SEND_FLAGS, flags) @@ -778,7 +814,15 @@ local function socket_recv(self, size, flags) return nil end - size = size or 512 + if socket_is_udp_internal(self) then + size = size or socket_dgram_length_internal(self) + if size == nil then + return nil + end + else + size = size or 512 + end + self._errno = nil local buf = ffi.new("char[?]", size) @@ -799,7 +843,15 @@ local function socket_recvfrom(self, size, flags) return nil end - size = size or 512 + if socket_is_udp_internal(self) then + size = size or socket_dgram_length_internal(self) + if size == nil then + return nil + end + else + size = size or 512 + end + self._errno = nil local res, from = internal.recvfrom(fd, size, iflags) if res == nil then @@ -860,7 +912,7 @@ local function socket_new(domain, stype, proto) local fd = ffi.C.socket(idomain, itype, iproto) if fd >= 0 then - local socket = make_socket(fd) + local socket = make_socket(fd, itype) if not socket:nonblock(true) then socket:close() else @@ -1171,6 +1223,10 @@ socket_mt = { name = socket_name; peer = socket_peer; fd = socket_fd; + internal = { -- for testing + is_udp = socket_is_udp_internal; + dgram_length = socket_dgram_length_internal; + }; }; __tostring = function(self) local fd = check_socket(self) diff --git a/test/app/socket.result b/test/app/socket.result index b4d4a9a78..20ead8c6f 100644 --- a/test/app/socket.result +++ b/test/app/socket.result @@ -59,9 +59,11 @@ test_run:cmd("setopt delimiter ';'") - true ... for k in pairs(getmetatable(s).__index) do - local r, msg = pcall(s[k]) - if not msg:match('Usage:') then - error("Arguments is not checked for "..k) + if k ~= 'internal' then + local r, msg = pcall(s[k]) + if not msg:match('Usage:') then + error("Arguments is not checked for "..k) + end end end; --- @@ -1721,6 +1723,7 @@ test_run:cmd("setopt delimiter ''"); -------------------------------------------------------------------------------- -- Lua Socket Emulation -------------------------------------------------------------------------------- +-- {{{ test_run:cmd("push filter 'fd=([0-9]+)' to 'fd=<FD>'") --- - true @@ -2230,6 +2233,669 @@ s:close() --- - 1 ... +-- }}} +-- s.internal.is_udp() {{{ +-- case: udp socket +s = socket('AF_INET', 'SOCK_DGRAM', 'udp') +--- +... +s.internal.is_udp(s) -- expected true +--- +- true +... +s:close() +--- +- true +... +-- case: tcp socket +s = socket('AF_INET', 'SOCK_STREAM', 0) +--- +... +s.internal.is_udp(s) -- expected false +--- +- false +... +s:close() +--- +- true +... +-- case: tcp socket from :accept() +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() +--- +... +receiving_socket.internal.is_udp(receiving_socket) -- expected false +--- +- false +... +receiving_socket:close() +--- +- true +... +sending_socket:close() +--- +- true +... +listening_socket:close() +--- +- true +... +-- }}} +-- s.internal.dgram_length() {{{ +-- case: tcp socket is not supported +s = socket('AF_INET', 'SOCK_STREAM', 0) +--- +... +s.internal.dgram_length(s) -- expected an error +--- +- error: 'builtin/socket.lua: socket_dgram_length_internal() supports only a UDP + socket' +... +s:close() +--- +- true +... +receiving_socket = socket('AF_INET', 'SOCK_DGRAM', 'udp') +--- +... +receiving_socket:bind('127.0.0.1', 0) +--- +- true +... +receiving_socket_port = receiving_socket:name().port +--- +... +sending_socket = socket('AF_INET', 'SOCK_DGRAM', 'udp') +--- +... +-- case: no datagram +received_len = receiving_socket.internal.dgram_length(receiving_socket) +--- +... +e = receiving_socket:errno() +--- +... +received_len == nil -- expected true +--- +- true +... +received_len +--- +- null +... +e == errno.EAGAIN -- expected true +--- +- true +... +message_len = 0 +--- +... +message = '' +--- +... +-- case: zero-length datagram (the same as no datagram) +sending_socket:sendto('127.0.0.1', receiving_socket_port, message) +--- +- true +... +received_len = receiving_socket.internal.dgram_length(receiving_socket) +--- +... +e = receiving_socket:errno() +--- +... +received_len == nil -- expected true +--- +- true +... +received_len +--- +- null +... +e == errno.EAGAIN -- expected true +--- +- true +... +message_len = 1025 +--- +... +message = string.rep('x', message_len) +--- +... +-- case: non-zero length datagram +sending_socket:sendto('127.0.0.1', receiving_socket_port, message) +--- +- 1025 +... +received_len = receiving_socket.internal.dgram_length(receiving_socket) +--- +... +e = receiving_socket:errno() +--- +... +received_len == message_len -- expected true +--- +- true +... +received_len +--- +- 1025 +... +e == 0 -- expected true +--- +- true +... +e +--- +- 0 +... +receiving_socket:close() +--- +- true +... +sending_socket:close() +--- +- true +... +-- }}} +-- gh-3619: socket.recvfrom crops UDP packets {{{ +receiving_socket = socket('AF_INET', 'SOCK_DGRAM', 'udp') +--- +... +receiving_socket:bind('127.0.0.1', 0) +--- +- true +... +receiving_socket_port = receiving_socket:name().port +--- +... +sending_socket = socket('AF_INET', 'SOCK_DGRAM', 'udp') +--- +... +-- case: recv, no datagram +received_message = receiving_socket:recv() +--- +... +e = receiving_socket:errno() +--- +... +received_message == nil -- expected true +--- +- true +... +received_message +--- +- null +... +e == errno.EAGAIN -- expected true +--- +- true +... +-- case: recvfrom, no datagram +received_message, from = receiving_socket:recvfrom() +--- +... +e = receiving_socket:errno() +--- +... +received_message == nil -- expected true +--- +- true +... +received_message +--- +- null +... +from == nil -- expected true +--- +- true +... +from +--- +- null +... +e == errno.EAGAIN -- expected true +--- +- true +... +message_len = 0 +--- +... +message = '' +--- +... +-- case: recv, zero-length datagram (the same as no datagram) +sending_socket:sendto('127.0.0.1', receiving_socket_port, message) +--- +- true +... +received_message = receiving_socket:recv() +--- +... +e = receiving_socket:errno() +--- +... +received_message == nil -- expected true +--- +- true +... +received_message +--- +- null +... +e == errno.EAGAIN -- expected true +--- +- true +... +-- case: recvfrom, zero-length datagram (the same as no datagram) +sending_socket:sendto('127.0.0.1', receiving_socket_port, message) +--- +- true +... +received_message, from = receiving_socket:recvfrom() +--- +... +e = receiving_socket:errno() +--- +... +received_message == nil -- expected true +--- +- true +... +received_message +--- +- null +... +from == nil -- expected true +--- +- true +... +from +--- +- null +... +e == errno.EAGAIN -- expected true +--- +- true +... +-- case: recv, zero-length datagram (the same as no datagram), explicit size +sending_socket:sendto('127.0.0.1', receiving_socket_port, message) +--- +- true +... +received_message = receiving_socket:recv(512) +--- +... +e = receiving_socket:errno() +--- +... +received_message == nil -- expected true +--- +- true +... +received_message +--- +- null +... +e == errno.EAGAIN -- expected true +--- +- true +... +-- case: recvfrom, zero-length datagram (the same as no datagram), explicit size +sending_socket:sendto('127.0.0.1', receiving_socket_port, message) +--- +- true +... +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 +--- +- null +... +e == errno.EAGAIN -- expected true +--- +- true +... +message_len = 1025 +--- +... +message = string.rep('x', message_len) +--- +... +-- case: recv, non-zero length datagram, the buffer size should be evaluated +sending_socket:sendto('127.0.0.1', receiving_socket_port, message) +--- +- 1025 +... +received_message = receiving_socket:recv() +--- +... +e = receiving_socket:errno() +--- +... +received_message == message -- expected true +--- +- true +... +received_message:len() +--- +- 1025 +... +received_message +--- +- xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx +... +e == 0 -- expected true +--- +- true +... +e +--- +- 0 +... +-- case: recvfrom, non-zero length datagram, the buffer size should be +-- evaluated +sending_socket:sendto('127.0.0.1', receiving_socket_port, message) +--- +- 1025 +... +received_message, from = receiving_socket:recvfrom() +--- +... +e = receiving_socket:errno() +--- +... +received_message == message -- expected true +--- +- true +... +received_message:len() +--- +- 1025 +... +received_message +--- +- xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx +... +from ~= nil -- expected true +--- +- true +... +from.host == '127.0.0.1' -- expected true +--- +- true +... +e == 0 -- expected true +--- +- true +... +e +--- +- 0 +... +-- case: recv truncates a datagram 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 == message:sub(1, 512) -- expected true +--- +- true +... +received_message:len() == 512 -- expected true +--- +- true +... +received_message +--- +- xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx +... +received_message:len() +--- +- 512 +... +e == 0 -- expected true +--- +- true +... +e +--- +- 0 +... +-- we set the different message to ensure the tail of the previous datagram was +-- discarded +message_len = 1025 +--- +... +message = string.rep('y', message_len) +--- +... +-- case: recvfrom truncates a datagram 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 == message:sub(1, 512) -- expected true +--- +- true +... +received_message:len() == 512 -- expected true +--- +- true +... +received_message +--- +- yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy +... +received_message:len() +--- +- 512 +... +from ~= nil -- expected true +--- +- true +... +from.host == '127.0.0.1' -- expected true +--- +- true +... +e == 0 -- expected true +--- +- true +... +e +--- +- 0 +... +receiving_socket:close() +--- +- true +... +sending_socket:close() +--- +- true +... +message_len = 513 +--- +... +message = string.rep('x', message_len) +--- +... +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() +--- +... +sending_socket:write(message) +--- +- 513 +... +-- case: recvfrom reads first 512 bytes from the message with tcp +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 +... +-- case: recvfrom does not discard the message tail with tcp +received_message, from = receiving_socket:recvfrom() +--- +... +e = receiving_socket:errno() +--- +... +received_message == message:sub(513, 513) -- expected true +--- +- true +... +received_message:len() == 1 -- expected true +--- +- true +... +received_message +--- +- x +... +received_message:len() +--- +- 1 +... +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") --- - true diff --git a/test/app/socket.test.lua b/test/app/socket.test.lua index f6539438b..cef71fc95 100644 --- a/test/app/socket.test.lua +++ b/test/app/socket.test.lua @@ -20,9 +20,11 @@ type(s) -- Invalid arguments test_run:cmd("setopt delimiter ';'") for k in pairs(getmetatable(s).__index) do - local r, msg = pcall(s[k]) - if not msg:match('Usage:') then - error("Arguments is not checked for "..k) + if k ~= 'internal' then + local r, msg = pcall(s[k]) + if not msg:match('Usage:') then + error("Arguments is not checked for "..k) + end end end; s:close(); @@ -591,6 +593,7 @@ test_run:cmd("setopt delimiter ''"); -------------------------------------------------------------------------------- -- Lua Socket Emulation -------------------------------------------------------------------------------- +-- {{{ test_run:cmd("push filter 'fd=([0-9]+)' to 'fd=<FD>'") @@ -736,4 +739,244 @@ sc:connect(host, port) sc:close() s:close() +-- }}} + +-- s.internal.is_udp() {{{ + +-- case: udp socket +s = socket('AF_INET', 'SOCK_DGRAM', 'udp') +s.internal.is_udp(s) -- expected true +s:close() + +-- case: tcp socket +s = socket('AF_INET', 'SOCK_STREAM', 0) +s.internal.is_udp(s) -- expected false +s:close() + +-- case: tcp socket from :accept() +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() +receiving_socket.internal.is_udp(receiving_socket) -- expected false +receiving_socket:close() +sending_socket:close() +listening_socket:close() + +-- }}} + +-- s.internal.dgram_length() {{{ + +-- case: tcp socket is not supported +s = socket('AF_INET', 'SOCK_STREAM', 0) +s.internal.dgram_length(s) -- expected an error +s:close() + +receiving_socket = socket('AF_INET', 'SOCK_DGRAM', 'udp') +receiving_socket:bind('127.0.0.1', 0) +receiving_socket_port = receiving_socket:name().port +sending_socket = socket('AF_INET', 'SOCK_DGRAM', 'udp') + +-- case: no datagram +received_len = receiving_socket.internal.dgram_length(receiving_socket) +e = receiving_socket:errno() +received_len == nil -- expected true +received_len +e == errno.EAGAIN -- expected true + +message_len = 0 +message = '' + +-- case: zero-length datagram (the same as no datagram) +sending_socket:sendto('127.0.0.1', receiving_socket_port, message) +received_len = receiving_socket.internal.dgram_length(receiving_socket) +e = receiving_socket:errno() +received_len == nil -- expected true +received_len +e == errno.EAGAIN -- expected true + +message_len = 1025 +message = string.rep('x', message_len) + +-- case: non-zero length datagram +sending_socket:sendto('127.0.0.1', receiving_socket_port, message) +received_len = receiving_socket.internal.dgram_length(receiving_socket) +e = receiving_socket:errno() +received_len == message_len -- expected true +received_len +e == 0 -- expected true +e + +receiving_socket:close() +sending_socket:close() + +-- }}} + +-- gh-3619: socket.recvfrom crops UDP packets {{{ + +receiving_socket = socket('AF_INET', 'SOCK_DGRAM', 'udp') +receiving_socket:bind('127.0.0.1', 0) +receiving_socket_port = receiving_socket:name().port +sending_socket = socket('AF_INET', 'SOCK_DGRAM', 'udp') + +-- case: recv, no datagram +received_message = receiving_socket:recv() +e = receiving_socket:errno() +received_message == nil -- expected true +received_message +e == errno.EAGAIN -- expected true + +-- case: recvfrom, no datagram +received_message, from = receiving_socket:recvfrom() +e = receiving_socket:errno() +received_message == nil -- expected true +received_message +from == nil -- expected true +from +e == errno.EAGAIN -- expected true + +message_len = 0 +message = '' + +-- case: recv, zero-length datagram (the same as no datagram) +sending_socket:sendto('127.0.0.1', receiving_socket_port, message) +received_message = receiving_socket:recv() +e = receiving_socket:errno() +received_message == nil -- expected true +received_message +e == errno.EAGAIN -- expected true + +-- case: recvfrom, zero-length datagram (the same as no datagram) +sending_socket:sendto('127.0.0.1', receiving_socket_port, message) +received_message, from = receiving_socket:recvfrom() +e = receiving_socket:errno() +received_message == nil -- expected true +received_message +from == nil -- expected true +from +e == errno.EAGAIN -- expected true + +-- case: recv, zero-length datagram (the same as no datagram), 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.EAGAIN -- expected true + +-- case: recvfrom, zero-length datagram (the same as no datagram), 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 +e == errno.EAGAIN -- expected true + +message_len = 1025 +message = string.rep('x', message_len) + +-- case: recv, non-zero length datagram, the buffer size should be evaluated +sending_socket:sendto('127.0.0.1', receiving_socket_port, message) +received_message = receiving_socket:recv() +e = receiving_socket:errno() +received_message == message -- expected true +received_message:len() +received_message +e == 0 -- expected true +e + +-- case: recvfrom, non-zero length datagram, the buffer size should be +-- evaluated +sending_socket:sendto('127.0.0.1', receiving_socket_port, message) +received_message, from = receiving_socket:recvfrom() +e = receiving_socket:errno() +received_message == message -- expected true +received_message:len() +received_message +from ~= nil -- expected true +from.host == '127.0.0.1' -- expected true +e == 0 -- expected true +e + +-- case: recv truncates a datagram 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 == message:sub(1, 512) -- expected true +received_message:len() == 512 -- expected true +received_message +received_message:len() +e == 0 -- expected true +e + +-- we set the different message to ensure the tail of the previous datagram was +-- discarded +message_len = 1025 +message = string.rep('y', message_len) + +-- case: recvfrom truncates a datagram 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 == message:sub(1, 512) -- expected true +received_message:len() == 512 -- expected true +received_message +received_message:len() +from ~= nil -- expected true +from.host == '127.0.0.1' -- expected true +e == 0 -- expected true +e + +receiving_socket:close() +sending_socket:close() + +message_len = 513 +message = string.rep('x', message_len) + +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() +sending_socket:write(message) + +-- case: recvfrom reads first 512 bytes from the message with tcp +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 + +-- case: recvfrom does not discard the message tail with tcp +received_message, from = receiving_socket:recvfrom() +e = receiving_socket:errno() +received_message == message:sub(513, 513) -- expected true +received_message:len() == 1 -- 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() + +-- }}} + test_run:cmd("clear filter") -- 2.17.1
next reply other threads:[~2018-08-27 21:05 UTC|newest] Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-08-27 21:05 Alexander Turenko [this message] 2018-08-28 11:32 ` Vladimir Davydov 2018-08-28 15:01 ` Alexander Turenko
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=de71fd8383b9c6f6827850cf09c818a6f2970ec4.1535401788.git.alexander.turenko@tarantool.org \ --to=alexander.turenko@tarantool.org \ --cc=tarantool-patches@freelists.org \ --cc=vdavydov.dev@gmail.com \ --cc=yaroslav.dynnikov@tarantool.org \ --subject='Re: [PATCH v2] socket: evaluate buffer size in recv / recvfrom' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox