diff --git a/CHANGELOG b/CHANGELOG index f4104c0..e8abc8f 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,13 @@ +2021-01-08 + - 2.27.3 + - [BUGFIX] gQUIC: do not destroy critical streams when connection is + closed. See issue #201. + - [BUGFIX] Drop #if LSQUIC_CONN_STATS from lsquic.h. See issue #211. + - [BUGFIX] Challenge cancellation when path validation fails. + - [BUGFIX] Do not send FIN if RST is scheduled to be sent on a stream. + - [BUGFIX] gQUIC's is_tickable() when connection is closing. + - [BUGFIX] Q050 processing of GOAWAY frames. + 2021-01-06 - 2.27.2 - [BUGFIX] Memory corruption in receive history copy-ranges function. diff --git a/docs/conf.py b/docs/conf.py index dd7d232..ced2f49 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -26,7 +26,7 @@ author = u'LiteSpeed Technologies' # The short X.Y version version = u'2.27' # The full version, including alpha/beta/rc tags -release = u'2.27.2' +release = u'2.27.3' # -- General configuration --------------------------------------------------- diff --git a/include/lsquic.h b/include/lsquic.h index 506f62d..d9ea9e3 100644 --- a/include/lsquic.h +++ b/include/lsquic.h @@ -25,7 +25,7 @@ extern "C" { #define LSQUIC_MAJOR_VERSION 2 #define LSQUIC_MINOR_VERSION 27 -#define LSQUIC_PATCH_VERSION 2 +#define LSQUIC_PATCH_VERSION 3 /** * Engine flags: @@ -1320,13 +1320,13 @@ struct lsquic_engine_api */ const struct lsquic_hset_if *ea_hsi_if; void *ea_hsi_ctx; -#if LSQUIC_CONN_STATS + /** * If set, engine will print cumulative connection statistics to this - * file just before it is destroyed. + * file just before it is destroyed. (Must be compiled with + * -DLSQUIC_CONN_STATS=1). */ void /* FILE, really */ *ea_stats_fh; -#endif /** * The optional ALPN string is used by the client if @ref LSENG_HTTP diff --git a/src/liblsquic/lsquic_full_conn.c b/src/liblsquic/lsquic_full_conn.c index bb7c22b..b50d647 100644 --- a/src/liblsquic/lsquic_full_conn.c +++ b/src/liblsquic/lsquic_full_conn.c @@ -84,6 +84,9 @@ enum stream_if { STREAM_IF_STD, STREAM_IF_HSK, STREAM_IF_HDR, N_STREAM_IFS }; /* Maximum number of ACK ranges that can fit into gQUIC ACK frame */ #define MAX_ACK_RANGES 256 +/* HANDSHAKE and HEADERS streams are always open in gQUIC connection */ +#define N_SPECIAL_STREAMS 2 + /* IMPORTANT: Keep values of FC_SERVER and FC_HTTP same as LSENG_SERVER * and LSENG_HTTP. */ @@ -1828,7 +1831,7 @@ reset_local_streams_over_goaway (struct full_conn *conn) el = lsquic_hash_next(conn->fc_pub.all_streams)) { stream = lsquic_hashelem_getdata(el); - if (stream->id > conn->fc_goaway_stream_id && + if ((int64_t) stream->id > (int64_t) conn->fc_goaway_stream_id && ((stream->id & 1) ^ is_server /* Locally initiated? */)) { lsquic_stream_received_goaway(stream); @@ -2083,8 +2086,6 @@ static unsigned process_connection_close_frame (struct full_conn *conn, lsquic_packet_in_t *packet_in, const unsigned char *p, size_t len) { - lsquic_stream_t *stream; - struct lsquic_hash_elem *el; uint64_t error_code; uint16_t reason_len; uint8_t reason_off; @@ -2101,17 +2102,7 @@ process_connection_close_frame (struct full_conn *conn, lsquic_packet_in_t *pack if (conn->fc_stream_ifs[STREAM_IF_STD].stream_if->on_conncloseframe_received) conn->fc_stream_ifs[STREAM_IF_STD].stream_if->on_conncloseframe_received( &conn->fc_conn, -1, error_code, (const char *) p + reason_off, reason_len); - conn->fc_flags |= FC_RECV_CLOSE; - if (!(conn->fc_flags & FC_CLOSING)) - { - for (el = lsquic_hash_first(conn->fc_pub.all_streams); el; - el = lsquic_hash_next(conn->fc_pub.all_streams)) - { - stream = lsquic_hashelem_getdata(el); - lsquic_stream_shutdown_internal(stream); - } - conn->fc_flags |= FC_CLOSING; - } + conn->fc_flags |= FC_RECV_CLOSE|FC_CLOSING; return parsed_len; } @@ -2411,7 +2402,7 @@ process_regular_packet (struct full_conn *conn, lsquic_packet_in_t *packet_in) { frame_types = packet_in->pi_frame_types; if ((conn->fc_flags & FC_GOING_AWAY) - && lsquic_hash_count(conn->fc_pub.all_streams) < 3) + && lsquic_hash_count(conn->fc_pub.all_streams) <= N_SPECIAL_STREAMS) { /* Ignore PING frames if we are going away and there are no * active streams. (HANDSHAKE and HEADERS streams are the @@ -2627,7 +2618,7 @@ maybe_close_conn (struct full_conn *conn) if ((conn->fc_flags & (FC_CLOSING|FC_GOAWAY_SENT|FC_SERVER)) == (FC_GOAWAY_SENT|FC_SERVER) - && lsquic_hash_count(conn->fc_pub.all_streams) == 2) + && lsquic_hash_count(conn->fc_pub.all_streams) == N_SPECIAL_STREAMS) { #ifndef NDEBUG for (el = lsquic_hash_first(conn->fc_pub.all_streams); el; @@ -3193,7 +3184,7 @@ conn_ok_to_close (const struct full_conn *conn) || (conn->fc_flags & FC_RECV_CLOSE) || ( !lsquic_send_ctl_have_outgoing_stream_frames(&conn->fc_send_ctl) - && lsquic_hash_count(conn->fc_pub.all_streams) == 0 + && lsquic_hash_count(conn->fc_pub.all_streams) <= N_SPECIAL_STREAMS && lsquic_send_ctl_have_unacked_stream_frames(&conn->fc_send_ctl) == 0); } @@ -3807,7 +3798,8 @@ full_conn_ci_close (struct lsquic_conn *lconn) el = lsquic_hash_next(conn->fc_pub.all_streams)) { stream = lsquic_hashelem_getdata(el); - lsquic_stream_shutdown_internal(stream); + if (!lsquic_stream_is_critical(stream)) + lsquic_stream_reset(stream, 0); } conn->fc_flags |= FC_CLOSING; if (!(conn->fc_flags & FC_GOAWAY_SENT)) @@ -4264,7 +4256,7 @@ full_conn_ci_is_tickable (lsquic_conn_t *lconn) !lsquic_send_ctl_sched_is_blocked(&conn->fc_send_ctl))) { const enum full_conn_flags send_flags = FC_SEND_GOAWAY - |FC_SEND_STOP_WAITING|FC_SEND_PING|FC_SEND_WUF|FC_CLOSING; + |FC_SEND_STOP_WAITING|FC_SEND_PING|FC_SEND_WUF; if (conn->fc_flags & send_flags) { LSQ_DEBUG("tickable: flags: 0x%X", conn->fc_flags & send_flags); diff --git a/src/liblsquic/lsquic_full_conn_ietf.c b/src/liblsquic/lsquic_full_conn_ietf.c index f5fec6f..3c13e7f 100644 --- a/src/liblsquic/lsquic_full_conn_ietf.c +++ b/src/liblsquic/lsquic_full_conn_ietf.c @@ -705,8 +705,10 @@ cid_throt_alarm_expired (enum alarm_id al_id, void *ctx, static void wipe_path (struct ietf_full_conn *conn, unsigned path_id) { + void *peer_ctx = conn->ifc_paths[path_id].cop_path.np_peer_ctx; memset(&conn->ifc_paths[path_id], 0, sizeof(conn->ifc_paths[0])); conn->ifc_paths[path_id].cop_path.np_path_id = path_id; + conn->ifc_paths[path_id].cop_path.np_peer_ctx = peer_ctx; } @@ -2916,7 +2918,7 @@ ietf_full_conn_ci_close (struct lsquic_conn *lconn) stream = lsquic_hashelem_getdata(el); sd = (stream->id >> SD_SHIFT) & 1; if (SD_BIDI == sd) - lsquic_stream_shutdown_internal(stream); + lsquic_stream_reset(stream, 0); } conn->ifc_flags |= IFC_CLOSING; conn->ifc_send_flags |= SF_SEND_CONN_CLOSE; @@ -5933,8 +5935,6 @@ static unsigned process_connection_close_frame (struct ietf_full_conn *conn, struct lsquic_packet_in *packet_in, const unsigned char *p, size_t len) { - lsquic_stream_t *stream; - struct lsquic_hash_elem *el; uint64_t error_code; uint16_t reason_len; uint8_t reason_off; @@ -5971,17 +5971,7 @@ process_connection_close_frame (struct ietf_full_conn *conn, if (conn->ifc_enpub->enp_stream_if->on_conncloseframe_received) conn->ifc_enpub->enp_stream_if->on_conncloseframe_received( &conn->ifc_conn, app_error, error_code, (const char *) p + reason_off, reason_len); - conn->ifc_flags |= IFC_RECV_CLOSE; - if (!(conn->ifc_flags & IFC_CLOSING)) - { - for (el = lsquic_hash_first(conn->ifc_pub.all_streams); el; - el = lsquic_hash_next(conn->ifc_pub.all_streams)) - { - stream = lsquic_hashelem_getdata(el); - lsquic_stream_shutdown_internal(stream); - } - conn->ifc_flags |= IFC_CLOSING; - } + conn->ifc_flags |= IFC_RECV_CLOSE|IFC_CLOSING; return parsed_len; } @@ -8890,6 +8880,7 @@ cancel_push_promise (struct ietf_full_conn *conn, struct push_promise *promise) /* But let stream dtor free the promise object as sm_promise may yet * be used by the stream in some ways. */ + /* TODO: drop lsquic_stream_shutdown_internal, use something else */ lsquic_stream_shutdown_internal(promise->pp_pushed_stream); if (0 != lsquic_hcso_write_cancel_push(&conn->ifc_hcso, promise->pp_id)) ABORT_WARN("cannot write CANCEL_PUSH"); diff --git a/src/liblsquic/lsquic_send_ctl.c b/src/liblsquic/lsquic_send_ctl.c index b6b5eef..dba590e 100644 --- a/src/liblsquic/lsquic_send_ctl.c +++ b/src/liblsquic/lsquic_send_ctl.c @@ -3617,9 +3617,10 @@ lsquic_send_ctl_cancel_chals (struct lsquic_send_ctl *ctl, packet_out = next) { next = TAILQ_NEXT(packet_out, po_next); - if (packet_out->po_path == path - && packet_out->po_frame_types == QUIC_FTBIT_PATH_CHALLENGE) + if (packet_out->po_path == path) { + assert(packet_out->po_frame_types & QUIC_FTBIT_PATH_CHALLENGE); + assert(!(packet_out->po_frame_types & ctl->sc_retx_frames)); send_ctl_maybe_renumber_sched_to_right(ctl, packet_out); send_ctl_sched_remove(ctl, packet_out); assert(packet_out->po_loss_chain == packet_out); diff --git a/src/liblsquic/lsquic_stream.c b/src/liblsquic/lsquic_stream.c index 6c29e77..fa57af0 100644 --- a/src/liblsquic/lsquic_stream.c +++ b/src/liblsquic/lsquic_stream.c @@ -1837,7 +1837,8 @@ stream_shutdown_write (lsquic_stream_t *stream) * the flags indicate that nothing else should be written. */ if (!(stream->sm_bflags & SMBF_CRYPTO) - && !(stream->stream_flags & (STREAM_FIN_SENT|STREAM_RST_SENT)) + && !((stream->stream_flags & (STREAM_FIN_SENT|STREAM_RST_SENT)) + || (stream->sm_qflags & SMQF_SEND_RST)) && !stream_is_incoming_unidir(stream) /* In gQUIC, receiving a RESET means "stop sending" */ && !(!(stream->sm_bflags & SMBF_IETF) diff --git a/tests/test_h3_framing.c b/tests/test_h3_framing.c index 377501d..bdfab34 100644 --- a/tests/test_h3_framing.c +++ b/tests/test_h3_framing.c @@ -331,7 +331,9 @@ static const struct conn_iface our_conn_if = }; +#if LSQUIC_CONN_STATS static struct conn_stats s_conn_stats; +#endif static void init_test_objs (struct test_objs *tobjs, unsigned initial_conn_window, @@ -361,7 +363,9 @@ init_test_objs (struct test_objs *tobjs, unsigned initial_conn_window, tobjs->conn_pub.packet_out_malo = lsquic_malo_create(sizeof(struct lsquic_packet_out)); tobjs->conn_pub.path = &network_path; +#if LSQUIC_CONN_STATS tobjs->conn_pub.conn_stats = &s_conn_stats; +#endif 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; diff --git a/tests/test_send_headers.c b/tests/test_send_headers.c index 137f6b5..762a427 100644 --- a/tests/test_send_headers.c +++ b/tests/test_send_headers.c @@ -174,7 +174,9 @@ static const struct conn_iface our_conn_if = static struct http1x_ctor_ctx ctor_ctx = { .is_server = 0, }; +#if LSQUIC_CONN_STATS static struct conn_stats s_conn_stats; +#endif static void init_test_objs (struct test_objs *tobjs, unsigned initial_conn_window, @@ -204,7 +206,9 @@ init_test_objs (struct test_objs *tobjs, unsigned initial_conn_window, tobjs->conn_pub.packet_out_malo = lsquic_malo_create(sizeof(struct lsquic_packet_out)); tobjs->conn_pub.path = &network_path; +#if LSQUIC_CONN_STATS tobjs->conn_pub.conn_stats = &s_conn_stats; +#endif tobjs->initial_stream_window = initial_stream_window; lsquic_send_ctl_init(&tobjs->send_ctl, &tobjs->alset, &tobjs->eng_pub, &tobjs->ver_neg, &tobjs->conn_pub, 0); diff --git a/tests/test_stream.c b/tests/test_stream.c index 011eb48..df5d3bd 100644 --- a/tests/test_stream.c +++ b/tests/test_stream.c @@ -350,7 +350,9 @@ static const struct conn_iface our_conn_if = .ci_write_ack = write_ack, }; +#if LSQUIC_CONN_STATS static struct conn_stats s_conn_stats; +#endif static void init_test_objs (struct test_objs *tobjs, unsigned initial_conn_window, @@ -381,7 +383,9 @@ init_test_objs (struct test_objs *tobjs, unsigned initial_conn_window, tobjs->conn_pub.packet_out_malo = lsquic_malo_create(sizeof(struct lsquic_packet_out)); tobjs->conn_pub.path = &network_path; +#if LSQUIC_CONN_STATS tobjs->conn_pub.conn_stats = &s_conn_stats; +#endif tobjs->initial_stream_window = initial_stream_window; lsquic_send_ctl_init(&tobjs->send_ctl, &tobjs->alset, &tobjs->eng_pub, &tobjs->ver_neg, &tobjs->conn_pub, 0); @@ -801,8 +805,15 @@ test_rem_data_loc_close_and_rst_in (struct test_objs *tobjs) s = lsquic_stream_shutdown(stream, 1); assert(0 == s); - assert(1 == lsquic_send_ctl_n_scheduled(&tobjs->send_ctl)); /* Shutdown performs a flush */ - assert(stream->n_unacked == 1); + if (stream->sm_bflags & SMBF_IETF) + { + assert(1 == lsquic_send_ctl_n_scheduled(&tobjs->send_ctl)); /* Shutdown performs a flush */ + assert(stream->n_unacked == 1); + } + else + { + /* gQUIC has RST scheduled to be sent, so no FIN is written */ + } assert(!TAILQ_EMPTY(&tobjs->conn_pub.service_streams)); assert((stream->sm_qflags & (SMQF_SERVICE_FLAGS)) == SMQF_CALL_ONCLOSE); @@ -820,12 +831,20 @@ test_rem_data_loc_close_and_rst_in (struct test_objs *tobjs) assert(stream->sm_qflags & SMQF_CALL_ONCLOSE); lsquic_stream_rst_frame_sent(stream); - stream->n_unacked++; /* RESET frame take a reference */ - assert(!(stream->sm_qflags & SMQF_FREE_STREAM)); /* Not yet, - because: */ assert(stream->n_unacked == 2); + if (stream->sm_bflags & SMBF_IETF) + { + stream->n_unacked++; /* RESET frame take a reference */ + assert(!(stream->sm_qflags & SMQF_FREE_STREAM)); /* Not yet, + because: */ assert(stream->n_unacked == 2); + } - lsquic_stream_acked(stream, QUIC_FRAME_STREAM); - lsquic_stream_acked(stream, QUIC_FRAME_RST_STREAM); + if (stream->sm_bflags & SMBF_IETF) + { + lsquic_stream_acked(stream, QUIC_FRAME_STREAM); + lsquic_stream_acked(stream, QUIC_FRAME_RST_STREAM); + } + else + assert(stream->n_unacked == 0); /* STREAM frame was elided */ assert(stream->sm_qflags & SMQF_FREE_STREAM); /* OK, now */ lsquic_stream_destroy(stream); @@ -869,13 +888,15 @@ test_rem_data_loc_close (struct test_objs *tobjs) s = lsquic_stream_shutdown(stream, 1); assert(0 == s); - assert(1 == lsquic_send_ctl_n_scheduled(&tobjs->send_ctl)); /* Shutdown performs a flush */ + if (stream->sm_bflags & SMBF_IETF) + assert(1 == lsquic_send_ctl_n_scheduled(&tobjs->send_ctl)); /* Shutdown performs a flush */ assert(!TAILQ_EMPTY(&tobjs->conn_pub.service_streams)); assert(stream->sm_qflags & SMQF_CALL_ONCLOSE); assert(!(stream->sm_qflags & SMQF_FREE_STREAM)); - lsquic_stream_acked(stream, QUIC_FRAME_STREAM); + if (stream->sm_bflags & SMBF_IETF) + lsquic_stream_acked(stream, QUIC_FRAME_STREAM); lsquic_stream_rst_frame_sent(stream); stream->n_unacked++; /* RESET frame take a reference */ @@ -1225,8 +1246,11 @@ test_loc_RST_rem_FIN (struct test_objs *tobjs) ++stream->n_unacked; /* Fake sending of packet with RST_STREAM */ assert(!TAILQ_EMPTY(&tobjs->conn_pub.sending_streams)); assert((stream->sm_qflags & SMQF_SENDING_FLAGS) == SMQF_SEND_RST); - sss = lsquic_stream_sending_state(stream); - assert(SSS_DATA_SENT == sss); /* FIN was packetized */ + if (stream->sm_bflags & SMBF_IETF) + { + sss = lsquic_stream_sending_state(stream); + assert(SSS_DATA_SENT == sss); /* FIN was packetized */ + } s = lsquic_stream_frame_in(stream, new_frame_in(tobjs, 0, 90, 1)); assert(s == 0); @@ -1246,8 +1270,12 @@ test_loc_RST_rem_FIN (struct test_objs *tobjs) assert(TAILQ_EMPTY(&tobjs->conn_pub.sending_streams)); lsquic_stream_call_on_close(stream); - assert(TAILQ_EMPTY(&tobjs->conn_pub.service_streams)); /* Not acked yet */ - lsquic_stream_acked(stream, QUIC_FRAME_STREAM); + + if (stream->sm_bflags & SMBF_IETF) + { + assert(TAILQ_EMPTY(&tobjs->conn_pub.service_streams)); /* Not acked yet */ + lsquic_stream_acked(stream, QUIC_FRAME_STREAM); + } assert(!TAILQ_EMPTY(&tobjs->conn_pub.service_streams)); assert((stream->sm_qflags & SMQF_SERVICE_FLAGS) == SMQF_FREE_STREAM);