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

Roman Arutyunyan arut at nginx.com
Mon Oct 18 12:48:28 UTC 2021


# HG changeset patch
# User Roman Arutyunyan <arut at 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;
     }
 
     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;
         }
 


More information about the nginx-devel mailing list