From 1c105cf288ee99de3bf119708981bcd6712052bf Mon Sep 17 00:00:00 2001 From: Dmitri Tikhonov Date: Fri, 24 Apr 2020 09:44:39 -0400 Subject: [PATCH] Release 2.14.4 - [BUGFIX] Heed es_rw_once for pushed HTTP/3 streams. - [BUGFIX] IETF client: set correct flags on bidirectional streams. - [BUGFIX] Generate Cancel Stream QPACK instructions for abandoned streams. - [BUGFIX] Do not call header callbacks after stream is closed. - Use ls-qpack 2.1.1 --- CHANGELOG | 9 ++++ docs/conf.py | 2 +- include/lsquic.h | 2 +- src/liblsquic/ls-qpack | 2 +- src/liblsquic/lsquic_full_conn_ietf.c | 17 +++++-- src/liblsquic/lsquic_qdec_hdl.c | 36 +++++++++++++- src/liblsquic/lsquic_qdec_hdl.h | 6 +-- src/liblsquic/lsquic_stream.c | 71 ++++++++++++++++++--------- test/unittests/test_h3_framing.c | 7 +++ 9 files changed, 119 insertions(+), 33 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index a3360cd79..913506e92 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,12 @@ +2020-04-24 + - 2.14.4 + - [BUGFIX] Heed es_rw_once for pushed HTTP/3 streams. + - [BUGFIX] IETF client: set correct flags on bidirectional streams. + - [BUGFIX] Generate Cancel Stream QPACK instructions for abandoned + streams. + - [BUGFIX] Do not call header callbacks after stream is closed. + - Use ls-qpack 2.1.1 + 2020-04-15 - 2.14.3 - [BUGFIX] gQUIC: pass correct stream to hsi_create_header_set() callback. diff --git a/docs/conf.py b/docs/conf.py index 34b160243..2df55c953 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -26,7 +26,7 @@ # The short X.Y version version = u'2.14' # The full version, including alpha/beta/rc tags -release = u'2.14.3' +release = u'2.14.4' # -- General configuration --------------------------------------------------- diff --git a/include/lsquic.h b/include/lsquic.h index 5f8ae8d2f..12ae0cefb 100644 --- a/include/lsquic.h +++ b/include/lsquic.h @@ -25,7 +25,7 @@ extern "C" { #define LSQUIC_MAJOR_VERSION 2 #define LSQUIC_MINOR_VERSION 14 -#define LSQUIC_PATCH_VERSION 3 +#define LSQUIC_PATCH_VERSION 4 /** * Engine flags: diff --git a/src/liblsquic/ls-qpack b/src/liblsquic/ls-qpack index 451bbc6c7..f41613843 160000 --- a/src/liblsquic/ls-qpack +++ b/src/liblsquic/ls-qpack @@ -1 +1 @@ -Subproject commit 451bbc6c710e058e1a409efb7bf45beac6767030 +Subproject commit f41613843f6164bb83993e435e8a164c1bb157e2 diff --git a/src/liblsquic/lsquic_full_conn_ietf.c b/src/liblsquic/lsquic_full_conn_ietf.c index ccb405870..ad5aa6068 100644 --- a/src/liblsquic/lsquic_full_conn_ietf.c +++ b/src/liblsquic/lsquic_full_conn_ietf.c @@ -896,14 +896,20 @@ create_bidi_stream_out (struct ietf_full_conn *conn) { struct lsquic_stream *stream; lsquic_stream_id_t stream_id; + enum stream_ctor_flags flags; + + flags = SCF_IETF|SCF_DI_AUTOSWITCH; + if (conn->ifc_enpub->enp_settings.es_rw_once) + flags |= SCF_DISP_RW_ONCE; + if (conn->ifc_flags & IFC_HTTP) + flags |= SCF_HTTP; stream_id = generate_stream_id(conn, SD_BIDI); stream = lsquic_stream_new(stream_id, &conn->ifc_pub, conn->ifc_enpub->enp_stream_if, conn->ifc_enpub->enp_stream_if_ctx, conn->ifc_settings->es_init_max_stream_data_bidi_local, - conn->ifc_cfg.max_stream_send, SCF_IETF - | (conn->ifc_flags & IFC_HTTP ? SCF_HTTP : 0)); + conn->ifc_cfg.max_stream_send, flags); if (!stream) return -1; if (!lsquic_hash_insert(conn->ifc_pub.all_streams, &stream->id, @@ -922,15 +928,20 @@ create_push_stream (struct ietf_full_conn *conn) { struct lsquic_stream *stream; lsquic_stream_id_t stream_id; + enum stream_ctor_flags flags; assert((conn->ifc_flags & (IFC_SERVER|IFC_HTTP)) == (IFC_SERVER|IFC_HTTP)); + flags = SCF_IETF|SCF_HTTP; + if (conn->ifc_enpub->enp_settings.es_rw_once) + flags |= SCF_DISP_RW_ONCE; + stream_id = generate_stream_id(conn, SD_UNI); stream = lsquic_stream_new(stream_id, &conn->ifc_pub, conn->ifc_enpub->enp_stream_if, conn->ifc_enpub->enp_stream_if_ctx, conn->ifc_settings->es_init_max_stream_data_bidi_local, - conn->ifc_cfg.max_stream_send, SCF_IETF|SCF_HTTP); + conn->ifc_cfg.max_stream_send, flags); if (!stream) return NULL; if (!lsquic_hash_insert(conn->ifc_pub.all_streams, &stream->id, diff --git a/src/liblsquic/lsquic_qdec_hdl.c b/src/liblsquic/lsquic_qdec_hdl.c index a4eeed545..1067f6d8c 100644 --- a/src/liblsquic/lsquic_qdec_hdl.c +++ b/src/liblsquic/lsquic_qdec_hdl.c @@ -594,6 +594,8 @@ lsquic_qdh_header_in_begin (struct qpack_dec_hdl *qdh, union hblock_ctx *u; unsigned char dec_buf[LSQPACK_LONGEST_HEADER_ACK]; + assert(!(stream->stream_flags & STREAM_U_READ_DONE)); + if (!(qdh->qdh_flags & QDH_INITIALIZED)) { LSQ_WARN("not initialized: cannot process header block"); @@ -636,6 +638,8 @@ lsquic_qdh_header_in_continue (struct qpack_dec_hdl *qdh, size_t dec_buf_sz; unsigned char dec_buf[LSQPACK_LONGEST_HEADER_ACK]; + assert(!(stream->stream_flags & STREAM_U_READ_DONE)); + if (qdh->qdh_flags & QDH_INITIALIZED) { dec_buf_sz = sizeof(dec_buf); @@ -651,7 +655,7 @@ lsquic_qdh_header_in_continue (struct qpack_dec_hdl *qdh, } -void +static void lsquic_qdh_unref_stream (struct qpack_dec_hdl *qdh, struct lsquic_stream *stream) { @@ -669,6 +673,9 @@ lsquic_qdh_cancel_stream (struct qpack_dec_hdl *qdh, ssize_t nw; unsigned char buf[LSQPACK_LONGEST_CANCEL]; + if (!qdh->qdh_dec_sm_out) + return; + nw = lsqpack_dec_cancel_stream(&qdh->qdh_decoder, stream, buf, sizeof(buf)); if (nw > 0) { @@ -687,6 +694,33 @@ lsquic_qdh_cancel_stream (struct qpack_dec_hdl *qdh, } +void +lsquic_qdh_cancel_stream_id (struct qpack_dec_hdl *qdh, + lsquic_stream_id_t stream_id) +{ + ssize_t nw; + unsigned char buf[LSQPACK_LONGEST_CANCEL]; + + if (!qdh->qdh_dec_sm_out) + return; + + nw = lsqpack_dec_cancel_stream_id(&qdh->qdh_decoder, stream_id, buf, + sizeof(buf)); + if (nw > 0) + { + if (0 == qdh_write_decoder(qdh, buf, nw)) + LSQ_DEBUG("wrote %zd-byte Cancel Stream instruction for " + "stream %"PRIu64" to the decoder stream", stream_id, nw); + } + else if (nw == 0) + LSQ_DEBUG("not generating Cancel Stream instruction for " + "stream %"PRIu64, stream_id); + else + LSQ_WARN("cannot generate Cancel Stream instruction for " + "stream %"PRIu64" -- not enough buffer space", stream_id); +} + + int lsquic_qdh_arm_if_unsent (struct qpack_dec_hdl *qdh, void (*func)(void *), void *ctx) diff --git a/src/liblsquic/lsquic_qdec_hdl.h b/src/liblsquic/lsquic_qdec_hdl.h index 560c58fe7..4740792e3 100644 --- a/src/liblsquic/lsquic_qdec_hdl.h +++ b/src/liblsquic/lsquic_qdec_hdl.h @@ -62,13 +62,13 @@ enum lsqpack_read_header_status lsquic_qdh_header_in_continue (struct qpack_dec_hdl *, struct lsquic_stream *, const unsigned char **, size_t); -void -lsquic_qdh_unref_stream (struct qpack_dec_hdl *, struct lsquic_stream *); - void lsquic_qdh_cancel_stream (struct qpack_dec_hdl *, struct lsquic_stream *); +void +lsquic_qdh_cancel_stream_id (struct qpack_dec_hdl *, lsquic_stream_id_t); + int lsquic_qdh_arm_if_unsent (struct qpack_dec_hdl *, void (*)(void *), void *); diff --git a/src/liblsquic/lsquic_stream.c b/src/liblsquic/lsquic_stream.c index e39ac860b..b0e3ffc01 100644 --- a/src/liblsquic/lsquic_stream.c +++ b/src/liblsquic/lsquic_stream.c @@ -572,7 +572,11 @@ lsquic_stream_destroy (lsquic_stream_t *stream) if (stream->sm_qflags & SMQF_SERVICE_FLAGS) TAILQ_REMOVE(&stream->conn_pub->service_streams, stream, next_service_stream); if (stream->sm_qflags & SMQF_QPACK_DEC) - lsquic_qdh_unref_stream(stream->conn_pub->u.ietf.qdh, stream); + lsquic_qdh_cancel_stream(stream->conn_pub->u.ietf.qdh, stream); + else if ((stream->sm_bflags & (SMBF_IETF|SMBF_USE_HEADERS)) + == (SMBF_IETF|SMBF_USE_HEADERS) + && !(stream->stream_flags & STREAM_FIN_REACHED)) + lsquic_qdh_cancel_stream_id(stream->conn_pub->u.ietf.qdh, stream->id); drop_buffered_data(stream); lsquic_sfcw_consume_rem(&stream->fc); drop_frames_in(stream); @@ -731,6 +735,44 @@ stream_readable_http_ietf (struct lsquic_stream *stream) } +static int +maybe_switch_data_in (struct lsquic_stream *stream) +{ + if ((stream->sm_bflags & SMBF_AUTOSWITCH) && + (stream->data_in->di_flags & DI_SWITCH_IMPL)) + { + stream->data_in = stream->data_in->di_if->di_switch_impl( + stream->data_in, stream->read_offset); + if (!stream->data_in) + { + stream->data_in = lsquic_data_in_error_new(); + return -1; + } + } + + return 0; +} + + +/* Drain and discard any incoming data */ +static int +stream_readable_discard (struct lsquic_stream *stream) +{ + struct data_frame *data_frame; + + while ((data_frame = stream->data_in->di_if->di_get_frame( + stream->data_in, stream->read_offset))) + { + data_frame->df_read_off = data_frame->df_size; + stream->data_in->di_if->di_frame_done(stream->data_in, data_frame); + } + + (void) maybe_switch_data_in(stream); + + return 0; /* Never readable */ +} + + int lsquic_stream_readable (struct lsquic_stream *stream) { @@ -940,17 +982,8 @@ lsquic_stream_frame_in (lsquic_stream_t *stream, stream_frame_t *frame) stream->sm_fin_off = DF_END(frame); maybe_finish_stream(stream); } - if ((stream->sm_bflags & SMBF_AUTOSWITCH) && - (stream->data_in->di_flags & DI_SWITCH_IMPL)) - { - stream->data_in = stream->data_in->di_if->di_switch_impl( - stream->data_in, stream->read_offset); - if (!stream->data_in) - { - stream->data_in = lsquic_data_in_error_new(); - goto end_ok; - } - } + if (0 != maybe_switch_data_in(stream)) + goto end_ok; if (got_next_offset) /* Checking the offset saves di_get_frame() call */ maybe_conn_to_tickable_if_readable(stream); @@ -1319,17 +1352,8 @@ read_data_frames (struct lsquic_stream *stream, int do_filtering, const int fin = data_frame->df_fin; stream->data_in->di_if->di_frame_done(stream->data_in, data_frame); data_frame = NULL; - if ((stream->sm_bflags & SMBF_AUTOSWITCH) && - (stream->data_in->di_flags & DI_SWITCH_IMPL)) - { - stream->data_in = stream->data_in->di_if->di_switch_impl( - stream->data_in, stream->read_offset); - if (!stream->data_in) - { - stream->data_in = lsquic_data_in_error_new(); - return -1; - } - } + if (0 != maybe_switch_data_in(stream)) + return -1; if (fin) { stream->stream_flags |= STREAM_FIN_REACHED; @@ -1525,6 +1549,7 @@ stream_shutdown_read (lsquic_stream_t *stream) { SM_HISTORY_APPEND(stream, SHE_SHUTDOWN_READ); stream->stream_flags |= STREAM_U_READ_DONE; + stream->sm_readable = stream_readable_discard; stream_wantread(stream, 0); maybe_finish_stream(stream); } diff --git a/test/unittests/test_h3_framing.c b/test/unittests/test_h3_framing.c index 3fc3c004a..bc0edd7f7 100644 --- a/test/unittests/test_h3_framing.c +++ b/test/unittests/test_h3_framing.c @@ -53,6 +53,8 @@ #include "lsxpack_header.h" #include "lsquic_frab_list.h" #include "lsquic_qenc_hdl.h" +#include "lsquic_http1x_if.h" +#include "lsquic_qdec_hdl.h" #include "lsquic_varint.h" #include "lsquic_hq.h" #include "lsquic_data_in_if.h" @@ -264,6 +266,8 @@ struct test_objs { unsigned initial_stream_window; enum stream_ctor_flags ctor_flags; struct qpack_enc_hdl qeh; + struct qpack_dec_hdl qdh; + struct lsquic_hset_if hsi_if; }; @@ -320,6 +324,7 @@ init_test_objs (struct test_objs *tobjs, unsigned initial_conn_window, tobjs->conn_pub.path = &network_path; tobjs->initial_stream_window = initial_stream_window; tobjs->eng_pub.enp_settings.es_cc_algo = 1; /* Cubic */ + tobjs->eng_pub.enp_hsi_if = &tobjs->hsi_if; lsquic_send_ctl_init(&tobjs->send_ctl, &tobjs->alset, &tobjs->eng_pub, &tobjs->ver_neg, &tobjs->conn_pub, 0); tobjs->send_ctl.sc_cong_u.cubic.cu_cwnd = ~0ull; @@ -332,6 +337,8 @@ init_test_objs (struct test_objs *tobjs, unsigned initial_conn_window, s = lsquic_qeh_settings(&tobjs->qeh, 0, 0, 0, 0); assert(0 == s); tobjs->conn_pub.u.ietf.qeh = &tobjs->qeh; + lsquic_qdh_init(&tobjs->qdh, &tobjs->lconn, 0, &tobjs->eng_pub, 0, 0); + tobjs->conn_pub.u.ietf.qdh = &tobjs->qdh; } }