From e542fd9da10634bf11b1b6185a380712909dbc10 Mon Sep 17 00:00:00 2001
From: Stefan Eissing <stefan@eissing.org>
Date: Tue, 4 Mar 2025 12:31:32 +0100
Subject: [PATCH] http2: detect session being closed on ingress handling

nghttp2 will on its own send GOAWAY frames, closing the connection, when
internal processing of frames runs into errors. This may not become
visible in a direct error code from a call to nghttp2.

Check for session being closed on ingress processing (on sending, we
already did that) and report an error if so. In addition, monitor
outgoing GOAWAY not initiated by us so that the user will get a fail
message when that happens.

Add some more long response header tests.

Closes #16544
---
 lib/http2.c                 | 21 +++++++++++++++-----
 tests/http/test_01_basic.py | 39 +++++++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 5 deletions(-)

diff --git a/lib/http2.c b/lib/http2.c
index c1b7876227..21e034c293 100644
--- a/lib/http2.c
+++ b/lib/http2.c
@@ -648,10 +648,8 @@ static int h2_process_pending_input(struct Curl_cfilter *cf,
 
     rv = nghttp2_session_mem_recv(ctx->h2, (const uint8_t *)buf, blen);
     if(rv < 0) {
-      failf(data,
-            "process_pending_input: nghttp2_session_mem_recv() returned "
-            "%zd:%s", rv, nghttp2_strerror((int)rv));
-      *err = CURLE_RECV_ERROR;
+      failf(data, "nghttp2 error %zd: %s", rv, nghttp2_strerror((int)rv));
+      *err = CURLE_HTTP2;
       return -1;
     }
     Curl_bufq_skip(&ctx->inbufq, (size_t)rv);
@@ -1292,6 +1290,7 @@ static int on_frame_send(nghttp2_session *session, const nghttp2_frame *frame,
                          void *userp)
 {
   struct Curl_cfilter *cf = userp;
+  struct cf_h2_ctx *ctx = cf->ctx;
   struct Curl_easy *data = CF_DATA_CURRENT(cf);
 
   (void)session;
@@ -1303,6 +1302,13 @@ static int on_frame_send(nghttp2_session *session, const nghttp2_frame *frame,
     buffer[len] = 0;
     CURL_TRC_CF(data, cf, "[%d] -> %s", frame->hd.stream_id, buffer);
   }
+  if((frame->hd.type == NGHTTP2_GOAWAY) && !ctx->sent_goaway) {
+    /* A GOAWAY not initiated by us, but by nghttp2 itself on detecting
+     * a protocol error on the connection */
+    failf(data, "nghttp2 shuts down connection with error %d: %s",
+          frame->goaway.error_code,
+          nghttp2_http2_strerror(frame->goaway.error_code));
+  }
   return 0;
 }
 #endif /* !CURL_DISABLE_VERBOSE_STRINGS */
@@ -2036,6 +2042,11 @@ static CURLcode h2_progress_ingress(struct Curl_cfilter *cf,
   CURLcode result = CURLE_OK;
   ssize_t nread;
 
+  if(should_close_session(ctx)) {
+    CURL_TRC_CF(data, cf, "progress ingress, session is closed");
+    return CURLE_HTTP2;
+  }
+
   /* Process network input buffer fist */
   if(!Curl_bufq_is_empty(&ctx->inbufq)) {
     CURL_TRC_CF(data, cf, "Process %zu bytes in connection buffer",
@@ -2610,6 +2621,7 @@ static CURLcode cf_h2_shutdown(struct Curl_cfilter *cf,
   CF_DATA_SAVE(save, cf, data);
 
   if(!ctx->sent_goaway) {
+    ctx->sent_goaway = TRUE;
     rv = nghttp2_submit_goaway(ctx->h2, NGHTTP2_FLAG_NONE,
                                ctx->local_max_sid, 0,
                                (const uint8_t *)"shutdown",
@@ -2620,7 +2632,6 @@ static CURLcode cf_h2_shutdown(struct Curl_cfilter *cf,
       result = CURLE_SEND_ERROR;
       goto out;
     }
-    ctx->sent_goaway = TRUE;
   }
   /* GOAWAY submitted, process egress and ingress until nghttp2 is done. */
   result = CURLE_OK;
diff --git a/tests/http/test_01_basic.py b/tests/http/test_01_basic.py
index 1f4b66100a..5a4adfe889 100644
--- a/tests/http/test_01_basic.py
+++ b/tests/http/test_01_basic.py
@@ -203,3 +203,42 @@ class TestBasic:
             r.check_exit_code(16)  # CURLE_HTTP2
         else:
             r.check_exit_code(100)  # CURLE_TOO_LARGE
+
+    # http: several response headers, together > 256 KB
+    # nghttp2 error -905: Too many CONTINUATION frames following a HEADER frame
+    @pytest.mark.skipif(condition=not Env.httpd_is_at_least('2.4.64'),
+                        reason='httpd must be at least 2.4.64')
+    @pytest.mark.parametrize("proto", ['http/1.1', 'h2'])
+    def test_01_14_gigalarge_resp_headers(self, env: Env, httpd, proto):
+        httpd.set_extra_config('base', [
+            'LogLevel http2:trace2',
+            f'H2MaxHeaderBlockLen {1024 * 1024}',
+        ])
+        httpd.reload()
+        curl = CurlClient(env=env)
+        url = f'https://{env.authority_for(env.domain1, proto)}' \
+              f'/curltest/tweak?x-hd={256 * 1024}'
+        r = curl.http_get(url=url, alpn_proto=proto, extra_args=[])
+        if proto == 'h2':
+            r.check_exit_code(16)  # CURLE_HTTP2
+        else:
+            r.check_exit_code(0)   # 1.1 can do
+
+    # http: one response header > 256 KB
+    @pytest.mark.skipif(condition=not Env.httpd_is_at_least('2.4.64'),
+                        reason='httpd must be at least 2.4.64')
+    @pytest.mark.parametrize("proto", ['http/1.1', 'h2'])
+    def test_01_15_gigalarge_resp_headers(self, env: Env, httpd, proto):
+        httpd.set_extra_config('base', [
+            'LogLevel http2:trace2',
+            f'H2MaxHeaderBlockLen {1024 * 1024}',
+        ])
+        httpd.reload()
+        curl = CurlClient(env=env)
+        url = f'https://{env.authority_for(env.domain1, proto)}' \
+              f'/curltest/tweak?x-hd1={256 * 1024}'
+        r = curl.http_get(url=url, alpn_proto=proto, extra_args=[])
+        if proto == 'h2':
+            r.check_exit_code(16)  # CURLE_HTTP2
+        else:
+            r.check_exit_code(100)  # CURLE_TOO_LARGE
-- 
2.47.3