[PATCH 4 of 4] HTTP/2: don't send SETTINGS ACK before already queued DATA frames

Valentin V. Bartenev vbart at nginx.com
Thu Jun 1 17:52:38 UTC 2017


On Monday 24 April 2017 15:48:26 Piotr Sikora via nginx-devel wrote:
> # HG changeset patch
> # User Piotr Sikora <piotrsikora at google.com>
> # Date 1493073310 25200
> #      Mon Apr 24 15:35:10 2017 -0700
> # Node ID 3624fa075acac110a08c0f1c928c545a58c5801f
> # Parent  b8d7f4a4d5abb4a27a772910358e263d49c618ef
> HTTP/2: don't send SETTINGS ACK before already queued DATA frames.
> 
> Previously, SETTINGS ACK was sent immediately upon receipt of SETTINGS
> frame, before already queued DATA frames created using old SETTINGS.
> 
> This incorrect behavior was source of interoperability issues, because
> peers rely on the fact that new SETTINGS are in effect after receiving
> SETTINGS ACK.
> 
> Reported by Feng Li.
> 
> Signed-off-by: Piotr Sikora <piotrsikora at google.com>
> 
> diff -r b8d7f4a4d5ab -r 3624fa075aca src/http/v2/ngx_http_v2.c
> --- a/src/http/v2/ngx_http_v2.c
> +++ b/src/http/v2/ngx_http_v2.c
> @@ -2043,7 +2043,7 @@ ngx_http_v2_state_settings_params(ngx_ht
>          return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_INTERNAL_ERROR);
>      }
>  
> -    ngx_http_v2_queue_blocked_frame(h2c, frame);
> +    ngx_http_v2_queue_ordered_frame(h2c, frame);
>  
>      if (adjustment) {
>          if (ngx_http_v2_adjust_windows(h2c, adjustment) != NGX_OK) {
> diff -r b8d7f4a4d5ab -r 3624fa075aca src/http/v2/ngx_http_v2.h
> --- a/src/http/v2/ngx_http_v2.h
> +++ b/src/http/v2/ngx_http_v2.h
> @@ -261,6 +261,15 @@ ngx_http_v2_queue_blocked_frame(ngx_http
>  }
>  
>  
> +static ngx_inline void
> +ngx_http_v2_queue_ordered_frame(ngx_http_v2_connection_t *h2c,
> +    ngx_http_v2_out_frame_t *frame)
> +{
> +    frame->next = h2c->last_out;
> +    h2c->last_out = frame;
> +}
> +
> +
>  void ngx_http_v2_init(ngx_event_t *rev);
>  void ngx_http_v2_request_headers_init(void);
>  

Looks good.

  wbr, Valentin V. Bartenev




More information about the nginx-devel mailing list