From 60564b23132b3ec9e558e290b7abc3a5a3cd1705 Mon Sep 17 00:00:00 2001 From: Roman Arutyunyan Date: Mon, 18 Oct 2021 15:22:33 +0300 Subject: [PATCH] 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. --- src/http/v3/ngx_http_v3.c | 12 ++++- src/http/v3/ngx_http_v3_request.c | 17 ++----- src/http/v3/ngx_http_v3_streams.c | 79 +++++++++++++++++++++++++------ src/http/v3/ngx_http_v3_tables.c | 4 ++ 4 files changed, 82 insertions(+), 30 deletions(-) diff --git a/src/http/v3/ngx_http_v3.c b/src/http/v3/ngx_http_v3.c index 867f3c25d..8850b2879 100644 --- 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_t *c) 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_t *c) 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_t *c) 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 index bb9a72248..5c905bc3a 100644 --- 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 *rev) 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_request_t *r, ngx_chain_t *in) } 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 index d30069923..934b9dd4c 100644 --- 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_connection_t *c, uint64_t push_id) sc = ngx_quic_open_stream(c, 0); if (sc == NULL) { - return NULL; + goto failed; } p = buf; @@ -320,7 +320,13 @@ ngx_http_v3_create_push_stream(ngx_connection_t *c, uint64_t push_id) 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; } @@ -370,7 +376,7 @@ ngx_http_v3_get_uni_stream(ngx_connection_t *c, ngx_uint_t type) sc = ngx_quic_open_stream(c, 0); if (sc == NULL) { - return NULL; + goto failed; } sc->quic->cancelable = 1; @@ -409,7 +415,13 @@ ngx_http_v3_get_uni_stream(ngx_connection_t *c, ngx_uint_t type) 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; } @@ -428,7 +440,7 @@ ngx_http_v3_send_settings(ngx_connection_t *c) 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); @@ -463,6 +475,10 @@ ngx_http_v3_send_settings(ngx_connection_t *c) 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; @@ -481,7 +497,7 @@ ngx_http_v3_send_goaway(ngx_connection_t *c, uint64_t id) cc = ngx_http_v3_get_uni_stream(c, NGX_HTTP_V3_STREAM_CONTROL); if (cc == NULL) { - return NGX_DECLINED; + return NGX_ERROR; } n = ngx_http_v3_encode_varlen_int(NULL, id); @@ -503,6 +519,10 @@ ngx_http_v3_send_goaway(ngx_connection_t *c, uint64_t id) 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; @@ -518,7 +538,7 @@ ngx_http_v3_send_ack_section(ngx_connection_t *c, ngx_uint_t stream_id) 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) { @@ -534,11 +554,21 @@ ngx_http_v3_send_ack_section(ngx_connection_t *c, ngx_uint_t stream_id) 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; } @@ -551,7 +581,7 @@ ngx_http_v3_send_cancel_stream(ngx_connection_t *c, ngx_uint_t stream_id) 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) { @@ -567,11 +597,20 @@ ngx_http_v3_send_cancel_stream(ngx_connection_t *c, ngx_uint_t stream_id) 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; } @@ -584,7 +623,7 @@ ngx_http_v3_send_inc_insert_count(ngx_connection_t *c, ngx_uint_t inc) 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) { @@ -600,11 +639,21 @@ ngx_http_v3_send_inc_insert_count(ngx_connection_t *c, ngx_uint_t inc) 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 index 348088774..c6d543ac4 100644 --- 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_connection_t *c, ngx_uint_t insert_count) 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; }