1.12.3: [BUGFIX] Fix duplicate STREAM frame detection

CHANGELOG

@@ -1,3 +1,8 @@
+2018-08-22
+
+    - 1.12.3
+    - [BUGFIX] Fix duplicate STREAM frame detection
+
 2018-08-20
 
     - 1.12.2

include/lsquic.h

@@ -25,7 +25,7 @@ extern "C" {
 
 #define LSQUIC_MAJOR_VERSION 1
 #define LSQUIC_MINOR_VERSION 12
-#define LSQUIC_PATCH_VERSION 2
+#define LSQUIC_PATCH_VERSION 3
 
 /**
  * Engine flags:

src/liblsquic/lsquic_di_nocopy.c

@@ -196,6 +196,11 @@ frame_list_is_sane (const struct nocopy_data_in *ncdi)
 
 #define CASE(letter) ((int) (letter) << 8)
 
+/* Not all errors are picked up by this function, as it is expensive (and
+ * potentially error-prone) to check for all possible error conditions.
+ * It an error be misclassified as an overlap or dup, in the worst case
+ * we end up with an application error instead of protocol violation.
+ */
 static int
 insert_frame (struct nocopy_data_in *ncdi, struct stream_frame *new_frame,
                                     uint64_t read_offset, unsigned *p_n_frames)
@@ -217,6 +222,13 @@ insert_frame (struct nocopy_data_in *ncdi, struct stream_frame *new_frame,
             return INS_FRAME_ERR                                | CASE('C');
         if (DF_END(new_frame) > ncdi->ncdi_fin_off)
             return INS_FRAME_ERR                                | CASE('D');
+        if (read_offset == DF_END(new_frame))
+            return INS_FRAME_DUP                                | CASE('M');
+    }
+    else
+    {
+        if (read_offset == DF_END(new_frame) && !DF_FIN(new_frame))
+            return INS_FRAME_DUP                                | CASE('L');
     }
 
     /* Find position in the list, going backwards.  We go backwards because
@@ -401,9 +413,17 @@ nocopy_di_get_frame (struct data_in *data_in, uint64_t read_offset)
     struct stream_frame *frame = TAILQ_FIRST(&ncdi->ncdi_frames_in);
     if (frame && frame->data_frame.df_offset +
                                 frame->data_frame.df_read_off == read_offset)
+    {
+        LSQ_DEBUG("get_frame: frame (off: %"PRIu64", size: %u, fin: %d), at "
+            "read offset %"PRIu64, DF_OFF(frame), DF_SIZE(frame), DF_FIN(frame),
+            read_offset);
         return &frame->data_frame;
+    }
     else
+    {
+        LSQ_DEBUG("get_frame: no frame at read offset %"PRIu64, read_offset);
         return NULL;
+    }
 }
 
 
@@ -424,6 +444,8 @@ nocopy_di_frame_done (struct data_in *data_in, struct data_frame *data_frame)
         ncdi->ncdi_flags |= NCDI_FIN_REACHED;
         LSQ_DEBUG("FIN has been reached at offset %"PRIu64, DF_END(frame));
     }
+    LSQ_DEBUG("frame (off: %"PRIu64", size: %u, fin: %d) done",
+                                DF_OFF(frame), DF_SIZE(frame), DF_FIN(frame));
     lsquic_packet_in_put(ncdi->ncdi_conn_pub->mm, frame->packet_in);
     lsquic_malo_put(frame);
 }

test/unittests/CMakeLists.txt

@@ -221,6 +221,10 @@ add_executable(test_cubic test_cubic.c)
 target_link_libraries(test_cubic lsquic pthread libssl.a libcrypto.a m ${LIBS})
 add_test(cubic test_cubic)
 
+add_executable(test_di_nocopy test_di_nocopy.c)
+target_link_libraries(test_di_nocopy lsquic pthread libssl.a libcrypto.a m ${LIBS})
+add_test(di_nocopy test_di_nocopy)
+
 add_executable(test_dec test_dec.c)
 target_link_libraries(test_dec libssl.a libcrypto.a z m pthread ${LIBS})
 
@@ -437,6 +441,10 @@ add_executable(test_cubic test_cubic.c ../../wincompat/getopt.c ../../wincompat/
 target_link_libraries(test_cubic lsquic ${LIBS_LIST})
 add_test(cubic test_cubic)
 
+add_executable(test_di_nocopy test_di_nocopy.c)
+target_link_libraries(test_di_nocopy lsquic pthread libssl.a libcrypto.a m ${LIBS})
+add_test(di_nocopy test_di_nocopy)
+
 add_executable(test_dec test_dec.c ../../wincompat/getopt.c ../../wincompat/getopt1.c)
 target_link_libraries(test_dec ${LIBS_LIST})