From 5650ee6cd621f3c4fb0e99db3445dd19268d1b51 Mon Sep 17 00:00:00 2001 From: Dmitri Tikhonov Date: Wed, 27 Jan 2021 10:36:25 -0500 Subject: [PATCH] Release 2.27.6 - [BUGFIX] Replace dispatch read/write events assertion with a check. - [BUGFIX] gQUIC connection close: there is no HEADERS stream without HTTP flag, see issue #220. - http_client, http_server: add hq protocol support and other flags for use with QUIC Interop Runner. - Fix: use IP_PMTUDISC_PROBE (not IP_PMTUDISC_DO) on Linux to set Don't-Fragment flag on outgoing packets. - Fix send_packets_one_by_one on Windows platform when sending multiple iovs, see issue #218. - Exit echo_client on Windows immediately, see issue #219. --- CHANGELOG | 13 +++ CONTRIBUTORS.txt | 1 + bin/echo_client.c | 6 ++ bin/http_client.c | 96 ++++++++++++++++++++- bin/http_server.c | 119 ++++++++++++++++++++++++++- bin/test_common.c | 4 +- docs/conf.py | 2 +- include/lsquic.h | 2 +- src/liblsquic/lsquic_full_conn.c | 4 +- src/liblsquic/lsquic_parse_ietf_v1.c | 24 ++++++ src/liblsquic/lsquic_stream.c | 17 ++-- 11 files changed, 274 insertions(+), 14 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 4474204..f0c3877 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -1,3 +1,16 @@ +2021-01-27 + - 2.27.6 + - [BUGFIX] Replace dispatch read/write events assertion with a check. + - [BUGFIX] gQUIC connection close: there is no HEADERS stream without + HTTP flag, see issue #220. + - http_client, http_server: add hq protocol support and other flags + for use with QUIC Interop Runner. + - Fix: use IP_PMTUDISC_PROBE (not IP_PMTUDISC_DO) on Linux to set + Don't-Fragment flag on outgoing packets. + - Fix send_packets_one_by_one on Windows platform when sending + multiple iovs, see issue #218. + - Exit echo_client on Windows immediately, see issue #219. + 2021-01-18 - 2.27.5 - [BUGFIX] Assertion in send controller when path validation fails. diff --git a/CONTRIBUTORS.txt b/CONTRIBUTORS.txt index 4b41a2e..d5fd6a7 100644 --- a/CONTRIBUTORS.txt +++ b/CONTRIBUTORS.txt @@ -14,6 +14,7 @@ to the LiteSpeed QUIC and HTTP/3 Library: - 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 + - Weiwei Wang -- Some fixes on Windows Thank you! diff --git a/bin/echo_client.c b/bin/echo_client.c index 896f0ae..d0c9b68 100644 --- a/bin/echo_client.c +++ b/bin/echo_client.c @@ -208,6 +208,12 @@ main (int argc, char **argv) struct prog prog; struct echo_client_ctx client_ctx; +#ifdef WIN32 + fprintf(stderr, "%s does not work on Windows, see\n" + "https://github.com/litespeedtech/lsquic/issues/219\n", argv[0]); + exit(EXIT_FAILURE); +#endif + memset(&client_ctx, 0, sizeof(client_ctx)); client_ctx.prog = &prog; diff --git a/bin/http_client.c b/bin/http_client.c index 9b0fa4d..ff2c7ec 100644 --- a/bin/http_client.c +++ b/bin/http_client.c @@ -190,6 +190,7 @@ struct http_client_ctx { unsigned hcc_n_open_conns; unsigned hcc_reset_after_nbytes; unsigned hcc_retire_cid_after_nbytes; + const char *hcc_download_dir; char *hcc_sess_resume_file_name; @@ -487,6 +488,7 @@ struct lsquic_stream_ctx { * lsquic_stream_read* functions. */ unsigned count; + FILE *download_fh; struct lsquic_reader reader; }; @@ -552,6 +554,23 @@ http_client_on_new_stream (void *stream_if_ctx, lsquic_stream_t *stream) st_h->sh_flags |= ABANDON; } + if (st_h->client_ctx->hcc_download_dir) + { + char path[PATH_MAX]; + snprintf(path, sizeof(path), "%s/%s", + st_h->client_ctx->hcc_download_dir, st_h->path); + st_h->download_fh = fopen(path, "wb"); + if (st_h->download_fh) + LSQ_NOTICE("downloading %s to %s", st_h->path, path); + else + { + LSQ_ERROR("cannot open %s for writing: %s", path, strerror(errno)); + lsquic_stream_close(stream); + } + } + else + st_h->download_fh = NULL; + return st_h; } @@ -785,7 +804,8 @@ http_client_on_read (lsquic_stream_t *stream, lsquic_stream_ctx_t *st_h) st_h->sh_flags |= PROCESSED_HEADERS; } if (!s_discard_response) - fwrite(buf, 1, nread, stdout); + fwrite(buf, 1, nread, st_h->download_fh + ? st_h->download_fh : stdout); if (randomly_reprioritize_streams && (st_h->count++ & 0x3F) == 0) { if ((1 << lsquic_conn_quic_version(lsquic_stream_conn(stream))) @@ -887,6 +907,8 @@ http_client_on_close (lsquic_stream_t *stream, lsquic_stream_ctx_t *st_h) } if (st_h->reader.lsqr_ctx) destroy_lsquic_reader_ctx(st_h->reader.lsqr_ctx); + if (st_h->download_fh) + fclose(st_h->download_fh); free(st_h); } @@ -902,6 +924,65 @@ static struct lsquic_stream_if http_client_if = { }; +/* XXX This function assumes we can send the request in one shot. This is + * not a realistic assumption to make in general, but will work for our + * limited use case (QUIC Interop Runner). + */ +static void +hq_client_on_write (struct lsquic_stream *stream, lsquic_stream_ctx_t *st_h) +{ + if (st_h->client_ctx->payload) + { + LSQ_ERROR("payload is not supported in HQ client"); + lsquic_stream_close(stream); + return; + } + + lsquic_stream_write(stream, "GET ", 4); + lsquic_stream_write(stream, st_h->path, strlen(st_h->path)); + lsquic_stream_write(stream, "\r\n", 2); + lsquic_stream_shutdown(stream, 1); + lsquic_stream_wantread(stream, 1); +} + + +static size_t +hq_client_print_to_file (void *user_data, const unsigned char *buf, + size_t buf_len, int fin_unused) +{ + fwrite(buf, 1, buf_len, user_data); + return buf_len; +} + + +static void +hq_client_on_read (struct lsquic_stream *stream, lsquic_stream_ctx_t *st_h) +{ + FILE *out = st_h->download_fh ? st_h->download_fh : stdout; + ssize_t nread; + + nread = lsquic_stream_readf(stream, hq_client_print_to_file, out); + if (nread <= 0) + { + if (nread < 0) + LSQ_WARN("error reading response for %s: %s", st_h->path, + strerror(errno)); + lsquic_stream_close(stream); + } +} + + +/* The "hq" set of callbacks differs only in the read and write routines */ +static struct lsquic_stream_if hq_client_if = { + .on_new_conn = http_client_on_new_conn, + .on_conn_closed = http_client_on_conn_closed, + .on_new_stream = http_client_on_new_stream, + .on_read = hq_client_on_read, + .on_write = hq_client_on_write, + .on_close = http_client_on_close, +}; + + static void usage (const char *prog) { @@ -947,6 +1028,8 @@ usage (const char *prog) " -9 SPEC Priority specification. May be specified several times.\n" " SPEC takes the form stream_id:nread:UI, where U is\n" " urgency and I is incremental. Matched \\d+:\\d+:[0-7][01]\n" +" -7 DIR Save fetched resources into this directory.\n" +" -Q ALPN Use hq ALPN. Specify, for example, \"h3-29\".\n" , prog); } @@ -1535,6 +1618,8 @@ main (int argc, char **argv) "46Br:R:IKu:EP:M:n:w:H:p:0:q:e:hatT:b:d:" "3:" /* 3 is 133+ for "e" ("e" for "early") */ "9:" /* 9 sort of looks like P... */ + "7:" /* Download directory */ + "Q:" /* ALPN, e.g. h3-29 */ #ifndef WIN32 "C:" #endif @@ -1694,6 +1779,15 @@ main (int argc, char **argv) s_priority_specs = priority_specs; break; } + case '7': + client_ctx.hcc_download_dir = optarg; + break; + case 'Q': + /* XXX A bit hacky, as `prog' has already been initialized... */ + prog.prog_engine_flags &= ~LSENG_HTTP; + prog.prog_api.ea_alpn = optarg; + prog.prog_api.ea_stream_if = &hq_client_if; + break; default: if (0 != prog_set_opt(&prog, opt, optarg)) exit(1); diff --git a/bin/http_server.c b/bin/http_server.c index 452e68d..0404c97 100644 --- a/bin/http_server.c +++ b/bin/http_server.c @@ -1066,6 +1066,116 @@ const struct lsquic_stream_if http_server_if = { }; +/* XXX Assume we can always read the request in one shot. This is not a + * good assumption to make in a real product. + */ +static void +hq_server_on_read (struct lsquic_stream *stream, lsquic_stream_ctx_t *st_h) +{ + char buf[0x400]; + ssize_t nread; + char *path, *end, *filename; + + nread = lsquic_stream_read(stream, buf, sizeof(buf)); + if (nread >= (ssize_t) sizeof(buf)) + { + LSQ_WARN("request too large, at least %zd bytes", sizeof(buf)); + lsquic_stream_close(stream); + return; + } + else if (nread < 0) + { + LSQ_WARN("error reading request from stream: %s", strerror(errno)); + lsquic_stream_close(stream); + return; + } + buf[nread] = '\0'; + path = strchr(buf, ' '); + if (!path) + { + LSQ_WARN("invalid request (no space character): `%s'", buf); + lsquic_stream_close(stream); + return; + } + if (!(path - buf == 3 && 0 == strncasecmp(buf, "GET", 3))) + { + LSQ_NOTICE("unsupported method `%.*s'", (int) (path - buf), buf); + lsquic_stream_close(stream); + return; + } + ++path; + for (end = path + nread - 5; end > path + && (*end == '\r' || *end == '\n'); --end) + *end = '\0'; + LSQ_NOTICE("parsed out request path: %s", path); + + filename = malloc(strlen(st_h->server_ctx->document_root) + 1 + strlen(path) + 1); + strcpy(filename, st_h->server_ctx->document_root); + strcat(filename, "/"); + strcat(filename, path); + LSQ_NOTICE("file to fetch: %s", filename); + /* XXX This copy pasta is getting a bit annoying now: two mallocs of the + * same thing? + */ + st_h->req_filename = filename; + st_h->req_path = strdup(filename); + st_h->reader.lsqr_read = test_reader_read; + st_h->reader.lsqr_size = test_reader_size; + st_h->reader.lsqr_ctx = create_lsquic_reader_ctx(st_h->req_path); + if (!st_h->reader.lsqr_ctx) + { + lsquic_stream_close(stream); + return; + } + lsquic_stream_shutdown(stream, 0); + lsquic_stream_wantwrite(stream, 1); +} + + +static void +hq_server_on_write (struct lsquic_stream *stream, lsquic_stream_ctx_t *st_h) +{ + ssize_t nw; + + nw = lsquic_stream_writef(stream, &st_h->reader); + if (nw < 0) + { + struct lsquic_conn *conn = lsquic_stream_conn(stream); + lsquic_conn_ctx_t *conn_h = lsquic_conn_get_ctx(conn); + if (conn_h->flags & RECEIVED_GOAWAY) + { + LSQ_NOTICE("cannot write: goaway received"); + lsquic_stream_close(stream); + } + else + { + LSQ_ERROR("write error: %s", strerror(errno)); + lsquic_stream_close(stream); + } + } + else if (bytes_left(st_h) > 0) + { + st_h->written += (size_t) nw; + lsquic_stream_wantwrite(stream, 1); + } + else + { + lsquic_stream_shutdown(stream, 1); + lsquic_stream_wantread(stream, 1); + } +} + + +const struct lsquic_stream_if hq_server_if = { + .on_new_conn = http_server_on_new_conn, + .on_conn_closed = http_server_on_conn_closed, + .on_new_stream = http_server_on_new_stream, + .on_read = hq_server_on_read, + .on_write = hq_server_on_write, + .on_close = http_server_on_close, +}; + + #if HAVE_REGEX struct req_map { @@ -1655,6 +1765,7 @@ usage (const char *prog) " Incompatible with -w.\n" #endif " -y DELAY Delay response for this many seconds -- use for debugging\n" +" -Q ALPN Use hq mode; ALPN could be \"hq-29\", for example.\n" , prog); } @@ -1811,7 +1922,7 @@ main (int argc, char **argv) prog_init(&prog, LSENG_SERVER|LSENG_HTTP, &server_ctx.sports, &http_server_if, &server_ctx); - while (-1 != (opt = getopt(argc, argv, PROG_OPTS "y:Y:n:p:r:w:P:h"))) + while (-1 != (opt = getopt(argc, argv, PROG_OPTS "y:Y:n:p:r:w:P:hQ:"))) { switch (opt) { case 'n': @@ -1854,6 +1965,12 @@ main (int argc, char **argv) usage(argv[0]); prog_print_common_options(&prog, stdout); exit(0); + case 'Q': + /* XXX A bit hacky, as `prog' has already been initialized... */ + prog.prog_engine_flags &= ~LSENG_HTTP; + prog.prog_api.ea_stream_if = &hq_server_if; + add_alpn(optarg); + break; default: if (0 != prog_set_opt(&prog, opt, optarg)) exit(1); diff --git a/bin/test_common.c b/bin/test_common.c index 76e3ad5..8a595a1 100644 --- a/bin/test_common.c +++ b/bin/test_common.c @@ -947,7 +947,7 @@ sport_init_server (struct service_port *sport, struct lsquic_engine *engine, if (AF_INET == sa_local->sa_family) { #if __linux__ - on = IP_PMTUDISC_DO; + on = IP_PMTUDISC_PROBE; s = setsockopt(sockfd, IPPROTO_IP, IP_MTU_DISCOVER, &on, sizeof(on)); #else @@ -1136,7 +1136,7 @@ sport_init_client (struct service_port *sport, struct lsquic_engine *engine, { int on; #if __linux__ - on = IP_PMTUDISC_DO; + on = IP_PMTUDISC_PROBE; s = setsockopt(sockfd, IPPROTO_IP, IP_MTU_DISCOVER, &on, sizeof(on)); #elif WIN32 diff --git a/docs/conf.py b/docs/conf.py index 2569879..bb8850a 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.5' +release = u'2.27.6' # -- General configuration --------------------------------------------------- diff --git a/include/lsquic.h b/include/lsquic.h index 28c962e..f2643b6 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 5 +#define LSQUIC_PATCH_VERSION 6 /** * Engine flags: diff --git a/src/liblsquic/lsquic_full_conn.c b/src/liblsquic/lsquic_full_conn.c index b50d647..caace14 100644 --- a/src/liblsquic/lsquic_full_conn.c +++ b/src/liblsquic/lsquic_full_conn.c @@ -2615,10 +2615,12 @@ maybe_close_conn (struct full_conn *conn) struct lsquic_stream *stream; struct lsquic_hash_elem *el; #endif + const unsigned n_special_streams = N_SPECIAL_STREAMS + - !(conn->fc_flags & FC_HTTP); if ((conn->fc_flags & (FC_CLOSING|FC_GOAWAY_SENT|FC_SERVER)) == (FC_GOAWAY_SENT|FC_SERVER) - && lsquic_hash_count(conn->fc_pub.all_streams) == N_SPECIAL_STREAMS) + && lsquic_hash_count(conn->fc_pub.all_streams) == n_special_streams) { #ifndef NDEBUG for (el = lsquic_hash_first(conn->fc_pub.all_streams); el; diff --git a/src/liblsquic/lsquic_parse_ietf_v1.c b/src/liblsquic/lsquic_parse_ietf_v1.c index 0871e82..69c1542 100644 --- a/src/liblsquic/lsquic_parse_ietf_v1.c +++ b/src/liblsquic/lsquic_parse_ietf_v1.c @@ -1699,6 +1699,22 @@ static const enum header_type bits2ht[4] = }; +#if LSQUIC_QIR +/* Return true if the parsing function is to enforce the minimum DCID + * length requirement as specified in IETF v1 and the I-Ds. + */ +static int +enforce_initial_dcil (lsquic_ver_tag_t tag) +{ + enum lsquic_version version; + + version = lsquic_tag2ver(tag); + return version != (enum lsquic_version) -1 + && ((1 << version) & LSQUIC_IETF_VERSIONS); +} +#endif + + int lsquic_ietf_v1_parse_packet_in_long_begin (struct lsquic_packet_in *packet_in, size_t length, int is_server, unsigned cid_len, @@ -1751,6 +1767,14 @@ lsquic_ietf_v1_parse_packet_in_long_begin (struct lsquic_packet_in *packet_in, switch (header_type) { case HETY_INITIAL: +#if LSQUIC_QIR + if (!enforce_initial_dcil(tag)) + { + /* Count even zero-length DCID as having DCID */ + packet_in->pi_flags |= PI_CONN_ID; + } + else +#endif if (is_server && dcil < MIN_INITIAL_DCID_LEN) return -1; r = vint_read(p, end, &token_len); diff --git a/src/liblsquic/lsquic_stream.c b/src/liblsquic/lsquic_stream.c index 6929fdc..8a9580c 100644 --- a/src/liblsquic/lsquic_stream.c +++ b/src/liblsquic/lsquic_stream.c @@ -2363,12 +2363,13 @@ maybe_mark_as_blocked (lsquic_stream_t *stream) void lsquic_stream_dispatch_read_events (lsquic_stream_t *stream) { - assert(stream->sm_qflags & SMQF_WANT_READ); - - if (stream->sm_bflags & SMBF_RW_ONCE) - stream_dispatch_read_events_once(stream); - else - stream_dispatch_read_events_loop(stream); + if (stream->sm_qflags & SMQF_WANT_READ) + { + if (stream->sm_bflags & SMBF_RW_ONCE) + stream_dispatch_read_events_once(stream); + else + stream_dispatch_read_events_loop(stream); + } } @@ -2381,7 +2382,9 @@ lsquic_stream_dispatch_write_events (lsquic_stream_t *stream) unsigned short n_buffered; enum stream_q_flags q_flags; - assert(stream->sm_qflags & SMQF_WRITE_Q_FLAGS); + if (!(stream->sm_qflags & SMQF_WRITE_Q_FLAGS)) + return; + q_flags = stream->sm_qflags & SMQF_WRITE_Q_FLAGS; tosend_off = stream->tosend_off; n_buffered = stream->sm_n_buffered;