diff options
Diffstat (limited to '')
-rw-r--r-- | changes-entries/proxy_http2_retries.txt | 5 | ||||
-rw-r--r-- | modules/http2/h2_proxy_session.c | 9 | ||||
-rw-r--r-- | test/modules/http2/test_003_get.py | 11 | ||||
-rw-r--r-- | test/modules/http2/test_104_padding.py | 42 |
4 files changed, 34 insertions, 33 deletions
diff --git a/changes-entries/proxy_http2_retries.txt b/changes-entries/proxy_http2_retries.txt new file mode 100644 index 0000000000..4c66c4eb19 --- /dev/null +++ b/changes-entries/proxy_http2_retries.txt @@ -0,0 +1,5 @@ + *) mod_proxy_http2: fix retry handling to not leak temporary errors. + On detecting that that an existing connection was shutdown by the other + side, a 503 response leaked even though the request was retried on a + fresh connection. + [Stefan Eissing]
\ No newline at end of file diff --git a/modules/http2/h2_proxy_session.c b/modules/http2/h2_proxy_session.c index c3f2ff3010..378a1ded69 100644 --- a/modules/http2/h2_proxy_session.c +++ b/modules/http2/h2_proxy_session.c @@ -1375,12 +1375,9 @@ static void ev_stream_done(h2_proxy_session *session, int stream_id, session->id, stream_id, touched, stream->error_code); if (status != APR_SUCCESS) { - b = ap_bucket_error_create(HTTP_SERVICE_UNAVAILABLE, NULL, stream->r->pool, - stream->r->connection->bucket_alloc); - APR_BRIGADE_INSERT_TAIL(stream->output, b); - b = apr_bucket_eos_create(stream->r->connection->bucket_alloc); - APR_BRIGADE_INSERT_TAIL(stream->output, b); - ap_pass_brigade(stream->r->output_filters, stream->output); + /* stream failed, error reporting is done by caller + * of proxy_session, e.g. mod_proxy_http2 which also + * decides about retries. */ } else if (!stream->data_received) { /* if the response had no body, this is the time to flush diff --git a/test/modules/http2/test_003_get.py b/test/modules/http2/test_003_get.py index 77afa4feb9..cc7e511795 100644 --- a/test/modules/http2/test_003_get.py +++ b/test/modules/http2/test_003_get.py @@ -197,11 +197,7 @@ content-type: text/html def test_h2_003_50(self, env, path, repeat): # check that the resource supports ranges and we see its raw content-length url = env.mkurl("https", "test1", path) - # TODO: sometimes we see a 503 here from h2proxy - for i in range(10): - r = env.curl_get(url, 5) - if r.response["status"] != 503: - break + r = env.curl_get(url, 5) assert r.response["status"] == 200 assert "HTTP/2" == r.response["protocol"] h = r.response["header"] @@ -210,10 +206,7 @@ content-type: text/html assert "content-length" in h clen = h["content-length"] # get the first 1024 bytes of the resource, 206 status, but content-length as original - for i in range(10): - r = env.curl_get(url, 5, options=["-H", "range: bytes=0-1023"]) - if r.response["status"] != 503: - break + r = env.curl_get(url, 5, options=["-H", "range: bytes=0-1023"]) assert 206 == r.response["status"] assert "HTTP/2" == r.response["protocol"] assert 1024 == len(r.response["body"]) diff --git a/test/modules/http2/test_104_padding.py b/test/modules/http2/test_104_padding.py index 7b874ed9ce..401804ade8 100644 --- a/test/modules/http2/test_104_padding.py +++ b/test/modules/http2/test_104_padding.py @@ -13,57 +13,63 @@ class TestPadding: @pytest.fixture(autouse=True, scope='class') def _class_scope(self, env): + def add_echo_handler(conf): + conf.add([ + "<Location \"/h2test/echo\">", + " SetHandler h2test-echo", + "</Location>", + ]) + conf = H2Conf(env) conf.start_vhost(domains=[f"ssl.{env.http_tld}"], port=env.https_port, doc_root="htdocs/cgi") - conf.add("AddHandler cgi-script .py") + add_echo_handler(conf) conf.end_vhost() conf.start_vhost(domains=[f"pad0.{env.http_tld}"], port=env.https_port, doc_root="htdocs/cgi") conf.add("H2Padding 0") - conf.add("AddHandler cgi-script .py") + add_echo_handler(conf) conf.end_vhost() conf.start_vhost(domains=[f"pad1.{env.http_tld}"], port=env.https_port, doc_root="htdocs/cgi") conf.add("H2Padding 1") - conf.add("AddHandler cgi-script .py") + add_echo_handler(conf) conf.end_vhost() conf.start_vhost(domains=[f"pad2.{env.http_tld}"], port=env.https_port, doc_root="htdocs/cgi") conf.add("H2Padding 2") - conf.add("AddHandler cgi-script .py") + add_echo_handler(conf) conf.end_vhost() conf.start_vhost(domains=[f"pad3.{env.http_tld}"], port=env.https_port, doc_root="htdocs/cgi") conf.add("H2Padding 3") - conf.add("AddHandler cgi-script .py") + add_echo_handler(conf) conf.end_vhost() conf.start_vhost(domains=[f"pad8.{env.http_tld}"], port=env.https_port, doc_root="htdocs/cgi") conf.add("H2Padding 8") - conf.add("AddHandler cgi-script .py") + add_echo_handler(conf) conf.end_vhost() conf.install() assert env.apache_restart() == 0 # default paddings settings: 0 bits - def test_h2_104_01(self, env): - url = env.mkurl("https", "ssl", "/echo.py") + def test_h2_104_01(self, env, repeat): + url = env.mkurl("https", "ssl", "/h2test/echo") # we get 2 frames back: one with data and an empty one with EOF # check the number of padding bytes is as expected for data in ["x", "xx", "xxx", "xxxx", "xxxxx", "xxxxxx", "xxxxxxx", "xxxxxxxx"]: r = env.nghttp().post_data(url, data, 5) assert r.response["status"] == 200 - assert r.results["paddings"] == [ - frame_padding(len(data)+1, 0), - frame_padding(0, 0) - ] + for i in r.results["paddings"]: + assert i == frame_padding(len(data)+1, 0) # 0 bits of padding def test_h2_104_02(self, env): - url = env.mkurl("https", "pad0", "/echo.py") + url = env.mkurl("https", "pad0", "/h2test/echo") for data in ["x", "xx", "xxx", "xxxx", "xxxxx", "xxxxxx", "xxxxxxx", "xxxxxxxx"]: r = env.nghttp().post_data(url, data, 5) assert r.response["status"] == 200 - assert r.results["paddings"] == [0, 0] + for i in r.results["paddings"]: + assert i == 0 # 1 bit of padding def test_h2_104_03(self, env): - url = env.mkurl("https", "pad1", "/echo.py") + url = env.mkurl("https", "pad1", "/h2test/echo") for data in ["x", "xx", "xxx", "xxxx", "xxxxx", "xxxxxx", "xxxxxxx", "xxxxxxxx"]: r = env.nghttp().post_data(url, data, 5) assert r.response["status"] == 200 @@ -72,7 +78,7 @@ class TestPadding: # 2 bits of padding def test_h2_104_04(self, env): - url = env.mkurl("https", "pad2", "/echo.py") + url = env.mkurl("https", "pad2", "/h2test/echo") for data in ["x", "xx", "xxx", "xxxx", "xxxxx", "xxxxxx", "xxxxxxx", "xxxxxxxx"]: r = env.nghttp().post_data(url, data, 5) assert r.response["status"] == 200 @@ -81,7 +87,7 @@ class TestPadding: # 3 bits of padding def test_h2_104_05(self, env): - url = env.mkurl("https", "pad3", "/echo.py") + url = env.mkurl("https", "pad3", "/h2test/echo") for data in ["x", "xx", "xxx", "xxxx", "xxxxx", "xxxxxx", "xxxxxxx", "xxxxxxxx"]: r = env.nghttp().post_data(url, data, 5) assert r.response["status"] == 200 @@ -90,7 +96,7 @@ class TestPadding: # 8 bits of padding def test_h2_104_06(self, env): - url = env.mkurl("https", "pad8", "/echo.py") + url = env.mkurl("https", "pad8", "/h2test/echo") for data in ["x", "xx", "xxx", "xxxx", "xxxxx", "xxxxxx", "xxxxxxx", "xxxxxxxx"]: r = env.nghttp().post_data(url, data, 5) assert r.response["status"] == 200 |