*) 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

diff --git a/STATUS b/STATUS
index c8eb5b4867f1169aa6bf8020d3c9e92da90bd520..127f04645bd37b2826f6327c35f3457992818ec3 100644 (file)
--- a/STATUS
+++ b/STATUS
@@ -157,18 +157,6 @@ RELEASE SHOWSTOPPERS:
 PATCHES ACCEPTED TO BACKPORT FROM TRUNK:
   [ start all new proposals below, under PATCHES PROPOSED. ]
 
-  *) mod_proxy_fcgi: Fix proxy-fcgi-pathinfo=full
-     trunk patch: https://svn.apache.org/r1919547
-                  https://svn.apache.org/r1921238
-     2.4.x patch: https://patch-diff.githubusercontent.com/raw/apache/httpd/pull/489.diff
-     PR: https://github.com/apache/httpd/pull/489
-     +1: ylavic, covener, jim
-
-  *) mod_http2: Fix failed request counting and keepalive timeout
-     trunk patch: https://svn.apache.org/r1921805
-     2.4.x patch: svn merge -c r1921805 ^/httpd/httpd/trunk .
-     +1: icing
-
   *) mod_log_config: Fix LogFormat directive merging
      trunk patch: https://svn.apache.org/r1921305
                   https://svn.apache.org/r1921306

diff --git a/modules/http2/h2_session.c b/modules/http2/h2_session.c
index ba248d0cc2780a9fb50df6d3754f5b9fc15cd19e..7c73e230699b564b4024050e1529c35c399e2719 100644 (file)
--- 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 ee87555f9f3cd0d4b3de2a5a9a3dcfbc62dd5650..9698f4b35b6a013eaf03232c6504b6348ccbdcd4 100644 (file)
--- 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 04c022c362dc88045c2ec04f7318454565c55b4f..cbc4b6c9fa8721b873039553b54d0413487abb55 100644 (file)
--- 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
+