Release 2.8.8

- [BUGFIX] Invalid read when parsing IETF transport parameters
  (this was benign).
- [OPTIMIZATION] Frame bundling when using buffered packets in
  IETF QUIC: a) flush QPACK decoder stream and b) include ACKs
  in opportunistic fashion.
- Fix HTTP/3 framing unit test.
- Code cleanup.

CHANGELOG

@@ -1,3 +1,13 @@
+2020-01-14
+    - 2.8.8
+    - [BUGFIX] Invalid read when parsing IETF transport parameters
+      (this was benign).
+    - [OPTIMIZATION] Frame bundling when using buffered packets in
+      IETF QUIC: a) flush QPACK decoder stream and b) include ACKs
+      in opportunistic fashion.
+    - Fix HTTP/3 framing unit test.
+    - Code cleanup.
+
 2020-01-09
     - 2.8.7
     - [BUGFIX] Initial packet size check for IETF mini conn applies to

include/lsquic.h

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

src/liblsquic/lsquic_frame_writer.c

@@ -35,6 +35,11 @@
                                         lsquic_stream_conn(fw->fw_stream))
 #include "lsquic_logger.h"
 
+/* Size of the buffer passed to lshpack_enc_encode() -- this limits the size
+ * of a single compressed header field.
+ */
+#define MAX_COMP_HEADER_FIELD_SIZE (64 * 1024)
+
 
 struct lsquic_frame_writer
 {
@@ -457,10 +462,10 @@ lsquic_frame_writer_write_headers (struct lsquic_frame_writer *fw,
             return s;
     }
 
-    buf = malloc(MAX_HEADERS_SIZE);
+    buf = malloc(MAX_COMP_HEADER_FIELD_SIZE);
     if (!buf)
         return -1;
-    s = write_headers(fw, headers, &hfc, buf, MAX_HEADERS_SIZE);
+    s = write_headers(fw, headers, &hfc, buf, MAX_COMP_HEADER_FIELD_SIZE);
     free(buf);
     if (0 == s)
     {
@@ -529,11 +534,11 @@ lsquic_frame_writer_write_promise (struct lsquic_frame_writer *fw,
     if (s < 0)
         return s;
 
-    buf = malloc(MAX_HEADERS_SIZE);
+    buf = malloc(MAX_COMP_HEADER_FIELD_SIZE);
     if (!buf)
         return -1;
 
-    s = write_headers(fw, &mpas, &hfc, buf, MAX_HEADERS_SIZE);
+    s = write_headers(fw, &mpas, &hfc, buf, MAX_COMP_HEADER_FIELD_SIZE);
     if (s != 0)
     {
         free(buf);
@@ -541,7 +546,7 @@ lsquic_frame_writer_write_promise (struct lsquic_frame_writer *fw,
     }
 
     if (extra_headers)
-        s = write_headers(fw, extra_headers, &hfc, buf, MAX_HEADERS_SIZE);
+        s = write_headers(fw, extra_headers, &hfc, buf, MAX_COMP_HEADER_FIELD_SIZE);
 
     free(buf);
 

src/liblsquic/lsquic_frame_writer.h

@@ -9,9 +9,6 @@
 #include <stddef.h>
 #include <stdint.h>
 
-/* Same as H2_TMP_HDR_BUFF_SIZE */
-#define MAX_HEADERS_SIZE (64 * 1024)
-
 struct iovec;
 struct lshpack_enc;
 struct lsquic_mm;

src/liblsquic/lsquic_full_conn_ietf.c

@@ -1338,7 +1338,13 @@ static int
 ietf_full_conn_ci_can_write_ack (struct lsquic_conn *lconn)
 {
     struct ietf_full_conn *conn = (struct ietf_full_conn *) lconn;
-    return should_generate_ack(conn, IFC_ACK_QUED_APP);
+
+    /* Follow opportunistic ACK logic.  Because this method is only used by
+     * buffered packets code path, no need to check whether anything is
+     * writing: we know it is.
+     */
+    return conn->ifc_n_slack_akbl[PNS_APP] > 0
+        && lsquic_send_ctl_can_send(&conn->ifc_send_ctl);
 }
 
 

src/liblsquic/lsquic_packet_ietf.h

@@ -23,4 +23,9 @@
 /* [draft-ietf-quic-transport-24] Section 8.1 */
 #define IQUIC_MIN_INIT_PACKET_SZ 1200
 
+/* Our stream code makes an assumption that packet size is smaller than the
+ * maximum HTTP/3 DATA frame size we can generate.
+ */
+#define IQUIC_MAX_OUT_PACKET_SZ ((1u << 14) - 1)
+
 #endif

src/liblsquic/lsquic_qdec_hdl.c

@@ -139,7 +139,7 @@ lsquic_qdh_init (struct qpack_dec_hdl *qdh, struct lsquic_conn *conn,
     {
         qdh->qdh_h1x_ctor_ctx = (struct http1x_ctor_ctx) {
             .conn           = conn,
-            .max_headers_sz = 0x10000,  /* XXX */
+            .max_headers_sz = MAX_HTTP1X_HEADERS_SIZE,
             .is_server      = is_server,
         };
         qdh->qdh_hsi_ctx = &qdh->qdh_h1x_ctor_ctx;

src/liblsquic/lsquic_send_ctl.c

@@ -45,6 +45,10 @@
 #include "lsquic_hash.h"
 #include "lsquic_malo.h"
 #include "lsquic_attq.h"
+#include "lsquic_http1x_if.h"
+#include "lsqpack.h"
+#include "lsquic_frab_list.h"
+#include "lsquic_qdec_hdl.h"
 
 #define LSQUIC_LOGGER_MODULE LSQLM_SENDCTL
 #define LSQUIC_LOG_CONN_ID lsquic_conn_log_cid(ctl->sc_conn_pub->lconn)
@@ -2435,6 +2439,25 @@ send_ctl_get_buffered_packet (lsquic_send_ctl_t *ctl,
 }
 
 
+static void
+send_ctl_maybe_flush_decoder (struct lsquic_send_ctl *ctl,
+                                        const struct lsquic_stream *caller)
+{
+    struct lsquic_stream *decoder;
+
+    if ((ctl->sc_flags & SC_IETF) && ctl->sc_conn_pub->u.ietf.qdh)
+    {
+        decoder = ctl->sc_conn_pub->u.ietf.qdh->qdh_dec_sm_out;
+        if (decoder && decoder != caller
+                                && lsquic_stream_has_data_to_flush(decoder))
+        {
+            LSQ_DEBUG("flushing decoder stream");
+            lsquic_stream_flush(decoder);
+        }
+    }
+}
+
+
 lsquic_packet_out_t *
 lsquic_send_ctl_get_packet_for_stream (lsquic_send_ctl_t *ctl,
                 unsigned need_at_least, const struct network_path *path,
@@ -2447,6 +2470,8 @@ lsquic_send_ctl_get_packet_for_stream (lsquic_send_ctl_t *ctl,
                                                 need_at_least, path, 0, NULL);
     else
     {
+        if (!lsquic_send_ctl_has_buffered(ctl))
+            send_ctl_maybe_flush_decoder(ctl, stream);
         packet_type = send_ctl_lookup_bpt(ctl, stream);
         return send_ctl_get_buffered_packet(ctl, packet_type, need_at_least,
                                             path, stream);

src/liblsquic/lsquic_stream.c

@@ -3418,7 +3418,7 @@ stream_write_buf (struct lsquic_stream *stream, const void *buf, size_t sz)
 }
 
 
-/* XXX Move this define elsewhere? */
+/* This limits the cumulative size of the compressed header fields */
 #define MAX_HEADERS_SIZE (64 * 1024)
 
 static int

src/liblsquic/lsquic_trans_params.c

@@ -329,7 +329,7 @@ lsquic_tp_decode (const unsigned char *const buf, size_t bufsz,
 } while (0)
 
     set_of_ids = 0;
-    while (p < end)
+    while (p + 4 <= end)
     {
         READ_UINT(param_id, 16, p, 2);
         p += 2;

test/unittests/CMakeLists.txt

@@ -49,7 +49,6 @@ SET(TESTS
     lsquic_hash
     packet_out
     packno_len
-    parse
     parse_packet_in
     purga
     qlog

test/unittests/test_h3_framing.c

@@ -589,7 +589,11 @@ test_hq_framing (int sched_immed, int dispatch_once, unsigned wsize,
             frame_type = *src++;
             s = vint_read(src, buf_out + buf_out_sz, &sz);
             assert(s > 0);
-            assert(sz > 0);
+            /* In some rare circumstances it is possible to produce zero-length
+             * DATA frames:
+             *
+             * assert(sz > 0);
+             */
             assert(sz < (1 << 14));
             src += s;
             if (src == buf_out + s + 1)
@@ -646,6 +650,77 @@ main_test_hq_framing (void)
 }
 
 
+/* Instead of the not-very-random testing done in main_test_hq_framing(),
+ * the fuzz-guided testing initializes parameters based on the fuzz input
+ * file.  This allows afl-fuzz explore the code paths.
+ */
+void
+fuzz_guided_testing (const char *input)
+{
+                                /* Range */                 /* Bytes from file */
+    unsigned short packet_sz;   /* [200, 0x3FFF] */         /* 2 */
+    unsigned wsize;             /* [1, 20000] */            /* 2 */
+    unsigned n_packets;         /* [1, 255] and UINT_MAX */ /* 1 */
+    size_t conn_limit;          /* [1, 33K] */              /* 2 */
+    int sched_immed;            /* 0 or 1 */                /* 1 */
+    int dispatch_once;          /* 0 or 1 */                /* 0 (same as above) */
+    int flush_after_each_write; /* 0 or 1 */                /* 0 (same as above) */
+                                                     /* TOTAL: 8 bytes */
+
+    FILE *f;
+    size_t nread;
+    uint16_t tmp;
+    unsigned char buf[9];
+
+    f = fopen(input, "rb");
+    if (!f)
+    {
+        assert(0);
+        return;
+    }
+
+    nread = fread(buf, 1, sizeof(buf), f);
+    if (nread != 8)
+        goto cleanup;
+
+    memcpy(&tmp, &buf[0], 2);
+    if (tmp < 200)
+        tmp = 200;
+    else if (tmp > IQUIC_MAX_OUT_PACKET_SZ)
+        tmp = IQUIC_MAX_OUT_PACKET_SZ;
+    packet_sz = tmp;
+
+    memcpy(&tmp, &buf[2], 2);
+    if (tmp < 1)
+        tmp = 1;
+    else if (tmp > 20000)
+        tmp = 20000;
+    wsize = tmp;
+
+    if (buf[4])
+        n_packets = buf[4];
+    else
+        n_packets = UINT_MAX;
+
+    memcpy(&tmp, &buf[5], 2);
+    if (tmp < 1)
+        tmp = 1;
+    else if (tmp > 33 * 1024)
+        tmp = 33 * 1024;
+    conn_limit = tmp;
+
+    sched_immed             = !!(buf[7] & 1);
+    dispatch_once           = !!(buf[7] & 2);
+    flush_after_each_write  = !!(buf[7] & 4);
+
+    test_hq_framing(sched_immed, dispatch_once, wsize,
+        flush_after_each_write, conn_limit, n_packets, packet_sz);
+
+  cleanup:
+    (void) fclose(f);
+}
+
+
 static void
 test_frame_header_split (unsigned n_packets)
 {
@@ -877,14 +952,18 @@ test_zero_size_frame (void)
 int
 main (int argc, char **argv)
 {
+    const char *fuzz_input = NULL;
     int opt;
 
     lsquic_global_init(LSQUIC_GLOBAL_SERVER);
 
-    while (-1 != (opt = getopt(argc, argv, "l:")))
+    while (-1 != (opt = getopt(argc, argv, "f:l:")))
     {
         switch (opt)
         {
+        case 'f':
+            fuzz_input = optarg;
+            break;
         case 'l':
             lsquic_log_to_fstream(stderr, 0);
             lsquic_logger_lopt(optarg);
@@ -896,10 +975,15 @@ main (int argc, char **argv)
 
     init_test_ctl_settings(&g_ctl_settings);
 
+    if (fuzz_input)
+        fuzz_guided_testing(fuzz_input);
+    else
+    {
         main_test_hq_framing();
         test_frame_header_split(1);
         test_frame_header_split(2);
         test_zero_size_frame();
+    }
 
     return 0;
 }

test/unittests/test_parse.c

@@ -1,6 +0,0 @@
-/* Copyright (c) 2017 - 2020 LiteSpeed Technologies Inc.  See LICENSE. */
-int
-main (void)
-{
-    return 0;
-}