pseudotcp: Fix retransmission of segments before handling a FIN

Previously, if peer A transmitted one or more data segments (1),
followed by a FIN segment (2) to peer B, and segments 1 were
dropped, peer B would not request retransmission of them and would
instead continue with the FIN handshake. This effectively meant
segments 1 were lost without peer B realising.

Fix this by only handling the FIN segment once its sequence number is
acknowledged in the receive window.

1 files changed, 86 insertions, 2 deletions

diff --git a/tests/test-pseudotcp-fin.c b/tests/test-pseudotcp-fin.c
index b769161..96e6d30 100644
--- a/tests/test-pseudotcp-fin.c
+++ b/tests/test-pseudotcp-fin.c
@@ -1,7 +1,7 @@
 /*
  * This file is part of the Nice GLib ICE library.
  *
- * (C) 2014 Collabora Ltd.
+ * © 2014, 2015 Collabora Ltd.
  *  Contact: Philip Withnall
  *
  * The contents of this file are subject to the Mozilla Public License Version
@@ -269,6 +269,13 @@ expect_syn_received (Data *data)
   expect_segment (data->right, data->right_sent, 0, 7, 7, FLAG_SYN);
 }
 
+static void
+assert_empty_queues (Data *data)
+{
+  g_assert_cmpuint (g_queue_get_length (data->left_sent), ==, 0);
+  g_assert_cmpuint (g_queue_get_length (data->right_sent), ==, 0);
+}
+
 /* Return whether the socket accepted the packet. */
 static gboolean
 forward_segment (GQueue/*<owned GBytes>*/ *from, PseudoTcpSocket *to)
@@ -453,6 +460,8 @@ establish_connection (Data *data)
   expect_ack (data->left,  data->left_sent, 7, 7);
   forward_segment_ltr (data);
   expect_sockets_connected (data);
+
+  assert_empty_queues (data);
 }
 
 /* Helper to close the LHS of a socket pair which has not transmitted any
@@ -753,6 +762,76 @@ pseudotcp_close_normal_recovery4 (void)
   data_clear (&data);
 }
 
+/* Check that closing a connection recovers from a data segment being dropped
+ * immediately before the first FIN is sent. Based on: RFC 793, Figure 13. */
+static void
+pseudotcp_close_normal_recovery_data (void)
+{
+  Data data = { 0, };
+
+  /* Establish a connection. */
+  establish_connection (&data);
+
+  /* Send some data from LHS to RHS, but drop the segment. */
+  g_assert_cmpint (pseudo_tcp_socket_send (data.left, "foo", 3), ==, 3);
+  expect_data (data.left, data.left_sent, 7, 7, 3);
+  drop_segment (data.left, data.left_sent);
+
+  assert_empty_queues(&data);
+
+  /* Close the LHS. */
+  g_assert_cmpint (pseudo_tcp_socket_get_available_bytes (data.left), ==, 0);
+  g_assert_cmpint (pseudo_tcp_socket_get_available_bytes (data.right), ==, 0);
+  close_socket (data.left);
+
+  expect_socket_state (data.left, TCP_FIN_WAIT_1);
+  expect_fin (data.left, data.left_sent, 10, 7);
+  forward_segment_ltr (&data);
+
+  expect_socket_state (data.right, TCP_ESTABLISHED);
+  expect_ack (data.right, data.right_sent, 7, 7);
+  forward_segment_rtl (&data);
+
+  expect_socket_state (data.left, TCP_FIN_WAIT_1);
+
+  assert_empty_queues(&data);
+
+  /* Close the RHS. */
+  close_socket (data.right);
+
+  expect_socket_state (data.right, TCP_FIN_WAIT_1);
+
+  expect_fin (data.right, data.right_sent, 7, 7);
+  forward_segment_rtl (&data);
+
+  expect_socket_state (data.left, TCP_CLOSING);
+
+  expect_ack (data.left, data.left_sent, 11, 8);
+  forward_segment_ltr (&data);
+
+  expect_socket_state (data.right, TCP_FIN_WAIT_2);
+
+  expect_data (data.right, data.right_sent, 8, 7, 0);
+  forward_segment_rtl (&data);
+  expect_socket_state (data.left, TCP_CLOSING);
+
+  expect_data (data.left, data.left_sent, 7, 8, 3);
+  forward_segment_ltr (&data);
+  expect_socket_state (data.right, TCP_TIME_WAIT);
+
+  increment_time_both (&data, 100);  /* Delayed ACK */
+
+  expect_ack (data.right, data.right_sent, 8, 11);
+  forward_segment_rtl (&data);
+  expect_socket_state (data.left, TCP_TIME_WAIT);
+
+  increment_time_both (&data, 10);  /* TIME-WAIT */
+
+  expect_sockets_closed (&data);
+
+  data_clear (&data);
+}
+
 /* Check that if both FIN segments from a simultaneous FIN handshake are
  * dropped, the handshake recovers and completes successfully.
  * See: RFC 793, Figure 14. */
@@ -977,11 +1056,14 @@ pseudotcp_close_rst_afterwards (void)
 
   /* Close the LHS. */
   g_assert_cmpint (pseudo_tcp_socket_get_available_bytes (data.left), ==, 0);
+  pseudo_tcp_socket_close (data.left, TRUE);
   close_socket (data.left);
 
-  expect_fin (data.left, data.left_sent, 7, 7);
+  expect_rst (data.left, data.left_sent, 7, 7);
   drop_segment (data.left, data.left_sent);  /* just to get it out of the way */
 
+  assert_empty_queues(&data);
+
   /* Send some data from RHS to LHS, which should result in an RST. */
   g_assert_cmpint (pseudo_tcp_socket_send (data.right, "foo", 3), ==, 3);
   expect_data (data.right, data.right_sent, 7, 7, 3);
@@ -1109,6 +1191,8 @@ main (int argc, char *argv[])
       pseudotcp_close_normal_recovery3);
   g_test_add_func ("/pseudotcp/close/normal/recovery4",
       pseudotcp_close_normal_recovery4);
+  g_test_add_func ("/pseudotcp/close/normal/recovery-data",
+      pseudotcp_close_normal_recovery_data);
   g_test_add_func ("/pseudotcp/close/simultaneous/recovery1",
       pseudotcp_close_simultaneous_recovery1);
   g_test_add_func ("/pseudotcp/close/simultaneous/recovery2",