[PATCH 0/3] *** [#3619] socket.recvfrom crops UDP packets ***

[PATCH 0/3] *** [#3619] socket.recvfrom crops UDP packets ***

[Alexander Turenko]

Hi!

Vladimir, please review the patchset.

Yaroslav, please confirm that the new behaviour is one you want.

The changes in short:

* evaluate buffer size for a UDP socket in recv / recvfrom when it was
  not passed;
* discard a datagram in recv / recvfrom when it is larger then the input
  buffer, when the size was passed;
* prevent defererence of a pointer to non-initialized data in case of
  recvfrom was called for a TCP socket.

WBR, Alexander Turenko.

branch: Totktonada/gh-3619-socket-recvfrom-detect-a-cropped-packet
travis-ci: https://travis-ci.org/tarantool/tarantool/builds/419931085
issue: https://github.com/tarantool/tarantool/issues/3619

Alexander Turenko (3):
  socket: evaluate buffer size in recv / recvfrom
  socket: don't truncate a datagram in recv/recvfrom
  socket: prevent recvfrom from returning garbage

 src/lua/socket.c         |  25 ++
 src/lua/socket.lua       |  84 +++++-
 test/app/socket.result   | 540 ++++++++++++++++++++++++++++++++++++++-
 test/app/socket.test.lua | 198 ++++++++++++++
 4 files changed, 842 insertions(+), 5 deletions(-)

-- 
2.17.1

[PATCH 1/3] socket: evaluate buffer size in recv / recvfrom

[Alexander Turenko]

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 recv syscall on Linux.

Added socket:is_udp() and socket:dgram_length() methods to the public
API.

Part of #3619.
---
 src/lua/socket.c         |   3 +
 src/lua/socket.lua       |  57 +++++-
 test/app/socket.result   | 401 +++++++++++++++++++++++++++++++++++++++
 test/app/socket.test.lua | 154 +++++++++++++++
 4 files changed, 613 insertions(+), 2 deletions(-)

diff --git a/src/lua/socket.c b/src/lua/socket.c
index 14d49d106..bf23eae1f 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..946e11e9e 100644
--- a/src/lua/socket.lua
+++ b/src/lua/socket.lua
@@ -770,6 +770,41 @@ local function socket_send(self, octets, flags)
     return tonumber(res)
 end
 
+local function socket_is_udp(self)
+    local fd = check_socket(self)
+    if self.itype == nil then
+        self.itype = self:getsockopt('SOL_SOCKET', 'SO_TYPE')
+    end
+    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(self)
+    local fd = check_socket(self)
+    if not self:is_udp() then
+        error("socket:dgram_length() 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 +813,15 @@ local function socket_recv(self, size, flags)
         return nil
     end
 
-    size = size or 512
+    if self:is_udp() then
+        size = size or self:dgram_length()
+        if size == nil then
+            return nil
+        end
+    else
+        size = size or 512
+    end
+
     self._errno = nil
     local buf = ffi.new("char[?]", size)
 
@@ -799,7 +842,15 @@ local function socket_recvfrom(self, size, flags)
         return nil
     end
 
-    size = size or 512
+    if self:is_udp() then
+        size = size or self:dgram_length()
+        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
@@ -1165,6 +1216,8 @@ socket_mt   = {
         read = socket_read;
         write = socket_write;
         send = socket_send;
+        is_udp = socket_is_udp;
+        dgram_length = socket_dgram_length;
         recv = socket_recv;
         recvfrom = socket_recvfrom;
         sendto = socket_sendto;
diff --git a/test/app/socket.result b/test/app/socket.result
index b4d4a9a78..07cd24c15 100644
--- a/test/app/socket.result
+++ b/test/app/socket.result
@@ -1721,6 +1721,7 @@ test_run:cmd("setopt delimiter ''");
 --------------------------------------------------------------------------------
 -- Lua Socket Emulation
 --------------------------------------------------------------------------------
+-- {{{
 test_run:cmd("push filter 'fd=([0-9]+)' to 'fd=<FD>'")
 ---
 - true
@@ -2230,6 +2231,406 @@ s:close()
 ---
 - 1
 ...
+-- }}}
+-- s:is_udp() {{{
+s = socket('AF_INET', 'SOCK_DGRAM', 'udp')
+---
+...
+s:is_udp() -- expected true
+---
+- true
+...
+s:close()
+---
+- true
+...
+s = socket('AF_INET', 'SOCK_STREAM', 0)
+---
+...
+s:is_udp() -- expected false
+---
+- false
+...
+s:close()
+---
+- true
+...
+-- }}}
+-- s:dgram_length() {{{
+-- case: tcp socket is not supported
+s = socket('AF_INET', 'SOCK_STREAM', 0)
+---
+...
+s:dgram_length() -- expected an error
+---
+- error: 'builtin/socket.lua: socket:dgram_length() 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:dgram_length()
+---
+...
+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:dgram_length()
+---
+...
+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:dgram_length()
+---
+...
+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
+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
+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
+...
+receiving_socket:close()
+---
+- true
+...
+sending_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..c2dc7ea90 100644
--- a/test/app/socket.test.lua
+++ b/test/app/socket.test.lua
@@ -591,6 +591,7 @@ test_run:cmd("setopt delimiter ''");
 --------------------------------------------------------------------------------
 -- Lua Socket Emulation
 --------------------------------------------------------------------------------
+-- {{{
 
 test_run:cmd("push filter 'fd=([0-9]+)' to 'fd=<FD>'")
 
@@ -736,4 +737,157 @@ sc:connect(host, port)
 sc:close()
 s:close()
 
+-- }}}
+
+-- s:is_udp() {{{
+
+s = socket('AF_INET', 'SOCK_DGRAM', 'udp')
+s:is_udp() -- expected true
+s:close()
+
+s = socket('AF_INET', 'SOCK_STREAM', 0)
+s:is_udp() -- expected false
+s:close()
+
+-- }}}
+
+-- s:dgram_length() {{{
+
+-- case: tcp socket is not supported
+s = socket('AF_INET', 'SOCK_STREAM', 0)
+s:dgram_length() -- 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:dgram_length()
+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:dgram_length()
+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:dgram_length()
+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
+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
+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
+
+receiving_socket:close()
+sending_socket:close()
+
+-- }}}
+
 test_run:cmd("clear filter")
-- 
2.17.1

Re: [PATCH 1/3] socket: evaluate buffer size in recv / recvfrom

[Vladimir Davydov]

On Fri, Aug 24, 2018 at 05:47:37AM +0300, Alexander Turenko wrote:
> diff --git a/src/lua/socket.lua b/src/lua/socket.lua
> index 06306eae2..946e11e9e 100644
> --- a/src/lua/socket.lua
> +++ b/src/lua/socket.lua
> @@ -1165,6 +1216,8 @@ socket_mt   = {
>          read = socket_read;
>          write = socket_write;
>          send = socket_send;
> +        is_udp = socket_is_udp;
> +        dgram_length = socket_dgram_length;

I don't think it's a good idea to add these two methods to the public
API. Nobody asked for this. Let's please remove them.

>          recv = socket_recv;
>          recvfrom = socket_recvfrom;
>          sendto = socket_sendto;

Re: [PATCH 1/3] socket: evaluate buffer size in recv / recvfrom

[Vladimir Davydov]

On Fri, Aug 24, 2018 at 05:47:37AM +0300, Alexander Turenko wrote:
> diff --git a/src/lua/socket.lua b/src/lua/socket.lua
> index 06306eae2..946e11e9e 100644
> --- a/src/lua/socket.lua
> +++ b/src/lua/socket.lua
> @@ -770,6 +770,41 @@ local function socket_send(self, octets, flags)
>      return tonumber(res)
>  end
>  
> +local function socket_is_udp(self)
> +    local fd = check_socket(self)
> +    if self.itype == nil then
> +        self.itype = self:getsockopt('SOL_SOCKET', 'SO_TYPE')

I think you could instead set itype in socket constructor.

> +    end
> +    return self.itype == internal.SO_TYPE['SOCK_DGRAM']
> +end

Re: [PATCH 1/3] socket: evaluate buffer size in recv / recvfrom

[Alexander Turenko]

On Fri, Aug 24, 2018 at 06:25:42PM +0300, Vladimir Davydov wrote:
> On Fri, Aug 24, 2018 at 05:47:37AM +0300, Alexander Turenko wrote:
> > diff --git a/src/lua/socket.lua b/src/lua/socket.lua
> > index 06306eae2..946e11e9e 100644
> > --- a/src/lua/socket.lua
> > +++ b/src/lua/socket.lua
> > @@ -770,6 +770,41 @@ local function socket_send(self, octets, flags)
> >      return tonumber(res)
> >  end
> >  
> > +local function socket_is_udp(self)
> > +    local fd = check_socket(self)
> > +    if self.itype == nil then
> > +        self.itype = self:getsockopt('SOL_SOCKET', 'SO_TYPE')
> 
> I think you could instead set itype in socket constructor.
> 

That will increase memory consumption in case when it is not needed to
know itype: say, for sending sockets (I should add comment about this,
of course). However I think it is worth to cache it when revc / recvfrom
is used intensively to avoid extra syscall.

Do you think we can ignore extra memory comsumption in this case?

WBR, Alexander Turenko.

Re: [PATCH 1/3] socket: evaluate buffer size in recv / recvfrom

[Vladimir Davydov]

On Mon, Aug 27, 2018 at 03:08:49AM +0300, Alexander Turenko wrote:
> On Fri, Aug 24, 2018 at 06:25:42PM +0300, Vladimir Davydov wrote:
> > On Fri, Aug 24, 2018 at 05:47:37AM +0300, Alexander Turenko wrote:
> > > diff --git a/src/lua/socket.lua b/src/lua/socket.lua
> > > index 06306eae2..946e11e9e 100644
> > > --- a/src/lua/socket.lua
> > > +++ b/src/lua/socket.lua
> > > @@ -770,6 +770,41 @@ local function socket_send(self, octets, flags)
> > >      return tonumber(res)
> > >  end
> > >  
> > > +local function socket_is_udp(self)
> > > +    local fd = check_socket(self)
> > > +    if self.itype == nil then
> > > +        self.itype = self:getsockopt('SOL_SOCKET', 'SO_TYPE')
> > 
> > I think you could instead set itype in socket constructor.
> > 
> 
> That will increase memory consumption in case when it is not needed to
> know itype: say, for sending sockets (I should add comment about this,
> of course). However I think it is worth to cache it when revc / recvfrom
> is used intensively to avoid extra syscall.
> 
> Do you think we can ignore extra memory comsumption in this case?

How much of memory overhead is it going to be? Several bytes per socket?
IMHO we can ignore it.

Re: [PATCH 1/3] socket: evaluate buffer size in recv / recvfrom

[Alexander Turenko]

On Mon, Aug 27, 2018 at 12:20:59PM +0300, Vladimir Davydov wrote:
> On Mon, Aug 27, 2018 at 03:08:49AM +0300, Alexander Turenko wrote:
> > On Fri, Aug 24, 2018 at 06:25:42PM +0300, Vladimir Davydov wrote:
> > > On Fri, Aug 24, 2018 at 05:47:37AM +0300, Alexander Turenko wrote:
> > > > diff --git a/src/lua/socket.lua b/src/lua/socket.lua
> > > > index 06306eae2..946e11e9e 100644
> > > > --- a/src/lua/socket.lua
> > > > +++ b/src/lua/socket.lua
> > > > @@ -770,6 +770,41 @@ local function socket_send(self, octets, flags)
> > > >      return tonumber(res)
> > > >  end
> > > >  
> > > > +local function socket_is_udp(self)
> > > > +    local fd = check_socket(self)
> > > > +    if self.itype == nil then
> > > > +        self.itype = self:getsockopt('SOL_SOCKET', 'SO_TYPE')
> > > 
> > > I think you could instead set itype in socket constructor.
> > > 
> > 
> > That will increase memory consumption in case when it is not needed to
> > know itype: say, for sending sockets (I should add comment about this,
> > of course). However I think it is worth to cache it when revc / recvfrom
> > is used intensively to avoid extra syscall.
> > 
> > Do you think we can ignore extra memory comsumption in this case?
> 
> How much of memory overhead is it going to be? Several bytes per socket?
> IMHO we can ignore it.

8 bytes (numbers in lua are doubles). Ok, I'll proceed.

WBR, Alexander Turenko.

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

[Alexander Turenko]

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

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

[Vladimir Davydov]

On Fri, Aug 24, 2018 at 05:47:38AM +0300, Alexander Turenko wrote:
> 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
> +

I don't like how the logic is spread between Lua and C. Let's try to put
everything in C. I suggest to do the following.

First, let's add a wrapper around recvfrom to lua/socket.c. Let's call
it do_recvfrom, but the name is not of an importance here. The function
will take a buffer, its size, flags, and a boolean flag whether a
datagram is expected or not.

In case a datagram is expected, do_recvfrom will auto-detect the size in
case size == 0. If size != 0, it should check that the message fits in
the buffer with MSG_TRUNC or getsockopt depending on the OS. I think
it's possible to implement this function with just one ifdef
TARGET_OS_DARWIN.

Next, we implement both socket.recv and socket.recvfrom in C with the
aid of this function. They will differ only by the Lua return value.

I'm not 100% sure it's the right way, but as I said spreading logic
doesn't look good to me. Please try to gather all related bits one
place.

[PATCH 3/3] socket: prevent recvfrom from returning garbage

[Alexander Turenko]

In C recvfrom function sets addrlen parameter to zero when called on TCP
socket (at least on Linux). The src_addr parameter can contain garbage
in the case, so we should not dereference it.

Before this commit socket:recvfrom() can return 'from' table with only
family field (don't sure why, but addr->sa_family often contain PF_INET
value in my case) or return nil depending on the garbage at the address.
Now it always return nil.
---
 src/lua/socket.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/src/lua/socket.c b/src/lua/socket.c
index c716979f4..c9ed7cfdd 100644
--- a/src/lua/socket.c
+++ b/src/lua/socket.c
@@ -680,6 +680,11 @@ static int
 lbox_socket_push_addr(struct lua_State *L,
 			 const struct sockaddr *addr, socklen_t alen)
 {
+	if (alen == 0) {
+		lua_pushnil(L);
+		return 1;
+	}
+
 	lua_newtable(L);
 
 	lua_pushliteral(L, "family");
-- 
2.17.1

Re: [PATCH 3/3] socket: prevent recvfrom from returning garbage

[Vladimir Davydov]

On Fri, Aug 24, 2018 at 05:47:39AM +0300, Alexander Turenko wrote:
> In C recvfrom function sets addrlen parameter to zero when called on TCP
> socket (at least on Linux). The src_addr parameter can contain garbage
> in the case, so we should not dereference it.
> 
> Before this commit socket:recvfrom() can return 'from' table with only
> family field (don't sure why, but addr->sa_family often contain PF_INET
> value in my case) or return nil depending on the garbage at the address.
> Now it always return nil.
> ---
>  src/lua/socket.c | 5 +++++
>  1 file changed, 5 insertions(+)

Could you please add a test case?

Re: [PATCH 3/3] socket: prevent recvfrom from returning garbage

[Alexander Turenko]

On Fri, Aug 24, 2018 at 04:07:48PM +0300, Vladimir Davydov wrote:
> On Fri, Aug 24, 2018 at 05:47:39AM +0300, Alexander Turenko wrote:
> > In C recvfrom function sets addrlen parameter to zero when called on TCP
> > socket (at least on Linux). The src_addr parameter can contain garbage
> > in the case, so we should not dereference it.
> > 
> > Before this commit socket:recvfrom() can return 'from' table with only
> > family field (don't sure why, but addr->sa_family often contain PF_INET
> > value in my case) or return nil depending on the garbage at the address.
> > Now it always return nil.
> > ---
> >  src/lua/socket.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> 
> Could you please add a test case?

Added in the previous commit ('case: recvfrom truncate the message with
tcp'). The main purpose of the test case is another, but when added I
found the flakiness ('from' could be {family = 'AF_INET'} or nil from
time to time) and fixed it here.

How to better handle this? I suppose I should add a comment to the test
case that it also test this case?

WBR, Alexander Turenko.

Re: [PATCH 3/3] socket: prevent recvfrom from returning garbage

[Vladimir Davydov]

On Fri, Aug 24, 2018 at 04:44:41PM +0300, Alexander Turenko wrote:
> On Fri, Aug 24, 2018 at 04:07:48PM +0300, Vladimir Davydov wrote:
> > On Fri, Aug 24, 2018 at 05:47:39AM +0300, Alexander Turenko wrote:
> > > In C recvfrom function sets addrlen parameter to zero when called on TCP
> > > socket (at least on Linux). The src_addr parameter can contain garbage
> > > in the case, so we should not dereference it.
> > > 
> > > Before this commit socket:recvfrom() can return 'from' table with only
> > > family field (don't sure why, but addr->sa_family often contain PF_INET
> > > value in my case) or return nil depending on the garbage at the address.
> > > Now it always return nil.
> > > ---
> > >  src/lua/socket.c | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > 
> > Could you please add a test case?
> 
> Added in the previous commit ('case: recvfrom truncate the message with
> tcp'). The main purpose of the test case is another, but when added I
> found the flakiness ('from' could be {family = 'AF_INET'} or nil from
> time to time) and fixed it here.
> 
> How to better handle this? I suppose I should add a comment to the test
> case that it also test this case?

This patch is pretty straightforward and raises no questions, in
contrast to patches 1 and 2. If you equipped it with a test and
rebased on top of 1.10 (or should it be 1.9?), I'd pushed it right
away.

Re: [PATCH 3/3] socket: prevent recvfrom from returning garbage

[Vladimir Davydov]

As Alexander explained to me verbally, implementing a test case that
reproduces this issue is extremely difficult (there has to be a specific
integer value in the stack for this problem to happen). So we agreed to
push this one without a test - it's a 'one-liner' anyway.

Pushed to 1.9