Another fix for bug #28241 (ooseq processing) and adapted corresponding unit test

diff --git a/CHANGELOG b/CHANGELOG
index 860d1c470e8799402f2b6b368d2785609b89aa46..650a71fb9146c3e01b2d9270eccca157e75b9927 100644 (file)
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -22,6 +22,10 @@ HISTORY
 
   ++ Bugfixes:
 
+  2009-12-27: Simon Goldschmidt
+  * tcp_in.c: Another fix for bug #28241 (ooseq processing) and adapted
+    unit test
+
 
 (STABLE-1.3.2)
 

diff --git a/src/core/tcp_in.c b/src/core/tcp_in.c
index 51e8775901059eb0323d39088fa9cbf5302ec992..54e19d1e4c7fc730295ab6af5c4b2a55e1154010 100644 (file)
--- a/src/core/tcp_in.c
+++ b/src/core/tcp_in.c
@@ -1167,11 +1167,7 @@ tcp_receive(struct tcp_pcb *pcb)
                         ("tcp_receive: received in-order FIN, binning ooseq queue\n"));
             /* Received in-order FIN means anything that was received
              * out of order must now have been received in-order, so
-             * bin the ooseq queue
-             * rcv_nxt
-             * .    |--ooseq--|
-             * .==seg============|FIN
-             */
+             * bin the ooseq queue */
             while (pcb->ooseq != NULL) {
               struct tcp_seg *old_ooseq = pcb->ooseq;
               pcb->ooseq = pcb->ooseq->next;
@@ -1179,42 +1175,37 @@ tcp_receive(struct tcp_pcb *pcb)
             }               
           } 
           else {
-            struct tcp_seg* next = pcb->ooseq;
-            struct tcp_seg *old_seg;
-            /* rcv_nxt
-             * .    |--ooseq--|
-             * .==seg============|
-             */
+            next = pcb->ooseq;
+            /* Remove all segments on ooseq that are covered by inseg already.
+             * FIN is copied from ooseq to inseg if present. */
             while (next &&
                    TCP_SEQ_GEQ(seqno + tcplen,
                                next->tcphdr->seqno + next->len)) {
-              /* inseg doesn't have FIN (already processed) */
+              /* inseg cannot have FIN here (already processed above) */
               if (TCPH_FLAGS(next->tcphdr) & TCP_FIN &&
                   (TCPH_FLAGS(inseg.tcphdr) & TCP_SYN) == 0) {
                 TCPH_FLAGS_SET(inseg.tcphdr, 
                                TCPH_FLAGS(inseg.tcphdr) | TCP_FIN);
                 tcplen = TCP_TCPLEN(&inseg);
               }
-              old_seg = next;
+              prev = next;
               next = next->next;
-              tcp_seg_free(old_seg);
+              tcp_seg_free(prev);
             }
-            /* rcv_nxt
-             * .             |--ooseq--|
-             * .==seg============|
-             */
+            /* Now trim right side of inseg if it overlaps with the first
+             * segment on ooseq */
             if (next &&
                 TCP_SEQ_GT(seqno + tcplen,
                            next->tcphdr->seqno)) {
-              /* FIN in inseg already handled by dropping whole ooseq queue */
-              inseg.len = (u16_t)(pcb->ooseq->tcphdr->seqno - seqno);
+              /* inseg cannot have FIN here (already processed above) */
+              inseg.len = (u16_t)(next->tcphdr->seqno - seqno);
               if (TCPH_FLAGS(inseg.tcphdr) & TCP_SYN) {
                 inseg.len -= 1;
               }
               pbuf_realloc(inseg.p, inseg.len);
               tcplen = TCP_TCPLEN(&inseg);
               LWIP_ASSERT("tcp_receive: segment not trimmed correctly to ooseq queue\n",
-                          (seqno + tcplen) == pcb->ooseq->tcphdr->seqno);
+                          (seqno + tcplen) == next->tcphdr->seqno);
             }
             pcb->ooseq = next;
           }

