Release 2.14.3

- [BUGFIX] gQUIC: pass correct stream to hsi_create_header_set() callback.
- [BUGFIX] Use ls-hpack 2.1.1
- Improve stream code readability.
- Use ls-qpack 2.0.5

CHANGELOG

@@ -1,3 +1,10 @@
+2020-04-15
+    - 2.14.3
+    - [BUGFIX] gQUIC: pass correct stream to hsi_create_header_set() callback.
+    - [BUGFIX] Use ls-hpack 2.1.1
+    - Improve stream code readability.
+    - Use ls-qpack 2.0.5
+
 2020-04-08
     - 2.14.2
     - [BUGFIX] Use ls-qpack 2.0.4

docs/apiref.rst

@@ -1477,7 +1477,8 @@ fields yourself.  In that case, the header set must be "read" from the stream vi
     .. member::  void * (*hsi_create_header_set)(void *hsi_ctx, lsquic_stream_t *stream, int is_push_promise)
 
         :param hsi_ctx: User context.  This is the pointer specifed in ``ea_hsi_ctx``.
-        :param stream: Stream with which the header set is associated.
+        :param stream: Stream with which the header set is associated.  May be set
+                       to NULL in server mode.
         :param is_push_promise: Boolean value indicating whether this header set is
                                 for a push promise.
         :return: Pointer to user-defined header set object.

docs/conf.py

@@ -26,7 +26,7 @@ author = u'LiteSpeed Technologies'
 # The short X.Y version
 version = u'2.14'
 # The full version, including alpha/beta/rc tags
-release = u'2.14.2'
+release = u'2.14.3'
 
 
 # -- General configuration ---------------------------------------------------

include/lsquic.h

@@ -25,7 +25,7 @@ extern "C" {
 
 #define LSQUIC_MAJOR_VERSION 2
 #define LSQUIC_MINOR_VERSION 14
-#define LSQUIC_PATCH_VERSION 2
+#define LSQUIC_PATCH_VERSION 3
 
 /**
  * Engine flags:
@@ -896,6 +896,8 @@ struct lsquic_hset_if
      * Create a new header set.  This object is (and must be) fetched from a
      * stream by calling @ref lsquic_stream_get_hset() before the stream can
      * be read.
+     *
+     * `stream' may be set to NULL in server mode.
      */
     void * (*hsi_create_header_set)(void *hsi_ctx, lsquic_stream_t *stream,
                                     int is_push_promise);

include/lsxpack_header.h

@@ -1,6 +1,6 @@
 /* Copyright (c) 2017 - 2020 LiteSpeed Technologies Inc.  See LICENSE. */
-#ifndef LSXPACK_HEADER_H_v204
-#define LSXPACK_HEADER_H_v204
+#ifndef LSXPACK_HEADER_H_v205
+#define LSXPACK_HEADER_H_v205
 
 #ifdef __cplusplus
 extern "C" {
@@ -175,8 +175,14 @@ static inline size_t
 lsxpack_header_get_dec_size(const lsxpack_header_t *hdr)
 {   return hdr->name_len + hdr->val_len + hdr->dec_overhead;    }
 
+static inline void
+lsxpack_header_mark_val_changed(lsxpack_header_t *hdr)
+{
+    hdr->flags = (enum lsxpack_flag)(hdr->flags &
+                ~(LSXPACK_VAL_MATCHED|LSXPACK_NAMEVAL_HASH));
+}
 #ifdef __cplusplus
 }
 #endif
 
-#endif //LSXPACK_HEADER_H_v204
+#endif //LSXPACK_HEADER_H_v205

src/liblsquic/ls-qpack

@@ -1 +1 @@
-Subproject commit 5b9001252447c26f861f4b1979c677272c7b541c
+Subproject commit 451bbc6c710e058e1a409efb7bf45beac6767030

src/liblsquic/lsquic_conn.h

@@ -156,6 +156,10 @@ struct conn_iface
     int
     (*ci_is_push_enabled) (struct lsquic_conn *);
 
+    /* Optional: only used by gQUIC frames reader */
+    struct lsquic_stream *
+    (*ci_get_stream_by_id) (struct lsquic_conn *, lsquic_stream_id_t stream_id);
+
     struct lsquic_engine *
     (*ci_get_engine) (struct lsquic_conn *);
 

src/liblsquic/lsquic_frame_reader.c

@@ -509,6 +509,21 @@ skip_headers_padding (struct lsquic_frame_reader *fr)
 }
 
 
