Release 2.26.2

- [BUGFIX] Do not drop incoming data when STOP_SENDING is received.
- [BUGFIX] Receipt of STOP_SENDING should not cause read-reset.
- [BUGFIX] Allow stream writes after receiving RESET.
- [BUGFIX] Typo in stream: ANDing enum with wrong flag.
- [BUGFIX] Reset elision: do not use zero as special stream ID value,
  for zero is a valid stream ID in IETF QUIC.
- [API] Add optional on_conncloseframe_received() callback.
- Use zero error code in RESET stream sent in response to STOP_SENDING.
This commit is contained in:
Dmitri Tikhonov 2020-12-23 09:06:05 -05:00
parent efa7f95dff
commit 292abba1f8
11 changed files with 165 additions and 33 deletions

View File

@ -1,3 +1,14 @@
2020-12-23
- 2.26.2
- [BUGFIX] Do not drop incoming data when STOP_SENDING is received.
- [BUGFIX] Receipt of STOP_SENDING should not cause read-reset.
- [BUGFIX] Allow stream writes after receiving RESET.
- [BUGFIX] Typo in stream: ANDing enum with wrong flag.
- [BUGFIX] Reset elision: do not use zero as special stream ID value,
for zero is a valid stream ID in IETF QUIC.
- [API] Add optional on_conncloseframe_received() callback.
- Use zero error code in RESET stream sent in response to STOP_SENDING.
2020-12-17
- 2.26.1
- [BUGFIX] Migration corner cases: drop or pad over path challenge

View File

@ -13,6 +13,7 @@ to the LiteSpeed QUIC and HTTP/3 Library:
- Victor Stewart -- Generate SCIDs API (connection ID steering)
- Aaron France -- Shared library support and Lisp bindings
- Suma Subbarao -- Use callback to supply client's SSL_CTX
- Paul Sheer -- Callback on arrival of CONNECTION_CLOSE frame
Thank you!

View File

@ -1294,7 +1294,7 @@ the engine to communicate with the user code:
signals the user to stop reading, writing, or both.
Note that resets differ in gQUIC and IETF QUIC. In gQUIC, `how` is
always 2; in IETF QUIC, `how` is either 0 or 1 because on can reset
always 2; in IETF QUIC, `how` is either 0 or 1 because one can reset
just one direction in IETF QUIC.
This callback is optional. The reset error can still be collected

View File

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

View File

