Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Running nginx_1.13.1_http2_hpack.patch fails on Nginx 1.13.4 #77

Open
fmunteanu opened this issue Aug 10, 2017 · 5 comments
Open

Running nginx_1.13.1_http2_hpack.patch fails on Nginx 1.13.4 #77

fmunteanu opened this issue Aug 10, 2017 · 5 comments

Comments

@fmunteanu
Copy link

fmunteanu commented Aug 10, 2017

@vkrasnov I fixed the broken hunks:

$ diff -Naur src/http/v2/ngx_http_v2_filter_module.c.floren src/http/v2/ngx_http_v2_filter_module.c
--- src/http/v2/ngx_http_v2_filter_module.c.floren	2017-08-08 15:00:13.000000000 +0000
+++ src/http/v2/ngx_http_v2_filter_module.c	2017-08-10 15:47:40.734471424 +0000
@@ -53,10 +48,6 @@
 #define NGX_HTTP_V2_NO_TRAILERS           (ngx_http_v2_out_frame_t *) -1


-static u_char *ngx_http_v2_string_encode(u_char *dst, u_char *src, size_t len,
-    u_char *tmp, ngx_uint_t lower);
-static u_char *ngx_http_v2_write_int(u_char *pos, ngx_uint_t prefix,
-    ngx_uint_t value);
 static ngx_http_v2_out_frame_t *ngx_http_v2_create_headers_frame(
     ngx_http_request_t *r, u_char *pos, u_char *end, ngx_uint_t fin);
 static ngx_http_v2_out_frame_t *ngx_http_v2_create_trailers_frame(
@@ -597,23 +588,6 @@
             continue;
         }

-#if (NGX_DEBUG)
-        if (fc->log->log_level & NGX_LOG_DEBUG_HTTP) {
-            ngx_strlow(tmp, header[i].key.data, header[i].key.len);
-
-            ngx_log_debug3(NGX_LOG_DEBUG_HTTP, fc->log, 0,
-                           "http2 output header: \"%*s: %V\"",
-                           header[i].key.len, tmp, &header[i].value);
-        }
-#endif
-
-        *pos++ = 0;
-
-        pos = ngx_http_v2_write_name(pos, header[i].key.data,
-                                     header[i].key.len, tmp);
-
-        pos = ngx_http_v2_write_value(pos, header[i].value.data,
-                                      header[i].value.len, tmp);
     }

     frame = ngx_http_v2_create_headers_frame(r, start, pos, r->header_only);

I'm attaching the patch, can you please commit it?
nginx-1.13.4-http2-hpack.patch.zip

@injust
Copy link
Contributor

injust commented Aug 11, 2017

nginx_1.13.1_http2_hpack.patch applies cleanly onto nginx 1.13.4.
Are you trying to apply it after other patches by any chance?

@Starbix
Copy link

Starbix commented Aug 12, 2017

It also fails for me, I haven't applied any other patch

patch -p1 < nginx_1.13.1_http2_hpack.patch
patching file auto/modules
patching file auto/options
patching file src/core/ngx_murmurhash.c
patching file src/core/ngx_murmurhash.h
patching file src/http/v2/ngx_http_v2.c
patching file src/http/v2/ngx_http_v2.h
patching file src/http/v2/ngx_http_v2_filter_module.c
Hunk 2 FAILED 51/46.
 #define NGX_HTTP_V2_VARY_INDEX            59


-static u_char *ngx_http_v2_string_encode(u_char *dst, u_char *src, size_t len,
-    u_char *tmp, ngx_uint_t lower);
-static u_char *ngx_http_v2_write_int(u_char *pos, ngx_uint_t prefix,
-    ngx_uint_t value);
 static ngx_http_v2_out_frame_t *ngx_http_v2_create_headers_frame(
     ngx_http_request_t *r, u_char *pos, u_char *end);

patching file src/http/v2/ngx_http_v2_table.c

@centminmod
Copy link

patches and compiles fine here on CentOS 7.3 64bit with nginx 1.13.4 and openssl 1.0.2l

patch -p1 < nginx_1.13.1_http2_hpack.patch
patching file auto/modules
patching file auto/options
Hunk #2 succeeded at 223 (offset 1 line).
Hunk #3 succeeded at 434 (offset 2 lines).
patching file src/core/ngx_murmurhash.c
patching file src/core/ngx_murmurhash.h
patching file src/http/v2/ngx_http_v2.c
Hunk #2 succeeded at 2020 (offset 5 lines).
patching file src/http/v2/ngx_http_v2.h
Hunk #5 succeeded at 401 (offset 9 lines).
patching file src/http/v2/ngx_http_v2_filter_module.c
Hunk #2 succeeded at 48 with fuzz 2 (offset 2 lines).
Hunk #3 succeeded at 133 (offset 4 lines).
Hunk #4 succeeded at 142 (offset 4 lines).
Hunk #5 succeeded at 405 (offset 4 lines).
Hunk #6 succeeded at 413 (offset 4 lines).
Hunk #7 succeeded at 433 (offset 4 lines).
Hunk #8 succeeded at 480 (offset 4 lines).
Hunk #9 succeeded at 532 with fuzz 1 (offset 4 lines).
Hunk #10 succeeded at 674 (offset 116 lines).
Hunk #11 succeeded at 700 (offset 116 lines).
patching file src/http/v2/ngx_http_v2_table.c

nginx -V
nginx version: nginx/1.13.4
built by gcc 6.2.1 20160916 (Red Hat 6.2.1-3) (GCC)
built with OpenSSL 1.0.2l 25 May 2017
TLS SNI support enabled
configure arguments: --with-ld-opt='-ljemalloc -lpcre -Wl,-z,relro -Wl,-rpath,/usr/local/lib' --with-cc-opt='-m64 -march=native -DTCP_FASTOPEN=23 -g -O3 -Wno-error=strict-aliasing -fstack-protector-strong -flto -fuse-ld=gold --param=ssp-buffer-size=4 -Wformat -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wno-deprecated-declarations -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_brotli --add-dynamic-module=../ngx_pagespeed-1.12.34.2-stable --with-http_sub_module --with-http_addition_module --with-http_image_filter_module=dynamic --with-http_geoip_module --with-stream_geoip_module --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.0 --add-module=../ngx_cache_purge-2.3 --add-module=../ngx_devel_kit-0.3.0 --add-module=../set-misc-nginx-module-0.31 --add-module=../echo-nginx-module-0.61 --add-module=../redis2-nginx-module-0.14 --add-module=../ngx_http_redis-0.3.7 --add-dynamic-module=../lua-nginx-module-0.10.10 --add-module=../memc-nginx-module-0.18 --add-module=../srcache-nginx-module-0.31 --add-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-http_v2_hpack_enc --with-openssl=../openssl-1.0.2l --with-openssl-opt='enable-ec_nistp_64_gcc_128'

@kn007
Copy link

kn007 commented Aug 12, 2017

Same as @centminmod , patch working fine for me.
CentOS 7.3.1611 X64, Nginx 1.13.4, OpenSSL 1.0.2l.

@SoftCreatR
Copy link

Works even on Nginx 1.13.5. However, it's NOT compatible with the http2/spdy patch. I've created a single patch that combines all three nginx patches and which is compatible with nginx 1.13.5 (and lower, i guess):

https://gist.github.com/SoftCreatR/b5047bda93e10ef048c7f7d4ffba3ec6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants