mod_http2: reset truncated HTTP/2 responses; spare header-only ones

modules/http2/h2_c2.c

@@ -397,6 +397,7 @@ static apr_status_t h2_c2_filter_out(ap_filter_t* f, apr_bucket_brigade* bb)
                 ap_bucket_response *resp = e->data;
                 if (resp->status >= HTTP_OK) {
                     conn_ctx->has_final_response = 1;
+                    conn_ctx->header_only = AP_STATUS_IS_HEADER_ONLY(resp->status);
                     break;
                 }
             }
@@ -804,7 +805,20 @@ static apr_status_t c2_process(h2_conn_ctx_t *conn_ctx, conn_rec *c)
      * request pool may have been deleted. */
     r = NULL;
     if (conn_ctx->beam_out) {
-        h2_beam_close(conn_ctx->beam_out, c);
+        if (conn_ctx->has_final_response
+            && !h2_beam_is_complete(conn_ctx->beam_out)
+            && !conn_ctx->header_only) {
+            /* A final response was started but its body never completed (no
+             * EOS), e.g. the CGI/handler timed out mid-body. Abort so the
+             * stream is RST_STREAM'd. Closing here would mark the beam complete
+             * and s_c2_done() would see nothing to reset, leaving the client
+             * hanging. Header-only responses (204/304/HEAD) carry no body EOS
+             * and are closed normally, not reset (PR 69580). */
+            h2_beam_abort(conn_ctx->beam_out, c);
+        }
+        else {
+            h2_beam_close(conn_ctx->beam_out, c);
+        }
     }
 
     ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, c,

modules/http2/h2_c2_filter.c

@@ -549,6 +549,7 @@ static apr_status_t pass_response(h2_conn_ctx_t *conn_ctx, ap_filter_t *f,
 
     if (response->status >= HTTP_OK) {
         conn_ctx->has_final_response = 1;
+        conn_ctx->header_only = AP_STATUS_IS_HEADER_ONLY(response->status);
     }
     ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, parser->c,
                   APLOGNO(03197) "h2_c2(%s): passed response %d",
@@ -757,6 +758,7 @@ apr_status_t h2_c2_filter_response_out(ap_filter_t *f, apr_bucket_brigade *bb)
     }
 
     if (r->header_only || AP_STATUS_IS_HEADER_ONLY(r->status)) {
+        conn_ctx->header_only = 1;
         ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, f->c,
                       "h2_c2(%s): headers only, cleanup output brigade", conn_ctx->id);
         b = body_bucket? body_bucket : APR_BRIGADE_FIRST(bb);

modules/http2/h2_conn_ctx.h

@@ -60,6 +60,7 @@ struct h2_conn_ctx_t {
     apr_pollfd_t pfd;                /* c1: poll socket input, c2: NUL */
 
     int has_final_response;          /* final HTTP response passed on out */
+    int header_only;                 /* c2: final response is header-only (204/304/HEAD), no body EOS will come */
     apr_status_t last_err;           /* APR_SUCCES or last error encountered in filters */
 
     apr_off_t bytes_sent;            /* c2: bytes acutaly sent via c1 */

modules/http2/h2_mplx.c

@@ -1005,7 +1005,13 @@ static void s_c2_done(h2_mplx *m, conn_rec *c2, h2_conn_ctx_t *conn_ctx)
         if (conn_ctx->beam_out)
           h2_beam_abort(conn_ctx->beam_out, c2);
     }
