[PATCH 2/3] socket: don't truncate a datagram in recv/recvfrom
Vladimir Davydov
vdavydov.dev at gmail.com
Fri Aug 24 18:24:39 MSK 2018
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.
More information about the Tarantool-patches
mailing list