Finally fix bug #50088 (socket/netconn: data before RST should be readable) and added a unit test for it

src/api/api_lib.c

@@ -524,25 +524,30 @@ netconn_recv_data(struct netconn *conn, void **new_buf, u8_t apiflags)
 {
   void *buf = NULL;
   u16_t len;
-  err_t err;
 
   LWIP_ERROR("netconn_recv: invalid pointer", (new_buf != NULL), return ERR_ARG;);
   *new_buf = NULL;
   LWIP_ERROR("netconn_recv: invalid conn",    (conn != NULL),    return ERR_ARG;);
 
-  err = netconn_err(conn);
-  if (err != ERR_OK) {
-    /* return pending error */
-    return err;
-  }
   if (!sys_mbox_valid(&conn->recvmbox)) {
-    return ERR_CLSD;
+    err_t err = netconn_err(conn);
+    if (err != ERR_OK) {
+      /* return pending error */
+      return err;
+    }
+    return ERR_CONN;
   }
 
-  if (netconn_is_nonblocking(conn) || (apiflags & NETCONN_DONTBLOCK) || (conn->flags & NETCONN_FLAG_MBOXCLOSED)) {
+  if (netconn_is_nonblocking(conn) || (apiflags & NETCONN_DONTBLOCK) ||
+    (conn->flags & NETCONN_FLAG_MBOXCLOSED) || (conn->pending_err != ERR_OK)) {
     if (sys_arch_mbox_tryfetch(&conn->recvmbox, &buf) == SYS_ARCH_TIMEOUT) {
+      err_t err = netconn_err(conn);
+      if (err != ERR_OK) {
+        /* return pending error */
+        return err;
+      }
       if (conn->flags & NETCONN_FLAG_MBOXCLOSED) {
-        return ERR_CLSD;
+        return ERR_CONN;
       }
       return ERR_WOULDBLOCK;
     }
@@ -561,6 +566,7 @@ netconn_recv_data(struct netconn *conn, void **new_buf, u8_t apiflags)
   if (NETCONNTYPE_GROUP(conn->type) == NETCONN_TCP)
 #endif /* (LWIP_UDP || LWIP_RAW) */
   {
+    err_t err;
     /* Check if this is an error message or a pbuf */
     if (lwip_netconn_is_err_msg(buf, &err)) {
       /* new_buf has been zeroed above already */

test/unit/api/test_sockets.c

@@ -8,6 +8,7 @@
 
 #include "lwip/tcpip.h"
 #include "lwip/priv/tcp_priv.h"
+#include "lwip/api.h"
 
 
 static int
@@ -641,6 +642,149 @@ START_TEST(test_sockets_select)
 }
 END_TEST
 
+START_TEST(test_sockets_recv_after_rst)
+{
+  int sl, sact;
+  int spass = -1;
+  int ret;
+  struct sockaddr_in sa_listen;
+  const u16_t port = 1234;
+  int arg;
+  const char txbuf[] = "something";
+  char rxbuf[16];
+  struct lwip_sock *sact_sock;
+  int err;
+  LWIP_UNUSED_ARG(_i);
+
+  fail_unless(test_sockets_get_used_count() == 0);
+
+  memset(&sa_listen, 0, sizeof(sa_listen));
+  sa_listen.sin_family = AF_INET;
+  sa_listen.sin_port = PP_HTONS(port);
+  sa_listen.sin_addr.s_addr = PP_HTONL(INADDR_LOOPBACK);
+
+  /* set up the listener */
+  sl = lwip_socket(AF_INET, SOCK_STREAM, 0);
+  fail_unless(sl >= 0);
+  fail_unless(test_sockets_get_used_count() == 0);
+
+  ret = lwip_bind(sl, (struct sockaddr *)&sa_listen, sizeof(sa_listen));
+  fail_unless(ret == 0);
+  ret = lwip_listen(sl, 0);
+  fail_unless(ret == 0);
+
+  /* set up the client */
+  sact = lwip_socket(AF_INET, SOCK_STREAM, 0);
+  fail_unless(sact >= 0);
+  fail_unless(test_sockets_get_used_count() == 0);
+  /* set the client to nonblocking to simplify this test */
+  arg = 1;
+  ret = lwip_ioctl(sact, FIONBIO, &arg);
+  fail_unless(ret == 0);
+  /* connect */
+  do {
+    ret = lwip_connect(sact, (struct sockaddr *)&sa_listen, sizeof(sa_listen));
+    err = errno;
+    fail_unless((ret == 0) || (ret == -1));
+    if (ret != 0) {
+      if (err == EISCONN) {
+        /* Although this is not valid, use EISCONN as an indicator for successful connection.
+           This marks us as "connect phase is done". On error, we would either have a different
+           errno code or "send" fails later... -> good enough for this test. */
+        ret = 0;
+      } else {
+        fail_unless(err == EINPROGRESS);
+        if (err != EINPROGRESS) {
+          goto cleanup;
+        }
+        /* we're in progress: little side check: test for EALREADY */
+        ret = lwip_connect(sact, (struct sockaddr *)&sa_listen, sizeof(sa_listen));
+        err = errno;
+        fail_unless(ret == -1);
+        fail_unless(err == EALREADY);
+        if ((ret != -1) || (err != EALREADY)) {
+          goto cleanup;
+        }
+      }
+      tcpip_thread_poll_one();
+      tcpip_thread_poll_one();
+      tcpip_thread_poll_one();
+      tcpip_thread_poll_one();
+    }
+  } while (ret != 0);
+  fail_unless(ret == 0);
+
+  /* accept the server connection part */
+  spass = lwip_accept(sl, NULL, NULL);
+  fail_unless(spass >= 0);
+
+  /* write data from client */
+  ret = lwip_send(sact, txbuf, sizeof(txbuf), 0);
+  fail_unless(ret == sizeof(txbuf));
+
+  tcpip_thread_poll_one();
+  tcpip_thread_poll_one();
+
+  /* issue RST (This is a HACK, don't try this in your own app!) */
+  sact_sock = lwip_socket_dbg_get_socket(sact);
+  fail_unless(sact_sock != NULL);
+  if (sact_sock != NULL) {
+    struct netconn *sact_conn = sact_sock->conn;
+    fail_unless(sact_conn != NULL);
+    if (sact_conn != NULL) {
+      struct tcp_pcb *pcb = sact_conn->pcb.tcp;
+      fail_unless(pcb != NULL);
+      if (pcb != NULL) {
+        tcp_rst(pcb, pcb->snd_nxt, pcb->rcv_nxt, &pcb->local_ip, &pcb->remote_ip,
+                     pcb->local_port, pcb->remote_port);
+      }
+    }
+  }
+  tcpip_thread_poll_one();
+  tcpip_thread_poll_one();
+
+  /* expect to receive data first */
+  ret = lwip_recv(spass, rxbuf, sizeof(rxbuf), 0);
+  fail_unless(ret > 0);
+  tcpip_thread_poll_one();
+  tcpip_thread_poll_one();
+
+  /* expect to receive RST indication */
+  ret = lwip_recv(spass, rxbuf, sizeof(rxbuf), 0);
+  fail_unless(ret == -1);
+  err = errno;
+  fail_unless(err == ECONNRESET);
+  tcpip_thread_poll_one();
+  tcpip_thread_poll_one();
+
+  /* expect to receive ENOTCONN indication */
+  ret = lwip_recv(spass, rxbuf, sizeof(rxbuf), 0);
+  fail_unless(ret == -1);
+  err = errno;
+  fail_unless(err == ENOTCONN);
+  tcpip_thread_poll_one();
+  tcpip_thread_poll_one();
+
+  /* expect to receive ENOTCONN indication */
+  ret = lwip_recv(spass, rxbuf, sizeof(rxbuf), 0);
+  fail_unless(ret == -1);
+  err = errno;
+  fail_unless(err == ENOTCONN);
+  tcpip_thread_poll_one();
+  tcpip_thread_poll_one();
+
+cleanup:
+  ret = lwip_close(sl);
+  fail_unless(ret == 0);
+  ret = lwip_close(sact);
+  fail_unless(ret == 0);
+  if (spass >= 0) {
+    ret = lwip_close(spass);
+    fail_unless(ret == 0);
+  }
+}
+END_TEST
+
 /** Create the suite including all tests for this module */
 Suite *
 sockets_suite(void)
@@ -650,6 +794,7 @@ sockets_suite(void)
     TESTFUNC(test_sockets_allfunctions_basic),
     TESTFUNC(test_sockets_msgapis),
     TESTFUNC(test_sockets_select),
+    TESTFUNC(test_sockets_recv_after_rst),
   };
   return create_suite("SOCKETS", tests, sizeof(tests)/sizeof(testfunc), sockets_setup, sockets_teardown);
 }