diff --git a/test/unit/tcp/test_tcp_oos.c b/test/unit/tcp/test_tcp_oos.c
index 8e4c7f4d0e703833bd130864d2b7b13772f67945..29fb6b96cf04e59295b13cc7b0ed7abaf11ff7d8 100644 (file)
--- a/test/unit/tcp/test_tcp_oos.c
+++ b/test/unit/tcp/test_tcp_oos.c
@@ -208,11 +208,9 @@ START_TEST(test_tcp_recv_ooseq_FIN_OOSEQ)
     EXPECT(counters.recved_bytes == 0);
     EXPECT(counters.err_calls == 0);
     /* check ooseq queue */
-    EXPECT_OOSEQ(tcp_oos_count(pcb) == 2);
+    EXPECT_OOSEQ(tcp_oos_count(pcb) == 1);
     EXPECT_OOSEQ(tcp_oos_seg_seqno(pcb, 0) == 2);
-    EXPECT_OOSEQ(tcp_oos_seg_tcplen(pcb, 0) == 6);
-    EXPECT_OOSEQ(tcp_oos_seg_seqno(pcb, 1) == 8);
-    EXPECT_OOSEQ(tcp_oos_seg_tcplen(pcb, 1) == 9); /* includes FIN */
+    EXPECT_OOSEQ(tcp_oos_seg_tcplen(pcb, 0) == 15); /* includes FIN */
 
     /* pass the segment to tcp_input */
     tcp_input(p_fin, &netif);
@@ -222,11 +220,9 @@ START_TEST(test_tcp_recv_ooseq_FIN_OOSEQ)
     EXPECT(counters.recved_bytes == 0);
     EXPECT(counters.err_calls == 0);
     /* ooseq queue: unchanged */
-    EXPECT_OOSEQ(tcp_oos_count(pcb) == 2);
+    EXPECT_OOSEQ(tcp_oos_count(pcb) == 1);
     EXPECT_OOSEQ(tcp_oos_seg_seqno(pcb, 0) == 2);
-    EXPECT_OOSEQ(tcp_oos_seg_tcplen(pcb, 0) == 6);
-    EXPECT_OOSEQ(tcp_oos_seg_seqno(pcb, 1) == 8);
-    EXPECT_OOSEQ(tcp_oos_seg_tcplen(pcb, 1) == 9); /* includes FIN */
+    EXPECT_OOSEQ(tcp_oos_seg_tcplen(pcb, 0) == 15); /* includes FIN */
 
     /* pass the segment to tcp_input */
     tcp_input(pinseq, &netif);
@@ -363,13 +359,11 @@ START_TEST(test_tcp_recv_ooseq_FIN_INSEQ)
     EXPECT(counters.recved_bytes == 0);
     EXPECT(counters.err_calls == 0);
     /* check ooseq queue */
-    EXPECT_OOSEQ(tcp_oos_count(pcb) == 3);
+    EXPECT_OOSEQ(tcp_oos_count(pcb) == 2);
     EXPECT_OOSEQ(tcp_oos_seg_seqno(pcb, 0) == 1);
     EXPECT_OOSEQ(tcp_oos_seg_tcplen(pcb, 0) == 1);
     EXPECT_OOSEQ(tcp_oos_seg_seqno(pcb, 1) == 2);
-    EXPECT_OOSEQ(tcp_oos_seg_tcplen(pcb, 1) == 1);
-    EXPECT_OOSEQ(tcp_oos_seg_seqno(pcb, 2) == 3);
-    EXPECT_OOSEQ(tcp_oos_seg_tcplen(pcb, 2) == 11);
+    EXPECT_OOSEQ(tcp_oos_seg_tcplen(pcb, 1) == 12);
 
     /* pass the segment to tcp_input */
     tcp_input(pinseq, &netif);