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

3 files changed, 69 insertions, 5 deletions

diff --git a/modules/http2/h2_session.c b/modules/http2/h2_session.c
index ba248d0cc2..7c73e23069 100644
--- a/modules/http2/h2_session.c
+++ b/modules/http2/h2_session.c
@@ -326,6 +326,10 @@ static int on_header_cb(nghttp2_session *ngh2, const nghttp2_frame *frame,
           * with an informative HTTP error response like 413. But of the
           * client is too wrong, we RESET the stream */
          stream->request_headers_failed > 100)) {
+        ap_log_cerror(APLOG_MARK, APLOG_TRACE1, 0, session->c1,
+                      H2_SSSN_STRM_MSG(session, frame->hd.stream_id,
+                      "RST stream, header failures: %d"),
+                      (int)stream->request_headers_failed);
         return NGHTTP2_ERR_TEMPORAL_CALLBACK_FAILURE;
     }
     return 0;
@@ -1498,7 +1502,8 @@ static void h2_session_ev_conn_error(h2_session *session, int arg, const char *m
         default:
             ap_log_cerror(APLOG_MARK, APLOG_DEBUG, 0, session->c1,
                           H2_SSSN_LOG(APLOGNO(03401), session, 
-                          "conn error -> shutdown"));
+                          "conn error -> shutdown, remote.emitted=%d"),
+                          (int)session->remote.emitted_count);
             h2_session_shutdown(session, arg, msg, 0);
             break;
     }
@@ -1591,9 +1596,7 @@ static void ev_stream_created(h2_session *session, h2_stream *stream)
 static void ev_stream_open(h2_session *session, h2_stream *stream)
 {
     if (H2_STREAM_CLIENT_INITIATED(stream->id)) {
-        ++session->remote.emitted_count;
-        if (stream->id > session->remote.emitted_max) {
-            session->remote.emitted_max = stream->id;
+        if (stream->id > session->remote.accepted_max) {
             session->local.accepted_max = stream->id;
         }
     }
@@ -1890,7 +1893,8 @@ apr_status_t h2_session_process(h2_session *session, int async,
                         /* Not an async mpm, we must continue waiting
                          * for client data to arrive until the configured
                          * server Timeout/KeepAliveTimeout happens */
-                        apr_time_t timeout = (session->open_streams == 0)?
+                        apr_time_t timeout = ((session->open_streams == 0) &&
+                                              session->remote.emitted_count)?
                             session->s->keep_alive_timeout :
                             session->s->timeout;
                         ap_log_cerror(APLOG_MARK, APLOG_TRACE2, status, c,
diff --git a/modules/http2/h2_stream.c b/modules/http2/h2_stream.c
index ee87555f9f..9698f4b35b 100644
--- a/modules/http2/h2_stream.c
+++ b/modules/http2/h2_stream.c
@@ -761,6 +761,11 @@ apr_status_t h2_stream_add_header(h2_stream *stream,
     }
     else if (H2_SS_IDLE == stream->state) {
         if (!stream->rtmp) {
+            if (H2_STREAM_CLIENT_INITIATED(stream->id)) {
+                ++stream->session->remote.emitted_count;
+              if (stream->id > stream->session->remote.emitted_max)
+                  session->remote.emitted_max = stream->id;
+            }
             stream->rtmp = h2_request_create(stream->id, stream->pool,
                                              NULL, NULL, NULL, NULL, NULL);
         }
diff --git a/test/modules/http2/test_200_header_invalid.py b/test/modules/http2/test_200_header_invalid.py
index 04c022c362..cbc4b6c9fa 100644
--- a/test/modules/http2/test_200_header_invalid.py
+++ b/test/modules/http2/test_200_header_invalid.py
@@ -1,3 +1,4 @@
+import re
 import pytest
 
 from .env import H2Conf, H2TestEnv
@@ -227,3 +228,57 @@ class TestInvalidHeaders:
         r = env.nghttp().get(url, options=opt)
         assert r.exit_code == 0, r
         assert r.response is None
+
+    # test few failed headers, should
+    def test_h2_200_17(self, env):
+        url = env.mkurl("https", "cgi", "/")
+
+    # test few failed headers, should give response
+    def test_h2_200_17(self, env):
+        conf = H2Conf(env)
+        conf.add("""
+            LimitRequestFieldSize 20
+            LogLevel http2:debug
+            """)
+        conf.add_vhost_cgi()
+        conf.install()
+        assert env.apache_restart() == 0
+        re_emitted = re.compile(r'.* AH03401: .* shutdown, remote.emitted=1')
+        url = env.mkurl("https", "cgi", "/")
+        opt = []
+        for i in range(10):
+            opt += ["-H", f"x{i}: 012345678901234567890123456789"]
+        r = env.curl_get(url, options=opt)
+        assert r.response
+        assert r.response["status"] == 431
+        assert env.httpd_error_log.scan_recent(re_emitted)
+
+    # test too many failed headers, should give RST
+    def test_h2_200_18(self, env):
+        conf = H2Conf(env)
+        conf.add("""
+            LimitRequestFieldSize 20
+            LogLevel http2:debug
+            """)
+        conf.add_vhost_cgi()
+        conf.install()
+        assert env.apache_restart() == 0
+        re_emitted = re.compile(r'.* AH03401: .* shutdown, remote.emitted=1')
+        url = env.mkurl("https", "cgi", "/")
+        opt = []
+        for i in range(100):
+            opt += ["-H", f"x{i}: 012345678901234567890123456789"]
+        r = env.curl_get(url, options=opt)
+        assert r.response is None
+        assert env.httpd_error_log.scan_recent(re_emitted)
+
+    # test header 10 invalid headers, should trigger stream RST
+    def test_h2_200_19(self, env):
+        url = env.mkurl("https", "cgi", "/")
+        opt = []
+        invalid = '\x7f'
+        for i in range(10):
+            opt += ["-H", f"x{i}: {invalid}"]
+        r = env.curl_get(url, options=opt)
+        assert r.response is None
+