Welcome! Log In Create A New Profile

Advanced

Re: [PATCH] HTTP/2: emit PROTOCOL_ERROR on invalid WINDOW_UPDATE increments

Valentin V. Bartenev
March 27, 2017 03:38PM
On Sunday 26 March 2017 01:41:15 Piotr Sikora via nginx-devel wrote:
> # HG changeset patch
> # User Piotr Sikora <piotrsikora@google.com>
> # Date 1490516706 25200
> # Sun Mar 26 01:25:06 2017 -0700
> # Node ID 9bbcacbdf6bd858a34a9dfd1ac2185eb8fc8c82f
> # Parent 22be63bf21edaa1b8ea916c7d8cd4e5fe4892061
> HTTP/2: emit PROTOCOL_ERROR on invalid WINDOW_UPDATE increments.
>
> Signed-off-by: Piotr Sikora <piotrsikora@google.com>
>
> diff -r 22be63bf21ed -r 9bbcacbdf6bd src/http/v2/ngx_http_v2.c
> --- a/src/http/v2/ngx_http_v2.c
> +++ b/src/http/v2/ngx_http_v2.c
> @@ -2173,6 +2173,22 @@ ngx_http_v2_state_window_update(ngx_http
>
> stream = node->stream;
>
> + if (window == 0) {
> + ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0,
> + "client sent WINDOW_UPDATE frame for stream %ui "
> + "with incorrect window increment 0", h2c->state.sid);
> +
> + if (ngx_http_v2_terminate_stream(h2c, stream,
> + NGX_HTTP_V2_PROTOCOL_ERROR)
> + == NGX_ERROR)
> + {
> + return ngx_http_v2_connection_error(h2c,
> + NGX_HTTP_V2_INTERNAL_ERROR);
> + }
> +
> + return ngx_http_v2_state_complete(h2c, pos, end);
> + }
> +
> if (window > (size_t) (NGX_HTTP_V2_MAX_WINDOW - stream->send_window)) {
>
> ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0,
> @@ -2211,6 +2227,14 @@ ngx_http_v2_state_window_update(ngx_http
> return ngx_http_v2_state_complete(h2c, pos, end);
> }
>
> + if (window == 0) {
> + ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0,
> + "client sent WINDOW_UPDATE frame "
> + "with incorrect window increment 0");
> +
> + return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_PROTOCOL_ERROR);
> + }
> +
> if (window > NGX_HTTP_V2_MAX_WINDOW - h2c->send_window) {
> ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0,
> "client violated connection flow control: "


I'm not sure that strictly following RFC here is worth the effort.

It seems there's no other "harm" from zero window updates except that it
allows to reset timers without any progress. That's only slightly worse
than 1-bytes window updates.

The downside is additional code and intolerance to potential client bugs.

Also note that in your implementation if zero window update is received
for unknown stream then it's silently ignored.

wbr, Valentin V. Bartenev

_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel
Subject Author Views Posted

[PATCH] HTTP/2: emit PROTOCOL_ERROR on invalid WINDOW_UPDATE increments

Piotr Sikora via nginx-devel 514 March 26, 2017 04:44AM

Re: [PATCH] HTTP/2: emit PROTOCOL_ERROR on invalid WINDOW_UPDATE increments

Valentin V. Bartenev 214 March 27, 2017 03:38PM

Re: [PATCH] HTTP/2: emit PROTOCOL_ERROR on invalid WINDOW_UPDATE increments

Piotr Sikora via nginx-devel 247 March 28, 2017 06:38AM

[PATCH] HTTP/2: emit PROTOCOL_ERROR on invalid WINDOW_UPDATE increments

Piotr Sikora via nginx-devel 239 March 28, 2017 06:54AM

Re: [PATCH] HTTP/2: emit PROTOCOL_ERROR on invalid WINDOW_UPDATE increments

Valentin V. Bartenev 300 March 28, 2017 01:26PM



Sorry, you do not have permission to post/reply in this forum.

Online Users

Guests: 82
Record Number of Users: 6 on February 13, 2018
Record Number of Guests: 421 on December 02, 2018
Powered by nginx      Powered by FreeBSD      PHP Powered      Powered by MariaDB      ipv6 ready