Skip to content

Commit fcd23ef

Browse files
committed
*) mod_http2: Fix failed request counting and keepalive timeout
git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/branches/2.4.x@1922960 13f79535-47bb-0310-9956-ffa450edef68
1 parent 20f3423 commit fcd23ef

File tree

4 files changed

+69
-17
lines changed

4 files changed

+69
-17
lines changed

STATUS

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -157,18 +157,6 @@ RELEASE SHOWSTOPPERS:
157157
PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
158158
[ start all new proposals below, under PATCHES PROPOSED. ]
159159

160-
*) mod_proxy_fcgi: Fix proxy-fcgi-pathinfo=full
161-
trunk patch: https://svn.apache.org/r1919547
162-
https://svn.apache.org/r1921238
163-
2.4.x patch: https://patch-diff.githubusercontent.com/raw/apache/httpd/pull/489.diff
164-
PR: https://github.com/apache/httpd/pull/489
165-
+1: ylavic, covener, jim
166-
167-
*) mod_http2: Fix failed request counting and keepalive timeout
168-
trunk patch: https://svn.apache.org/r1921805
169-
2.4.x patch: svn merge -c r1921805 ^/httpd/httpd/trunk .
170-
+1: icing
171-
172160
*) mod_log_config: Fix LogFormat directive merging
173161
trunk patch: https://svn.apache.org/r1921305
174162
https://svn.apache.org/r1921306

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)