Skip to content

Commit e68020e

Browse files
committed
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() reports such a closed beam complete, so s_c2_done() sends no reset. The only remaining reset, stream_data_cb() on APR_EOF, is starved: h2_beam_receive() returns APR_EOF only on a receive that transfers zero buckets from a closed beam. The single wakeup at close drives one receive, which usually transfers the response's trailing flush and returns APR_SUCCESS, so the stream re-suspends; no further wakeup follows and it parks, and the client hangs waiting for data that never comes. Abort the beam instead when a final response was started but never completed and is not header-only, the same test s_c2_done() applies. An aborted beam returns APR_ECONNABORTED from the top of h2_beam_receive(), before the transfer, so the terminal is observed on that one receive regardless of any trailing buckets. Header-only responses (204/304/HEAD) are still closed normally; only genuinely truncated responses are reset.
1 parent 68447bc commit e68020e

3 files changed

Lines changed: 68 additions & 2 deletions

File tree

‎modules/http2/h2_c2.c‎

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -805,7 +805,20 @@ static apr_status_t c2_process(h2_conn_ctx_t *conn_ctx, conn_rec *c)
805805
* request pool may have been deleted. */
806806
r = NULL;
807807
if (conn_ctx->beam_out) {
808-
h2_beam_close(conn_ctx->beam_out, c);
808+
if (conn_ctx->has_final_response
809+
&& !h2_beam_is_complete(conn_ctx->beam_out)
810+
&& !conn_ctx->header_only) {
811+
/* A final response was started but its body never completed (no
812+
* EOS), e.g. the CGI/handler timed out mid-body. Abort so the
813+
* stream is RST_STREAM'd. Closing here would mark the beam complete
814+
* and s_c2_done() would see nothing to reset, leaving the client
815+
* hanging. Header-only responses (204/304/HEAD) carry no body EOS
816+
* and are closed normally, not reset (PR 69580). */
817+
h2_beam_abort(conn_ctx->beam_out, c);
818+
}
819+
else {
820+
h2_beam_close(conn_ctx->beam_out, c);
821+
}
809822
}
810823

811824
ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, c,
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
#!/usr/bin/env python3
2+
# Start a response (status + headers + a little body), then go silent for
3+
# longer than the server's `Timeout`. Over HTTP/2 this makes Apache's content
4+
# filter time out reading from the CGI (AH01220): the response was started but
5+
# never completed (no EOS). A correct server RST_STREAMs the stream; a buggy
6+
# one leaves the client hanging. Used by test_105_timeout.py.
7+
import sys
8+
import time
9+
10+
sys.stdout.write("Content-Type: application/octet-stream\r\n\r\n")
11+
sys.stdout.write("X" * 16384)
12+
sys.stdout.flush()
13+
time.sleep(30)

‎test/modules/http2/test_105_timeout.py‎

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import socket
22
import time
3+
from datetime import timedelta
34

45
import pytest
56

@@ -165,7 +166,46 @@ def test_h2_105_12(self, env):
165166
assert piper.response, f'{piper}'
166167
assert piper.response['status'] == 408, f"{piper.response}"
167168

168-
# A body-less response that is missing an EOS
169+
# A CGI that starts a response (status + headers + some body) and then goes
170+
# silent past `Timeout` must lead to the HTTP/2 stream being RST_STREAM'd,
171+
# not leave the client hanging. Regression test for the mod_http2 hang where
172+
# a c2 that finished an *incomplete* response (no EOS, e.g. CGI timed out
173+
# mid-body) was treated as complete and no reset was ever sent.
174+
def test_h2_105_20(self, env):
175+
conf = H2Conf(env)
176+
conf.add("Timeout 1")
177+
conf.add_vhost_cgi()
178+
conf.install()
179+
assert env.apache_restart() == 0
180+
url = env.mkurl("https", "cgi", "/h2cgi_slow.py")
181+
# Open many concurrent streams on one h2 connection. The missed reset
182+
# is racy per stream: the single wakeup at close drives one receive,
183+
# which resets only if it finds the beam already drained but usually
184+
# still carries the response's trailing flush and re-suspends. So one
185+
# stream is unreliable while many in flight hang at least one every
186+
# run. Key on elapsed time, not curl's exit code (--parallel makes
187+
# that ambiguous): a hang is a multi-second stall, a correct reset a
188+
# sub-second teardown. --max-time bounds the client so a hang cannot
189+
# wedge the test. The per-stream hang is a timing race, so its odds
190+
# shift with hardware and load; 10 is cheap margin and, being
191+
# concurrent, costs no wall-clock.
192+
count = 10
193+
max_time = 10
194+
r = env.curl_raw([url] * count, options=[
195+
"--parallel", "--parallel-immediate", "--max-time", str(max_time)])
196+
assert r.duration < timedelta(seconds=max_time - 2), \
197+
f'streams hung waiting for RST_STREAM (batch took {r.duration}): {r}'
198+
# Resetting each silent CGI is logged as a CGI timeout (cgid) and the
199+
# failed body read that follows it (core); both are expected here.
200+
time.sleep(1) # let the log flush
201+
env.httpd_error_log.ignore_recent(
202+
lognos = [
203+
"AH01220", # cgid: Timeout waiting for output from CGI script
204+
"AH00574", # core: ap_content_length_filter, apr_bucket_read() failed
205+
]
206+
)
207+
208+
# Complement to test_h2_105_20: a body-less response that is missing an EOS
169209
# must NOT be reset over HTTP/2. mod_cache revalidation of a cached,
170210
# immediately-stale resource emits exactly such a 304 (EOR + Flush, no EOS).
171211
# This guards the incomplete-response reset against re-opening PR 69580:

0 commit comments

Comments
 (0)