Welcome! Log In Create A New Profile

Advanced

Re: [PATCH 1 of 3] HTTP/3: adjusted QUIC connection finalization

Sergey Kandaurov
November 08, 2021 11:26AM
On Mon, Oct 18, 2021 at 03:48:28PM +0300, Roman Arutyunyan wrote:
> # HG changeset patch
> # User Roman Arutyunyan <arut@nginx.com>
> # Date 1634559753 -10800
> # Mon Oct 18 15:22:33 2021 +0300
> # Branch quic
> # Node ID 8739f475583031399879ef0af2eb5af76008449e
> # Parent 404de224517e33f685613d6425dcdb3c8ef5b97e
> HTTP/3: adjusted QUIC connection finalization.
>
> When an HTTP/3 function returns an error in context of a QUIC stream, it's
> this function's responsibility now to finalize the entire QUIC connection
> with the right code, if required. Previously, QUIC connection finalization
> could be done both outside and inside such functions. The new rule follows
> a similar rule for logging, leads to cleaner code, and allows to provide more
> details about the error.
>
> While here, a few error cases are no longer treated as fatal and QUIC connection
> is no longer finalized in these cases. A few other cases now lead to
> stream reset instead of connection finalization.
>
> diff --git a/src/http/v3/ngx_http_v3.c b/src/http/v3/ngx_http_v3.c
> --- a/src/http/v3/ngx_http_v3.c
> +++ b/src/http/v3/ngx_http_v3.c
> @@ -33,7 +33,7 @@ ngx_http_v3_init_session(ngx_connection_
>
> h3c = ngx_pcalloc(pc->pool, sizeof(ngx_http_v3_session_t));
> if (h3c == NULL) {
> - return NGX_ERROR;
> + goto failed;
> }
>
> h3c->max_push_id = (uint64_t) -1;
> @@ -49,7 +49,7 @@ ngx_http_v3_init_session(ngx_connection_
>
> cln = ngx_pool_cleanup_add(pc->pool, 0);
> if (cln == NULL) {
> - return NGX_ERROR;
> + goto failed;
> }
>
> cln->handler = ngx_http_v3_cleanup_session;
> @@ -58,6 +58,14 @@ ngx_http_v3_init_session(ngx_connection_
> hc->v3_session = h3c;
>
> return ngx_http_v3_send_settings(c);
> +
> +failed:
> +
> + ngx_log_error(NGX_LOG_ERR, c->log, 0, "failed to create http3 session");
> +
> + ngx_http_v3_finalize_connection(c, NGX_HTTP_V3_ERR_INTERNAL_ERROR,
> + "failed to create http3 session");
> + return NGX_ERROR;
> }
>
>
> diff --git a/src/http/v3/ngx_http_v3_request.c b/src/http/v3/ngx_http_v3_request.c
> --- a/src/http/v3/ngx_http_v3_request.c
> +++ b/src/http/v3/ngx_http_v3_request.c
> @@ -65,8 +65,6 @@ ngx_http_v3_init(ngx_connection_t *c)
> ngx_http_core_srv_conf_t *cscf;
>
> if (ngx_http_v3_init_session(c) != NGX_OK) {
> - ngx_http_v3_finalize_connection(c, NGX_HTTP_V3_ERR_INTERNAL_ERROR,
> - "internal error");
> ngx_http_close_connection(c);
> return;
> }
> @@ -110,8 +108,6 @@ ngx_http_v3_init(ngx_connection_t *c)
> h3c->goaway = 1;
>
> if (ngx_http_v3_send_goaway(c, (n + 1) << 2) != NGX_OK) {
> - ngx_http_v3_finalize_connection(c, NGX_HTTP_V3_ERR_INTERNAL_ERROR,
> - "goaway error");
> ngx_http_close_connection(c);
> return;
> }
> @@ -287,15 +283,14 @@ ngx_http_v3_process_request(ngx_event_t
> rc = ngx_http_v3_parse_headers(c, st, b);
>
> if (rc > 0) {
> - ngx_http_v3_finalize_connection(c, rc,
> - "could not parse request headers");
> + ngx_quic_reset_stream(c, rc);
> + ngx_log_error(NGX_LOG_ERR, r->connection->log, 0,
> + "client sent invalid header");
> ngx_http_finalize_request(r, NGX_HTTP_BAD_REQUEST);
> break;
> }
>
> if (rc == NGX_ERROR) {
> - ngx_http_v3_finalize_connection(c, NGX_HTTP_V3_ERR_INTERNAL_ERROR,
> - "internal error");
> ngx_http_close_request(r, NGX_HTTP_INTERNAL_SERVER_ERROR);
> break;
> }
> @@ -1167,17 +1162,13 @@ ngx_http_v3_request_body_filter(ngx_http
> }
>
> if (rc > 0) {
> - ngx_http_v3_finalize_connection(r->connection, rc,
> - "client sent invalid body");
> + ngx_quic_reset_stream(r->connection, rc);
> ngx_log_error(NGX_LOG_ERR, r->connection->log, 0,
> "client sent invalid body");
> return NGX_HTTP_BAD_REQUEST;
> }
>
> if (rc == NGX_ERROR) {
> - ngx_http_v3_finalize_connection(r->connection,
> - NGX_HTTP_V3_ERR_INTERNAL_ERROR,
> - "internal error");
> return NGX_HTTP_INTERNAL_SERVER_ERROR;
> }
>
> diff --git a/src/http/v3/ngx_http_v3_streams.c b/src/http/v3/ngx_http_v3_streams.c
> --- a/src/http/v3/ngx_http_v3_streams.c
> +++ b/src/http/v3/ngx_http_v3_streams.c
> @@ -283,7 +283,7 @@ ngx_http_v3_create_push_stream(ngx_conne
>
> sc = ngx_quic_open_stream(c, 0);
> if (sc == NULL) {
> - return NULL;
> + goto failed;
> }
>
> p = buf;
> @@ -318,7 +318,13 @@ ngx_http_v3_create_push_stream(ngx_conne
>
> failed:
>
> - ngx_http_v3_close_uni_stream(sc);
> + ngx_log_error(NGX_LOG_ERR, c->log, 0, "failed to create push stream");
> +
> + ngx_http_v3_finalize_connection(c, NGX_HTTP_V3_ERR_STREAM_CREATION_ERROR,
> + "failed to create push stream");
> + if (sc) {
> + ngx_http_v3_close_uni_stream(sc);
> + }
>
> return NULL;
> }
> @@ -368,7 +374,7 @@ ngx_http_v3_get_uni_stream(ngx_connectio
>
> sc = ngx_quic_open_stream(c, 0);
> if (sc == NULL) {
> - return NULL;
> + goto failed;
> }
>
> sc->quic->cancelable = 1;
> @@ -405,7 +411,13 @@ ngx_http_v3_get_uni_stream(ngx_connectio
>
> failed:
>
> - ngx_http_v3_close_uni_stream(sc);
> + ngx_log_error(NGX_LOG_ERR, c->log, 0, "failed to create server stream");
> +
> + ngx_http_v3_finalize_connection(c, NGX_HTTP_V3_ERR_STREAM_CREATION_ERROR,
> + "failed to create server stream");
> + if (sc) {
> + ngx_http_v3_close_uni_stream(sc);
> + }
>
> return NULL;
> }
> @@ -424,7 +436,7 @@ ngx_http_v3_send_settings(ngx_connection
>
> cc = ngx_http_v3_get_uni_stream(c, NGX_HTTP_V3_STREAM_CONTROL);
> if (cc == NULL) {
> - return NGX_DECLINED;
> + return NGX_ERROR;
> }
>
> h3scf = ngx_http_v3_get_module_srv_conf(c, ngx_http_v3_module);
> @@ -457,6 +469,10 @@ ngx_http_v3_send_settings(ngx_connection
>
> failed:
>
> + ngx_log_error(NGX_LOG_ERR, c->log, 0, "failed to send settings");
> +
> + ngx_http_v3_finalize_connection(c, NGX_HTTP_V3_ERR_EXCESSIVE_LOAD,
> + "failed to send settings");
> ngx_http_v3_close_uni_stream(cc);
>
> return NGX_ERROR;
> @@ -475,7 +491,7 @@ ngx_http_v3_send_goaway(ngx_connection_t
>
> cc = ngx_http_v3_get_uni_stream(c, NGX_HTTP_V3_STREAM_CONTROL);
> if (cc == NULL) {
> - return NGX_DECLINED;
> + return NGX_ERROR;

This case now misses sending CONNECTION_CLOSE (no further stream processing),
Instead, the last allowed stream connection is now closed while initializing
with sending FIN with off:0 in its cleanup function which confuses clients
due to H3_FRAME_UNEXPECTED. It probably needs to be converted to goto failed
with conditional uni stream close.

Same in ngx_http_v3_send_settings() around ngx_http_v3_get_uni_stream(), but
there it just leads to control stream silent closure with no visible effects
on the wire. Yet, with teh 2nd patch applied that updates stream reset logic,
the error causes to emit RESET_STREAM(H3_INTERNAL_ERROR), which is a protocol
violation when operating on a critical (control) stream.

> }
>
> n = ngx_http_v3_encode_varlen_int(NULL, id);
> @@ -495,6 +511,10 @@ ngx_http_v3_send_goaway(ngx_connection_t
>
> failed:
>
> + ngx_log_error(NGX_LOG_ERR, c->log, 0, "failed to send goaway");
> +
> + ngx_http_v3_finalize_connection(c, NGX_HTTP_V3_ERR_EXCESSIVE_LOAD,
> + "failed to send goaway");
> ngx_http_v3_close_uni_stream(cc);
>
> return NGX_ERROR;
> @@ -510,7 +530,7 @@ ngx_http_v3_send_ack_section(ngx_connect
> ngx_http_v3_session_t *h3c;
>
> ngx_log_debug1(NGX_LOG_DEBUG_HTTP, c->log, 0,
> - "http3 client ack section %ui", stream_id);
> + "http3 send section acknowledgement %ui", stream_id);
>
> dc = ngx_http_v3_get_uni_stream(c, NGX_HTTP_V3_STREAM_DECODER);
> if (dc == NULL) {
> @@ -524,11 +544,21 @@ ngx_http_v3_send_ack_section(ngx_connect
> h3c->total_bytes += n;
>
> if (dc->send(dc, buf, n) != (ssize_t) n) {
> - ngx_http_v3_close_uni_stream(dc);
> - return NGX_ERROR;
> + goto failed;
> }
>
> return NGX_OK;
> +
> +failed:
> +
> + ngx_log_error(NGX_LOG_ERR, c->log, 0,
> + "failed to send section acknowledgement");
> +
> + ngx_http_v3_finalize_connection(c, NGX_HTTP_V3_ERR_EXCESSIVE_LOAD,
> + "failed to send section acknowledgement");
> + ngx_http_v3_close_uni_stream(dc);
> +
> + return NGX_ERROR;
> }
>
>
> @@ -541,7 +571,7 @@ ngx_http_v3_send_cancel_stream(ngx_conne
> ngx_http_v3_session_t *h3c;
>
> ngx_log_debug1(NGX_LOG_DEBUG_HTTP, c->log, 0,
> - "http3 client cancel stream %ui", stream_id);
> + "http3 send stream cancellation %ui", stream_id);
>
> dc = ngx_http_v3_get_uni_stream(c, NGX_HTTP_V3_STREAM_DECODER);
> if (dc == NULL) {
> @@ -555,11 +585,20 @@ ngx_http_v3_send_cancel_stream(ngx_conne
> h3c->total_bytes += n;
>
> if (dc->send(dc, buf, n) != (ssize_t) n) {
> - ngx_http_v3_close_uni_stream(dc);
> - return NGX_ERROR;
> + goto failed;
> }
>
> return NGX_OK;
> +
> +failed:
> +
> + ngx_log_error(NGX_LOG_ERR, c->log, 0, "failed to send stream cancellation");
> +
> + ngx_http_v3_finalize_connection(c, NGX_HTTP_V3_ERR_EXCESSIVE_LOAD,
> + "failed to send stream cancellation");
> + ngx_http_v3_close_uni_stream(dc);
> +
> + return NGX_ERROR;
> }
>
>
> @@ -572,7 +611,7 @@ ngx_http_v3_send_inc_insert_count(ngx_co
> ngx_http_v3_session_t *h3c;
>
> ngx_log_debug1(NGX_LOG_DEBUG_HTTP, c->log, 0,
> - "http3 client increment insert count %ui", inc);
> + "http3 send insert count increment %ui", inc);
>
> dc = ngx_http_v3_get_uni_stream(c, NGX_HTTP_V3_STREAM_DECODER);
> if (dc == NULL) {
> @@ -586,11 +625,21 @@ ngx_http_v3_send_inc_insert_count(ngx_co
> h3c->total_bytes += n;
>
> if (dc->send(dc, buf, n) != (ssize_t) n) {
> - ngx_http_v3_close_uni_stream(dc);
> - return NGX_ERROR;
> + goto failed;
> }
>
> return NGX_OK;
> +
> +failed:
> +
> + ngx_log_error(NGX_LOG_ERR, c->log, 0,
> + "failed to send insert count increment");
> +
> + ngx_http_v3_finalize_connection(c, NGX_HTTP_V3_ERR_EXCESSIVE_LOAD,
> + "failed to send insert count increment");
> + ngx_http_v3_close_uni_stream(dc);
> +
> + return NGX_ERROR;
> }
>
>
> diff --git a/src/http/v3/ngx_http_v3_tables.c b/src/http/v3/ngx_http_v3_tables.c
> --- a/src/http/v3/ngx_http_v3_tables.c
> +++ b/src/http/v3/ngx_http_v3_tables.c
> @@ -589,6 +589,10 @@ ngx_http_v3_check_insert_count(ngx_conne
> if (h3c->nblocked == h3scf->max_blocked_streams) {
> ngx_log_error(NGX_LOG_INFO, c->log, 0,
> "client exceeded http3_max_blocked_streams limit");
> +
> + ngx_http_v3_finalize_connection(c,
> + NGX_HTTP_V3_ERR_DECOMPRESSION_FAILED,
> + "too many blocked streams");
> return NGX_HTTP_V3_ERR_DECOMPRESSION_FAILED;
> }
>
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel
Subject Author Views Posted

[PATCH 0 of 3] HTTP/3 Stream Cancellation and friends

Roman Arutyunyan 333 October 18, 2021 08:50AM

[PATCH 1 of 3] HTTP/3: adjusted QUIC connection finalization

Roman Arutyunyan 91 October 18, 2021 08:50AM

Re: [PATCH 1 of 3] HTTP/3: adjusted QUIC connection finalization

Sergey Kandaurov 83 November 08, 2021 11:26AM

Re: [PATCH 1 of 3] HTTP/3: adjusted QUIC connection finalization

Roman Arutyunyan 124 November 09, 2021 02:34AM

Re: [PATCH 1 of 3] HTTP/3: adjusted QUIC connection finalization

Sergey Kandaurov 79 November 09, 2021 11:02AM

Re: [PATCH 1 of 3] HTTP/3: adjusted QUIC connection finalization

Roman Arutyunyan 76 November 10, 2021 07:22AM

[PATCH 2 of 3] HTTP/3: allowed QUIC stream connection reuse

Roman Arutyunyan 118 October 18, 2021 08:50AM

Re: [PATCH 2 of 3] HTTP/3: allowed QUIC stream connection reuse

Sergey Kandaurov 116 November 10, 2021 08:02AM

Re: [PATCH 2 of 3] HTTP/3: allowed QUIC stream connection reuse

Roman Arutyunyan 129 November 10, 2021 04:44PM

Re: [PATCH 2 of 3] HTTP/3: allowed QUIC stream connection reuse

Sergey Kandaurov 91 November 11, 2021 06:50AM

Re: [PATCH 2 of 3] HTTP/3: allowed QUIC stream connection reuse

Roman Arutyunyan 71 November 15, 2021 07:36AM

Re: [PATCH 2 of 3] HTTP/3: allowed QUIC stream connection reuse

Vladimir Homutov 68 November 16, 2021 04:20AM

Re: [PATCH 2 of 3] HTTP/3: allowed QUIC stream connection reuse

Roman Arutyunyan 192 November 17, 2021 02:14AM

Re: [PATCH 2 of 3] HTTP/3: allowed QUIC stream connection reuse

Vladimir Homutov 135 November 17, 2021 03:24AM

[PATCH 3 of 3] HTTP/3: send Stream Cancellation instruction

Roman Arutyunyan 94 October 18, 2021 08:50AM

Re: [PATCH 3 of 3] HTTP/3: send Stream Cancellation instruction

Sergey Kandaurov 124 November 10, 2021 11:00AM



Sorry, you do not have permission to post/reply in this forum.

Online Users

Guests: 79
Record Number of Users: 6 on February 13, 2018
Record Number of Guests: 421 on December 02, 2018
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready