[PATCH] HTTP/2: signal 0-byte HPACK's dynamic table size

[Maxim Dounin]

Hello!

On Wed, Aug 30, 2017 at 02:55:34PM -0700, Piotr Sikora via nginx-devel wrote:

> # HG changeset patch
> # User Piotr Sikora <piotrsikora at google.com>
> # Date 1504129931 25200
> #      Wed Aug 30 14:52:11 2017 -0700
> # Node ID 7e1e91f9ca063563cb293136e7b1cede36e63dc6
> # Parent  c7d4017c8876af6d8570e400320537d7d39e9578
> HTTP/2: signal 0-byte HPACK's dynamic table size.
> 
> This change lets NGINX talk to clients with SETTINGS_HEADER_TABLE_SIZE
> smaller than the default 4KB. Previously, NGINX would ACK the SETTINGS
> frame with a small dynamic table size, but it would never send dynamic
> table size update, leading to a connection-level COMPRESSION_ERROR.
> 
> Also, it allows clients to release 4KB of memory per connection, since
> NGINX doesn't use HPACK's dynamic table when encoding headers, however
> clients had to maintain it, since NGINX never signaled that it doesn't
> use it.
> 
> Signed-off-by: Piotr Sikora <piotrsikora at google.com>
> 
> diff -r c7d4017c8876 -r 7e1e91f9ca06 src/http/v2/ngx_http_v2.c
> --- a/src/http/v2/ngx_http_v2.c
> +++ b/src/http/v2/ngx_http_v2.c
> @@ -245,6 +245,8 @@ ngx_http_v2_init(ngx_event_t *rev)
>  
>      h2c->frame_size = NGX_HTTP_V2_DEFAULT_FRAME_SIZE;
>  
> +    h2c->table_update = 1;
> +
>      h2scf = ngx_http_get_module_srv_conf(hc->conf_ctx, ngx_http_v2_module);
>  
>      h2c->pool = ngx_create_pool(h2scf->pool_size, h2c->connection->log);
> diff -r c7d4017c8876 -r 7e1e91f9ca06 src/http/v2/ngx_http_v2.h
> --- a/src/http/v2/ngx_http_v2.h
> +++ b/src/http/v2/ngx_http_v2.h
> @@ -144,6 +144,7 @@ struct ngx_http_v2_connection_s {
>  
>      unsigned                         closed_nodes:8;
>      unsigned                         settings_ack:1;
> +    unsigned                         table_update:1;
>      unsigned                         blocked:1;
>      unsigned                         goaway:1;
>  };
> diff -r c7d4017c8876 -r 7e1e91f9ca06 src/http/v2/ngx_http_v2_filter_module.c
> --- a/src/http/v2/ngx_http_v2_filter_module.c
> +++ b/src/http/v2/ngx_http_v2_filter_module.c
> @@ -141,6 +141,7 @@ ngx_http_v2_header_filter(ngx_http_reque
>      ngx_http_v2_out_frame_t   *frame;
>      ngx_http_core_loc_conf_t  *clcf;
>      ngx_http_core_srv_conf_t  *cscf;
> +    ngx_http_v2_connection_t  *h2c;
>      u_char                     addr[NGX_SOCKADDR_STRLEN];
>  
>      static const u_char nginx[5] = "\x84\xaa\x63\x55\xe7";
> @@ -235,7 +236,11 @@ ngx_http_v2_header_filter(ngx_http_reque
>          }
>      }
>  
> -    len = status ? 1 : 1 + ngx_http_v2_literal_size("418");
> +    h2c = r->stream->connection;
> +
> +    len = h2c->table_update ? 1 : 0;
> +
> +    len += status ? 1 : 1 + ngx_http_v2_literal_size("418");
>  
>      clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module);
>  
> @@ -423,6 +428,13 @@ ngx_http_v2_header_filter(ngx_http_reque
>  
>      start = pos;
>  
> +    if (h2c->table_update) {
> +        ngx_log_debug0(NGX_LOG_DEBUG_HTTP, fc->log, 0,
> +                       "http2 sending dynamic table size update: 0");
> +        *pos++ = 32;
> +        h2c->table_update = 0;
> +    }
> +
>      ngx_log_debug1(NGX_LOG_DEBUG_HTTP, fc->log, 0,
>                     "http2 output header: \":status: %03ui\"",
>                     r->headers_out.status);

Thank you for the patch.  I've pushed a change which fixes tests 
with this patch (http://hg.nginx.org/nginx-tests/rev/24e175025ad8). 
Unless there are objections, I'll push this patch with the 
following mostly style changes:

--- a/src/http/v2/ngx_http_v2_filter_module.c
+++ b/src/http/v2/ngx_http_v2_filter_module.c
@@ -139,9 +139,9 @@ ngx_http_v2_header_filter(ngx_http_reque
     ngx_connection_t          *fc;
     ngx_http_cleanup_t        *cln;
     ngx_http_v2_out_frame_t   *frame;
+    ngx_http_v2_connection_t  *h2c;
     ngx_http_core_loc_conf_t  *clcf;
     ngx_http_core_srv_conf_t  *cscf;
-    ngx_http_v2_connection_t  *h2c;
     u_char                     addr[NGX_SOCKADDR_STRLEN];
 
     static const u_char nginx[5] = "\x84\xaa\x63\x55\xe7";
@@ -430,8 +430,8 @@ ngx_http_v2_header_filter(ngx_http_reque
 
     if (h2c->table_update) {
         ngx_log_debug0(NGX_LOG_DEBUG_HTTP, fc->log, 0,
-                       "http2 sending dynamic table size update: 0");
-        *pos++ = 32;
+                       "http2 table size update: 0");
+        *pos++ = (1 << 5) | 0;
         h2c->table_update = 0;
     }
 

-- 
Maxim Dounin
http://nginx.org/