-    else if (!conn_ctx->beam_out || !h2_beam_is_complete(conn_ctx->beam_out)) {
+    else if (!conn_ctx->beam_out
+             || (!h2_beam_is_complete(conn_ctx->beam_out)
+                 && !conn_ctx->header_only)) {
+        /* A header-only response (204/304/HEAD) produces no body EOS, so an
+         * incomplete out beam is expected for it, not a truncation. This is the
+         * same case the c1 output path skips via AP_STATUS_IS_HEADER_ONLY().
+         * Only reset a response that began a body and never finished it. */
         ap_log_cerror(APLOG_MARK, APLOG_TRACE1, conn_ctx->last_err, c2,
                       "h2_c2(%s-%d): processing finished with incomplete output",
                       conn_ctx->id, conn_ctx->stream_id);

test/modules/http2/htdocs/cgi/h2cgi_slow.py

@@ -0,0 +1,13 @@
+#!/usr/bin/env python3
+# Start a response (status + headers + a little body), then go silent for
+# longer than the server's `Timeout`. Over HTTP/2 this makes Apache's content
+# filter time out reading from the CGI (AH01220): the response was started but
+# never completed (no EOS). A correct server RST_STREAMs the stream; a buggy
+# one leaves the client hanging. Used by test_105_timeout.py.
+import sys
+import time
+
+sys.stdout.write("Content-Type: application/octet-stream\r\n\r\n")
+sys.stdout.write("X" * 16384)
+sys.stdout.flush()
+time.sleep(30)

test/modules/http2/test_105_timeout.py

@@ -1,5 +1,6 @@
 import socket
 import time
+from datetime import timedelta
 
 import pytest
 
@@ -164,3 +165,70 @@ def test_h2_105_12(self, env):
         piper.close()
         assert piper.response, f'{piper}'
         assert piper.response['status'] == 408, f"{piper.response}"
+
+    # A CGI that starts a response (status + headers + some body) and then goes
+    # silent past `Timeout` must lead to the HTTP/2 stream being RST_STREAM'd,
+    # not leave the client hanging. Regression test for the mod_http2 hang where
+    # a c2 that finished an *incomplete* response (no EOS, e.g. CGI timed out
+    # mid-body) was treated as complete and no reset was ever sent.
+    def test_h2_105_20(self, env):
+        conf = H2Conf(env)
+        conf.add("Timeout 1")
+        conf.add_vhost_cgi()
+        conf.install()
+        assert env.apache_restart() == 0
+        url = env.mkurl("https", "cgi", "/h2cgi_slow.py")
+        # Open many concurrent streams on one h2 connection: the missed reset
+        # is racy per stream, so a single stream reproduces the hang only
+        # sometimes, but many in flight hit it reliably. Key on elapsed time,
+        # not curl's exit code (--parallel makes that ambiguous): a hang is a
+        # multi-second stall, a correct reset a sub-second teardown. --max-time
+        # bounds the client so a hang cannot wedge the test. 10 is well under
+        # the default 100 max concurrent streams and leaves margin on fast CI.
+        count = 10
+        max_time = 10
+        r = env.curl_raw([url] * count, options=[
+            "--parallel", "--parallel-immediate", "--max-time", str(max_time)])
+        assert r.duration < timedelta(seconds=max_time - 2), \
+            f'streams hung waiting for RST_STREAM (batch took {r.duration}): {r}'
+        # Resetting each silent CGI is logged as a CGI timeout (cgid) and the
+        # failed body read that follows it (core); both are expected here.
+        time.sleep(1)  # let the log flush
+        env.httpd_error_log.ignore_recent(
+            lognos = [
+                "AH01220",  # cgid: Timeout waiting for output from CGI script
+                "AH00574",  # core: ap_content_length_filter, apr_bucket_read() failed
+            ]
+        )
+
+    # Complement to test_h2_105_20: a body-less response that is missing an EOS
+    # must NOT be reset over HTTP/2. mod_cache revalidation of a cached,
+    # immediately-stale resource emits exactly such a 304 (EOR + Flush, no EOS).
+    # This guards the incomplete-response reset against re-opening PR 69580:
+    # only a response that began a body and is missing EOS should be
+    # reset, never a header-only one.
+    def test_h2_105_21(self, env):
+        cacheroot = f"{env.server_dir}/cacheroot"
+        env.mkpath(cacheroot)
+        conf = H2Conf(env)
+        conf.add(f"""
+            CacheRoot "{cacheroot}"
+            CacheEnable disk /
+            Header set Cache-Control "public, max-age=0"
+            """)
+        conf.add_vhost_test1()
+        conf.install()
+        assert env.apache_restart() == 0
+        url = env.mkurl("https", "test1", "/006/006.css")
+        # prime the cache (stored, immediately stale via max-age=0)
+        r = env.curl_get(url)
+        assert r.exit_code == 0, f'{r}'
+        assert r.response["status"] == 200
+        lm = r.response["header"]["last-modified"]
+        # revalidate: mod_cache freshens the stale entry and the origin returns a
+        # body-less 304. The h2 stream must close cleanly; a regression shows up
+        # as a curl exit (92, "stream not closed cleanly"), not as a 304.
+        r = env.curl_get(url, options=["-H", "Cache-Control: max-age=0",
+                                       "-H", f"if-modified-since: {lm}"])
+        assert r.exit_code == 0, f'304 stream was reset (curl {r.exit_code}): {r}'
+        assert r.response["status"] == 304, f'{r.response}'