HPACK patch broken for Nginx 1.13.6

[centminmod]

Seems Nginx 1.13.6 update broken HPACK patch https://community.centminmod.com/threads/nginx-announce-nginx-1-13-6.13079/#post-55493

                -o objs/src/http/ngx_http_postpone_filter_module.o \
                src/http/ngx_http_postpone_filter_module.c
src/http/v2/ngx_http_v2_filter_module.c: In function ‘ngx_http_v2_header_filter’:
src/http/v2/ngx_http_v2_filter_module.c:137:32: error: redeclaration of ‘h2c’ with no linkage
     ngx_http_v2_connection_t  *h2c;
                                ^~~
src/http/v2/ngx_http_v2_filter_module.c:133:32: note: previous declaration of ‘h2c’ was here
     ngx_http_v2_connection_t  *h2c;
                                ^~~
make[1]: *** [objs/src/http/v2/ngx_http_v2_filter_module.o] Error 1
make[1]: *** Waiting for unfinished jobs....
make[1]: Leaving directory `/svr-setup/nginx-1.13.6'
make: *** [build] Error 2

after disabling HPACK patch, Nginx 1.13.6 compiled fine on CentOS 6.9 64bit

nginx -V
nginx version: nginx/1.13.6
built by gcc 6.3.1 20170216 (Red Hat 6.3.1-3) (GCC) 
built with OpenSSL 1.1.0f  25 May 2017
TLS SNI support enabled

configure arguments: --with-ld-opt='-ljemalloc -Wl,-z,relro -Wl,-rpath,/usr/local/lib' --with-cc-opt='-m64 -march=native -DTCP_FASTOPEN=23 -g -O3 -fstack-protector-strong -flto -fuse-ld=gold --param=ssp-buffer-size=4 -Wformat -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -gsplit-dwarf' --sbin-path=/usr/local/sbin/nginx --conf-path=/usr/local/nginx/conf/nginx.conf --with-compat --with-http_stub_status_module --with-http_secure_link_module --with-libatomic --with-http_gzip_static_module --add-dynamic-module=../ngx_pagespeed-1.12.34.3-stable --with-http_sub_module --with-http_addition_module --with-http_image_filter_module=dynamic --with-http_geoip_module=dynamic --with-stream_geoip_module=dynamic --with-stream_realip_module --with-stream_ssl_preread_module --with-threads --with-stream=dynamic --with-stream_ssl_module --with-http_realip_module --add-dynamic-module=../ngx-fancyindex-0.4.2 --add-module=../ngx_cache_purge-2.4.2 --add-module=../ngx_devel_kit-0.3.0 --add-dynamic-module=../set-misc-nginx-module-0.31 --add-dynamic-module=../echo-nginx-module-0.61 --add-module=../redis2-nginx-module-0.14 --add-module=../ngx_http_redis-0.3.7 --add-module=../memc-nginx-module-0.18 --add-module=../srcache-nginx-module-0.31 --add-dynamic-module=../headers-more-nginx-module-0.32 --with-pcre=../pcre-8.41 --with-pcre-jit --with-zlib=../zlib-1.2.11 --with-http_ssl_module --with-http_v2_module --with-openssl=../openssl-1.1.0f --with-openssl-opt='enable-ec_nistp_64_gcc_128'

[kn007]

Same ERROR here

nginx version: nginx/1.13.5
built by gcc 6.3.1 20170216 (Red Hat 6.3.1-3) (GCC)
built with OpenSSL 1.0.2l  25 May 2017
TLS SNI support enabled

Nginx 1.13.6 has already declaration of ‘h2c’.

[pamamolf]

Same error for me also:

nginx version: nginx/1.13.6
built by gcc 6.3.1 20170216 (Red Hat 6.3.1-3) (GCC)
built with LibreSSL 2.5.5
TLS SNI support enabled

[injust]

Removing line 324 from the patch will make it work.

For a version of the HPACK patch that works on nginx 1.13.6, see https://github.com/Injust/hws/blob/master/individual-patches/nginx__http2-hpack.patch.

[HansVanEijsden]

Thanks for fixing! 😃 But unfortunately the patch is giving me problems now, with Nginx 1.13.6. At random I get curl errors: "Stream error in the HTTP/2 framing layer" with images. And, Chrome works fine but Safari (newest, on iOS and macOS) sometimes also doesn't load images. After doing a clean build without the patch, all is working again.

The clean working build without your patch: nginx version: nginx/1.13.6 built by gcc 6.3.0 20170516 (Debian 6.3.0-18) built with OpenSSL 1.1.0f 25 May 2017 TLS SNI support enabled configure arguments: --add-module=/usr/local/src/ngx_brotli_module --add-module=/home/hans/ngx_pagespeed-1.12.34.3-stable --prefix=/opt/nginx --user=www-data --group=www-data --with-http_ssl_module --with-http_v2_module --with-openssl=/usr/local/src/openssl-1.1.0f --with-openssl-opt='enable-ec_nistp_64_gcc_128 -DCFLAGS='-O3 -march=native -flto -fuse-linker-plugin'' --with-pcre-jit --with-file-aio --with-http_flv_module --with-http_geoip_module --with-http_gunzip_module --with-http_mp4_module --with-http_realip_module --with-http_stub_status_module --with-threads --with-libatomic --add-module=/usr/local/src/headers-more-nginx-module --add-module=/usr/local/src/echo-nginx-module --add-module=/usr/local/src/ngx_http_substitutions_filter_module --add-module=/usr/local/src/srcache-nginx-module --add-module=/usr/local/src/redis2-nginx-module --add-module=/usr/local/src/ngx_http_redis-0.3.8 --add-module=/usr/local/src/ngx_devel_kit --add-module=/usr/local/src/set-misc-nginx-module --add-module=/usr/local/src/nginx-module-vts --with-cc-opt='-DTCP_FASTOPEN=23 -march=native -flto -O3 -fuse-linker-plugin -Wno-error=strict-aliasing -fstack-protector-strong -D_FORTIFY_SOURCE=2' --with-ld-opt='-lrt -z relro -fstack-protector-strong'

[vkrasnov]

I will take a look at it

[HansVanEijsden]

@vkrasnov thanks, let me know if you need more info (I can reproduce it on all of my servers).

[HansVanEijsden]

@vkrasnov are you able to reproduce the issue? Is there anything I can help you with, to make the patch working again? @centminmod Did you have testing-time for it already (I'm following https://community.centminmod.com/threads/nginx-announce-nginx-1-13-6.13079/) too?

[vkrasnov]

@HansVanEijsden, no. Can you try removing:

    if (h2c->table_update) {
        ngx_log_debug0(NGX_LOG_DEBUG_HTTP, fc->log, 0,
                       "http2 table size update: 0");
        *pos++ = (1 << 5) | 0;
        h2c->table_update = 0;
    }

from ngx_http_v2_filter_module.c ? It conflicts with the patch logic.

[HansVanEijsden]

Thanks for the update @vkrasnov - I've followed your advice and removed exactly those lines from ngx_http_v2_filter_module.c and it gives me these errors when building now:

cc -c -pipe  -O -W -Wall -Wpointer-arith -Wno-unused-parameter -Werror -g -DTCP_FASTOPEN=23 -march=native -flto -O3 -fuse-linker-plugin -Wno-error=strict-aliasing -fstack-protector-strong -D_FORTIFY_SOURCE=2  -D_GLIBCXX_USE_CXX11_ABI=0 -DNDK_SET_VAR -DNDK_UPSTREAM_LIST -I src/core -I src/event -I src/event/modules -I src/os/unix -I /usr/local/src/ngx_brotli_module/brotli/include/ -I /usr/local/src/ngx_devel_kit/objs -I objs/addon/ndk -I /usr/local/src/openssl-1.1.0f/.openssl/include -I objs -I src/http -I src/http/modules -I src/http/v2 -I /home/hans/ngx_pagespeed-1.12.34.3-stable/psol/include -I /home/hans/ngx_pagespeed-1.12.34.3-stable/psol/include/third_party/chromium/src -I /home/hans/ngx_pagespeed-1.12.34.3-stable/psol/include/third_party/google-sparsehash/src/src -I /home/hans/ngx_pagespeed-1.12.34.3-stable/psol/include/third_party/google-sparsehash/gen/arch/linux/x64/include -I /home/hans/ngx_pagespeed-1.12.34.3-stable/psol/include/third_party/grpc/src/include -I /home/hans/ngx_pagespeed-1.12.34.3-stable/psol/include/third_party/protobuf/src/src -I /home/hans/ngx_pagespeed-1.12.34.3-stable/psol/include/third_party/re2/src -I /home/hans/ngx_pagespeed-1.12.34.3-stable/psol/include/out/Release/obj/gen -I /home/hans/ngx_pagespeed-1.12.34.3-stable/psol/include/out/Release/obj/gen/protoc_out/instaweb -I /home/hans/ngx_pagespeed-1.12.34.3-stable/psol/include/third_party/apr/src/include -I /home/hans/ngx_pagespeed-1.12.34.3-stable/psol/include/third_party/aprutil/src/include -I /home/hans/ngx_pagespeed-1.12.34.3-stable/psol/include/third_party/apr/gen/arch/linux/x64/include -I /home/hans/ngx_pagespeed-1.12.34.3-stable/psol/include/third_party/aprutil/gen/arch/linux/x64/include -I /home/hans/ngx_pagespeed-1.12.34.3-stable/psol/include/url -I /usr/local/src/ngx_devel_kit/src -I /usr/local/src/ngx_devel_kit/src -I /usr/local/src/ngx_devel_kit/objs -I objs/addon/ndk \
    -o objs/src/http/modules/ngx_http_gzip_filter_module.o \
    src/http/modules/ngx_http_gzip_filter_module.c
src/http/v2/ngx_http_v2_filter_module.c: In function ‘ngx_http_v2_header_filter’:
src/http/v2/ngx_http_v2_filter_module.c:132:32: error: unused variable ‘frame’ [-Werror=unused-variable]
     ngx_http_v2_out_frame_t   *frame;
                                ^~~~~
src/http/v2/ngx_http_v2_filter_module.c:131:32: error: unused variable ‘cln’ [-Werror=unused-variable]
     ngx_http_cleanup_t        *cln;
                                ^~~
src/http/v2/ngx_http_v2_filter_module.c:124:47: error: unused variable ‘start’ [-Werror=unused-variable]
     u_char                     status, *pos, *start, *p, *tmp;
                                               ^~~~~
src/http/v2/ngx_http_v2_filter_module.c: At top level:
src/http/v2/ngx_http_v2_filter_module.c:421:5: error: data definition has no type or storage class [-Werror]
     h2c = r->stream->connection;
     ^~~
src/http/v2/ngx_http_v2_filter_module.c:421:5: error: type defaults to ‘int’ in declaration of ‘h2c’ [-Werror=implicit-int]
src/http/v2/ngx_http_v2_filter_module.c:421:11: error: ‘r’ undeclared here (not in a function)
     h2c = r->stream->connection;
           ^
src/http/v2/ngx_http_v2_filter_module.c:423:5: error: expected identifier or ‘(’ before ‘if’
     if (h2c->indicate_resize) {
     ^~
src/http/v2/ngx_http_v2_filter_module.c:437:5: error: expected identifier or ‘(’ before ‘if’
     if (status) {
     ^~
src/http/v2/ngx_http_v2_filter_module.c:440:7: error: expected identifier or ‘(’ before ‘else’
     } else {
       ^~~~
src/http/v2/ngx_http_v2_filter_module.c:446:5: error: expected identifier or ‘(’ before ‘if’
     if (r->headers_out.server == NULL) {
     ^~
src/http/v2/ngx_http_v2_filter_module.c:458:5: error: expected identifier or ‘(’ before ‘if’
     if (r->headers_out.date == NULL) {
     ^~
src/http/v2/ngx_http_v2_filter_module.c:462:5: error: expected identifier or ‘(’ before ‘if’
     if (r->headers_out.content_type.len) {
     ^~
src/http/v2/ngx_http_v2_filter_module.c:492:5: error: expected identifier or ‘(’ before ‘if’
     if (r->headers_out.content_length == NULL
     ^~
src/http/v2/ngx_http_v2_filter_module.c:501:5: error: expected identifier or ‘(’ before ‘if’
     if (r->headers_out.last_modified == NULL
     ^~
src/http/v2/ngx_http_v2_filter_module.c:511:5: error: expected identifier or ‘(’ before ‘if’
     if (r->headers_out.location && r->headers_out.location->value.len) {
     ^~
src/http/v2/ngx_http_v2_filter_module.c:516:5: error: expected identifier or ‘(’ before ‘if’
     if (r->gzip_vary) {
     ^~
src/http/v2/ngx_http_v2_filter_module.c:521:5: error: data definition has no type or storage class [-Werror]
     part = &r->headers_out.headers.part;
     ^~~~
src/http/v2/ngx_http_v2_filter_module.c:521:5: error: type defaults to ‘int’ in declaration of ‘part’ [-Werror=implicit-int]
src/http/v2/ngx_http_v2_filter_module.c:522:5: error: data definition has no type or storage class [-Werror]
     header = part->elts;
     ^~~~~~
src/http/v2/ngx_http_v2_filter_module.c:522:5: error: type defaults to ‘int’ in declaration of ‘header’ [-Werror=implicit-int]
src/http/v2/ngx_http_v2_filter_module.c:522:18: error: invalid type argument of ‘->’ (have ‘int’)
     header = part->elts;
                  ^~
src/http/v2/ngx_http_v2_filter_module.c:524:5: error: expected identifier or ‘(’ before ‘for’
     for (i = 0; /* void */; i++) {
     ^~~
src/http/v2/ngx_http_v2_filter_module.c:524:30: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘++’ token
     for (i = 0; /* void */; i++) {
                              ^~
src/http/v2/ngx_http_v2_filter_module.c:546:5: error: data definition has no type or storage class [-Werror]
     frame = ngx_http_v2_create_headers_frame(r, start, pos, r->header_only);
     ^~~~~
src/http/v2/ngx_http_v2_filter_module.c:546:5: error: type defaults to ‘int’ in declaration of ‘frame’ [-Werror=implicit-int]
src/http/v2/ngx_http_v2_filter_module.c:546:49: error: ‘start’ undeclared here (not in a function)
     frame = ngx_http_v2_create_headers_frame(r, start, pos, r->header_only);
                                                 ^~~~~
src/http/v2/ngx_http_v2_filter_module.c:546:56: error: ‘pos’ undeclared here (not in a function)
     frame = ngx_http_v2_create_headers_frame(r, start, pos, r->header_only);
                                                        ^~~
src/http/v2/ngx_http_v2_filter_module.c:547:5: error: expected identifier or ‘(’ before ‘if’
     if (frame == NULL) {
     ^~
src/http/v2/ngx_http_v2_filter_module.c:551:38: error: expected ‘)’ before ‘->’ token
     ngx_http_v2_queue_blocked_frame(r->stream->connection, frame);
                                      ^~
src/http/v2/ngx_http_v2_filter_module.c:553:6: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘->’ token
     r->stream->queued = 1;
      ^~
src/http/v2/ngx_http_v2_filter_module.c:555:5: error: data definition has no type or storage class [-Werror]
     cln = ngx_http_cleanup_add(r, 0);
     ^~~
src/http/v2/ngx_http_v2_filter_module.c:555:5: error: type defaults to ‘int’ in declaration of ‘cln’ [-Werror=implicit-int]
src/http/v2/ngx_http_v2_filter_module.c:556:5: error: expected identifier or ‘(’ before ‘if’
     if (cln == NULL) {
     ^~
src/http/v2/ngx_http_v2_filter_module.c:560:8: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘->’ token
     cln->handler = ngx_http_v2_filter_cleanup;
        ^~
src/http/v2/ngx_http_v2_filter_module.c:561:8: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘->’ token
     cln->data = r->stream;
        ^~
src/http/v2/ngx_http_v2_filter_module.c:563:7: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘->’ token
     fc->send_chain = ngx_http_v2_send_chain;
       ^~
src/http/v2/ngx_http_v2_filter_module.c:564:7: error: expected ‘=’, ‘,’, ‘;’, ‘asm’ or ‘__attribute__’ before ‘->’ token
     fc->need_last_buf = 1;
       ^~
src/http/v2/ngx_http_v2_filter_module.c:566:5: error: expected identifier or ‘(’ before ‘return’
     return ngx_http_v2_filter_send(fc, r->stream);
     ^~~~~~
src/http/v2/ngx_http_v2_filter_module.c:567:1: error: expected identifier or ‘(’ before ‘}’ token
 }
 ^
src/http/v2/ngx_http_v2_filter_module.c: In function ‘ngx_http_v2_header_filter’:
src/http/v2/ngx_http_v2_filter_module.c:419:5: error: control reaches end of non-void function [-Werror=return-type]
     }
     ^
At top level:
src/http/v2/ngx_http_v2_filter_module.c:1435:1: error: ‘ngx_http_v2_filter_cleanup’ defined but not used [-Werror=unused-function]
 ngx_http_v2_filter_cleanup(void *data)
 ^~~~~~~~~~~~~~~~~~~~~~~~~~
src/http/v2/ngx_http_v2_filter_module.c:836:1: error: ‘ngx_http_v2_send_chain’ defined but not used [-Werror=unused-function]
 ngx_http_v2_send_chain(ngx_connection_t *fc, ngx_chain_t *in, off_t limit)
 ^~~~~~~~~~~~~~~~~~~~~~
cc1: all warnings being treated as errors
objs/Makefile:1221: recept voor doel 'objs/src/http/v2/ngx_http_v2_filter_module.o' is mislukt
make[1]: *** [objs/src/http/v2/ngx_http_v2_filter_module.o] Fout 1
make[1]: *** Wachten op onvoltooide taken...
make[1]: Map '/usr/local/src/nginx-1.13.6' wordt verlaten
Makefile:11: recept voor doel 'install' is mislukt
make: *** [install] Fout 2

[vkrasnov]

Probably cause it is missing the closing }. You should remove it too.

[HansVanEijsden]

My original ngx_http_v2_filter_module.c looks like this:

    start = pos;

    if (h2c->table_update) {
        ngx_log_debug0(NGX_LOG_DEBUG_HTTP, fc->log, 0,
                       "http2 table size update: 0");
        *pos++ = (1 << 5) | 0;
        h2c->table_update = 0;
    }

According to your example I assume I should leave h2c->table_update = 0; there?

[vkrasnov]

It doesn't matter, as long as *pos++ = (1 << 5) | 0; is not there. I just need to know if that is the problem.

[vkrasnov]

I corrected the code to be removed. The start = pos should stay.

[HansVanEijsden]

I confirm to have it working @vkrasnov, after removing those lines. No more errors (yet), and HPACK works great again too. Thanks for the effort! 👍🏻

$ h2load https://www.weblogzwolle.nl -n 100 | tail -6 |head -1
traffic: 7.94MB (8329738) total, 2.04KB (2089) headers (space savings 97.40%), 7.93MB (8315900) data

[ahmed-sigmalux]

Any plans to make this fix in the master branch?

[vkrasnov]

Working on it.

[kevin25]

Any update on this guys?

objs/Makefile:983: recipe for target 'objs/src/http/v2/ngx_http_v2_filter_module.o' failed make[1]: [objs/src/http/v2/ngx_http_v2_filter_module.o] Error 1 make[1]: Leaving directory '/usr/local/src/nginx-1.13.6' Makefile:8: recipe for target 'build' failed make: [build] Error 2

[IThinkIGottaGo]

I search google very hard finally find here....And thanks to friends above work to this problem. I am using Nginx 1.13.8 and today was Jan 9th ,met same problem. Because my English not very well ,so at began I didn't figure out HansVanEijsden which code that he deleted. But finally i found it. Actually it was a duplicated variable name error. So for convenient if anyone also met same thing and came here before the master branch solve this problem, There is a easy way to solve the problem here.

First, if you came here you must already patched "HPACK 1.13.6". So get into your nginx dir, and move to XXX/nginx-1.13.8/src/http/v2 dir. You will find there will be a file named ngx_http_v2_filter_module.c. And use vim or other method open and edit it. Then find line number #122 and check the function named ngx_http_v2_header_filter. Then you will find there will be two same name variable named ngx_http_v2_connection_t *h2c; in the line #133 and #137. Finally delete anyone and save it. Back to your XXX/nginx-1.13.8/ dir and continue make it. if there wasn't others problem, everything should be OK now. Hope my poor grammar didn't affect you try to fix the program. : )

[jarredou]

@FreeStyleNight Thanks for the simple recap/update of the thread ! It solved my problem !

[PikachuEXE]

Created a PR for 1.13.9

[kn007]

For nginx 1.13.9, could using this patch: https://github.com/kn007/patch/blob/master/nginx.patch

[injust]

This (standalone HPACK patch) should work for nginx 1.13.9.

[PikachuEXE]

Update: I am using #92 now Fix code injection position for usage of indicate_resize

[centminmod]

FYI, 1.3.10 will also break this hpack patch https://github.com/cloudflare/sslconfig/issues/93

[kevin25]

Is there any update for Nginx 1.16.1?

[PikachuEXE]

@kevin25 I am using https://raw.githubusercontent.com/hakasenyang/openssl-patch/master/nginx_hpack_push_1.15.3.patch for 1.17.x