From 68447bcc24497665d1625d9a010c8148b41c4f86 Mon Sep 17 00:00:00 2001 From: Michael Montalbo Date: Tue, 30 Jun 2026 19:51:21 -0700 Subject: [PATCH 1/2] mod_http2: do not RST_STREAM header-only HTTP/2 responses A 204/304 (and HEAD) response carries no body and ends without a body EOS, so h2_beam_is_complete() reports the output beam as incomplete. s_c2_done() then RST_STREAM'd such a stream as if it were truncated, re-opening PR 69580 for body-less mod_cache 304 revalidations. Track in conn_ctx whether the final response is header-only (set from AP_STATUS_IS_HEADER_ONLY where the response passes out) and skip the incomplete-output reset for it in s_c2_done(). This mirrors the c1 output path, which already excludes header-only responses via AP_STATUS_IS_HEADER_ONLY(). Only a response that began a body and never finished it is reset. --- modules/http2/h2_c2.c | 1 + modules/http2/h2_c2_filter.c | 2 ++ modules/http2/h2_conn_ctx.h | 1 + modules/http2/h2_mplx.c | 8 ++++++- test/modules/http2/test_105_timeout.py | 32 ++++++++++++++++++++++++++ 5 files changed, 43 insertions(+), 1 deletion(-) diff --git a/modules/http2/h2_c2.c b/modules/http2/h2_c2.c index 79f76c0a70f..d8ecfb45a91 100644 --- a/modules/http2/h2_c2.c +++ b/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; } } diff --git a/modules/http2/h2_c2_filter.c b/modules/http2/h2_c2_filter.c index 17a2c131d8c..108f6c144fe 100644 --- a/modules/http2/h2_c2_filter.c +++ b/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); diff --git a/modules/http2/h2_conn_ctx.h b/modules/http2/h2_conn_ctx.h index 3b44856f95a..ac6decc36f4 100644 --- a/modules/http2/h2_conn_ctx.h +++ b/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 */ diff --git a/modules/http2/h2_mplx.c b/modules/http2/h2_mplx.c index c50e15ccd5d..a45e030cf98 100644 --- a/modules/http2/h2_mplx.c +++ b/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); diff --git a/test/modules/http2/test_105_timeout.py b/test/modules/http2/test_105_timeout.py index 22160b45853..16dd7ba6ea1 100644 --- a/test/modules/http2/test_105_timeout.py +++ b/test/modules/http2/test_105_timeout.py @@ -164,3 +164,35 @@ def test_h2_105_12(self, env): piper.close() assert piper.response, f'{piper}' assert piper.response['status'] == 408, f"{piper.response}" + + # 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}' From 29c2d2cc3e1761e59b2565b6b76b3c4e7cfcb8db Mon Sep 17 00:00:00 2001 From: Michael Montalbo Date: Tue, 30 Jun 2026 19:51:44 -0700 Subject: [PATCH 2/2] mod_http2: RST_STREAM an incomplete response on the legacy c2 path On the legacy c2 path, c2_process() always h2_beam_close()d the output beam when the handler returned. A response whose body never completed, e.g. a CGI that sent status, headers and a partial body and then timed out, was closed with no EOS. h2_beam_is_complete() then reported it complete, so no RST_STREAM was sent and the client hung waiting for data that never came. Abort the beam instead when a final response was started but never completed and is not header-only, the same condition s_c2_done() uses. Header-only responses (204/304/HEAD) are still closed normally; only genuinely truncated responses are reset. --- modules/http2/h2_c2.c | 15 +++++++- test/modules/http2/htdocs/cgi/h2cgi_slow.py | 13 +++++++ test/modules/http2/test_105_timeout.py | 38 ++++++++++++++++++++- 3 files changed, 64 insertions(+), 2 deletions(-) create mode 100755 test/modules/http2/htdocs/cgi/h2cgi_slow.py diff --git a/modules/http2/h2_c2.c b/modules/http2/h2_c2.c index d8ecfb45a91..e442a55dff6 100644 --- a/modules/http2/h2_c2.c +++ b/modules/http2/h2_c2.c @@ -805,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, diff --git a/test/modules/http2/htdocs/cgi/h2cgi_slow.py b/test/modules/http2/htdocs/cgi/h2cgi_slow.py new file mode 100755 index 00000000000..e48588c7f50 --- /dev/null +++ b/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) diff --git a/test/modules/http2/test_105_timeout.py b/test/modules/http2/test_105_timeout.py index 16dd7ba6ea1..8031590c243 100644 --- a/test/modules/http2/test_105_timeout.py +++ b/test/modules/http2/test_105_timeout.py @@ -1,5 +1,6 @@ import socket import time +from datetime import timedelta import pytest @@ -165,7 +166,42 @@ def test_h2_105_12(self, env): assert piper.response, f'{piper}' assert piper.response['status'] == 408, f"{piper.response}" - # A body-less response that is missing an EOS + # 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: