Skip to content

Commit d94933b

Browse files
committed
mod_http2, fix keepalive timeout on reset requests
Count failed requests that are RST'ed, so that the connection enters keepalive timeout instead of the regular timeout if the first request fails. Add tests to verify. git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@1921805 13f79535-47bb-0310-9956-ffa450edef68
1 parent f5c4355 commit d94933b

File tree

3 files changed

+69
-5
lines changed

3 files changed

+69
-5
lines changed

modules/http2/h2_session.c

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -326,6 +326,10 @@ static int on_header_cb(nghttp2_session *ngh2, const nghttp2_frame *frame,
326326
* with an informative HTTP error response like 413. But of the
327327
* client is too wrong, we RESET the stream */
328328
stream->request_headers_failed > 100)) {
329+
ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, session->c1,
330+
H2_SSSN_STRM_MSG(session, frame->hd.stream_id,
331+
"RST stream, header failures: %d"),
332+
(int)stream->request_headers_failed);
329333
return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
330334
}
331335
return 0;
@@ -1498,7 +1502,8 @@ static void h2_session_ev_conn_error(h2_session *session, int arg, const char *m
14981502
default:
14991503
ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, session->c1,
15001504
H2_SSSN_LOG(APLOGNO(03401), session,
1501-
"conn error -> shutdown"));
1505+
"conn error -> shutdown, remote.emitted=%d"),
1506+
(int)session->remote.emitted_count);
15021507
h2_session_shutdown(session, arg, msg, 0);
15031508
break;
15041509
}
@@ -1591,9 +1596,7 @@ static void ev_stream_created(h2_session *session, h2_stream *stream)
15911596
static void ev_stream_open(h2_session *session, h2_stream *stream)
15921597
{
15931598
if (H2_STREAM_CLIENT_INITIATED(stream->id)) {
1594-
++session->remote.emitted_count;
1595-
if (stream->id > session->remote.emitted_max) {
1596-
session->remote.emitted_max = stream->id;
1599+
if (stream->id > session->remote.accepted_max) {
15971600
session->local.accepted_max = stream->id;
15981601
}
15991602
}
@@ -1890,7 +1893,8 @@ apr_status_t h2_session_process(h2_session *session, int async,
18901893
/* Not an async mpm, we must continue waiting
18911894
* for client data to arrive until the configured
18921895
* server Timeout/KeepAliveTimeout happens */
1893-
apr_time_t timeout = (session->open_streams == 0)?
1896+
apr_time_t timeout = ((session->open_streams == 0) &&
1897+
session->remote.emitted_count)?
18941898
session->s->keep_alive_timeout :
18951899
session->s->timeout;
18961900
ap_log_cerror(APLOG_MARK, APLOG_TRACE2, status, c,

modules/http2/h2_stream.c

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -761,6 +761,11 @@ apr_status_t h2_stream_add_header(h2_stream *stream,
761761
}
762762
else if (H2_SS_IDLE == stream->state) {
763763
if (!stream->rtmp) {
764+
if (H2_STREAM_CLIENT_INITIATED(stream->id)) {
765+
++stream->session->remote.emitted_count;
766+
if (stream->id > stream->session->remote.emitted_max)
767+
session->remote.emitted_max = stream->id;
768+
}
764769
stream->rtmp = h2_request_create(stream->id, stream->pool,
765770
NULL, NULL, NULL, NULL, NULL);
766771
}

test/modules/http2/test_200_header_invalid.py

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import re
12
import pytest
23

34
from .env import H2Conf, H2TestEnv
@@ -227,3 +228,57 @@ def test_h2_200_16(self, env):
227228
r = env.nghttp().get(url, options=opt)
228229
assert r.exit_code == 0, r
229230
assert r.response is None
231+
232+
# test few failed headers, should
233+
def test_h2_200_17(self, env):
234+
url = env.mkurl("https", "cgi", "/")
235+
236+
# test few failed headers, should give response
237+
def test_h2_200_17(self, env):
238+
conf = H2Conf(env)
239+
conf.add("""
240+
LimitRequestFieldSize 20
241+
LogLevel http2:debug
242+
""")
243+
conf.add_vhost_cgi()
244+
conf.install()
245+
assert env.apache_restart() == 0
246+
re_emitted = re.compile(r'.* AH03401: .* shutdown, remote.emitted=1')
247+
url = env.mkurl("https", "cgi", "/")
248+
opt = []
249+
for i in range(10):
250+
opt += ["-H", f"x{i}: 012345678901234567890123456789"]
251+
r = env.curl_get(url, options=opt)
252+
assert r.response
253+
assert r.response["status"] == 431
254+
assert env.httpd_error_log.scan_recent(re_emitted)
255+
256+
# test too many failed headers, should give RST
257+
def test_h2_200_18(self, env):
258+
conf = H2Conf(env)
259+
conf.add("""
260+
LimitRequestFieldSize 20
261+
LogLevel http2:debug
262+
""")
263+
conf.add_vhost_cgi()
264+
conf.install()
265+
assert env.apache_restart() == 0
266+
re_emitted = re.compile(r'.* AH03401: .* shutdown, remote.emitted=1')
267+
url = env.mkurl("https", "cgi", "/")
268+
opt = []
269+
for i in range(100):
270+
opt += ["-H", f"x{i}: 012345678901234567890123456789"]
271+
r = env.curl_get(url, options=opt)
272+
assert r.response is None
273+
assert env.httpd_error_log.scan_recent(re_emitted)
274+
275+
# test header 10 invalid headers, should trigger stream RST
276+
def test_h2_200_19(self, env):
277+
url = env.mkurl("https", "cgi", "/")
278+
opt = []
279+
invalid = '\x7f'
280+
for i in range(10):
281+
opt += ["-H", f"x{i}: {invalid}"]
282+
r = env.curl_get(url, options=opt)
283+
assert r.response is None
284+

0 commit comments

Comments
 (0)