diff --git a/bin/varnishd/http2/cache_http2_proto.c b/bin/varnishd/http2/cache_http2_proto.c index 910fc8c0306..b69b659121e 100644 --- a/bin/varnishd/http2/cache_http2_proto.c +++ b/bin/varnishd/http2/cache_http2_proto.c @@ -336,7 +336,8 @@ h2_rx_rst_stream(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2) return (0); h2_kill_req(wrk, h2, r2, h2_streamerror(vbe32dec(h2->rxf_data))); - if (cache_param->h2_rapid_reset_limit == 0) + if (cache_param->h2_rapid_reset == 0 || + cache_param->h2_rapid_reset_limit == 0) return (0); now = VTIM_real(); @@ -353,7 +354,8 @@ h2_rx_rst_stream(struct worker *wrk, struct h2_sess *h2, struct h2_req *r2) Lck_Unlock(&h2->sess->mtx); return (H2CE_ENHANCE_YOUR_CALM); } - h2->rst_allowance -= 1.0; + if (d < cache_param->h2_rapid_reset) + h2->rst_allowance -= 1.0; return (0); } diff --git a/bin/varnishtest/tests/r03996.vtc b/bin/varnishtest/tests/r03996.vtc index a6eef1dcc50..758391512cd 100644 --- a/bin/varnishtest/tests/r03996.vtc +++ b/bin/varnishtest/tests/r03996.vtc @@ -7,6 +7,7 @@ server s1 { varnish v1 -cliok "param.set feature +http2" varnish v1 -cliok "param.set h2_rapid_reset_limit 3" +varnish v1 -cliok "param.set h2_rapid_reset 0.1" varnish v1 -vcl+backend {} -start diff --git a/include/tbl/params.h b/include/tbl/params.h index 5c6df310964..d27500c928d 100644 --- a/include/tbl/params.h +++ b/include/tbl/params.h @@ -1257,17 +1257,34 @@ PARAM_SIMPLE( "HTTP2 maximum size of an uncompressed header list." ) +PARAM_SIMPLE( + /* name */ h2_rapid_reset, + /* typ */ timeout, + /* min */ "0.000", + /* max */ NULL, + /* def */ "0.001", + /* units */ NULL, + /* descr */ + "Interval for classification as HTTP2 rapid reset.\n" + "Specifies the interval between successive HTTP2 RST_STREAM frames " + "to qualify for rate limiting by rapid_reset_limit over " + "h2_rapid_reset_period." + "Setting this parameter to 0 disables the limit.", + /* flags */ EXPERIMENTAL, +) + PARAM_SIMPLE( /* name */ h2_rapid_reset_limit, /* typ */ uint, /* min */ "0", /* max */ NULL, - /* def */ "0", + /* def */ "0", // XXX SHOULD WE HAVE A SAFE DEFAULT ? /* units */ NULL, /* descr */ - "HTTP2 RST Allowance.\n" - "Specifies the maximum number of allowed stream resets issued by\n" - "a client over a time period before the connection is closed.\n" + "HTTP2 rapid reset limit.\n" + "Specifies the maximum number of allowed stream resets classified as " + "\"rapid\" per the h2_rapid_reset prameter issued by\n" + "a client over h2_rapid_reset_period before the connection is closed.\n" "Setting this parameter to 0 disables the limit.", /* flags */ EXPERIMENTAL, ) @@ -1280,7 +1297,7 @@ PARAM_SIMPLE( /* def */ "60.000", /* units */ "seconds", /* descr */ - "HTTP2 RST allowance time period.", + "Period to which h2_rapid_reset_limit applies.", /* flags */ EXPERIMENTAL|WIZARD, )