@ -25,7 +25,7 @@ extern "C" {
#define LSQUIC_MAJOR_VERSION 2
#define LSQUIC_MINOR_VERSION 26
#define LSQUIC_PATCH_VERSION 1
#define LSQUIC_PATCH_VERSION 2
/**
* Engine flags:
@ -217,7 +217,7 @@ struct lsquic_stream_if {
* signals the user to stop reading, writing, or both.
*
* Note that resets differ in gQUIC and IETF QUIC. In gQUIC, `how' is
* always 2; in IETF QUIC, `how' is either 0 or 1 because on can reset
* always 2; in IETF QUIC, `how' is either 0 or 1 because one can reset
* just one direction in IETF QUIC.
*/
void (*on_reset) (lsquic_stream_t *s, lsquic_stream_ctx_t *h, int how);

View File

@ -297,7 +297,7 @@ lsquic_packet_out_destroy (lsquic_packet_out_t *packet_out,
}
/* If `stream_id' is zero, stream frames from all reset streams are elided.
/* If `stream_id' is UINT64_MAX, stream frames from all reset streams are elided.
* Otherwise, elision is limited to the specified stream.
*/
unsigned
@ -320,16 +320,16 @@ lsquic_packet_out_elide_reset_stream_frames (lsquic_packet_out_t *packet_out,
{
++n_stream_frames;
if (stream_id)
if (stream_id != UINT64_MAX)
{
victim = frec->fe_stream->id == stream_id;
if (victim)
{
assert(lsquic_stream_is_reset(frec->fe_stream));
assert(lsquic_stream_is_write_reset(frec->fe_stream));
}
}
else
victim = lsquic_stream_is_reset(frec->fe_stream);
victim = lsquic_stream_is_write_reset(frec->fe_stream);
if (victim)
{

View File

@ -1430,7 +1430,8 @@ send_ctl_next_lost (lsquic_send_ctl_t *ctl)
{
if (0 == (lost_packet->po_flags & PO_MINI))
{
lsquic_packet_out_elide_reset_stream_frames(lost_packet, 0);
lsquic_packet_out_elide_reset_stream_frames(lost_packet,
UINT64_MAX);
if (lost_packet->po_regen_sz >= lost_packet->po_data_sz)
{
LSQ_DEBUG("Dropping packet %"PRIu64" from lost queue",

View File

@ -872,6 +872,17 @@ stream_readable_discard (struct lsquic_stream *stream)
}
static int
stream_is_read_reset (const struct lsquic_stream *stream)
{
if (stream->sm_bflags & SMBF_IETF)
return stream->stream_flags & STREAM_RST_RECVD;
else
return (stream->stream_flags & (STREAM_RST_RECVD|STREAM_RST_SENT))
|| (stream->sm_qflags & SMQF_SEND_RST);
}
int
lsquic_stream_readable (struct lsquic_stream *stream)
{
@ -885,13 +896,27 @@ lsquic_stream_readable (struct lsquic_stream *stream)
* lsquic_stream_read() will return -1 (we want the user to be
* able to collect the error).
*/
|| lsquic_stream_is_reset(stream)
|| stream_is_read_reset(stream)
/* Type-dependent readability check: */
|| stream->sm_readable(stream);
;
}
/* Return true if write end of the stream has been reset.
* Note that the logic for gQUIC is the same for write and read resets.
*/
int
lsquic_stream_is_write_reset (const struct lsquic_stream *stream)
{
if (stream->sm_bflags & SMBF_IETF)
return stream->stream_flags & STREAM_SS_RECVD;
else
return (stream->stream_flags & (STREAM_RST_RECVD|STREAM_RST_SENT))
|| (stream->sm_qflags & SMQF_SEND_RST);
}
static int
stream_writeable (struct lsquic_stream *stream)
{
@ -901,7 +926,7 @@ stream_writeable (struct lsquic_stream *stream)
* lsquic_stream_write() will return -1 (we want the user to be
* able to collect the error).
*/
lsquic_stream_is_reset(stream)
lsquic_stream_is_write_reset(stream)
/* - Data can be written to stream: */
|| lsquic_stream_write_avail(stream)
;
@ -1294,13 +1319,12 @@ lsquic_stream_stop_sending_in (struct lsquic_stream *stream,
maybe_conn_to_tickable_if_writeable(stream, 0);
lsquic_sfcw_consume_rem(&stream->fc);
drop_frames_in(stream);
drop_buffered_data(stream);
maybe_elide_stream_frames(stream);
if (!(stream->stream_flags & (STREAM_RST_SENT|STREAM_FIN_SENT))
&& !(stream->sm_qflags & SMQF_SEND_RST))
lsquic_stream_reset_ext(stream, error_code, 0);
lsquic_stream_reset_ext(stream, 0, 0);
maybe_finish_stream(stream);
maybe_schedule_call_on_close(stream);
@ -1628,7 +1652,7 @@ lsquic_stream_readf (struct lsquic_stream *stream,
SM_HISTORY_APPEND(stream, SHE_USER_READ);
if (lsquic_stream_is_reset(stream))
if (stream_is_read_reset(stream))
{
if (stream->stream_flags & STREAM_RST_RECVD)
stream->stream_flags |= STREAM_RST_READ;
@ -1816,7 +1840,7 @@ stream_shutdown_write (lsquic_stream_t *stream)
&& !(stream->stream_flags & (STREAM_FIN_SENT|STREAM_RST_SENT))
&& !stream_is_incoming_unidir(stream)
/* In gQUIC, receiving a RESET means "stop sending" */
&& !(!(stream->sm_qflags & SMBF_IETF)
&& !(!(stream->sm_bflags & SMBF_IETF)
&& (stream->stream_flags & STREAM_RST_RECVD)))
{
if ((stream->sm_bflags & SMBF_USE_HEADERS)
@ -2530,7 +2554,7 @@ lsquic_stream_flush_threshold (const struct lsquic_stream *stream,
return -1; \
} \
} \
if (lsquic_stream_is_reset(stream)) \
if (lsquic_stream_is_write_reset(stream)) \
{ \
LSQ_INFO("Attempt to write to stream after it had been reset"); \
errno = ECONNRESET; \
@ -4600,7 +4624,7 @@ lsquic_stream_get_hset (struct lsquic_stream *stream)
{
void *hset;
if (lsquic_stream_is_reset(stream))
if (stream_is_read_reset(stream))
{
LSQ_INFO("%s: stream is reset, no headers returned", __func__);
errno = ECONNRESET;

View File

@ -429,16 +429,15 @@ lsquic_stream_call_on_new (lsquic_stream_t *);
void
lsquic_stream_destroy (lsquic_stream_t *);
/* Any of these flags will cause user-facing read and write and
* shutdown calls to return an error. They also make the stream
* both readable and writeable, as we want the user to collect
* the error.
*/
/* True if either read or write side of the stream has been reset */
#define lsquic_stream_is_reset(stream) \
(((stream)->stream_flags & \
(STREAM_RST_RECVD|STREAM_RST_SENT|STREAM_SS_RECVD)) \
|| ((stream)->sm_qflags & SMQF_SEND_RST))
int
lsquic_stream_is_write_reset (const struct lsquic_stream *);
/* Data that from the network gets inserted into the stream using
* lsquic_stream_frame_in() function. Returns 0 on success, -1 on
* failure. The latter may be caused by flow control violation or

View File

@ -124,7 +124,7 @@ elide_single_stream_frame (void)
streams[0].stream_flags |= STREAM_RST_SENT;
lsquic_packet_out_elide_reset_stream_frames(packet_out, 0);
lsquic_packet_out_elide_reset_stream_frames(packet_out, UINT64_MAX);
assert(0 == streams[0].n_unacked);
assert(0 == packet_out->po_frame_types);
assert(!lsquic_pofi_first(&pofi, packet_out));
@ -193,7 +193,7 @@ shrink_packet_post_elision (void)
streams[0].stream_flags |= STREAM_RST_SENT;
lsquic_packet_out_elide_reset_stream_frames(packet_out, 0);
lsquic_packet_out_elide_reset_stream_frames(packet_out, UINT64_MAX);
assert(0 == streams[0].n_unacked);
assert(QUIC_FTBIT_STREAM == packet_out->po_frame_types);
@ -364,7 +364,7 @@ elide_three_stream_frames (int chop_regen)
if (chop_regen)
lsquic_packet_out_chop_regen(packet_out);
lsquic_packet_out_elide_reset_stream_frames(packet_out, 0);
lsquic_packet_out_elide_reset_stream_frames(packet_out, UINT64_MAX);
assert(ref_out->po_data_sz == packet_out->po_data_sz + (chop_regen ? 5 : 0));
assert(ref_out->po_regen_sz == packet_out->po_regen_sz + (chop_regen ? 5 : 0));

View File

@ -1030,16 +1030,11 @@ test_loc_data_rem_RST (struct test_objs *tobjs)
s = lsquic_stream_frame_in(stream, new_frame_in(tobjs, 0, 100, 0));
assert(0 == s);
s_onreset_called = (struct reset_call_ctx) { NULL, -1, };
if (stream->sm_bflags & SMBF_IETF)
lsquic_stream_stop_sending_in(stream, 12345);
else
{
s = lsquic_stream_rst_in(stream, 200, 0);
assert(0 == s);
}
s = lsquic_stream_rst_in(stream, 200, 0);
assert(0 == s);
assert(s_onreset_called.stream == stream);
if (stream->sm_bflags & SMBF_IETF)
assert(s_onreset_called.how == 1);
assert(s_onreset_called.how == 0);
else
assert(s_onreset_called.how == 2);
@ -1055,6 +1050,106 @@ test_loc_data_rem_RST (struct test_objs *tobjs)
assert(TAILQ_EMPTY(&tobjs->conn_pub.service_streams));
assert(0 == (stream->sm_qflags & SMQF_SERVICE_FLAGS));
n = lsquic_stream_write(stream, buf, 100);
if (stream->sm_bflags & SMBF_IETF)
assert(100 == n); /* Write successful after reset in IETF */
else
assert(-1 == n); /* Error collected */
s = lsquic_stream_close(stream);
assert(0 == s); /* Stream successfully closed */
if (stream->sm_bflags & SMBF_IETF)
assert(stream->n_unacked == 1);
assert(!TAILQ_EMPTY(&tobjs->conn_pub.service_streams));
assert((stream->sm_qflags & SMQF_SERVICE_FLAGS) == SMQF_CALL_ONCLOSE);
if (!(stream->sm_bflags & SMBF_IETF))
lsquic_stream_rst_frame_sent(stream);
lsquic_stream_call_on_close(stream);
assert(TAILQ_EMPTY(&tobjs->conn_pub.sending_streams));
if (stream->sm_bflags & SMBF_IETF)
{
/* FIN packet has not been acked yet: */
assert(TAILQ_EMPTY(&tobjs->conn_pub.service_streams));
/* Now ack it: */
ack_packet(&tobjs->send_ctl, 1);
}
assert(!TAILQ_EMPTY(&tobjs->conn_pub.service_streams));
assert((stream->sm_qflags & SMQF_SERVICE_FLAGS) == SMQF_FREE_STREAM);
lsquic_stream_destroy(stream);
assert(TAILQ_EMPTY(&tobjs->conn_pub.service_streams));
assert(200 == tobjs->conn_pub.cfcw.cf_max_recv_off);
assert(200 == tobjs->conn_pub.cfcw.cf_read_off);
}
/* Client: we send some data (no FIN), and remote end sends some data and
* then sends STOP_SENDING
*/
static void
test_loc_data_rem_SS (struct test_objs *tobjs)
{
lsquic_packet_out_t *packet_out;
lsquic_stream_t *stream;
char buf_out[0x100];
unsigned char buf[0x100];
ssize_t n;
int s, fin;
init_buf(buf_out, sizeof(buf_out));
stream = new_stream(tobjs, 345);
assert(stream->sm_bflags & SMBF_IETF); /* STOP_SENDING is IETF-only */
n = lsquic_stream_write(stream, buf_out, 100);
assert(n == 100);
assert(0 == lsquic_send_ctl_n_scheduled(&tobjs->send_ctl));
s = lsquic_stream_flush(stream);
assert(1 == lsquic_send_ctl_n_scheduled(&tobjs->send_ctl));
n = read_from_scheduled_packets(&tobjs->send_ctl, stream->id, buf,
sizeof(buf), 0, &fin, 0);
assert(100 == n);
assert(0 == memcmp(buf_out, buf, 100));
assert(!fin);
/* Pretend we sent out a packet: */
packet_out = lsquic_send_ctl_next_packet_to_send(&tobjs->send_ctl, 0);
lsquic_send_ctl_sent_packet(&tobjs->send_ctl, packet_out);
s = lsquic_stream_frame_in(stream, new_frame_in(tobjs, 0, 100, 0));
assert(0 == s);
s_onreset_called = (struct reset_call_ctx) { NULL, -1, };
lsquic_stream_stop_sending_in(stream, 12345);
assert(s_onreset_called.stream == stream);
assert(s_onreset_called.how == 1);
/* Incoming STOP_SENDING should not affect the ability to read from
* stream.
*/
unsigned char mybuf[123];
const ssize_t nread = lsquic_stream_read(stream, mybuf, sizeof(mybuf));
assert(nread == 100);
ack_packet(&tobjs->send_ctl, 1);
if (!(stream->sm_bflags & SMBF_IETF))
{
assert(!TAILQ_EMPTY(&tobjs->conn_pub.sending_streams));
assert((stream->sm_qflags & SMQF_SENDING_FLAGS) == SMQF_SEND_RST);
}
/* Not yet closed: error needs to be collected */
assert(TAILQ_EMPTY(&tobjs->conn_pub.service_streams));
assert(0 == (stream->sm_qflags & SMQF_SERVICE_FLAGS));
n = lsquic_stream_write(stream, buf, 100);
assert(-1 == n); /* Error collected */
s = lsquic_stream_close(stream);
@ -1474,6 +1569,7 @@ test_termination (void)
{ 1, 0, test_rem_data_loc_close, },
{ 1, 1, test_loc_FIN_rem_RST, },
{ 1, 1, test_loc_data_rem_RST, },
{ 0, 1, test_loc_data_rem_SS, },
{ 1, 0, test_loc_RST_rem_FIN, },
{ 1, 1, test_gapless_elision_beginning, },
{ 1, 1, test_gapless_elision_middle, },