Skip to content

Commit 85a7f40

Browse files
committed
mod_http2: do not RST_STREAM header-only HTTP/2 responses
A mod_cache revalidation 304 (EOR + Flush, no body EOS) reaches s_c2_done() with an output beam that is neither closed nor EOS'd, so h2_beam_is_complete() reports it incomplete and s_c2_done() aborts it, resetting the stream (the client sees "stream not closed cleanly"). But a header-only response (204/304, and HEAD) is complete without a body EOS. Track response completeness in conn_ctx->response_eos_seen, set when an EOS passes out (h2_c2_filter_out) or the response is header-only, and spare such responses from the s_c2_done() abort. stream_data_cb() already carries the same header-only exemption; this brings s_c2_done() in line. Adds test_h2_105_21: a mod_cache revalidation 304 must close cleanly.
1 parent 0188772 commit 85a7f40

4 files changed

Lines changed: 63 additions & 1 deletion

File tree

‎modules/http2/h2_c2.c‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -397,6 +397,13 @@ static apr_status_t h2_c2_filter_out(ap_filter_t* f, apr_bucket_brigade* bb)
397397
ap_bucket_response *resp = e->data;
398398
if (resp->status >= HTTP_OK) {
399399
conn_ctx->has_final_response = 1;
400+
if (AP_STATUS_IS_HEADER_ONLY(resp->status)) {
401+
/* A header-only response (204/304) is complete without
402+
* a body EOS; mark it complete so c2_process() does not
403+
* abort/reset it as an "incomplete" response (which
404+
* would re-open PR 69580). */
405+
conn_ctx->response_eos_seen = 1;
406+
}
400407
break;
401408
}
402409
}
@@ -406,6 +413,19 @@ static apr_status_t h2_c2_filter_out(ap_filter_t* f, apr_bucket_brigade* bb)
406413
}
407414
}
408415
#endif /* AP_HAS_RESPONSE_BUCKETS */
416+
/* Note when the response body completes (an EOS passes out). A c2 that
417+
* finishes WITHOUT having seen an EOS (e.g. the CGI/handler timed out
418+
* mid-body) produced an incomplete response and must reset the stream. */
419+
if (!conn_ctx->response_eos_seen) {
420+
apr_bucket *e;
421+
for (e = APR_BRIGADE_FIRST(bb); e != APR_BRIGADE_SENTINEL(bb);
422+
e = APR_BUCKET_NEXT(e)) {
423+
if (APR_BUCKET_IS_EOS(e)) {
424+
conn_ctx->response_eos_seen = 1;
425+
break;
426+
}
427+
}
428+
}
409429
rv = beam_out(f->c, conn_ctx, bb);
410430

411431
ap_log_cerror(APLOG_MARK, APLOG_TRACE2, rv, f->c,

‎modules/http2/h2_conn_ctx.h‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,7 @@ struct h2_conn_ctx_t {
6060
apr_pollfd_t pfd; /* c1: poll socket input, c2: NUL */
6161

6262
int has_final_response; /* final HTTP response passed on out */
63+
int response_eos_seen; /* c2: response body completed (EOS seen on out) */
6364
apr_status_t last_err; /* APR_SUCCES or last error encountered in filters */
6465

6566
apr_off_t bytes_sent; /* c2: bytes acutaly sent via c1 */

‎modules/http2/h2_mplx.c‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1005,7 +1005,14 @@ static void s_c2_done(h2_mplx *m, conn_rec *c2, h2_conn_ctx_t *conn_ctx)
10051005
if (conn_ctx->beam_out)
10061006
h2_beam_abort(conn_ctx->beam_out, c2);
10071007
}
1008-
else if (!conn_ctx->beam_out || !h2_beam_is_complete(conn_ctx->beam_out)) {
1008+
else if (!conn_ctx->beam_out
1009+
|| (!h2_beam_is_complete(conn_ctx->beam_out)
1010+
&& !conn_ctx->response_eos_seen)) {
1011+
/* h2_beam_is_complete() cannot tell a header-only response (204/304,
1012+
* legitimately no body EOS) from a truncated one. response_eos_seen is
1013+
* set when an EOS passed out OR the response was header-only, so spare
1014+
* those here; only abort genuinely incomplete output. Without this, a
1015+
* body-less mod_cache 304 revalidation gets RST_STREAM'd. */
10091016
ap_log_cerror(APLOG_MARK, APLOG_TRACE1, conn_ctx->last_err, c2,
10101017
"h2_c2(%s-%d): processing finished with incomplete output",
10111018
conn_ctx->id, conn_ctx->stream_id);

‎test/modules/http2/test_105_timeout.py‎

Lines changed: 34 additions & 0 deletions
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

@@ -164,3 +165,36 @@ def test_h2_105_12(self, env):
164165
piper.close()
165166
assert piper.response, f'{piper}'
166167
assert piper.response['status'] == 408, f"{piper.response}"
168+
169+
# A body-less response that is missing an EOS
170+
# must NOT be reset over HTTP/2. mod_cache revalidation of a cached,
171+
# immediately-stale resource emits exactly such a 304 (EOR + Flush, no EOS).
172+
# This guards the incomplete-response reset against re-opening PR 69580:
173+
# only a response that began a body and is missing EOS should be
174+
# reset, never a header-only one.
175+
def test_h2_105_21(self, env):
176+
cacheroot = f"{env.server_dir}/cacheroot"
177+
env.mkpath(cacheroot)
178+
conf = H2Conf(env)
179+
conf.add(f"""
180+
CacheRoot "{cacheroot}"
181+
CacheEnable disk /
182+
Header set Cache-Control "public, max-age=0"
183+
""")
184+
conf.add_vhost_test1()
185+
conf.install()
186+
assert env.apache_restart() == 0
187+
url = env.mkurl("https", "test1", "/006/006.css")
188+
# prime the cache (stored, immediately stale via max-age=0)
189+
r = env.curl_get(url)
190+
assert r.exit_code == 0, f'{r}'
191+
assert r.response["status"] == 200
192+
lm = r.response["header"]["last-modified"]
193+
# revalidate: mod_cache freshens the stale entry and the origin returns a
194+
# body-less 304. The h2 stream must close cleanly; a regression shows up
195+
# as a curl exit (92, "stream not closed cleanly"), not as a 304.
196+
for _ in range(5):
197+
r = env.curl_get(url, options=["-H", "Cache-Control: max-age=0",
198+
"-H", f"if-modified-since: {lm}"])
199+
assert r.exit_code == 0, f'304 stream was reset (curl {r.exit_code}): {r}'
200+
assert r.response["status"] == 304, f'{r.response}'

0 commit comments

Comments
 (0)