From 11d73fbf3011d8960d39b424630b34c2796d2898 Mon Sep 17 00:00:00 2001 From: Simon Brummer Date: Sat, 14 Aug 2021 15:05:25 +0200 Subject: [PATCH] gnrc_tcp: handle zero size buffers --- sys/include/net/gnrc/tcp.h | 2 +- sys/net/gnrc/transport_layer/tcp/gnrc_tcp.c | 14 ++++++ tests/gnrc_tcp/main.c | 16 ++++--- tests/gnrc_tcp/tests-as-root/01-run.py | 48 +++++++++++++++++++++ tests/gnrc_tcp/tests-as-root/helpers.py | 9 ++-- 5 files changed, 79 insertions(+), 10 deletions(-) diff --git a/sys/include/net/gnrc/tcp.h b/sys/include/net/gnrc/tcp.h index 0e4c330ac2..db2bbb28cf 100644 --- a/sys/include/net/gnrc/tcp.h +++ b/sys/include/net/gnrc/tcp.h @@ -248,7 +248,7 @@ ssize_t gnrc_tcp_send(gnrc_tcp_tcb_t *tcb, const void *data, const size_t len, * occurred. * * @return The number of bytes read into @p data. - * @return 0, if the connection is closing and no further data can be read. + * @return 0, if the connection is closing and no further data can be read or @p max_len was 0. * @return -ENOTCONN if connection is not established. * @return -EAGAIN if user_timeout_duration_us is zero and no data is available. * @return -ECONNRESET if connection was reset by the peer. diff --git a/sys/net/gnrc/transport_layer/tcp/gnrc_tcp.c b/sys/net/gnrc/transport_layer/tcp/gnrc_tcp.c index 9385d0701c..3f354bbdf6 100644 --- a/sys/net/gnrc/transport_layer/tcp/gnrc_tcp.c +++ b/sys/net/gnrc/transport_layer/tcp/gnrc_tcp.c @@ -687,6 +687,13 @@ ssize_t gnrc_tcp_send(gnrc_tcp_tcb_t *tcb, const void *data, const size_t len, return -ENOTCONN; } + /* Early return for zero length payloads to send */ + if (!len) { + mutex_unlock(&(tcb->function_lock)); + TCP_DEBUG_LEAVE; + return 0; + } + /* Setup messaging */ _gnrc_tcp_fsm_set_mbox(tcb, &mbox); @@ -815,6 +822,13 @@ ssize_t gnrc_tcp_recv(gnrc_tcp_tcb_t *tcb, void *data, const size_t max_len, return -ENOTCONN; } + /* Early return for zero length buffers to store received data */ + if (!max_len) { + mutex_unlock(&(tcb->function_lock)); + TCP_DEBUG_LEAVE; + return 0; + } + /* If FIN was received (CLOSE_WAIT), no further data can be received. */ /* Copy received data into given buffer and return number of bytes. Can be zero. */ if (state == FSM_STATE_CLOSE_WAIT) { diff --git a/tests/gnrc_tcp/main.c b/tests/gnrc_tcp/main.c index 67f8c06e46..98b88ffe3e 100644 --- a/tests/gnrc_tcp/main.c +++ b/tests/gnrc_tcp/main.c @@ -248,13 +248,17 @@ int gnrc_tcp_send_cmd(int argc, char **argv) { dump_args(argc, argv); - int timeout = atol(argv[1]); - size_t to_send = strlen(buffer); + size_t timeout = atol(argv[1]); + size_t to_send = atol(argv[2]); size_t sent = 0; - while (sent < to_send) { + do { int ret = gnrc_tcp_send(tcb, buffer + sent, to_send - sent, timeout); switch (ret) { + case 0: + printf("%s: returns 0\n", argv[0]); + return ret; + case -ENOTCONN: printf("%s: returns -ENOTCONN\n", argv[0]); return ret; @@ -272,7 +276,7 @@ int gnrc_tcp_send_cmd(int argc, char **argv) return ret; } sent += ret; - } + } while (sent < to_send); printf("%s: sent %u\n", argv[0], (unsigned)sent); return sent; @@ -286,7 +290,7 @@ int gnrc_tcp_recv_cmd(int argc, char **argv) size_t to_receive = atol(argv[2]); size_t rcvd = 0; - while (rcvd < to_receive) { + do { int ret = gnrc_tcp_recv(tcb, buffer + rcvd, to_receive - rcvd, timeout); switch (ret) { @@ -315,7 +319,7 @@ int gnrc_tcp_recv_cmd(int argc, char **argv) return ret; } rcvd += ret; - } + } while (rcvd < to_receive); printf("%s: received %u\n", argv[0], (unsigned)rcvd); return 0; diff --git a/tests/gnrc_tcp/tests-as-root/01-run.py b/tests/gnrc_tcp/tests-as-root/01-run.py index 8f7efc701a..2e415c6a1e 100755 --- a/tests/gnrc_tcp/tests-as-root/01-run.py +++ b/tests/gnrc_tcp/tests-as-root/01-run.py @@ -255,6 +255,54 @@ def test_gnrc_tcp_recv_behavior_on_closed_connection(child): child.expect_exact('gnrc_tcp_recv: returns 0') +@Runner(timeout=5) +def test_gnrc_tcp_recv_behavior_on_zero_length_buffer_size(child): + """ This test verifies that gnrc_tcp_recv accepts zero as buffer size + and returns 0. + """ + with HostTcpServer(generate_port_number()) as host_srv: + riot_cli = RiotTcpClient(child, host_srv) + + # Call gnrc_tcp_recv with on data and no timeout on unconnected TCB + # This must return -ENOTCONN + child.sendline('gnrc_tcp_recv 0 0') + child.expect_exact('gnrc_tcp_recv: returns -ENOTCONN') + + with riot_cli: + host_srv.accept() + + # Call gnrc_tcp_recv with on data and no timeout + # This must return 0 not -EAGAIN + child.sendline('gnrc_tcp_recv 0 0') + child.expect_exact('gnrc_tcp_recv: returns 0') + + host_srv.close() + + +@Runner(timeout=5) +def test_gnrc_tcp_send_behavior_on_zero_length_buffer_size(child): + """ This test verifies that gnrc_tcp_send accepts zero length payload + and returns 0. + """ + with HostTcpServer(generate_port_number()) as host_srv: + riot_cli = RiotTcpClient(child, host_srv) + + # Call gnrc_tcp_send with on data and no timeout on unconnected TCB + # This must return -ENOTCONN + child.sendline('gnrc_tcp_send 0 0') + child.expect_exact('gnrc_tcp_send: returns -ENOTCONN') + + with riot_cli: + host_srv.accept() + + # Call gnrc_tcp_send with on data and no timeout + # This must return 0 not -EAGAIN + child.sendline('gnrc_tcp_send 0 0') + child.expect_exact('gnrc_tcp_send: returns 0') + + host_srv.close() + + @Runner(timeout=1) def test_gnrc_tcp_ep_from_str(child): """ Verify Endpoint construction from string """ diff --git a/tests/gnrc_tcp/tests-as-root/helpers.py b/tests/gnrc_tcp/tests-as-root/helpers.py index 2dc55f492d..199f5a1900 100644 --- a/tests/gnrc_tcp/tests-as-root/helpers.py +++ b/tests/gnrc_tcp/tests-as-root/helpers.py @@ -137,9 +137,12 @@ class _RiotTcpNode: self.child.sendline('gnrc_tcp_tcb_init') self.child.expect_exact('gnrc_tcp_tcb_init: returns') - def send(self, timeout_ms, payload_to_send): + def send(self, timeout_ms, payload_to_send, bytes_to_send=None): total_bytes = len(payload_to_send) + if bytes_to_send is None: + bytes_to_send = total_bytes + # Verify that internal buffer can hold the given amount of data assert self._setup_internal_buffer() >= total_bytes @@ -147,8 +150,8 @@ class _RiotTcpNode: self._write_data_to_internal_buffer(payload_to_send) # Send buffer contents via tcp - self.child.sendline('gnrc_tcp_send {}'.format(str(timeout_ms))) - self.child.expect_exact('gnrc_tcp_send: sent {}'.format(str(total_bytes))) + self.child.sendline('gnrc_tcp_send {} {}'.format(timeout_ms, bytes_to_send)) + self.child.expect_exact('gnrc_tcp_send: sent {}'.format(bytes_to_send)) # Verify that packet buffer is empty self._verify_pktbuf_empty()