+static struct lsquic_stream *
+find_target_stream (const struct lsquic_frame_reader *fr)
+{
+    lsquic_stream_id_t stream_id;
+    struct lsquic_conn *lconn;
+
+    stream_id = fr_get_stream_id(fr);
+    lconn = lsquic_stream_conn(fr->fr_stream);
+    if (lconn->cn_if->ci_get_stream_by_id)
+        return lconn->cn_if->ci_get_stream_by_id(lconn, stream_id);
+
+    return NULL;
+}
+
+
 static int
 decode_and_pass_payload (struct lsquic_frame_reader *fr)
 {
@@ -521,8 +536,11 @@ decode_and_pass_payload (struct lsquic_frame_reader *fr)
     void *hset = NULL;
     struct lsxpack_header *hdr = NULL;
     size_t req_space = 0;
+    lsquic_stream_t *target_stream = NULL;
 
-    hset = fr->fr_hsi_if->hsi_create_header_set(fr->fr_hsi_ctx, fr->fr_stream,
+    if (!(fr->fr_flags & FRF_SERVER))
+        target_stream = find_target_stream(fr);
+    hset = fr->fr_hsi_if->hsi_create_header_set(fr->fr_hsi_ctx, target_stream,
                             READER_PUSH_PROMISE == fr->fr_state.reader_type);
     if (!hset)
     {

src/liblsquic/lsquic_full_conn.c

@@ -1407,6 +1407,15 @@ find_stream_by_id (struct full_conn *conn, lsquic_stream_id_t stream_id)
 }
 
 
+static struct lsquic_stream *
+full_conn_ci_get_stream_by_id (struct lsquic_conn *lconn,
+                               lsquic_stream_id_t stream_id)
+{
+    struct full_conn *conn = (struct full_conn *) lconn;
+    return find_stream_by_id(conn, stream_id);
+}
+
+
 static struct lsquic_engine *
 full_conn_ci_get_engine (struct lsquic_conn *lconn)
 {
@@ -4324,6 +4333,7 @@ static const struct conn_iface full_conn_iface = {
     .ci_close                =  full_conn_ci_close,
     .ci_destroy              =  full_conn_ci_destroy,
     .ci_get_ctx              =  full_conn_ci_get_ctx,
+    .ci_get_stream_by_id     =  full_conn_ci_get_stream_by_id,
     .ci_get_engine           =  full_conn_ci_get_engine,
     .ci_get_path             =  full_conn_ci_get_path,
 #if LSQUIC_CONN_STATS

src/liblsquic/lsquic_stream.c

@@ -1276,15 +1276,7 @@ stream_consumed_bytes (struct lsquic_stream *stream)
 }
 
 
-struct read_frames_status
-{
-    int     error;
-    int     processed_frames;
-    size_t  total_nread;
-};
-
-
-static struct read_frames_status
+static ssize_t
 read_data_frames (struct lsquic_stream *stream, int do_filtering,
         size_t (*readf)(void *, const unsigned char *, size_t, int), void *ctx)
 {
@@ -1335,7 +1327,7 @@ read_data_frames (struct lsquic_stream *stream, int do_filtering,
                     if (!stream->data_in)
                     {
                         stream->data_in = lsquic_data_in_error_new();
-                        return (struct read_frames_status) { .error = 1, };
+                        return -1;
                     }
                 }
                 if (fin)
@@ -1356,11 +1348,7 @@ read_data_frames (struct lsquic_stream *stream, int do_filtering,
     if (processed_frames)
         stream_consumed_bytes(stream);
 
-    return (struct read_frames_status) {
-        .error = 0,
-        .processed_frames = processed_frames,
-        .total_nread = total_nread,
-    };
+    return total_nread;
 }
 
 
@@ -1368,8 +1356,8 @@ static ssize_t
 stream_readf (struct lsquic_stream *stream,
         size_t (*readf)(void *, const unsigned char *, size_t, int), void *ctx)
 {
-    size_t total_nread, nread;
-    int read_unc_headers;
+    size_t total_nread;
+    ssize_t nread;
 
     total_nread = 0;
 
@@ -1399,9 +1387,7 @@ stream_readf (struct lsquic_stream *stream,
     {
         if (stream->uh->uh_flags & UH_H1H)
         {
-            nread = read_uh(stream, readf, ctx);
-            read_unc_headers = nread > 0;
-            total_nread += nread;
+            total_nread += read_uh(stream, readf, ctx);
             if (stream->uh)
                 return total_nread;
         }
@@ -1418,25 +1404,22 @@ stream_readf (struct lsquic_stream *stream,
         errno = EWOULDBLOCK;
         return -1;
     }
-    else
-        read_unc_headers = 0;
 
-    const struct read_frames_status rfs
-                        = read_data_frames(stream, 1, readf, ctx);
-    if (rfs.error)
-        return -1;
-    total_nread += rfs.total_nread;
+    nread = read_data_frames(stream, 1, readf, ctx);
+    if (nread < 0)
+        return nread;
+    total_nread += (size_t) nread;
 
-    LSQ_DEBUG("%s: read %zd bytes, read offset %"PRIu64, __func__,
-                                        total_nread, stream->read_offset);
+    LSQ_DEBUG("%s: read %zd bytes, read offset %"PRIu64", reached fin: %d",
+        __func__, total_nread, stream->read_offset,
+        !!(stream->stream_flags & STREAM_FIN_REACHED));
 
-    if (rfs.processed_frames || read_unc_headers)
-    {
+    if (total_nread)
         return total_nread;
-    }
+    else if (stream->stream_flags & STREAM_FIN_REACHED)
+        return 0;
     else
     {
-        assert(0 == total_nread);
         errno = EWOULDBLOCK;
         return -1;
     }
@@ -4342,17 +4325,17 @@ static int
 hq_filter_readable (struct lsquic_stream *stream)
 {
     struct hq_filter *const filter = &stream->sm_hq_filter;
-    struct read_frames_status rfs;
+    ssize_t nread;
 
     if (filter->hqfi_flags & HQFI_FLAG_BLOCKED)
         return 0;
 
     if (!hq_filter_readable_now(stream))
     {
-        rfs = read_data_frames(stream, 0, hq_read, stream);
-        if (rfs.total_nread == 0)
+        nread = read_data_frames(stream, 0, hq_read, stream);
+        if (nread <= 0)
         {
-            if (rfs.error)
+            if (nread < 0)
             {
                 filter->hqfi_flags |= HQFI_FLAG_ERROR;
                 abort_connection(stream);   /* XXX Overkill? */

src/lshpack

@@ -1 +1 @@
-Subproject commit f209bb5e9af8dbb14c22b214519c8ea5ebaecb5d
+Subproject commit 8d4b795ff661e73e10b7d46ce06cba3b88499769

test/unittests/test_frame_reader.c

@@ -1111,6 +1111,7 @@ test_one_frt (const struct frame_reader_test *frt)
     struct lsquic_conn_public conn_pub;
     struct lsquic_stream stream;
     int s;
+    struct conn_iface my_conn_if;
 
 #if LSQUIC_CONN_STATS
     struct conn_stats conn_stats;
@@ -1120,6 +1121,8 @@ test_one_frt (const struct frame_reader_test *frt)
     memset(&stream, 0, sizeof(stream));
     memset(&lconn, 0, sizeof(lconn));
     memset(&conn_pub, 0, sizeof(conn_pub));
+    memset(&my_conn_if, 0, sizeof(my_conn_if));
+    lconn.cn_if = &my_conn_if;
     stream.conn_pub = &conn_pub;
     conn_pub.lconn = &lconn;
 

test/unittests/test_frame_rw.c

@@ -31,10 +31,9 @@
 #include "lsquic_frame_reader.h"
 #include "lsquic_headers.h"
 #include "lsquic_http1x_if.h"
-#if LSQUIC_CONN_STATS
 #include "lsquic_int_types.h"
+#include "lsquic_hash.h"
 #include "lsquic_conn.h"
-#endif
 
 
 struct lsquic_stream
@@ -59,6 +58,8 @@ lsquic_conn_id (const lsquic_conn_t *lconn)
     return &my_cid;
 }
 
+static const struct conn_iface s_if;
+static struct lsquic_conn s_conn = { .cn_if = &s_if, };
 
 #if !defined(NDEBUG) && __GNUC__
 __attribute__((weak))
@@ -66,7 +67,7 @@ __attribute__((weak))
 lsquic_conn_t *
 lsquic_stream_conn (const lsquic_stream_t *stream)
 {
-    return NULL;
+    return &s_conn;
 }
 
 

test/unittests/test_h3_framing.c

@@ -950,6 +950,216 @@ test_zero_size_frame (void)
 }
 
 
+/* Create a new stream frame.  Each stream frame has a real packet_in to
+ * back it up, just like in real code.  The contents of the packet do
+ * not matter.
+ */
+static stream_frame_t *
+new_frame_in_ext (struct test_objs *tobjs, size_t off, size_t sz, int fin,
+                                                            const void *data)
+{
+    lsquic_packet_in_t *packet_in;
+    stream_frame_t *frame;
+
+    assert(sz <= 1370);
+
+    packet_in = lsquic_mm_get_packet_in(&tobjs->eng_pub.enp_mm);
+    if (data)
+        packet_in->pi_data = (void *) data;
+    else
+    {
+        packet_in->pi_data = lsquic_mm_get_packet_in_buf(&tobjs->eng_pub.enp_mm, 1370);
+        packet_in->pi_flags |= PI_OWN_DATA;
+        memset(packet_in->pi_data, 'A', sz);
+    }
+    /* This is not how stream frame looks in the packet: we have no
+     * header.  In our test case it does not matter, as we only care
+     * about stream frame.
+     */
+    packet_in->pi_data_sz = sz;
+    packet_in->pi_refcnt = 1;
+
+    frame = lsquic_malo_get(tobjs->eng_pub.enp_mm.malo.stream_frame);
+    memset(frame, 0, sizeof(*frame));
+    frame->packet_in = packet_in;
+    frame->data_frame.df_offset = off;
+    frame->data_frame.df_size = sz;
+    frame->data_frame.df_data = &packet_in->pi_data[0];
+    frame->data_frame.df_fin  = fin;
+
+    return frame;
+}
+
+
+/* Receiving DATA frame with zero payload should result in lsquic_stream_read()
+ * returning -1.
+ */
+static void
+test_reading_zero_size_data_frame (void)
+{
+    struct test_objs tobjs;
+    struct lsquic_stream *stream;
+    struct stream_frame *frame;
+    ssize_t nr;
+    int s;
+    unsigned char buf[2];
+
+    init_test_ctl_settings(&g_ctl_settings);
+
+    stream_ctor_flags |= SCF_IETF;
+    init_test_objs(&tobjs, 0x1000, 0x2000, 1252);
+    tobjs.ctor_flags |= SCF_HTTP|SCF_IETF;
+
+    stream = new_stream(&tobjs, 0, 0x1000);
+
+    /* Fake out reading of HEADERS frame: */
+    stream->stream_flags |= STREAM_HAVE_UH;
+    stream->sm_hq_filter.hqfi_hist_buf = 1 /* CODE_HEADER */;
+    stream->sm_hq_filter.hqfi_hist_idx++;
+
+    /* One-byte DATA frame */
+    frame = new_frame_in_ext(&tobjs, 0, 3, 0, (uint8_t[3]){ 0, 1, 'a', });
+    s = lsquic_stream_frame_in(stream, frame);
+    assert(s == 0);     /* Self-check */
+
+    /* Zero-length DATA frame */
+    frame = new_frame_in_ext(&tobjs, 3, 2, 0, (uint8_t[2]){ 0, 0, });
+    s = lsquic_stream_frame_in(stream, frame);
+    assert(s == 0);     /* Self-check */
+
+    assert(stream->read_offset == 2);   /* Self-check */
+
+    /* Read 'a' */
+    nr = lsquic_stream_read(stream, buf, 1);
+    assert(nr == 1);
+    assert(buf[0] == 'a');
+
+    /* Check that read returns -1 */
+    nr = lsquic_stream_read(stream, buf, sizeof(buf));
+    assert(nr == -1);
+
+    /* DATA frame was consumed: */
+    assert(stream->read_offset == 5);
+
+    lsquic_stream_destroy(stream);
+    deinit_test_objs(&tobjs);
+
+    stream_ctor_flags &= ~SCF_IETF;
+}
+
+
+/* Receiving DATA frame with zero payload should result in lsquic_stream_read()
+ * returning -1.
+ */
+static void
+test_reading_zero_size_data_frame_scenario2 (void)
+{
+    struct test_objs tobjs;
+    struct lsquic_stream *stream;
+    struct stream_frame *frame;
+    ssize_t nr;
+    int s;
+    unsigned char buf[2];
+
+    init_test_ctl_settings(&g_ctl_settings);
+
+    stream_ctor_flags |= SCF_IETF;
+    init_test_objs(&tobjs, 0x1000, 0x2000, 1252);
+    tobjs.ctor_flags |= SCF_HTTP|SCF_IETF;
+
+    stream = new_stream(&tobjs, 0, 0x1000);
+
+    /* Fake out reading of HEADERS frame: */
+    stream->stream_flags |= STREAM_HAVE_UH;
+    stream->sm_hq_filter.hqfi_hist_buf = 1 /* CODE_HEADER */;
+    stream->sm_hq_filter.hqfi_hist_idx++;
+
+    /* Zero-length DATA frame */
+    frame = new_frame_in_ext(&tobjs, 0, 5, 0, (uint8_t[5]){ 0, 1, 'a', 0, 0, });
+    s = lsquic_stream_frame_in(stream, frame);
+    assert(s == 0);     /* Self-check */
+
+    assert(stream->read_offset == 2);   /* Self-check */
+
+    /* Read 'a' */
+    nr = lsquic_stream_read(stream, buf, 1);
+    assert(nr == 1);
+    assert(buf[0] == 'a');
+
+    /* Check that read returns -1 */
+    nr = lsquic_stream_read(stream, buf, sizeof(buf));
+    assert(nr == -1);
+
+    /* DATA frame was consumed: */
+    assert(stream->read_offset == 5);
+
+    lsquic_stream_destroy(stream);
+    deinit_test_objs(&tobjs);
+
+    stream_ctor_flags &= ~SCF_IETF;
+}
+
+
+/* Receiving DATA frame with zero payload should result in lsquic_stream_read()
+ * returning -1.
+ */
+static void
+test_reading_zero_size_data_frame_scenario3 (void)
+{
+    struct test_objs tobjs;
+    struct lsquic_stream *stream;
+    struct stream_frame *frame;
+    ssize_t nr;
+    int s;
+    unsigned char buf[2];
+
+    init_test_ctl_settings(&g_ctl_settings);
+
+    stream_ctor_flags |= SCF_IETF;
+    init_test_objs(&tobjs, 0x1000, 0x2000, 1252);
+    tobjs.ctor_flags |= SCF_HTTP|SCF_IETF;
+
+    stream = new_stream(&tobjs, 0, 0x1000);
+
+    /* Fake out reading of HEADERS frame: */
+    stream->stream_flags |= STREAM_HAVE_UH;
+    stream->sm_hq_filter.hqfi_hist_buf = 1 /* CODE_HEADER */;
+    stream->sm_hq_filter.hqfi_hist_idx++;
+
+    /* Zero-length DATA frame */
+    frame = new_frame_in_ext(&tobjs, 0, 4, 0, (uint8_t[4]){ 0, 1, 'a', 0, });
+    s = lsquic_stream_frame_in(stream, frame);
+    assert(s == 0);     /* Self-check */
+
+    assert(stream->read_offset == 2);   /* Self-check */
+
+    /* Read 'a' */
+    nr = lsquic_stream_read(stream, buf, 1);
+    assert(nr == 1);
+    assert(buf[0] == 'a');
+
+    /* Check that read returns -1 */
+    nr = lsquic_stream_read(stream, buf, sizeof(buf));
+    assert(nr == -1);
+
+    /* Zero-length DATA frame */
+    frame = new_frame_in_ext(&tobjs, 4, 1, 0, (uint8_t[1]){ 0, });
+    s = lsquic_stream_frame_in(stream, frame);
+    assert(s == 0);     /* Self-check */
+
+    /* Check that read returns -1 */
+    nr = lsquic_stream_read(stream, buf, sizeof(buf));
+    assert(nr == -1);
+
+    /* DATA frame was consumed: */
+    assert(stream->read_offset == 5);
+
+    lsquic_stream_destroy(stream);
+    deinit_test_objs(&tobjs);
+
+    stream_ctor_flags &= ~SCF_IETF;
+}
+
 
 int
 main (int argc, char **argv)
@@ -988,6 +1198,9 @@ main (int argc, char **argv)
                 for (add_one_more = 0; add_one_more <= 1; ++add_one_more)
                     test_frame_header_split(n_packets, extra_sz, add_one_more);
         test_zero_size_frame();
+        test_reading_zero_size_data_frame();
+        test_reading_zero_size_data_frame_scenario2();
+        test_reading_zero_size_data_frame_scenario3();
     }
 
     return 0;