[PATCH] Configure: increase the default optimization level to -O2

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora 
# Date 1714589900 0
#  Wed May 01 18:58:20 2024 +
# Node ID 429191aad8d3ab8f70d40dfdd7684a256357bc48
# Parent  49dce50fad40bf09db81ca2a35983ecd7b740e43
Configure: increase the default optimization level to -O2.

Signed-off-by: Piotr Sikora 

diff -r 49dce50fad40 -r 429191aad8d3 auto/cc/clang
--- a/auto/cc/clang Tue Apr 16 18:29:59 2024 +0400
+++ b/auto/cc/clang Wed May 01 18:58:20 2024 +
@@ -19,9 +19,9 @@
 
 # optimizations
 
-#NGX_CLANG_OPT="-O2"
+NGX_CLANG_OPT="-O2"
 #NGX_CLANG_OPT="-Oz"
-NGX_CLANG_OPT="-O"
+#NGX_CLANG_OPT="-O"
 
 case $CPU in
 pentium)
diff -r 49dce50fad40 -r 429191aad8d3 auto/cc/gcc
--- a/auto/cc/gcc   Tue Apr 16 18:29:59 2024 +0400
+++ b/auto/cc/gcc   Wed May 01 18:58:20 2024 +
@@ -48,9 +48,9 @@
 
 # optimizations
 
-#NGX_GCC_OPT="-O2"
+NGX_GCC_OPT="-O2"
 #NGX_GCC_OPT="-Os"
-NGX_GCC_OPT="-O"
+#NGX_GCC_OPT="-O"
 
 #CFLAGS="$CFLAGS -fomit-frame-pointer"
 
___
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel

[PATCH] Configure: postpone running the test binary

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora 
# Date 1714589827 0
#  Wed May 01 18:57:07 2024 +
# Node ID 2d5e754e3a4e7a59eaf7f653ad4fd5346f53eab4
# Parent  49dce50fad40bf09db81ca2a35983ecd7b740e43
Configure: postpone running the test binary.

Previously, the ./configure script would attempt to execute the test
binary during the C compiler check, and it would abort with an error
saying that the compiler doesn't work if there was any issue running
the test binary (e.g. due to missing compiler and/or linker flags).

This change delays that check and runs the test binary compiled with
all provided compiler and linker flags.

Signed-off-by: Piotr Sikora 

diff -r 49dce50fad40 -r 2d5e754e3a4e auto/cc/conf
--- a/auto/cc/conf  Tue Apr 16 18:29:59 2024 +0400
+++ b/auto/cc/conf  Wed May 01 18:57:07 2024 +
@@ -164,6 +164,23 @@
 fi
 
 
+ngx_feature="working test binary"
+ngx_feature_name=
+ngx_feature_run=yes
+ngx_feature_incs=
+ngx_feature_path=
+ngx_feature_libs=
+ngx_feature_test=
+. auto/feature
+
+if [ $ngx_found = no ]; then
+echo
+echo $0: error: cannot execute test binary
+echo
+exit 1
+fi
+
+
 ngx_feature="-Wl,-E switch"
 ngx_feature_name=
 ngx_feature_run=no
diff -r 49dce50fad40 -r 2d5e754e3a4e auto/cc/name
--- a/auto/cc/name  Tue Apr 16 18:29:59 2024 +0400
+++ b/auto/cc/name  Wed May 01 18:57:07 2024 +
@@ -7,7 +7,7 @@
 
 ngx_feature="C compiler"
 ngx_feature_name=
-ngx_feature_run=yes
+ngx_feature_run=no
 ngx_feature_incs=
 ngx_feature_path=
 ngx_feature_libs=
___
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel

[PATCH] Configure: build C++ test module using C++ compiler

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora 
# Date 1714589815 0
#  Wed May 01 18:56:55 2024 +
# Node ID 787e1adea9aa7f681884657f119e864af0be3e0d
# Parent  49dce50fad40bf09db81ca2a35983ecd7b740e43
Configure: build C++ test module using C++ compiler.

This fixes build when using C compiler that doesn't support C++.

Signed-off-by: Piotr Sikora 

diff -r 49dce50fad40 -r 787e1adea9aa auto/cc/conf
--- a/auto/cc/conf  Tue Apr 16 18:29:59 2024 +0400
+++ b/auto/cc/conf  Wed May 01 18:56:55 2024 +
@@ -142,6 +142,7 @@
 fi
 
 CFLAGS="$CFLAGS $NGX_CC_OPT"
+CXXFLAGS="$CXXFLAGS $NGX_CXX_OPT"
 NGX_TEST_LD_OPT="$NGX_LD_OPT"
 
 if [ "$NGX_PLATFORM" != win32 ]; then
diff -r 49dce50fad40 -r 787e1adea9aa auto/make
--- a/auto/make Tue Apr 16 18:29:59 2024 +0400
+++ b/auto/make Wed May 01 18:56:55 2024 +
@@ -23,6 +23,8 @@
 
 CC =   $CC
 CFLAGS = $CFLAGS
+CXX =  $CXX
+CXXFLAGS = $CXXFLAGS
 CPP =  $CPP
 LINK = $LINK
 
@@ -384,22 +386,36 @@
 if test -n "$MISC_SRCS"; then
 
 ngx_cc="\$(CC) $ngx_compile_opt \$(CFLAGS) $ngx_use_pch \$(ALL_INCS)"
+ngx_cxx="\$(CXX) $ngx_compile_opt \$(CXXFLAGS) $ngx_use_pch \$(ALL_INCS)"
 
-for ngx_src in $MISC_SRCS
+for ngx_source in $MISC_SRCS
 do
-ngx_src=`echo $ngx_src | sed -e "s/\//$ngx_regex_dirsep/g"`
+ngx_src=`echo $ngx_source | sed -e "s/\//$ngx_regex_dirsep/g"`
 ngx_obj=`echo $ngx_src \
 | sed -e "s#^\(.*\.\)cpp\\$#$ngx_objs_dir\1$ngx_objext#g" \
   -e "s#^\(.*\.\)cc\\$#$ngx_objs_dir\1$ngx_objext#g" \
   -e "s#^\(.*\.\)c\\$#$ngx_objs_dir\1$ngx_objext#g" \
   -e "s#^\(.*\.\)S\\$#$ngx_objs_dir\1$ngx_objext#g"`
 
-cat << END>> $NGX_MAKEFILE
+if [ $ngx_source = src/misc/ngx_cpp_test_module.cpp ]; then
+
+cat << END>> $NGX_MAKEFILE
+
+$ngx_obj:  \$(CORE_DEPS) $ngx_cont$ngx_src
+   $ngx_cxx$ngx_tab$ngx_objout$ngx_obj$ngx_tab$ngx_src$NGX_AUX
+
+END
+
+else
+
+cat << END>> $NGX_MAKEFILE
 
 $ngx_obj:  \$(CORE_DEPS) $ngx_cont$ngx_src
$ngx_cc$ngx_tab$ngx_objout$ngx_obj$ngx_tab$ngx_src$NGX_AUX
 
 END
+
+fi
 done
 
 fi
diff -r 49dce50fad40 -r 787e1adea9aa auto/options
--- a/auto/options  Tue Apr 16 18:29:59 2024 +0400
+++ b/auto/options  Wed May 01 18:56:55 2024 +
@@ -18,11 +18,13 @@
 NGX_BUILD=
 
 CC=${CC:-cc}
+CXX=${CXX:-c++}
 CPP=
 NGX_OBJS=objs
 
 NGX_DEBUG=NO
 NGX_CC_OPT=
+NGX_CXX_OPT=
 NGX_LD_OPT=
 CPU=NO
 
@@ -358,8 +360,10 @@
 --with-compat)   NGX_COMPAT=YES ;;
 
 --with-cc=*) CC="$value";;
+--with-cxx=*)CXX="$value"   ;;
 --with-cpp=*)CPP="$value"   ;;
 --with-cc-opt=*) NGX_CC_OPT="$value";;
+--with-cxx-opt=*)NGX_CXX_OPT="$value"   ;;
 --with-ld-opt=*) NGX_LD_OPT="$value";;
 --with-cpu-opt=*)CPU="$value"   ;;
 --with-debug)NGX_DEBUG=YES  ;;
@@ -578,8 +582,10 @@
   --with-compat  dynamic modules compatibility
 
   --with-cc=PATH set C compiler pathname
+  --with-cxx=PATHset C++ compiler pathname
   --with-cpp=PATHset C preprocessor pathname
   --with-cc-opt=OPTIONS  set additional C compiler options
+  --with-cxx-opt=OPTIONS set additional C++ compiler options
   --with-ld-opt=OPTIONS  set additional linker options
   --with-cpu-opt=CPU build for the specified CPU, valid values:
  pentium, pentiumpro, pentium3, pentium4,
___
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel

[PATCH] Configure: fix build on Windows using non-MSVC compilers

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora 
# Date 1714589717 0
#  Wed May 01 18:55:17 2024 +
# Node ID 43b04ece77b7132db868122a20c99c8ba89adfb5
# Parent  49dce50fad40bf09db81ca2a35983ecd7b740e43
Configure: fix build on Windows using non-MSVC compilers.

Previously, kernel32 and user32 libraries were linked only when
using MSVC, but they are needed regardless of the compiler.

Signed-off-by: Piotr Sikora 

diff -r 49dce50fad40 -r 43b04ece77b7 auto/cc/msvc
--- a/auto/cc/msvc  Tue Apr 16 18:29:59 2024 +0400
+++ b/auto/cc/msvc  Wed May 01 18:55:17 2024 +
@@ -114,8 +114,6 @@
 
 CFLAGS="$CFLAGS $LIBC"
 
-CORE_LIBS="$CORE_LIBS kernel32.lib user32.lib"
-
 # Win32 GUI mode application
 #CORE_LINK="$CORE_LINK -subsystem:windows -entry:mainCRTStartup"
 
diff -r 49dce50fad40 -r 43b04ece77b7 auto/os/win32
--- a/auto/os/win32 Tue Apr 16 18:29:59 2024 +0400
+++ b/auto/os/win32 Wed May 01 18:55:17 2024 +
@@ -19,14 +19,14 @@
 case "$NGX_CC_NAME" in
 
 clang | gcc)
-CORE_LIBS="$CORE_LIBS -ladvapi32 -lws2_32"
+CORE_LIBS="$CORE_LIBS -lkernel32 -luser32 -ladvapi32 -lws2_32"
 MAIN_LINK="$MAIN_LINK -Wl,--export-all-symbols"
 MAIN_LINK="$MAIN_LINK -Wl,--out-implib=$NGX_OBJS/libnginx.a"
 MODULE_LINK="-shared -L $NGX_OBJS -lnginx"
 ;;
 
 *)
-CORE_LIBS="$CORE_LIBS advapi32.lib ws2_32.lib"
+CORE_LIBS="$CORE_LIBS kernel32.lib user32.lib advapi32.lib ws2_32.lib"
 ;;
 
 esac
___
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel

[PATCH] Configure: always test with provided C compiler options

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora 
# Date 1714589692 0
#  Wed May 01 18:54:52 2024 +
# Node ID df39b5d3c3a0c670f3d94e623351b6c659f5be84
# Parent  49dce50fad40bf09db81ca2a35983ecd7b740e43
Configure: always test with provided C compiler options.

Previously, build in auto/include didn't use C compiler options,
which didn't allow using a custom sysroot.

While there, update auto/types/uintptr_t to match others.

Signed-off-by: Piotr Sikora 

diff -r 49dce50fad40 -r df39b5d3c3a0 auto/include
--- a/auto/include  Tue Apr 16 18:29:59 2024 +0400
+++ b/auto/include  Wed May 01 18:54:52 2024 +
@@ -27,7 +27,8 @@
 END
 
 
-ngx_test="$CC -o $NGX_AUTOTEST $NGX_AUTOTEST.c"
+ngx_test="$CC $CC_TEST_FLAGS $CC_AUX_FLAGS \
+  -o $NGX_AUTOTEST $NGX_AUTOTEST.c $NGX_LD_OPT $ngx_feature_libs"
 
 eval "$ngx_test >> $NGX_AUTOCONF_ERR 2>&1"
 
diff -r 49dce50fad40 -r df39b5d3c3a0 auto/types/uintptr_t
--- a/auto/types/uintptr_t  Tue Apr 16 18:29:59 2024 +0400
+++ b/auto/types/uintptr_t  Wed May 01 18:54:52 2024 +
@@ -27,7 +27,7 @@
 END
 
 ngx_test="$CC $CC_TEST_FLAGS $CC_AUX_FLAGS \
-  -o $NGX_AUTOTEST $NGX_AUTOTEST.c $NGX_LD_OPT"
+  -o $NGX_AUTOTEST $NGX_AUTOTEST.c $NGX_LD_OPT $ngx_feature_libs"
 
 eval "$ngx_test >> $NGX_AUTOCONF_ERR 2>&1"
 
___
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel

[PATCH] Configure: fix warnings in a few feature tests

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora 
# Date 1714589586 0
#  Wed May 01 18:53:06 2024 +
# Node ID c083cd8ead811426c6f7bd7c4ab58a413d80be52
# Parent  49dce50fad40bf09db81ca2a35983ecd7b740e43
Configure: fix warnings in a few feature tests.

Signed-off-by: Piotr Sikora 

diff -r 49dce50fad40 -r c083cd8ead81 auto/unix
--- a/auto/unix Tue Apr 16 18:29:59 2024 +0400
+++ b/auto/unix Wed May 01 18:53:06 2024 +
@@ -731,7 +731,7 @@
 ngx_feature_incs=
 ngx_feature_path=
 ngx_feature_libs=
-ngx_feature_test="char buf[1]; ssize_t n; n = pwrite(1, buf, 1, 0);
+ngx_feature_test="const char buf[1] = \"\"; ssize_t n; n = pwrite(1, buf, 1, 
0);
   if (n == -1) return 1"
 . auto/feature
 
@@ -760,7 +760,7 @@
 ngx_feature_incs='#include '
 ngx_feature_path=
 ngx_feature_libs=
-ngx_feature_test="char *p; p = strerrordesc_np(0);
+ngx_feature_test="const char *p; p = strerrordesc_np(0);
   if (p == NULL) return 1"
 . auto/feature
 
___
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel

[PATCH] Configure: test --with-cc-opt options

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora 
# Date 1714589524 0
#  Wed May 01 18:52:04 2024 +
# Node ID 0d5498e86bf8a7f119ed83dbc0789be37d728334
# Parent  49dce50fad40bf09db81ca2a35983ecd7b740e43
Configure: test --with-cc-opt options.

Previously, invalid C compiler options would fail with an
unrelated error much later in the ./configure tests.

This matches the existing test for --with-ld-opt options.

Signed-off-by: Piotr Sikora 

diff -r 49dce50fad40 -r 0d5498e86bf8 auto/cc/conf
--- a/auto/cc/conf  Tue Apr 16 18:29:59 2024 +0400
+++ b/auto/cc/conf  Wed May 01 18:52:04 2024 +
@@ -142,6 +142,29 @@
 fi
 
 CFLAGS="$CFLAGS $NGX_CC_OPT"
+
+if [ "$NGX_PLATFORM" != win32 ]; then
+
+if test -n "$NGX_CC_OPT"; then
+ngx_feature=--with-cc-opt=\"$NGX_CC_OPT\"
+ngx_feature_name=
+ngx_feature_run=no
+ngx_feature_incs=
+ngx_feature_path=
+ngx_feature_libs=
+ngx_feature_test=
+. auto/feature
+
+if [ $ngx_found = no ]; then
+echo
+echo $0: error: the invalid value in --with-cc-opt=\"$NGX_CC_OPT\"
+echo
+exit 1
+fi
+fi
+
+fi
+
 NGX_TEST_LD_OPT="$NGX_LD_OPT"
 
 if [ "$NGX_PLATFORM" != win32 ]; then
___
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel

[PATCH] QUIC: fix build against musl-libc when using Clang

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora 
# Date 1714589495 0
#  Wed May 01 18:51:35 2024 +
# Node ID 962cddbaecf02b9c213dca492a74b23924b8f24c
# Parent  49dce50fad40bf09db81ca2a35983ecd7b740e43
QUIC: fix build against musl-libc when using Clang.

Signed-off-by: Piotr Sikora 

diff -r 49dce50fad40 -r 962cddbaecf0 src/event/ngx_event_udp.c
--- a/src/event/ngx_event_udp.c Tue Apr 16 18:29:59 2024 +0400
+++ b/src/event/ngx_event_udp.c Wed May 01 18:51:35 2024 +
@@ -138,6 +138,14 @@
 ngx_memcpy(, local_sockaddr, local_socklen);
 local_sockaddr = 
 
+#ifndef __GLIBC__
+/* Silence warnings from expanded CMSG_NXTHDR macro when using musl-libc. 
*/
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wunknown-pragmas"
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wsign-compare"
+#endif
+
 for (cmsg = CMSG_FIRSTHDR();
  cmsg != NULL;
  cmsg = CMSG_NXTHDR(, cmsg))
@@ -148,6 +156,11 @@
 }
 }
 
+#ifndef __GLIBC__
+#pragma clang diagnostic pop
+#pragma GCC diagnostic pop
+#endif
+
 #endif
 
 c = ngx_lookup_udp_connection(ls, sockaddr, socklen, local_sockaddr,
diff -r 49dce50fad40 -r 962cddbaecf0 src/event/quic/ngx_event_quic_output.c
--- a/src/event/quic/ngx_event_quic_output.cTue Apr 16 18:29:59 2024 +0400
+++ b/src/event/quic/ngx_event_quic_output.cWed May 01 18:51:35 2024 +
@@ -434,10 +434,25 @@
 *valp = segment;
 
 #if (NGX_HAVE_ADDRINFO_CMSG)
+
+#ifndef __GLIBC__
+/* Silence warnings from expanded CMSG_NXTHDR macro when using musl-libc. 
*/
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wunknown-pragmas"
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wsign-compare"
+#endif
+
 if (c->listening && c->listening->wildcard && c->local_sockaddr) {
 cmsg = CMSG_NXTHDR(, cmsg);
 clen += ngx_set_srcaddr_cmsg(cmsg, c->local_sockaddr);
 }
+
+#ifndef __GLIBC__
+#pragma clang diagnostic pop
+#pragma GCC diagnostic pop
+#endif
+
 #endif
 
 msg.msg_controllen = clen;
diff -r 49dce50fad40 -r 962cddbaecf0 src/event/quic/ngx_event_quic_udp.c
--- a/src/event/quic/ngx_event_quic_udp.c   Tue Apr 16 18:29:59 2024 +0400
+++ b/src/event/quic/ngx_event_quic_udp.c   Wed May 01 18:51:35 2024 +
@@ -140,6 +140,14 @@
 ngx_memcpy(, local_sockaddr, local_socklen);
 local_sockaddr = 
 
+#ifndef __GLIBC__
+/* Silence warnings from expanded CMSG_NXTHDR macro when using musl-libc. 
*/
+#pragma GCC diagnostic push
+#pragma GCC diagnostic ignored "-Wunknown-pragmas"
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wsign-compare"
+#endif
+
 for (cmsg = CMSG_FIRSTHDR();
  cmsg != NULL;
  cmsg = CMSG_NXTHDR(, cmsg))
@@ -150,6 +158,11 @@
 }
 }
 
+#ifndef __GLIBC__
+#pragma clang diagnostic pop
+#pragma GCC diagnostic pop
+#endif
+
 #endif
 
 if (ngx_quic_get_packet_dcid(ev->log, buffer, n, ) != NGX_OK) {
___
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel

Re: [PATCH 2 of 2] SSL: add $ssl_curve when using AWS-LC

[Piotr Sikora via nginx-devel]

Hi Sergey,

> Not sure nginx needs such complexity.
> Instead, I'd expect the missing API to be added to the library,
> especially that both parent and grandparent implement it.
> 
> BoringSSL didn't have such API for quite some time until recently.

Right, this patch predates addition of this API to BoringSSL, and
I only removed BoringSSL's #ifdef when submitting it last month.

Feel free to skip it.

> Since AWS-LC is a fork of BoringSSL, it is welcome to sync up.
> As far as I can see, they used to sync quite often, so it is ought
> to be resolved automagically after the next sync.  To ease to task,
> here I provide certain git hashes:
> 
> 6cf98208371e5c2c8b9d34ce3b8c452ea90e2963 - SSL_get_negotiated_group
> 28c24092e39bfd70852afa2923a3d12d2e9be2f5 - TLSEXT_nid_unknown

You'd assume that's the case, but they've merged a lot of commits more
recent than those two, so they've skipped them (intentionally or not).

Best regards,
Piotr Sikora
___
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel

Re: [PATCH] Configure: link libcrypt when a feature using it is detected

[Piotr Sikora via nginx-devel]

Hi Sergey,

> I'd rewrote the description to be more specific:
> 
> : Configure: fixed Linux crypt_r() test to add libcrypt.
> :
> : Previously, the resulting binary was successfully linked
> : because libcrypt was added in a separate test for crypt().

That's fine with me.

Best regards,
Piotr Sikora
___
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel

Re: [PATCH] macOS: detect cache line size at runtime

[Piotr Sikora via nginx-devel]

Hi Sergey,

> I prefer not to introduce more ad-hoc prefixes in the log summary.
> Something like moving the "macOS" part to the end should be fine.

That's fine with me.

> style: this breaks a perfect indentation of two spaces after type;
> further, it appears to be unsorted by type; I'd put it after u_long

Good catch, thanks!

> This makes the following slight update to the patch.
> If you're okey with it, I will commit it then.

LGTM.

Best regards,
Piotr Sikora
___
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel

Re: [PATCH] Core: fix build without libcrypt

[Piotr Sikora via nginx-devel]

Hi Roman,

> Can you provide an example of a system where this fix is needed?

1. When linking against musl-libc on a system where libcrypt
   is linked against glibc and thus not linkable.

2. When cross-compiling, for similar reasons.

Best regards,
Piotr Sikora
___
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel

Re: [PATCH 1 of 2] SSL: add support for AWS-LC

[Piotr Sikora via nginx-devel]

Hi Roman,

> It looks like this library is not super popular, but the patch is relatively
> large.

Perhaps it's not as widely used as the forks that started ~10 years ago,
but it's basically a version of BoringSSL that's more suitable to use with
NGINX than BoringSSL itself:
- it ships releases and it's versioned,
- it supports OCSP stapling,
- it supports multiple TLS certificates,
- it supports big endian platforms supported by NGINX.

Also, the patch is pretty small.

> Also, compiling nginx with -DOPENSSL_IS_BORINGSSL should probably solve
> the issue.

For the time being, probably, but AWS folks are actively developing it,
so I'd expect it to led to issues sooner rather than later.

Best regards,
Piotr Sikora
___
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel

Re: [PATCH] Core: fix conversion of IPv4-mapped IPv6 addresses

[Piotr Sikora via nginx-devel]

Hi Sergey,

> The "shift" remark doesn't describe a problem in details.

It's not a remark, it's the name of the UndefinedBehaviorSanitizer
check that caught the issue [1].

> @@ -507,7 +507,7 @@ ngx_cidr_match(struct sockaddr *sa, ngx_
> 
>  p = inaddr6->s6_addr;
> 
> -inaddr = p[12] << 24;
> +inaddr = (in_addr_t) p[12] << 24;
>  inaddr += p[13] << 16;
>  inaddr += p[14] << 8;
>  inaddr += p[15];

While this minimizes the diff and silences the error at hand,
I find my version more readable.

But you're obviously welcome to commit either version.

[1] https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html

Best regards,
Piotr Sikora
___
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel

Re: [PATCH] Geo: fix uninitialized memory access

[Piotr Sikora via nginx-devel]

Hi Roman,

> Thanks for the patch, looks valid, except we no longer need to explicitly
> initialize fields to zero.

Right, I was going back-and-forth between which version I should send.

> Also, I think we need more details about the
> uninitialized memory access.  See updated patch.

LGTM, thanks!

Best regards,
Piotr Sikora
___
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel

Re: [PATCH] Core: free connections and read/write events at shutdown

[Piotr Sikora via nginx-devel]

Hi Sergey,

> While I agree that false positives do not allow to run LeakSanitizer
> in a clean fashion, I don't think it is nginx which should be fixed.
> Rather, sanitizer analysis could be improved instead to prevent FPs.

Patches welcome?

> Meanwhile, leak sanitizer can be used with suppressions as appropriate
> to run tests cleanly.  For example, this allows to suppress memory leak
> reports for allocations made during worker process init, such as cycle
> connections and read/write events:
> 
> $ cat suppr.txt
> leak:ngx_worker_process_init
> $ LSAN_OPTIONS=suppressions=suppr.txt prove -r

Right, but that's an unnecessary step that prevents potential contributors
from using LeakSanitizer with NGINX. Also, I don't think that you're using
those tools either, likely because of those few false positives.

Best regards,
Piotr Sikora
___
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel

Re: [PATCH] Core: free connections and read/write events at shutdown

[Piotr Sikora via nginx-devel]

Hi Sergey,

> Since this is not a real memory leak (the allocations made in the
> init_process method of the ngx_event_core_module module are used up
> to this function and already freed on exit automatically, as this
> function never returns), I don't think there is something to fix.

Agreed, this is not a "real" leak.

But fixing those "false" leaks allows you to run test with sanitizers
on the CI, which can protect you and/or external contributors from
introducing new bugs.

> (Further, the patch misses cycle->files for NGX_USE_FD_EVENT event
> methods.

Good catch, thanks!

> Also, it's probably better to free the memory in the
> exit_process method to obey the ownership (this would also fix
> missing ngx_free() calls on win32), but this would require moving
> code that uses these connections afterwards to catch socket leaks.)

Freeing memory in exit_process could result in use-after-free, since
cleanups for the cycle->pool might still access those connections.

Best regards,
Piotr Sikora
___
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel

Re: [PATCH] Configure: add support for Homebrew on Apple Silicon

[Piotr Sikora via nginx-devel]

Hi Sergey,

> An obvious question is why do you need this change.  Homebrew seems
> to be quite niche to pay attention.

Homebrew [1] is orders of magnitude more popular than MacPorts [2],
which is already supported by the configure script.

> Using appropriate paths in
> --with-cc-opt / --with-ld-opt should work (not tested).

Everything under auto/lib can be replaced with --with-{cc,ld}-opt,
so I don't really understand this reasoning.

> A quick grep for MacPorts search paths suggests that some libraries
> are missing in the change.  If this is on purpose, please reflect
> this in the description.

libxml2, libxslt, and libexslt are all installed as part of Xcode,
which is required to use Homebrew and compile anything on macOS.

I'll ship update patch in a moment.

> Apple Silicon is something from the marketing language,
> using Apple ARM instead should be fine.
> 
> Notably, Homebrew uses Hardware::CPU.arm Ruby language boolean
> to make the distinction.

There is no such thing as "Apple ARM".

The official documentation uses the term "Apple silicon" [3],
Homebrew refers to the supported platform as "Apple Silicon" [4],
and Wikipedia has an article about "Apple silicon" [5].

> Further, given the smooth decay on Intel-based hardware,
> I'd reduce this just to "Homebrew".

But that would be misleading, seeing that the new code path doesn't do
anything for Homebrew on Intel.

And then, there is Homebrew on Linux [6].

[1] https://formulae.brew.sh/analytics/install-on-request/30d/
[2] https://ports.macports.org/statistics/ports/
[3] https://support.apple.com/en-us/116943
[4] https://formulae.brew.sh/formula/nginx#default
[5] https://en.wikipedia.org/wiki/Apple_silicon
[6] https://docs.brew.sh/Homebrew-on-Linux

Best regards,
Piotr Sikora
___
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel

Re: [PATCH] HTTP: stop emitting server version by default

[Piotr Sikora via nginx-devel]

Hi Sergey,

> I don't think this is a good idea to change the default behaviour
> for the directive we have for a long-long time. 

But it's arguably a wrong behavior, and keeping it forever wrong
because of a decision made ~20 years ago, doesn't seem like
a particularly great idea.

Also, while I'm usually all for retaining backward-compatibility,
I cannot imagine this breaking anything.

> It's always possible
> to set `server_tokens off;' in the configuration file.

Right, but if you require majority of users to change the defaults,
then those defaults are not very good.

> Also, this change is required a corresponding change in the
> documentation on the nginx.org website.

I'm happy to submit the corresponding change if the patch
is accepted.

Best regards,
Piotr Sikora
___
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel

[PATCH] Configure: fix "make install" when cross-compiling to Windows

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora 
# Date 1708977646 0
#  Mon Feb 26 20:00:46 2024 +
# Branch patch018
# Node ID ea1ab31c166c52372b40429a1cccece9ec9e003b
# Parent  dd95daa55cf6131a7e845edd6ad3b429bcef6f98
Configure: fix "make install" when cross-compiling to Windows.

Signed-off-by: Piotr Sikora 

diff -r dd95daa55cf6 -r ea1ab31c166c auto/install
--- a/auto/install  Mon Feb 26 20:00:43 2024 +
+++ b/auto/install  Mon Feb 26 20:00:46 2024 +
@@ -112,7 +112,7 @@
test ! -f '\$(DESTDIR)$NGX_SBIN_PATH' \\
|| mv '\$(DESTDIR)$NGX_SBIN_PATH' \\
'\$(DESTDIR)$NGX_SBIN_PATH.old'
-   cp $NGX_OBJS/nginx '\$(DESTDIR)$NGX_SBIN_PATH'
+   cp $NGX_OBJS/nginx$ngx_binext '\$(DESTDIR)$NGX_SBIN_PATH'
 
test -d '\$(DESTDIR)$NGX_CONF_PREFIX' \\
|| mkdir -p '\$(DESTDIR)$NGX_CONF_PREFIX'
___
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel

[PATCH] Win32: fix unique file index calculations

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora 
# Date 1708977635 0
#  Mon Feb 26 20:00:35 2024 +
# Branch patch012
# Node ID 04e3155b3b9651fee708898aaf82ac35532806ee
# Parent  9b57470dc49f8d8d10abe30a5df628732d7618dc
Win32: fix unique file index calculations.

The old code was breaking strict aliasing rules.

Signed-off-by: Piotr Sikora 

diff -r 9b57470dc49f -r 04e3155b3b96 src/os/win32/ngx_files.h
--- a/src/os/win32/ngx_files.h  Mon Feb 26 20:00:33 2024 +
+++ b/src/os/win32/ngx_files.h  Mon Feb 26 20:00:35 2024 +
@@ -154,7 +154,8 @@
 (((off_t) (fi)->nFileSizeHigh << 32) | (fi)->nFileSizeLow)
 #define ngx_file_fs_size(fi)ngx_file_size(fi)
 
-#define ngx_file_uniq(fi)   (*(ngx_file_uniq_t *) &(fi)->nFileIndexHigh)
+#define ngx_file_uniq(fi)\
+(((ngx_file_uniq_t) (fi)->nFileIndexHigh << 32) | (fi)->nFileIndexLow)
 
 
 /* 1164447360 is commented in src/os/win32/ngx_time.c */
___
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel

[PATCH] Configure: allow cross-compiling to Windows using Clang

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora 
# Date 1708977648 0
#  Mon Feb 26 20:00:48 2024 +
# Branch patch019
# Node ID 77eab4d83413b053d9681611d243335a95ee5567
# Parent  ea1ab31c166c52372b40429a1cccece9ec9e003b
Configure: allow cross-compiling to Windows using Clang.

Signed-off-by: Piotr Sikora 

diff -r ea1ab31c166c -r 77eab4d83413 auto/os/win32
--- a/auto/os/win32 Mon Feb 26 20:00:46 2024 +
+++ b/auto/os/win32 Mon Feb 26 20:00:48 2024 +
@@ -18,7 +18,7 @@
 
 case "$NGX_CC_NAME" in
 
-gcc)
+clang | gcc)
 CORE_LIBS="$CORE_LIBS -ladvapi32 -lws2_32"
 MAIN_LINK="$MAIN_LINK -Wl,--export-all-symbols"
 MAIN_LINK="$MAIN_LINK -Wl,--out-implib=$NGX_OBJS/libnginx.a"
___
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel

[PATCH] Configure: add support for Homebrew on Apple Silicon

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora 
# Date 1708977643 0
#  Mon Feb 26 20:00:43 2024 +
# Branch patch017
# Node ID dd95daa55cf6131a7e845edd6ad3b429bcef6f98
# Parent  bb99cbe3a343ae581d2369b990aee66e69679ca2
Configure: add support for Homebrew on Apple Silicon.

Signed-off-by: Piotr Sikora 

diff -r bb99cbe3a343 -r dd95daa55cf6 auto/lib/geoip/conf
--- a/auto/lib/geoip/conf   Mon Feb 26 20:00:42 2024 +
+++ b/auto/lib/geoip/conf   Mon Feb 26 20:00:43 2024 +
@@ -64,6 +64,23 @@
 fi
 
 
+if [ $ngx_found = no ]; then
+
+# Homebrew on Apple Silicon
+
+ngx_feature="GeoIP library in /opt/homebrew/"
+ngx_feature_path="/opt/homebrew/include"
+
+if [ $NGX_RPATH = YES ]; then
+ngx_feature_libs="-R/opt/homebrew/lib -L/opt/homebrew/lib -lGeoIP"
+else
+ngx_feature_libs="-L/opt/homebrew/lib -lGeoIP"
+fi
+
+. auto/feature
+fi
+
+
 if [ $ngx_found = yes ]; then
 
 CORE_INCS="$CORE_INCS $ngx_feature_path"
diff -r bb99cbe3a343 -r dd95daa55cf6 auto/lib/google-perftools/conf
--- a/auto/lib/google-perftools/confMon Feb 26 20:00:42 2024 +
+++ b/auto/lib/google-perftools/confMon Feb 26 20:00:43 2024 +
@@ -46,6 +46,22 @@
 fi
 
 
+if [ $ngx_found = no ]; then
+
+# Homebrew on Apple Silicon
+
+ngx_feature="Google perftools in /opt/homebrew/"
+
+if [ $NGX_RPATH = YES ]; then
+ngx_feature_libs="-R/opt/homebrew/lib -L/opt/homebrew/lib -lprofiler"
+else
+ngx_feature_libs="-L/opt/homebrew/lib -lprofiler"
+fi
+
+. auto/feature
+fi
+
+
 if [ $ngx_found = yes ]; then
 CORE_LIBS="$CORE_LIBS $ngx_feature_libs"
 
diff -r bb99cbe3a343 -r dd95daa55cf6 auto/lib/libgd/conf
--- a/auto/lib/libgd/conf   Mon Feb 26 20:00:42 2024 +
+++ b/auto/lib/libgd/conf   Mon Feb 26 20:00:43 2024 +
@@ -65,6 +65,23 @@
 fi
 
 
+if [ $ngx_found = no ]; then
+
+# Homebrew on Apple Silicon
+
+ngx_feature="GD library in /opt/homebrew/"
+ngx_feature_path="/opt/homebrew/include"
+
+if [ $NGX_RPATH = YES ]; then
+ngx_feature_libs="-R/opt/homebrew/lib -L/opt/homebrew/lib -lgd"
+else
+ngx_feature_libs="-L/opt/homebrew/lib -lgd"
+fi
+
+. auto/feature
+fi
+
+
 if [ $ngx_found = yes ]; then
 
 CORE_INCS="$CORE_INCS $ngx_feature_path"
diff -r bb99cbe3a343 -r dd95daa55cf6 auto/lib/openssl/conf
--- a/auto/lib/openssl/conf Mon Feb 26 20:00:42 2024 +
+++ b/auto/lib/openssl/conf Mon Feb 26 20:00:43 2024 +
@@ -122,6 +122,24 @@
 . auto/feature
 fi
 
+if [ $ngx_found = no ]; then
+
+# Homebrew on Apple Silicon
+
+ngx_feature="OpenSSL library in /opt/homebrew/"
+ngx_feature_path="/opt/homebrew/include"
+
+if [ $NGX_RPATH = YES ]; then
+ngx_feature_libs="-R/opt/homebrew/lib -L/opt/homebrew/lib 
-lssl -lcrypto"
+else
+ngx_feature_libs="-L/opt/homebrew/lib -lssl -lcrypto"
+fi
+
+ngx_feature_libs="$ngx_feature_libs $NGX_LIBDL $NGX_LIBPTHREAD"
+
+. auto/feature
+fi
+
 if [ $ngx_found = yes ]; then
 have=NGX_SSL . auto/have
 CORE_INCS="$CORE_INCS $ngx_feature_path"
diff -r bb99cbe3a343 -r dd95daa55cf6 auto/lib/pcre/conf
--- a/auto/lib/pcre/confMon Feb 26 20:00:42 2024 +
+++ b/auto/lib/pcre/confMon Feb 26 20:00:43 2024 +
@@ -182,6 +182,22 @@
 . auto/feature
 fi
 
+if [ $ngx_found = no ]; then
+
+# Homebrew on Apple Silicon
+
+ngx_feature="PCRE library in /opt/homebrew/"
+ngx_feature_path="/opt/homebrew/include"
+
+if [ $NGX_RPATH = YES ]; then
+ngx_feature_libs="-R/opt/homebrew/lib -L/opt/homebrew/lib 
-lpcre"
+else
+ngx_feature_libs="-L/opt/homebrew/lib -lpcre"
+fi
+
+. auto/feature
+fi
+
 if [ $ngx_found = yes ]; then
 CORE_INCS="$CORE_INCS $ngx_feature_path"
 CORE_LIBS="$CORE_LIBS $ngx_feature_libs"
___
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel

[PATCH] Win32: include missing

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora 
# Date 1708977633 0
#  Mon Feb 26 20:00:33 2024 +
# Branch patch011
# Node ID 9b57470dc49f8d8d10abe30a5df628732d7618dc
# Parent  480071fe7251829912a4f42301e8fc85da2d1905
Win32: include missing .

Signed-off-by: Piotr Sikora 

diff -r 480071fe7251 -r 9b57470dc49f src/os/win32/ngx_win32_config.h
--- a/src/os/win32/ngx_win32_config.h   Mon Feb 26 20:00:32 2024 +
+++ b/src/os/win32/ngx_win32_config.h   Mon Feb 26 20:00:33 2024 +
@@ -62,6 +62,7 @@
 #include 
 #endif
 #include 
+#include 
 #include 
 
 #ifdef __WATCOMC__
___
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel

[PATCH] macOS: detect cache line size at runtime

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora 
# Date 1708977640 0
#  Mon Feb 26 20:00:40 2024 +
# Branch patch015
# Node ID f58bc1041ebca635517b919d58b49923bf24f76d
# Parent  570e97dddeeddb79c71587aa8a10150b64404beb
macOS: detect cache line size at runtime.

Notably, Apple Silicon CPUs have 128 byte cache line size,
which is twice the default configured for generic aarch64.

Signed-off-by: Piotr Sikora 

diff -r 570e97dddeed -r f58bc1041ebc src/os/unix/ngx_darwin_init.c
--- a/src/os/unix/ngx_darwin_init.c Mon Feb 26 20:00:38 2024 +
+++ b/src/os/unix/ngx_darwin_init.c Mon Feb 26 20:00:40 2024 +
@@ -11,6 +11,7 @@
 
 charngx_darwin_kern_ostype[16];
 charngx_darwin_kern_osrelease[128];
+int64_t ngx_darwin_hw_cachelinesize;
 int ngx_darwin_hw_ncpu;
 int ngx_darwin_kern_ipc_somaxconn;
 u_long  ngx_darwin_net_inet_tcp_sendspace;
@@ -44,6 +45,10 @@
 
 
 sysctl_t sysctls[] = {
+{ "hw.cachelinesize",
+  _darwin_hw_cachelinesize,
+  sizeof(ngx_darwin_hw_cachelinesize), 0 },
+
 { "hw.ncpu",
   _darwin_hw_ncpu,
   sizeof(ngx_darwin_hw_ncpu), 0 },
@@ -155,6 +160,7 @@
 return NGX_ERROR;
 }
 
+ngx_cacheline_size = ngx_darwin_hw_cachelinesize;
 ngx_ncpu = ngx_darwin_hw_ncpu;
 
 if (ngx_darwin_kern_ipc_somaxconn > 32767) {
diff -r 570e97dddeed -r f58bc1041ebc src/os/unix/ngx_posix_init.c
--- a/src/os/unix/ngx_posix_init.c  Mon Feb 26 20:00:38 2024 +
+++ b/src/os/unix/ngx_posix_init.c  Mon Feb 26 20:00:40 2024 +
@@ -51,7 +51,10 @@
 }
 
 ngx_pagesize = getpagesize();
-ngx_cacheline_size = NGX_CPU_CACHE_LINE;
+
+if (ngx_cacheline_size == 0) {
+ngx_cacheline_size = NGX_CPU_CACHE_LINE;
+}
 
 for (n = ngx_pagesize; n >>= 1; ngx_pagesize_shift++) { /* void */ }
 
___
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel

[PATCH] Configure: set cache line sizes for more architectures

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora 
# Date 1708977642 0
#  Mon Feb 26 20:00:42 2024 +
# Branch patch016
# Node ID bb99cbe3a343ae581d2369b990aee66e69679ca2
# Parent  f58bc1041ebca635517b919d58b49923bf24f76d
Configure: set cache line sizes for more architectures.

Signed-off-by: Piotr Sikora 

diff -r f58bc1041ebc -r bb99cbe3a343 auto/os/conf
--- a/auto/os/conf  Mon Feb 26 20:00:40 2024 +
+++ b/auto/os/conf  Mon Feb 26 20:00:42 2024 +
@@ -115,6 +115,21 @@
 NGX_MACH_CACHE_LINE=64
 ;;
 
+ppc64 | ppc64le)
+have=NGX_ALIGNMENT value=16 . auto/define
+NGX_MACH_CACHE_LINE=128
+;;
+
+riscv64)
+have=NGX_ALIGNMENT value=16 . auto/define
+NGX_MACH_CACHE_LINE=64
+;;
+
+s390x)
+have=NGX_ALIGNMENT value=16 . auto/define
+NGX_MACH_CACHE_LINE=256
+;;
+
 *)
 have=NGX_ALIGNMENT value=16 . auto/define
 NGX_MACH_CACHE_LINE=32
___
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel

[PATCH] Configure: link libcrypt when a feature using it is detected

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora 
# Date 1708977638 0
#  Mon Feb 26 20:00:38 2024 +
# Branch patch014
# Node ID 570e97dddeeddb79c71587aa8a10150b64404beb
# Parent  cdc173477ea99fd6c952a85e5cd11db66452076a
Configure: link libcrypt when a feature using it is detected.

Previously, this worked only because libcrypt was added in a separate
test for crypt() in auto/unix.

Signed-off-by: Piotr Sikora 

diff -r cdc173477ea9 -r 570e97dddeed auto/os/linux
--- a/auto/os/linux Mon Feb 26 20:00:37 2024 +
+++ b/auto/os/linux Mon Feb 26 20:00:38 2024 +
@@ -228,6 +228,10 @@
   crypt_r(\"key\", \"salt\", );"
 . auto/feature
 
+if [ $ngx_found = yes ]; then
+CRYPT_LIB="-lcrypt"
+fi
+
 
 ngx_include="sys/vfs.h"; . auto/include
 
___
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel

[PATCH] Core: fix build without libcrypt

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora 
# Date 1708977637 0
#  Mon Feb 26 20:00:37 2024 +
# Branch patch013
# Node ID cdc173477ea99fd6c952a85e5cd11db66452076a
# Parent  04e3155b3b9651fee708898aaf82ac35532806ee
Core: fix build without libcrypt.

libcrypt is no longer part of glibc, so it might not be available.

Signed-off-by: Piotr Sikora 

diff -r 04e3155b3b96 -r cdc173477ea9 auto/unix
--- a/auto/unix Mon Feb 26 20:00:35 2024 +
+++ b/auto/unix Mon Feb 26 20:00:37 2024 +
@@ -150,7 +150,7 @@
 
 
 ngx_feature="crypt()"
-ngx_feature_name=
+ngx_feature_name="NGX_HAVE_CRYPT"
 ngx_feature_run=no
 ngx_feature_incs=
 ngx_feature_path=
@@ -162,7 +162,7 @@
 if [ $ngx_found = no ]; then
 
 ngx_feature="crypt() in libcrypt"
-ngx_feature_name=
+ngx_feature_name="NGX_HAVE_CRYPT"
 ngx_feature_run=no
 ngx_feature_incs=
 ngx_feature_path=
diff -r 04e3155b3b96 -r cdc173477ea9 src/os/unix/ngx_linux_config.h
--- a/src/os/unix/ngx_linux_config.hMon Feb 26 20:00:35 2024 +
+++ b/src/os/unix/ngx_linux_config.hMon Feb 26 20:00:37 2024 +
@@ -52,7 +52,6 @@
 #include  /* memalign() */
 #include  /* IOV_MAX */
 #include 
-#include 
 #include /* uname() */
 
 #include 
@@ -61,6 +60,11 @@
 #include 
 
 
+#if (NGX_HAVE_CRYPT_H)
+#include 
+#endif
+
+
 #if (NGX_HAVE_POSIX_SEM)
 #include 
 #endif
diff -r 04e3155b3b96 -r cdc173477ea9 src/os/unix/ngx_user.c
--- a/src/os/unix/ngx_user.cMon Feb 26 20:00:35 2024 +
+++ b/src/os/unix/ngx_user.cMon Feb 26 20:00:37 2024 +
@@ -44,7 +44,7 @@
 return NGX_ERROR;
 }
 
-#else
+#elif (NGX_HAVE_CRYPT)
 
 ngx_int_t
 ngx_libc_crypt(ngx_pool_t *pool, u_char *key, u_char *salt, u_char **encrypted)
@@ -76,6 +76,14 @@
 return NGX_ERROR;
 }
 
+#else
+
+ngx_int_t
+ngx_libc_crypt(ngx_pool_t *pool, u_char *key, u_char *salt, u_char **encrypted)
+{
+return NGX_ERROR;
+}
+
 #endif
 
 #endif /* NGX_CRYPT */
___
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel

[PATCH 1 of 2] SSL: add support for AWS-LC

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora 
# Date 1708977630 0
#  Mon Feb 26 20:00:30 2024 +
# Branch patch009
# Node ID 5e923992006199748e79b08b1e65c4ef41f07495
# Parent  3cde11b747c08c69889edc014a700317fe4d1d88
SSL: add support for AWS-LC.

AWS-LC is a fork of BoringSSL with some performance improvements,
useful features (OCSP and multiple certificates), and support for
more platforms.

Signed-off-by: Piotr Sikora 

diff -r 3cde11b747c0 -r 5e9239920061 src/event/ngx_event_openssl.h
--- a/src/event/ngx_event_openssl.h Mon Feb 26 20:00:28 2024 +
+++ b/src/event/ngx_event_openssl.h Mon Feb 26 20:00:30 2024 +
@@ -25,7 +25,7 @@
 #endif
 #include 
 #if (NGX_QUIC)
-#ifdef OPENSSL_IS_BORINGSSL
+#if defined(OPENSSL_IS_BORINGSSL) || defined(OPENSSL_IS_AWSLC)
 #include 
 #include 
 #else
diff -r 3cde11b747c0 -r 5e9239920061 src/event/quic/ngx_event_quic.c
--- a/src/event/quic/ngx_event_quic.c   Mon Feb 26 20:00:28 2024 +
+++ b/src/event/quic/ngx_event_quic.c   Mon Feb 26 20:00:30 2024 +
@@ -962,7 +962,7 @@
 return NGX_DECLINED;
 }
 
-#if !defined (OPENSSL_IS_BORINGSSL)
+#if !defined(OPENSSL_IS_BORINGSSL) && !defined(OPENSSL_IS_AWSLC)
 /* OpenSSL provides read keys for an application level before it's ready */
 
 if (pkt->level == ssl_encryption_application && !c->ssl->handshaked) {
diff -r 3cde11b747c0 -r 5e9239920061 src/event/quic/ngx_event_quic_protection.c
--- a/src/event/quic/ngx_event_quic_protection.cMon Feb 26 20:00:28 
2024 +
+++ b/src/event/quic/ngx_event_quic_protection.cMon Feb 26 20:00:30 
2024 +
@@ -30,7 +30,7 @@
 
 static ngx_int_t ngx_quic_crypto_open(ngx_quic_secret_t *s, ngx_str_t *out,
 u_char *nonce, ngx_str_t *in, ngx_str_t *ad, ngx_log_t *log);
-#ifndef OPENSSL_IS_BORINGSSL
+#if !defined(OPENSSL_IS_BORINGSSL) && !defined(OPENSSL_IS_AWSLC)
 static ngx_int_t ngx_quic_crypto_common(ngx_quic_secret_t *s, ngx_str_t *out,
 u_char *nonce, ngx_str_t *in, ngx_str_t *ad, ngx_log_t *log);
 #endif
@@ -55,7 +55,7 @@
 switch (id) {
 
 case TLS1_3_CK_AES_128_GCM_SHA256:
-#ifdef OPENSSL_IS_BORINGSSL
+#if defined(OPENSSL_IS_BORINGSSL) || defined(OPENSSL_IS_AWSLC)
 ciphers->c = EVP_aead_aes_128_gcm();
 #else
 ciphers->c = EVP_aes_128_gcm();
@@ -66,7 +66,7 @@
 break;
 
 case TLS1_3_CK_AES_256_GCM_SHA384:
-#ifdef OPENSSL_IS_BORINGSSL
+#if defined(OPENSSL_IS_BORINGSSL) || defined(OPENSSL_IS_AWSLC)
 ciphers->c = EVP_aead_aes_256_gcm();
 #else
 ciphers->c = EVP_aes_256_gcm();
@@ -77,12 +77,12 @@
 break;
 
 case TLS1_3_CK_CHACHA20_POLY1305_SHA256:
-#ifdef OPENSSL_IS_BORINGSSL
+#if defined(OPENSSL_IS_BORINGSSL) || defined(OPENSSL_IS_AWSLC)
 ciphers->c = EVP_aead_chacha20_poly1305();
 #else
 ciphers->c = EVP_chacha20_poly1305();
 #endif
-#ifdef OPENSSL_IS_BORINGSSL
+#if defined(OPENSSL_IS_BORINGSSL) || defined(OPENSSL_IS_AWSLC)
 ciphers->hp = (const EVP_CIPHER *) EVP_aead_chacha20_poly1305();
 #else
 ciphers->hp = EVP_chacha20();
@@ -91,7 +91,7 @@
 len = 32;
 break;
 
-#ifndef OPENSSL_IS_BORINGSSL
+#if !defined(OPENSSL_IS_BORINGSSL) && !defined(OPENSSL_IS_AWSLC)
 case TLS1_3_CK_AES_128_CCM_SHA256:
 ciphers->c = EVP_aes_128_ccm();
 ciphers->hp = EVP_aes_128_ctr();
@@ -259,7 +259,7 @@
 ngx_hkdf_expand(u_char *out_key, size_t out_len, const EVP_MD *digest,
 const uint8_t *prk, size_t prk_len, const u_char *info, size_t info_len)
 {
-#ifdef OPENSSL_IS_BORINGSSL
+#if defined(OPENSSL_IS_BORINGSSL) || defined(OPENSSL_IS_AWSLC)
 
 if (HKDF_expand(out_key, out_len, digest, prk, prk_len, info, info_len)
 == 0)
@@ -321,7 +321,7 @@
 const u_char *secret, size_t secret_len, const u_char *salt,
 size_t salt_len)
 {
-#ifdef OPENSSL_IS_BORINGSSL
+#if defined(OPENSSL_IS_BORINGSSL) || defined(OPENSSL_IS_AWSLC)
 
 if (HKDF_extract(out_key, out_len, digest, secret, secret_len, salt,
  salt_len)
@@ -384,7 +384,7 @@
 ngx_quic_md_t *key, ngx_int_t enc, ngx_log_t *log)
 {
 
-#ifdef OPENSSL_IS_BORINGSSL
+#if defined(OPENSSL_IS_BORINGSSL) || defined(OPENSSL_IS_AWSLC)
 EVP_AEAD_CTX  *ctx;
 
 ctx = EVP_AEAD_CTX_new(cipher, key->data, key->len,
@@ -444,7 +444,7 @@
 ngx_quic_crypto_open(ngx_quic_secret_t *s, ngx_str_t *out, u_char *nonce,
 ngx_str_t *in, ngx_str_t *ad, ngx_log_t *log)
 {
-#ifdef OPENSSL_IS_BORINGSSL
+#if defined(OPENSSL_IS_BORINGSSL) || defined(OPENSSL_IS_AWSLC)
 if (EVP_AEAD_CTX_open(s->ctx, out->data, >len, out->len, nonce,
   s->iv.len, in->data, in->len, ad->data, ad->len)
 != 1)
@@ -464,7 +464,7 @@
 ngx_quic_crypto_seal(ngx_quic_secret_t *s, ngx_str_t *out, u_char *nonce,
 ngx_str_t *in, ngx_str_t *ad, ngx_log_t *log)
 {
-#ifdef OPENSSL_IS_BORINGSSL
+#if defined(OPENSSL_IS_BORINGSSL) || defined(OPENSSL_IS_

[PATCH 2 of 2] SSL: add $ssl_curve when using AWS-LC

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora 
# Date 1708977632 0
#  Mon Feb 26 20:00:32 2024 +
# Branch patch009
# Node ID dfffc67d286b788204f60701ef4179566d933a1b
# Parent  5e923992006199748e79b08b1e65c4ef41f07495
SSL: add $ssl_curve when using AWS-LC.

Signed-off-by: Piotr Sikora 

diff -r 5e9239920061 -r dfffc67d286b src/event/ngx_event_openssl.c
--- a/src/event/ngx_event_openssl.c Mon Feb 26 20:00:30 2024 +
+++ b/src/event/ngx_event_openssl.c Mon Feb 26 20:00:32 2024 +
@@ -5163,6 +5163,72 @@
 return NGX_OK;
 }
 
+#elif defined(OPENSSL_IS_AWSLC)
+
+uint16_t  curve_id;
+
+curve_id = SSL_get_curve_id(c->ssl->connection);
+
+/*
+ * Hardcoded table with ANSI / SECG curve names (e.g. "prime256v1"),
+ * which is the same format that OpenSSL returns for $ssl_curve.
+ *
+ * Without this table, we'd need to make 3 additional library calls
+ * to convert from curve_id to ANSI / SECG curve name:
+ *
+ * nist_name = SSL_get_curve_name(curve_id);
+ * nid = EC_curve_nist2nid(nist_name);
+ * ansi_name = OBJ_nid2sn(nid);
+ */
+
+switch (curve_id) {
+
+#ifdef SSL_CURVE_SECP224R1
+case SSL_CURVE_SECP224R1:
+ngx_str_set(s, "secp224r1");
+return NGX_OK;
+#endif
+
+#ifdef SSL_CURVE_SECP256R1
+case SSL_CURVE_SECP256R1:
+ngx_str_set(s, "prime256v1");
+return NGX_OK;
+#endif
+
+#ifdef SSL_CURVE_SECP384R1
+case SSL_CURVE_SECP384R1:
+ngx_str_set(s, "secp384r1");
+return NGX_OK;
+#endif
+
+#ifdef SSL_CURVE_SECP521R1
+case SSL_CURVE_SECP521R1:
+ngx_str_set(s, "secp521r1");
+return NGX_OK;
+#endif
+
+#ifdef SSL_CURVE_X25519
+case SSL_CURVE_X25519:
+ngx_str_set(s, "x25519");
+return NGX_OK;
+#endif
+
+case 0:
+break;
+
+default:
+s->len = sizeof("0x") - 1;
+
+s->data = ngx_pnalloc(pool, s->len);
+if (s->data == NULL) {
+return NGX_ERROR;
+}
+
+ngx_sprintf(s->data, "0x%04xd", curve_id);
+
+return NGX_OK;
+}
+
 #endif
 
 ngx_str_null(s);
___
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel

[PATCH] Rewrite: fix "return" directive without response text

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora 
# Date 1708977628 0
#  Mon Feb 26 20:00:28 2024 +
# Branch patch008
# Node ID 3cde11b747c08c69889edc014a700317fe4d1d88
# Parent  5584232259d28489efba149f2f5ae730691ff0d4
Rewrite: fix "return" directive without response text.

Previously, the response text wasn't initialized and the rewrite module
was sending response body set to NULL.

Found with UndefinedBehaviorSanitizer (pointer-overflow).

Signed-off-by: Piotr Sikora 

diff -r 5584232259d2 -r 3cde11b747c0 src/http/modules/ngx_http_rewrite_module.c
--- a/src/http/modules/ngx_http_rewrite_module.cMon Feb 26 20:00:26 
2024 +
+++ b/src/http/modules/ngx_http_rewrite_module.cMon Feb 26 20:00:28 
2024 +
@@ -489,6 +489,7 @@
 }
 
 if (cf->args->nelts == 2) {
+ngx_str_set(>text.value, "");
 return NGX_CONF_OK;
 }
 
___
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel

[PATCH] Core: fix conversion of IPv4-mapped IPv6 addresses

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora 
# Date 1708977626 0
#  Mon Feb 26 20:00:26 2024 +
# Branch patch007
# Node ID 5584232259d28489efba149f2f5ae730691ff0d4
# Parent  03e5549976765912818120e11f6b08410a2af6a9
Core: fix conversion of IPv4-mapped IPv6 addresses.

Found with UndefinedBehaviorSanitizer (shift).

Signed-off-by: Piotr Sikora 

diff -r 03e554997676 -r 5584232259d2 src/core/ngx_inet.c
--- a/src/core/ngx_inet.c   Mon Feb 26 20:00:23 2024 +
+++ b/src/core/ngx_inet.c   Mon Feb 26 20:00:26 2024 +
@@ -507,10 +507,10 @@
 
 p = inaddr6->s6_addr;
 
-inaddr = p[12] << 24;
-inaddr += p[13] << 16;
-inaddr += p[14] << 8;
-inaddr += p[15];
+inaddr = (in_addr_t) p[12] << 24;
+inaddr += (in_addr_t) p[13] << 16;
+inaddr += (in_addr_t) p[14] << 8;
+inaddr += (in_addr_t) p[15];
 
 inaddr = htonl(inaddr);
 }
diff -r 03e554997676 -r 5584232259d2 src/http/modules/ngx_http_access_module.c
--- a/src/http/modules/ngx_http_access_module.c Mon Feb 26 20:00:23 2024 +
+++ b/src/http/modules/ngx_http_access_module.c Mon Feb 26 20:00:26 2024 +
@@ -148,10 +148,10 @@
 p = sin6->sin6_addr.s6_addr;
 
 if (alcf->rules && IN6_IS_ADDR_V4MAPPED(>sin6_addr)) {
-addr = p[12] << 24;
-addr += p[13] << 16;
-addr += p[14] << 8;
-addr += p[15];
+addr = (in_addr_t) p[12] << 24;
+addr += (in_addr_t) p[13] << 16;
+addr += (in_addr_t) p[14] << 8;
+addr += (in_addr_t) p[15];
 return ngx_http_access_inet(r, alcf, htonl(addr));
 }
 
diff -r 03e554997676 -r 5584232259d2 src/http/modules/ngx_http_geo_module.c
--- a/src/http/modules/ngx_http_geo_module.cMon Feb 26 20:00:23 2024 +
+++ b/src/http/modules/ngx_http_geo_module.cMon Feb 26 20:00:26 2024 +
@@ -199,10 +199,10 @@
 p = inaddr6->s6_addr;
 
 if (IN6_IS_ADDR_V4MAPPED(inaddr6)) {
-inaddr = p[12] << 24;
-inaddr += p[13] << 16;
-inaddr += p[14] << 8;
-inaddr += p[15];
+inaddr = (in_addr_t) p[12] << 24;
+inaddr += (in_addr_t) p[13] << 16;
+inaddr += (in_addr_t) p[14] << 8;
+inaddr += (in_addr_t) p[15];
 
 vv = (ngx_http_variable_value_t *)
   ngx_radix32tree_find(ctx->u.trees.tree, inaddr);
@@ -272,10 +272,10 @@
 if (IN6_IS_ADDR_V4MAPPED(inaddr6)) {
 p = inaddr6->s6_addr;
 
-inaddr = p[12] << 24;
-inaddr += p[13] << 16;
-inaddr += p[14] << 8;
-inaddr += p[15];
+inaddr = (in_addr_t) p[12] << 24;
+inaddr += (in_addr_t) p[13] << 16;
+inaddr += (in_addr_t) p[14] << 8;
+inaddr += (in_addr_t) p[15];
 
 } else {
 inaddr = INADDR_NONE;
diff -r 03e554997676 -r 5584232259d2 src/http/modules/ngx_http_geoip_module.c
--- a/src/http/modules/ngx_http_geoip_module.c  Mon Feb 26 20:00:23 2024 +
+++ b/src/http/modules/ngx_http_geoip_module.c  Mon Feb 26 20:00:26 2024 +
@@ -266,10 +266,10 @@
 if (IN6_IS_ADDR_V4MAPPED(inaddr6)) {
 p = inaddr6->s6_addr;
 
-inaddr = p[12] << 24;
-inaddr += p[13] << 16;
-inaddr += p[14] << 8;
-inaddr += p[15];
+inaddr = (in_addr_t) p[12] << 24;
+inaddr += (in_addr_t) p[13] << 16;
+inaddr += (in_addr_t) p[14] << 8;
+inaddr += (in_addr_t) p[15];
 
 return inaddr;
 }
diff -r 03e554997676 -r 5584232259d2 src/stream/ngx_stream_access_module.c
--- a/src/stream/ngx_stream_access_module.c Mon Feb 26 20:00:23 2024 +
+++ b/src/stream/ngx_stream_access_module.c Mon Feb 26 20:00:26 2024 +
@@ -144,10 +144,10 @@
 p = sin6->sin6_addr.s6_addr;
 
 if (ascf->rules && IN6_IS_ADDR_V4MAPPED(>sin6_addr)) {
-addr = p[12] << 24;
-addr += p[13] << 16;
-addr += p[14] << 8;
-addr += p[15];
+addr = (in_addr_t) p[12] << 24;
+addr += (in_addr_t) p[13] << 16;
+addr += (in_addr_t) p[14] << 8;
+addr += (in_addr_t) p[15];
 return ngx_stream_access_inet(s, ascf, htonl(addr));
 }
 
diff -r 03e554997676 -r 5584232259d2 src/stream/ngx_stream_geo_module.c
--- a/src/stream/ngx_stream_geo_module.cMon Feb 26 20:00:23 2024 +
+++ b/src/stream/ngx_stream_geo_module.cMon Feb 26 20:00:26 2024 +
@@ -190,10 +190,10 @@
 p = ina

[PATCH] Geo: fix uninitialized memory access

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora 
# Date 1708977621 0
#  Mon Feb 26 20:00:21 2024 +
# Branch patch005
# Node ID fe6f8a72d42970df176ea53f4f0aea16947ba5b8
# Parent  52936793ac076072c3544aa4e27f973d2f8fecda
Geo: fix uninitialized memory access.

Found with MemorySanitizer.

Signed-off-by: Piotr Sikora 

diff -r 52936793ac07 -r fe6f8a72d429 src/http/modules/ngx_http_geo_module.c
--- a/src/http/modules/ngx_http_geo_module.cMon Feb 26 20:00:19 2024 +
+++ b/src/http/modules/ngx_http_geo_module.cMon Feb 26 20:00:21 2024 +
@@ -1259,7 +1259,7 @@
 return gvvn->value;
 }
 
-val = ngx_palloc(ctx->pool, sizeof(ngx_http_variable_value_t));
+val = ngx_pcalloc(ctx->pool, sizeof(ngx_http_variable_value_t));
 if (val == NULL) {
 return NULL;
 }
diff -r 52936793ac07 -r fe6f8a72d429 src/stream/ngx_stream_geo_module.c
--- a/src/stream/ngx_stream_geo_module.cMon Feb 26 20:00:19 2024 +
+++ b/src/stream/ngx_stream_geo_module.cMon Feb 26 20:00:21 2024 +
@@ -1209,7 +1209,7 @@
 return gvvn->value;
 }
 
-val = ngx_palloc(ctx->pool, sizeof(ngx_stream_variable_value_t));
+val = ngx_pcalloc(ctx->pool, sizeof(ngx_stream_variable_value_t));
 if (val == NULL) {
 return NULL;
 }
___
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel

[PATCH] Correctly initialize ngx_str_t

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora 
# Date 1708977619 0
#  Mon Feb 26 20:00:19 2024 +
# Branch patch004
# Node ID 52936793ac076072c3544aa4e27f973d2f8fecda
# Parent  8edb4003177dac56301aed7f86f8d2a564b47552
Correctly initialize ngx_str_t.

Previously, only the "len" field was set, which resulted in
an uninitialized "data" field accessed elsewhere in the code.

Note that "r->uri" is initialized to an empty string to avoid
changing the existing value for "$uri" in case of invalid URI.

Found with MemorySanitizer.

Signed-off-by: Piotr Sikora 

diff -r 8edb4003177d -r 52936793ac07 src/event/ngx_event_openssl.c
--- a/src/event/ngx_event_openssl.c Mon Feb 26 20:00:18 2024 +
+++ b/src/event/ngx_event_openssl.c Mon Feb 26 20:00:19 2024 +
@@ -5064,7 +5064,7 @@
 n = SSL_get0_raw_cipherlist(c->ssl->connection, );
 
 if (n <= 0) {
-s->len = 0;
+ngx_str_null(s);
 return NGX_OK;
 }
 
@@ -5116,7 +5116,7 @@
 if (SSL_get_shared_ciphers(c->ssl->connection, (char *) buf, 4096)
 == NULL)
 {
-s->len = 0;
+ngx_str_null(s);
 return NGX_OK;
 }
 
@@ -5165,7 +5165,7 @@
 
 #endif
 
-s->len = 0;
+ngx_str_null(s);
 return NGX_OK;
 }
 
@@ -5182,7 +5182,7 @@
 n = SSL_get1_curves(c->ssl->connection, NULL);
 
 if (n <= 0) {
-s->len = 0;
+ngx_str_null(s);
 return NGX_OK;
 }
 
@@ -5233,7 +5233,7 @@
 
 #else
 
-s->len = 0;
+ngx_str_null(s);
 
 #endif
 
@@ -5250,7 +5250,7 @@
 
 sess = SSL_get0_session(c->ssl->connection);
 if (sess == NULL) {
-s->len = 0;
+ngx_str_null(s);
 return NGX_OK;
 }
 
@@ -5285,7 +5285,7 @@
 ngx_int_t
 ngx_ssl_get_early_data(ngx_connection_t *c, ngx_pool_t *pool, ngx_str_t *s)
 {
-s->len = 0;
+ngx_str_null(s);
 
 #ifdef SSL_ERROR_EARLY_DATA_REJECTED
 
@@ -5335,7 +5335,7 @@
 
 #endif
 
-s->len = 0;
+ngx_str_null(s);
 return NGX_OK;
 }
 
@@ -5365,7 +5365,7 @@
 
 #endif
 
-s->len = 0;
+ngx_str_null(s);
 return NGX_OK;
 }
 
@@ -5377,10 +5377,9 @@
 BIO *bio;
 X509*cert;
 
-s->len = 0;
-
 cert = SSL_get_peer_certificate(c->ssl->connection);
 if (cert == NULL) {
+ngx_str_null(s);
 return NGX_OK;
 }
 
@@ -5433,7 +5432,7 @@
 }
 
 if (cert.len == 0) {
-s->len = 0;
+ngx_str_null(s);
 return NGX_OK;
 }
 
@@ -5476,7 +5475,7 @@
 }
 
 if (cert.len == 0) {
-s->len = 0;
+ngx_str_null(s);
 return NGX_OK;
 }
 
@@ -5501,10 +5500,9 @@
 X509   *cert;
 X509_NAME  *name;
 
-s->len = 0;
-
 cert = SSL_get_peer_certificate(c->ssl->connection);
 if (cert == NULL) {
+ngx_str_null(s);
 return NGX_OK;
 }
 
@@ -,10 +5553,9 @@
 X509   *cert;
 X509_NAME  *name;
 
-s->len = 0;
-
 cert = SSL_get_peer_certificate(c->ssl->connection);
 if (cert == NULL) {
+ngx_str_null(s);
 return NGX_OK;
 }
 
@@ -5611,10 +5608,9 @@
 X509   *cert;
 X509_NAME  *name;
 
-s->len = 0;
-
 cert = SSL_get_peer_certificate(c->ssl->connection);
 if (cert == NULL) {
+ngx_str_null(s);
 return NGX_OK;
 }
 
@@ -5659,10 +5655,9 @@
 X509   *cert;
 X509_NAME  *name;
 
-s->len = 0;
-
 cert = SSL_get_peer_certificate(c->ssl->connection);
 if (cert == NULL) {
+ngx_str_null(s);
 return NGX_OK;
 }
 
@@ -5705,10 +5700,9 @@
 X509*cert;
 BIO *bio;
 
-s->len = 0;
-
 cert = SSL_get_peer_certificate(c->ssl->connection);
 if (cert == NULL) {
+ngx_str_null(s);
 return NGX_OK;
 }
 
@@ -5745,10 +5739,9 @@
 unsigned int   len;
 u_char buf[EVP_MAX_MD_SIZE];
 
-s->len = 0;
-
 cert = SSL_get_peer_certificate(c->ssl->connection);
 if (cert == NULL) {
+ngx_str_null(s);
 return NGX_OK;
 }
 
@@ -5818,10 +5811,9 @@
 X509*cert;
 size_t   len;
 
-s->len = 0;
-
 cert = SSL_get_peer_certificate(c->ssl->connection);
 if (cert == NULL) {
+ngx_str_null(s);
 return NGX_OK;
 }
 
@@ -5863,10 +5855,9 @@
 X509*cert;
 size_t   len;
 
-s->len = 0;
-
 cert = SSL_get_peer_certificate(c->ssl->connection);
 if (cert == NULL) {
+ngx_str_null(s);
 return NGX_OK;
 }
 
@@ -5907,10 +5898,9 @@
 X509*cert;
 time_t   now, end;
 
-s->len = 0;
-
 cert = SSL_get_peer_certificate(c->ssl->connection);
 if (cert == NULL) {
+ngx_str_null(s);
 return NGX_OK;
 }
 
diff -r 8edb4003177d -r 52936793ac07 src/event/quic/ngx_event_quic_streams.c
--- a/src/event/quic/ngx_event_quic_streams.c   Mon Feb 26 20:0

[PATCH] Upstream: cleanup at shutdown

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora 
# Date 1708977618 0
#  Mon Feb 26 20:00:18 2024 +
# Branch patch003
# Node ID 8edb4003177dac56301aed7f86f8d2a564b47552
# Parent  f8d9fb94eab212f6e640b7a68ed111562e3157d5
Upstream: cleanup at shutdown.

Add "free_upstream" callback called on worker exit to
free any per-upstream objects allocated from the heap.

Found with LeakSanitizer.

Signed-off-by: Piotr Sikora 

diff -r f8d9fb94eab2 -r 8edb4003177d 
src/http/modules/ngx_http_upstream_random_module.c
--- a/src/http/modules/ngx_http_upstream_random_module.cMon Feb 26 
20:00:16 2024 +
+++ b/src/http/modules/ngx_http_upstream_random_module.cMon Feb 26 
20:00:18 2024 +
@@ -114,6 +114,35 @@
 }
 
 
+static void
+ngx_http_upstream_free_random(ngx_http_upstream_srv_conf_t *us)
+{
+#if (NGX_HTTP_UPSTREAM_ZONE)
+
+ngx_http_upstream_rr_peers_t *peers;
+ngx_http_upstream_random_srv_conf_t  *rcf;
+
+peers = us->peer.data;
+
+if (peers->shpool) {
+
+rcf = ngx_http_conf_upstream_srv_conf(us,
+  ngx_http_upstream_random_module);
+
+if (rcf->ranges) {
+ngx_log_debug1(NGX_LOG_DEBUG_HTTP, ngx_cycle->log, 0,
+   "free ranges: %p", rcf->ranges);
+ngx_free(rcf->ranges);
+rcf->ranges = NULL;
+}
+}
+
+#endif
+
+ngx_http_upstream_free_round_robin(us);
+}
+
+
 static ngx_int_t
 ngx_http_upstream_update_random(ngx_pool_t *pool,
 ngx_http_upstream_srv_conf_t *us)
@@ -465,6 +494,7 @@
 }
 
 uscf->peer.init_upstream = ngx_http_upstream_init_random;
+uscf->peer.free_upstream = ngx_http_upstream_free_random;
 
 uscf->flags = NGX_HTTP_UPSTREAM_CREATE
   |NGX_HTTP_UPSTREAM_WEIGHT
diff -r f8d9fb94eab2 -r 8edb4003177d src/http/ngx_http_upstream.c
--- a/src/http/ngx_http_upstream.c  Mon Feb 26 20:00:16 2024 +
+++ b/src/http/ngx_http_upstream.c  Mon Feb 26 20:00:18 2024 +
@@ -189,6 +189,8 @@
 ngx_http_upstream_t *u, ngx_connection_t *c);
 #endif
 
+static void ngx_http_upstream_worker_cleanup(ngx_cycle_t *cycle);
+
 
 static ngx_http_upstream_header_t  ngx_http_upstream_headers_in[] = {
 
@@ -368,7 +370,7 @@
 NULL,  /* init process */
 NULL,  /* init thread */
 NULL,  /* exit thread */
-NULL,  /* exit process */
+ngx_http_upstream_worker_cleanup,  /* exit process */
 NULL,  /* exit master */
 NGX_MODULE_V1_PADDING
 };
@@ -6829,3 +6831,29 @@
 
 return NGX_CONF_OK;
 }
+
+
+static void
+ngx_http_upstream_worker_cleanup(ngx_cycle_t *cycle)
+{
+ngx_uint_t   i;
+ngx_http_upstream_free_ptfree;
+ngx_http_upstream_srv_conf_t   **uscfp;
+ngx_http_upstream_main_conf_t   *umcf;
+
+umcf = ngx_http_cycle_get_module_main_conf(cycle, 
ngx_http_upstream_module);
+
+if (umcf) {
+
+uscfp = umcf->upstreams.elts;
+
+for (i = 0; i < umcf->upstreams.nelts; i++) {
+
+free = uscfp[i]->peer.free_upstream
+   ? uscfp[i]->peer.free_upstream
+   : ngx_http_upstream_free_round_robin;
+
+free(uscfp[i]);
+}
+}
+}
diff -r f8d9fb94eab2 -r 8edb4003177d src/http/ngx_http_upstream.h
--- a/src/http/ngx_http_upstream.h  Mon Feb 26 20:00:16 2024 +
+++ b/src/http/ngx_http_upstream.h  Mon Feb 26 20:00:18 2024 +
@@ -82,11 +82,13 @@
 ngx_http_upstream_srv_conf_t *us);
 typedef ngx_int_t (*ngx_http_upstream_init_peer_pt)(ngx_http_request_t *r,
 ngx_http_upstream_srv_conf_t *us);
+typedef void (*ngx_http_upstream_free_pt)(ngx_http_upstream_srv_conf_t *us);
 
 
 typedef struct {
 ngx_http_upstream_init_ptinit_upstream;
 ngx_http_upstream_init_peer_pt   init;
+ngx_http_upstream_free_ptfree_upstream;
 void*data;
 } ngx_http_upstream_peer_t;
 
diff -r f8d9fb94eab2 -r 8edb4003177d src/http/ngx_http_upstream_round_robin.c
--- a/src/http/ngx_http_upstream_round_robin.c  Mon Feb 26 20:00:16 2024 +
+++ b/src/http/ngx_http_upstream_round_robin.c  Mon Feb 26 20:00:18 2024 +
@@ -851,3 +851,34 @@
 }
 
 #endif
+
+
+void
+ngx_http_upstream_free_round_robin(ngx_http_upstream_srv_conf_t *us)
+{
+#if (NGX_HTTP_SSL)
+
+ngx_uint_t i;
+ngx_http_upstream_rr_peer_t   *peer;
+ngx_http_upstream_rr_peers_t  *peers;
+
+peers = us->peer.data;
+
+#if (NGX_HTTP_UPSTREAM_ZONE)
+if (peers->shpool) {
+return;
+}
+#endif
+
+for (peer = peers->peer, i = 0; peer; peer = peer->next, i++) {
+
+if (peer->ssl_session) {
+ngx_log_debug1(NGX_LOG_DEBUG_HTTP, ngx_cycle->log, 0,
+   "fr

[PATCH] Core: free connections and read/write events at shutdown

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora 
# Date 1708977616 0
#  Mon Feb 26 20:00:16 2024 +
# Branch patch002
# Node ID f8d9fb94eab212f6e640b7a68ed111562e3157d5
# Parent  a8a592b9b62eff7bca03e8b46669f59d2da689ed
Core: free connections and read/write events at shutdown.

Found with LeakSanitizer.

Signed-off-by: Piotr Sikora 

diff -r a8a592b9b62e -r f8d9fb94eab2 src/os/unix/ngx_process_cycle.c
--- a/src/os/unix/ngx_process_cycle.c   Mon Feb 26 20:00:11 2024 +
+++ b/src/os/unix/ngx_process_cycle.c   Mon Feb 26 20:00:16 2024 +
@@ -940,6 +940,7 @@
 ngx_worker_process_exit(ngx_cycle_t *cycle)
 {
 ngx_uint_t i;
+ngx_event_t   *rev, *wev;
 ngx_connection_t  *c;
 
 for (i = 0; cycle->modules[i]; i++) {
@@ -989,8 +990,16 @@
 ngx_exit_cycle.files_n = ngx_cycle->files_n;
 ngx_cycle = _exit_cycle;
 
+c = cycle->connections;
+rev = cycle->read_events;
+wev = cycle->write_events;
+
 ngx_destroy_pool(cycle->pool);
 
+ngx_free(c);
+ngx_free(rev);
+ngx_free(wev);
+
 ngx_log_error(NGX_LOG_NOTICE, ngx_cycle->log, 0, "exit");
 
 exit(0);
___
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel

[PATCH] HTTP: stop emitting server version by default

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora 
# Date 1708977611 0
#  Mon Feb 26 20:00:11 2024 +
# Branch patch001
# Node ID a8a592b9b62eff7bca03e8b46669f59d2da689ed
# Parent  89bff782528a91ad123b63b624f798e6fd9c8e68
HTTP: stop emitting server version by default.

This information is only useful to attackers.

The previous behavior can be restored using "server_tokens on".

Signed-off-by: Piotr Sikora 

diff -r 89bff782528a -r a8a592b9b62e src/http/ngx_http_core_module.c
--- a/src/http/ngx_http_core_module.c   Wed Feb 14 20:03:00 2024 +0400
+++ b/src/http/ngx_http_core_module.c   Mon Feb 26 20:00:11 2024 +
@@ -3899,7 +3899,7 @@
 ngx_conf_merge_value(conf->etag, prev->etag, 1);
 
 ngx_conf_merge_uint_value(conf->server_tokens, prev->server_tokens,
-  NGX_HTTP_SERVER_TOKENS_ON);
+  NGX_HTTP_SERVER_TOKENS_OFF);
 
 ngx_conf_merge_ptr_value(conf->open_file_cache,
   prev->open_file_cache, NULL);
___
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel

Re: [nginx] Changed keepalive_requests default to 1000 (ticket #2155).

[Piotr Sikora]

Hi Maxim,

> It turns out no browsers implement HTTP/2 GOAWAY handling properly, and
> large enough number of resources on a page results in failures to load
> some resources.  In particular, Chrome seems to experience errors if
> loading of all resources requires more than 1 connection (while it
> is usually able to retry requests at least once, even with 2 connections
> there are occasional failures for some reason), Safari if loading requires
> more than 3 connections, and Firefox if loading requires more than 10
> connections (can be configured with network.http.request.max-attempts,
> defaults to 10).
>
> It does not seem to be possible to resolve this on nginx side, even strict
> limiting of maximum concurrency does not help, and loading issues seems to
> be triggered by merely queueing of a request for a particular connection.
> The only available mitigation seems to use higher keepalive_requests value.

Instead of blaming browsers, did you consider implementing graceful shutdown
using 2-stage GOAWAY? The process is clearly described in RFC7540, sec. 6.8:

  [...] A server that is attempting to gracefully shut down a
  connection SHOULD send an initial GOAWAY frame with the last stream
  identifier set to 2^31-1 and a NO_ERROR code.  This signals to the
  client that a shutdown is imminent and that initiating further
  requests is prohibited.  After allowing time for any in-flight stream
  creation (at least one round-trip time), the server can send another
  GOAWAY frame with an updated last stream identifier.  This ensures
  that a connection can be cleanly shut down without losing requests.

This is a solved problem, and the solution was pointed out years ago:
http://mailman.nginx.org/pipermail/nginx-devel/2017-August/010439.html
http://mailman.nginx.org/pipermail/nginx-devel/2018-March/010930.html

Best regards,
Piotr Sikora
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

[PATCH] HTTP/2: fixed handling of fully preread request bodies

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora 
# Date 1540435636 25200
#  Wed Oct 24 19:47:16 2018 -0700
# Node ID 466c154c5c53b956660211df96331b3c25669485
# Parent  be5cb9c67c05ccaf22dab7abba78aa4c1545a8ee
HTTP/2: fixed handling of fully preread request bodies.

Previously, fully preread request body of a request without the
"Content-Length" header was always written to a temporary file.

Reported by Wayne Zhang.

Signed-off-by: Piotr Sikora 

diff -r be5cb9c67c05 -r 466c154c5c53 src/http/v2/ngx_http_v2.c
--- a/src/http/v2/ngx_http_v2.c
+++ b/src/http/v2/ngx_http_v2.c
@@ -3863,6 +3863,12 @@ ngx_http_v2_read_request_body(ngx_http_r
 {
 rb->buf = ngx_create_temp_buf(r->pool, (size_t) len);
 
+} else if (len < 0 && stream->in_closed && stream->preread
+   && !r->request_body_in_file_only)
+{
+rb->buf = ngx_create_temp_buf(r->pool,
+  (size_t) ngx_buf_size(stream->preread));
+
 } else {
 rb->buf = ngx_calloc_buf(r->pool);
 
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

[PATCH] Upstream: added $upstream_bytes_sent variable

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora 
# Date 1494129075 25200
#  Sat May 06 20:51:15 2017 -0700
# Node ID fafbb3ee41e5bb03bcfba73f7d4367b8ab7d36cc
# Parent  be5cb9c67c05ccaf22dab7abba78aa4c1545a8ee
Upstream: added $upstream_bytes_sent variable.

Signed-off-by: Piotr Sikora 

diff -r be5cb9c67c05 -r fafbb3ee41e5 src/http/ngx_http_upstream.c
--- a/src/http/ngx_http_upstream.c
+++ b/src/http/ngx_http_upstream.c
@@ -162,8 +162,8 @@ static ngx_int_t ngx_http_upstream_statu
 ngx_http_variable_value_t *v, uintptr_t data);
 static ngx_int_t ngx_http_upstream_response_time_variable(ngx_http_request_t 
*r,
 ngx_http_variable_value_t *v, uintptr_t data);
-static ngx_int_t ngx_http_upstream_response_length_variable(
-ngx_http_request_t *r, ngx_http_variable_value_t *v, uintptr_t data);
+static ngx_int_t ngx_http_upstream_bytes_variable(ngx_http_request_t *r,
+ngx_http_variable_value_t *v, uintptr_t data);
 static ngx_int_t ngx_http_upstream_header_variable(ngx_http_request_t *r,
 ngx_http_variable_value_t *v, uintptr_t data);
 static ngx_int_t ngx_http_upstream_trailer_variable(ngx_http_request_t *r,
@@ -402,11 +402,15 @@ static ngx_http_variable_t  ngx_http_ups
   NGX_HTTP_VAR_NOCACHEABLE, 0 },
 
 { ngx_string("upstream_response_length"), NULL,
-  ngx_http_upstream_response_length_variable, 0,
+  ngx_http_upstream_bytes_variable, 0,
   NGX_HTTP_VAR_NOCACHEABLE, 0 },
 
 { ngx_string("upstream_bytes_received"), NULL,
-  ngx_http_upstream_response_length_variable, 1,
+  ngx_http_upstream_bytes_variable, 1,
+  NGX_HTTP_VAR_NOCACHEABLE, 0 },
+
+{ ngx_string("upstream_bytes_sent"), NULL,
+  ngx_http_upstream_bytes_variable, 2,
   NGX_HTTP_VAR_NOCACHEABLE, 0 },
 
 #if (NGX_HTTP_CACHE)
@@ -4134,6 +4138,10 @@ ngx_http_upstream_next(ngx_http_request_
 ngx_log_debug1(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
"http next upstream, %xi", ft_type);
 
+if (u->state && u->state->bytes_sent == 0 && u->peer.connection) {
+u->state->bytes_sent = u->peer.connection->sent;
+}
+
 if (u->peer.sockaddr) {
 
 if (ft_type == NGX_HTTP_UPSTREAM_FT_HTTP_403
@@ -4319,6 +4327,10 @@ ngx_http_upstream_finalize_request(ngx_h
 - u->pipe->preread_size;
 u->state->response_length = u->pipe->read_length;
 }
+
+if (u->state->bytes_sent == 0 && u->peer.connection) {
+u->state->bytes_sent = u->peer.connection->sent;
+}
 }
 
 u->finalize_request(r, rc);
@@ -5468,7 +5480,7 @@ ngx_http_upstream_response_time_variable
 
 
 static ngx_int_t
-ngx_http_upstream_response_length_variable(ngx_http_request_t *r,
+ngx_http_upstream_bytes_variable(ngx_http_request_t *r,
 ngx_http_variable_value_t *v, uintptr_t data)
 {
 u_char *p;
@@ -5499,7 +5511,10 @@ ngx_http_upstream_response_length_variab
 
 for ( ;; ) {
 
-if (data == 1) {
+if (data == 2) {
+p = ngx_sprintf(p, "%O", state[i].bytes_sent);
+
+} else if (data == 1) {
 p = ngx_sprintf(p, "%O", state[i].bytes_received);
 
 } else {
diff -r be5cb9c67c05 -r fafbb3ee41e5 src/http/ngx_http_upstream.h
--- a/src/http/ngx_http_upstream.h
+++ b/src/http/ngx_http_upstream.h
@@ -64,6 +64,7 @@ typedef struct {
 ngx_msec_t   queue_time;
 off_tresponse_length;
 off_tbytes_received;
+off_tbytes_sent;
 
 ngx_str_t   *peer;
 } ngx_http_upstream_state_t;
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

Re: [PATCH] HTTP/2: don't limit number of requests per HTTP/2 connection

[Piotr Sikora via nginx-devel]

Hey,
just as reminder, limiting HTTP/2 connections to 1000 requests without
graceful shutdown via 2-stage GOAWAY is still an issue and while this
might work with browsers, you're going to break gRPC-based
microservices proxied via NGINX pretty badly, so you should either
implement graceful shutdown or stop limiting number of requests by
default.

Best regards,
Piotr Sikora

On Wed, Aug 30, 2017 at 4:14 PM, Piotr Sikora <piotrsik...@google.com> wrote:
> Hey Valentin,
>
>> This opens a vector for dos attack.  There are some configurations
>> when memory can be allocated from connection pool for each request.
>> Removing a reasonable enough limit for requests per connection
>> potentially allow an attacker to grow this pool until a worker
>> process will be killed due to OOM.
>>
>> The problem should be solved by introducing "lingering close",
>> similar to HTTP/1.x.
>
> Yes, the proper solution is graceful shutdown via 2-stage GOAWAY,
> as defined in RFC7540, Section 6.8, but I don't have capacity to
> work on it now, and above patch is IMHO better than lost requests.
>
> Best regards,
> Piotr Sikora
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

Re: [PATCH] HTTP/2: copy additional headers in the pushed requests

[Piotr Sikora via nginx-devel]

Hi Ruslan,

> http header: "User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_3)
AppleWebKit/537.36 (KHTML, like Gecko) Chrome/64.0.3282.140 Safari/537.36"
> http header: "Accept:
text/html,application/xhtml+xml,application/xml;q=0.9,image/webp,image/apng,*/*;q=0.8"
> http header: "Accept-Encoding: gzip, deflate, br"
> http header: "Accept-Language:
ru,en-US;q=0.9,en;q=0.8,zh-TW;q=0.7,zh;q=0.6"
> (...)
> http header: "User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_13_3)
AppleWebKit/537.36 (KHTML, like Gecko) Chrome/64.0.3282.140 Safari/537.36"
> http header: "Accept: image/webp,image/apng,image/*,*/*;q=0.8"
> http header: "Accept-Encoding: gzip, deflate, br"
> http header: "Accept-Language:
ru,en-US;q=0.9,en;q=0.8,zh-TW;q=0.7,zh;q=0.6"

> http header: "User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13;
rv:52.0) Gecko/20100101 Firefox/52.0"
> http header: "Accept:
text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8"
> http header: "Accept-Language: ru-RU,ru;q=0.8,en-US;q=0.5,en;q=0.3"
> http header: "Accept-Encoding: gzip, deflate"
> (...)
> http header: "User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13;
rv:52.0) Gecko/20100101 Firefox/52.0"
> http header: "Accept: */*"
> http header: "Accept-Language: ru-RU,ru;q=0.8,en-US;q=0.5,en;q=0.3"
> http header: "Accept-Encoding: gzip, deflate"

In both examples, User-Agent, Accept-Encoding and Accept-Language are the
same, and Accept is (an extension-specific?) subset of the "/" request,
which proves that this is in fact a good idea.

Best regards,
Piotr Sikora
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

Re: yield 499 while reading client body and client prematurely closed connection

[Piotr Sikora via nginx-devel]

Hi Alex,

On Mon, Jan 15, 2018 at 9:59 PM, tokers <zchao1...@gmail.com> wrote:
> # HG changeset patch
> # User Alex Zhang <zchao1...@gmail.com>
> # Date 1516079440 -28800
> #  Tue Jan 16 13:10:40 2018 +0800
> # Node ID 9ca5af970d2296a02acefb3070237c5f52119708
> # Parent  93abb5a855d6534f0356882f45be49f8c6a95a8b
> yield 499 while reading client body and client prematurely closed
> connection.
>
> The function ngx_http_do_read_client_request_body returns
> NGX_HTTP_BAD_REQUEST (client prematurely closed connection),
> while the 400 status code cannot reflect that client closed connection
> prematurely. It should return code 499(NGX_HTTP_CLIENT_CLOSED_REQUEST)
> and it is helpful to troubleshoot some relevant problems.
>
> Signed-off-by: Alex Zhang <zchao1...@gmail.com>
>
> diff -r 93abb5a855d6 -r 9ca5af970d22 src/http/ngx_http_request_body.c
> --- a/src/http/ngx_http_request_body.c  Thu Jan 11 21:43:49 2018 +0300
> +++ b/src/http/ngx_http_request_body.c  Tue Jan 16 13:10:40 2018 +0800
> @@ -342,14 +342,17 @@
>  break;
>  }
>
> -if (n == 0) {
> +if (n == 0 || n == NGX_ERROR) {
> +c->error = 1;
> +
> +if (n == 0) {
> +return NGX_HTTP_BAD_REQUEST;
> +}
> +
>  ngx_log_error(NGX_LOG_INFO, c->log, 0,
>"client prematurely closed connection");
> -}
>
> -if (n == 0 || n == NGX_ERROR) {
> -c->error = 1;
> -return NGX_HTTP_BAD_REQUEST;
> +return NGX_HTTP_CLIENT_CLOSED_REQUEST;
>  }
>
>  rb->buf->last += n;

I agree with this change (in fact, I have similar code in my local
tree), but something like this is probably more readable:

diff -r 93abb5a855d6 src/http/ngx_http_request_body.c
--- a/src/http/ngx_http_request_body.c
+++ b/src/http/ngx_http_request_body.c
@@ -345,9 +345,11 @@ ngx_http_do_read_client_request_body(ngx
 if (n == 0) {
 ngx_log_error(NGX_LOG_INFO, c->log, 0,
   "client prematurely closed connection");
+c->error = 1;
+return NGX_HTTP_CLIENT_CLOSED_REQUEST;
 }

-if (n == 0 || n == NGX_ERROR) {
+if (n == NGX_ERROR) {
 c->error = 1;
 return NGX_HTTP_BAD_REQUEST;
 }

Having said that, handing of client errors before request body is
fully received is pretty inconsistent in NGINX, especially between
HTTP/1.1 and HTTP/2, so this is only partial fix.

Best regards,
Piotr Sikora
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

[PATCH] Upstream: flush low-level buffers on write retry

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Patryk Lesiewicz 
# Date 1512172754 28800
#  Fri Dec 01 15:59:14 2017 -0800
# Node ID 0e2e2da798261fe5105017d9678566267b07e2b9
# Parent  fc0d06224edac2c7cfbfd9a4def478f285d9957b
Upstream: flush low-level buffers on write retry.

If the data to write is bigger than what the socket can send, and the
reminder is smaller than NGX_SSL_BUFSIZE, then SSL_write() fails with
SSL_ERROR_WANT_WRITE. The reminder of payload however is successfully
copied to the low-level buffer and all the output chain buffers are
flushed. This means that retry logic doesn't work because
ngx_http_upstream_process_non_buffered_request() checks only if there's
anything in the output chain buffers and ignores the fact that something
may be buffered in low-level parts of the stack.

Signed-off-by: Patryk Lesiewicz 

diff -r fc0d06224eda -r 0e2e2da79826 src/http/ngx_http_upstream.c
--- a/src/http/ngx_http_upstream.c
+++ b/src/http/ngx_http_upstream.c
@@ -3533,7 +3533,7 @@ ngx_http_upstream_process_non_buffered_r
 
 if (do_write) {
 
-if (u->out_bufs || u->busy_bufs) {
+if (u->out_bufs || u->busy_bufs || downstream->buffered) {
 rc = ngx_http_output_filter(r, u->out_bufs);
 
 if (rc == NGX_ERROR) {
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

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

[Piotr Sikora via nginx-devel]

Hey Maxim,

> 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:

Those changes look fine, thanks.

Best regards,
Piotr Sikora
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

Re: [PATCH] HTTP/2: don't limit number of requests per HTTP/2 connection

[Piotr Sikora via nginx-devel]

Hey Valentin,

> This opens a vector for dos attack.  There are some configurations
> when memory can be allocated from connection pool for each request.
> Removing a reasonable enough limit for requests per connection
> potentially allow an attacker to grow this pool until a worker
> process will be killed due to OOM.
>
> The problem should be solved by introducing "lingering close",
> similar to HTTP/1.x.

Yes, the proper solution is graceful shutdown via 2-stage GOAWAY,
as defined in RFC7540, Section 6.8, but I don't have capacity to
work on it now, and above patch is IMHO better than lost requests.

Best regards,
Piotr Sikora
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

[PATCH] HTTP/2: don't limit number of requests per HTTP/2 connection

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora <piotrsik...@google.com>
# Date 1504129797 25200
#  Wed Aug 30 14:49:57 2017 -0700
# Node ID 49b677bf2ae7ab92499766e8184ddcbf7a4233f9
# Parent  c7d4017c8876af6d8570e400320537d7d39e9578
HTTP/2: don't limit number of requests per HTTP/2 connection.

Previous default limit (1000 requests) and lack of graceful shutdown
could result in loss of requests, when clients were unable to retry.

Signed-off-by: Piotr Sikora <piotrsik...@google.com>

diff -r c7d4017c8876 -r 49b677bf2ae7 src/http/v2/ngx_http_v2_module.c
--- a/src/http/v2/ngx_http_v2_module.c
+++ b/src/http/v2/ngx_http_v2_module.c
@@ -11,6 +11,9 @@
 #include 
 
 
+#define NGX_HTTP_V2_MAX_STREAMS  (1U << 30)
+
+
 static ngx_int_t ngx_http_v2_add_variables(ngx_conf_t *cf);
 
 static ngx_int_t ngx_http_v2_variable(ngx_http_request_t *r,
@@ -355,7 +358,8 @@ ngx_http_v2_merge_srv_conf(ngx_conf_t *c
 
 ngx_conf_merge_uint_value(conf->concurrent_streams,
   prev->concurrent_streams, 128);
-ngx_conf_merge_uint_value(conf->max_requests, prev->max_requests, 1000);
+ngx_conf_merge_uint_value(conf->max_requests, prev->max_requests,
+  NGX_HTTP_V2_MAX_STREAMS);
 
 ngx_conf_merge_size_value(conf->max_field_size, prev->max_field_size,
   4096);
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

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

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora <piotrsik...@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 <piotrsik...@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);
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

Re: [PATCH 12 of 14] Proxy: add HTTP/2 support

[Piotr Sikora via nginx-devel]

Hey Maxim,

On Tue, Jul 25, 2017 at 6:28 PM, Piotr Sikora <piotrsik...@google.com> wrote:
> Hey Maxim,
>
>> There are serious concerns about this and with other patches.
>> Most generic ones are outlined below.
>>
>> 1. Server-side HTTP/2 code complexity concerns.
>>
>> As per discussion with Valentin, introducing client-related code
>> paths in the server HTTP/2 code seems to complicate things.  Given
>> that the complexity of the code is already high, it might be
>> better idea to implement separate client-side processing instead.
>
> I strongly disagree. Changes introduced to ngx_http_v2.c and
> ngx_http_v2_filter_module.c are minimal.
>
> Also, having separate client-side and server-side code would result in
> thousands of lines of duplicated code, for no reason, really.
>
>> 2. Different protocols in proxy module.
>>
>> You've introduced a completely different protocol in the proxy
>> module, which contradicts the established practice of using
>> separate modules for different protocols.
>>
>> Reasons for having all (or at least may of) the protocols
>> available in the proxy module instead are well understood, and
>> there is a long-term plan to revise current practice.  The plan is
>> to preserve protocol-specific modules as a separate entities, but
>> let them share the common configuration directives, similar to how
>> it is currently done with upstream{} blocks and the overall
>> upstream infrastructure.  So one can write something like
>>
>> proxy_pass fastcgi://127.0.0.1:9000;
>>
>> and get an expected result.
>>
>> While benefits of having all protocols sharing the same
>> user-visible interface are understood, approach taken with HTTP/2
>> implementation is considered suboptimal, and will likely lead to
>> something hard to maintain.
>>
>> I see two possible solutions here:
>>
>> - make HTTP/2-to-upstreams a separate full-featured upstream-based
>>   module, similar to proxy or fastcgi;
>
> But that's basically:
>
> cat ngx_http_proxy_module.c \
> | sed 's/ngx_http_proxy/ngx_http_proxy_v2/g' \
> > ngx_http_proxy_v2_module.c
>
> I don't see how copying ~4500 lines of code is a good idea.
>
> Also, as mentioned elsewhere, one of the reasons for adding HTTP/2
> code to the proxy module was the ability to negotiate HTTP/1.1 or
> HTTP/2 via ALPN. Creating a separate HTTP/2-to-upstreams module won't
> allow to add such feature in the future.
>
>> - start working on the different protocols in the proxy module,
>>   and introduce HTTP/2 proxying after this work is done.
>
> How is that even an option here? It's going to take forever before
> such rewrite is done, and I have no desire nor time to work on that.
>
>> Additionally, there are also some minor / related comments:
>>
>> - Parts of the code, tightly coupled with upstream infrastructure,
>>   notably ngx_http_v2_upstream_output_filter(), are placed into
>>   v2/ directory.  Instead, they seem to belong to the
>>   HTTP/2-to-upstream module implementation, as suggested in (1).
>
> Sure, this and a few other functions could be added to different files.
>
>> - Upstream infrastructure as available in
>>   src/http/ngx_http_upstream.c is expected to be
>>   protocol-agnostic.  Introducing calls like
>>   ngx_http_v2_init_connection() there is a layering violation.
>>   Instead, there should be something more generic.
>
> I agree that ngx_http_v2_init_connection() isn't perfect, however,
> fake connections are an abstraction layer that needs to be added
> somewhere. The same is done for SSL, and it's somehow acceptable.
>
> I'm happy to use whatever generic mechanism is available, but there is
> none at the moment, and I don't see how adding even more code and
> complexity to this already big patchset is going to get us anywhere.
>
>> - Doing protocol parsing elsewhere instead of parsing things based
>>   on what nginx got from the connection results in some generic
>>   upstream mechanisms rendered not working - notably, it is no
>>   longer possible to simply write headers to a cache file.
>>   Serialization introduced instead, at least in its current form,
>>   looks more like a bandaid.
>
> Except that HTTP/2 headers as read from the wire, even if parsed in
> separate module, couldn't be simply written to a cache file, because
> of stateful HPACK encoding, so serialization would need to be done
> anyway.
>
> Anyway, it appears that your "serious concerns" are mostly about
> organization of the code, and not the code itself. What's unclear to
> me is how much of the code review this patchset actually received,
> i.e. if the existing code would be moved to a separate
> HTTP/2-to-upstreams module, would it be acceptable or do you have
> other issues with the code?

Ping.

Best regards,
Piotr Sikora
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

Re: [PATCH 12 of 14] Proxy: add HTTP/2 support

[Piotr Sikora via nginx-devel]

Hey Maxim,

> There are serious concerns about this and with other patches.
> Most generic ones are outlined below.
>
> 1. Server-side HTTP/2 code complexity concerns.
>
> As per discussion with Valentin, introducing client-related code
> paths in the server HTTP/2 code seems to complicate things.  Given
> that the complexity of the code is already high, it might be
> better idea to implement separate client-side processing instead.

I strongly disagree. Changes introduced to ngx_http_v2.c and
ngx_http_v2_filter_module.c are minimal.

Also, having separate client-side and server-side code would result in
thousands of lines of duplicated code, for no reason, really.

> 2. Different protocols in proxy module.
>
> You've introduced a completely different protocol in the proxy
> module, which contradicts the established practice of using
> separate modules for different protocols.
>
> Reasons for having all (or at least may of) the protocols
> available in the proxy module instead are well understood, and
> there is a long-term plan to revise current practice.  The plan is
> to preserve protocol-specific modules as a separate entities, but
> let them share the common configuration directives, similar to how
> it is currently done with upstream{} blocks and the overall
> upstream infrastructure.  So one can write something like
>
> proxy_pass fastcgi://127.0.0.1:9000;
>
> and get an expected result.
>
> While benefits of having all protocols sharing the same
> user-visible interface are understood, approach taken with HTTP/2
> implementation is considered suboptimal, and will likely lead to
> something hard to maintain.
>
> I see two possible solutions here:
>
> - make HTTP/2-to-upstreams a separate full-featured upstream-based
>   module, similar to proxy or fastcgi;

But that's basically:

cat ngx_http_proxy_module.c \
| sed 's/ngx_http_proxy/ngx_http_proxy_v2/g' \
> ngx_http_proxy_v2_module.c

I don't see how copying ~4500 lines of code is a good idea.

Also, as mentioned elsewhere, one of the reasons for adding HTTP/2
code to the proxy module was the ability to negotiate HTTP/1.1 or
HTTP/2 via ALPN. Creating a separate HTTP/2-to-upstreams module won't
allow to add such feature in the future.

> - start working on the different protocols in the proxy module,
>   and introduce HTTP/2 proxying after this work is done.

How is that even an option here? It's going to take forever before
such rewrite is done, and I have no desire nor time to work on that.

> Additionally, there are also some minor / related comments:
>
> - Parts of the code, tightly coupled with upstream infrastructure,
>   notably ngx_http_v2_upstream_output_filter(), are placed into
>   v2/ directory.  Instead, they seem to belong to the
>   HTTP/2-to-upstream module implementation, as suggested in (1).

Sure, this and a few other functions could be added to different files.

> - Upstream infrastructure as available in
>   src/http/ngx_http_upstream.c is expected to be
>   protocol-agnostic.  Introducing calls like
>   ngx_http_v2_init_connection() there is a layering violation.
>   Instead, there should be something more generic.

I agree that ngx_http_v2_init_connection() isn't perfect, however,
fake connections are an abstraction layer that needs to be added
somewhere. The same is done for SSL, and it's somehow acceptable.

I'm happy to use whatever generic mechanism is available, but there is
none at the moment, and I don't see how adding even more code and
complexity to this already big patchset is going to get us anywhere.

> - Doing protocol parsing elsewhere instead of parsing things based
>   on what nginx got from the connection results in some generic
>   upstream mechanisms rendered not working - notably, it is no
>   longer possible to simply write headers to a cache file.
>   Serialization introduced instead, at least in its current form,
>   looks more like a bandaid.

Except that HTTP/2 headers as read from the wire, even if parsed in
separate module, couldn't be simply written to a cache file, because
of stateful HPACK encoding, so serialization would need to be done
anyway.

Anyway, it appears that your "serious concerns" are mostly about
organization of the code, and not the code itself. What's unclear to
me is how much of the code review this patchset actually received,
i.e. if the existing code would be moved to a separate
HTTP/2-to-upstreams module, would it be acceptable or do you have
other issues with the code?

Best regards,
Piotr Sikora
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

Re: [PATCH 10 of 14] Proxy: always emit "Host" header first

[Piotr Sikora via nginx-devel]

Hey Maxim,

> As already outlined in the review here:
>
> http://mailman.nginx.org/pipermail/nginx-devel/2017-June/010087.html
>
> the approach taken looks very fragile.  There should be a better
> way to do this.

I disagree. The change is minimal (which is something that you seem to
always prefer) and works perfectly fine.

Yes, it's based on the assumption that "Host" header is the first of
default headers, but I don't think that's bad and I can't imagine the
order magically changing by itself... Also, the check if the first key
is "Host" could be probably removed.

As for your suggested alternatives:

Emitting default headers first, would - just like you mentioned - lose
the benefit of order defined by "proxy_set_header" directives, and as
such would change the order on the wire, so I'd rather avoid this
solution.

Emitting "Host" header separately, via different means, would require
more drastic code changes, for little to no benefit.

Anyway, you're more than welcome to provide alternative patch, but I
don't see anything wrong with the current approach, so I'm unlikely to
rewrite it myself.

Best regards,
Piotr Sikora
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

Re: [PATCH 03 of 14] HTTP/2: add debug logging of control frames

[Piotr Sikora via nginx-devel]

Hey Valentin,
comments below. I commented only on the first occurrence, but
obviously those comments apply to all similar changes.

>  ngx_log_debug0(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0,
> -   "http2 SETTINGS frame ack:1");
> +   "http2 SETTINGS ACK frame");

I don't particularly like this change, since it makes it look like
this is a "SETTINGS ACK" frame, not a "SETTINGS" frame with ACK flag
set. This is even more confusing since QUIC drafts had "SETTINGS_ACK"
frame at some point.

Maybe "http2 SETTINGS frame ACK" would be more acceptable?

>  ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0,
> -   "http2 SETTINGS frame params:%uz",
> +   "http2 SETTINGS frame with %uz params",
> h2c->state.length / NGX_HTTP_V2_SETTINGS_PARAM_SIZE);

The extra "with" doesn't add any information, and it doesn't seem to
be inline with other debug logs, but I don't mind this change too
much.

>  ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0,
> -   "http2 SETTINGS param HEADER_TABLE_SIZE:%ui "
> +   "http2 SETTINGS param HEADER_TABLE_SIZE: %ui "
> "(ignored)", value);

This is fine with me, but again, the extra space doesn't seem to be
inline with other debug logs, which usually don't include space
between "key:" and "value".

>  ngx_log_debug2(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0,
> -   "http2 SETTINGS param 0x%Xi:%ui "
> -   "(ignored)", id, value);
> +   "http2 unknown SETTINGS param id:0x%Xi value:%ui",
> +   id, value);

I'd prefer if we could stick to the "http2 SETTINGS" prefix here,
otherwise it won't look like part of the same frame, i.e.:

http2 SETTINGS frame with 3 params
http2 SETTINGS param HEADER_TABLE_SIZE: 4096
http2 SETTINGS param INITIAL_WINDOW_SIZE: 65536
http2 unknown SETTINGS param id:0xAA value:1234

Also, " (ignored)" part was removed, which makes it inconsistent with
other ignored parameters.

Maybe "http2 SETTINGS unknown param id:0x%Xi value:%ui (ignored)" or
"http2 SETTINGS param id:0x%Xi value:%ui (unknown, ignored)"?

Thanks!

Best regards,
Piotr Sikora
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

[PATCH 14 of 14] Cache: add HTTP/2 support

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora <piotrsik...@google.com>
# Date 1491954701 25200
#  Tue Apr 11 16:51:41 2017 -0700
# Node ID 432abcf285745ec6b6ee14d6487a4a42b51b7bce
# Parent  cde1f42da7b26b7d2b788f916685e736b919138e
Cache: add HTTP/2 support.

Signed-off-by: Piotr Sikora <piotrsik...@google.com>

diff -r cde1f42da7b2 -r 432abcf28574 src/http/modules/ngx_http_proxy_module.c
--- a/src/http/modules/ngx_http_proxy_module.c
+++ b/src/http/modules/ngx_http_proxy_module.c
@@ -132,6 +132,14 @@ static ngx_int_t ngx_http_proxy_reinit_r
 static ngx_int_t ngx_http_proxy_body_output_filter(void *data, ngx_chain_t 
*in);
 static ngx_int_t ngx_http_proxy_process_status_line(ngx_http_request_t *r);
 static ngx_int_t ngx_http_proxy_process_header(ngx_http_request_t *r);
+static ngx_int_t ngx_http_proxy_process_trailer(ngx_http_request_t *r,
+ngx_buf_t *buf);
+#if (NGX_HTTP_V2 && NGX_HTTP_CACHE)
+static ngx_int_t ngx_http_proxy_serialize_headers(ngx_http_request_t *r,
+ngx_http_upstream_t *u);
+static ngx_chain_t *ngx_http_proxy_serialize_trailers(ngx_http_request_t *r,
+ngx_http_upstream_t *u);
+#endif
 static ngx_int_t ngx_http_proxy_input_filter_init(void *data);
 static ngx_int_t ngx_http_proxy_copy_filter(ngx_event_pipe_t *p,
 ngx_buf_t *buf);
@@ -917,6 +925,7 @@ ngx_http_proxy_handler(ngx_http_request_
 u->create_request = ngx_http_proxy_create_request;
 u->reinit_request = ngx_http_proxy_reinit_request;
 u->process_header = ngx_http_proxy_process_status_line;
+u->process_trailer = ngx_http_proxy_process_trailer;
 u->abort_request = ngx_http_proxy_abort_request;
 u->finalize_request = ngx_http_proxy_finalize_request;
 r->state = 0;
@@ -929,6 +938,14 @@ ngx_http_proxy_handler(ngx_http_request_
 u->create_request = ngx_http_proxy_create_v2_request;
 u->output.output_filter = ngx_http_v2_upstream_output_filter;
 u->output.filter_ctx = r;
+
+#if (NGX_HTTP_CACHE)
+
+u->serialize_headers = ngx_http_proxy_serialize_headers;
+u->serialize_trailers = ngx_http_proxy_serialize_trailers;
+
+#endif
+
 }
 
 #endif
@@ -2482,6 +2499,227 @@ ngx_http_proxy_process_header(ngx_http_r
 
 
 static ngx_int_t
+ngx_http_proxy_process_trailer(ngx_http_request_t *r, ngx_buf_t *buf)
+{
+ngx_int_t rc;
+ngx_table_elt_t  *h;
+
+for ( ;; ) {
+
+rc = ngx_http_parse_header_line(r, buf, 1);
+
+if (rc == NGX_OK) {
+h = ngx_list_push(>upstream->headers_in.trailers);
+if (h == NULL) {
+return NGX_ERROR;
+}
+
+h->hash = r->header_hash;
+
+h->key.len = r->header_name_end - r->header_name_start;
+h->value.len = r->header_end - r->header_start;
+
+h->key.data = ngx_pnalloc(r->pool,
+   h->key.len + 1 + h->value.len + 1 + h->key.len);
+if (h->key.data == NULL) {
+return NGX_ERROR;
+}
+
+h->value.data = h->key.data + h->key.len + 1;
+h->lowcase_key = h->key.data + h->key.len + 1 + h->value.len + 1;
+
+ngx_memcpy(h->key.data, r->header_name_start, h->key.len);
+h->key.data[h->key.len] = '\0';
+ngx_memcpy(h->value.data, r->header_start, h->value.len);
+h->value.data[h->value.len] = '\0';
+
+if (h->key.len == r->lowcase_index) {
+ngx_memcpy(h->lowcase_key, r->lowcase_header, h->key.len);
+
+} else {
+ngx_strlow(h->lowcase_key, h->key.data, h->key.len);
+}
+
+ngx_log_debug2(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
+   "http proxy trailer: \"%V: %V\"",
+   >key, >value);
+
+continue;
+}
+
+if (rc == NGX_HTTP_PARSE_HEADER_DONE) {
+ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
+   "http proxy trailer done");
+
+return NGX_OK;
+}
+
+ngx_log_error(NGX_LOG_ERR, r->connection->log, 0,
+  "upstream sent invalid trailer");
+
+return NGX_HTTP_UPSTREAM_INVALID_HEADER;
+}
+}
+
+
+#if (NGX_HTTP_V2 && NGX_HTTP_CACHE)
+
+static ngx_int_t
+ngx_http_proxy_serialize_headers(ngx_http_request_t *r, ngx_http_upstream_t *u)
+{
+size_tlen;
+ngx_buf_t*b;
+ngx_uint_ti;
+ngx_list_part_t  *part;
+ngx_table_elt_t  *header;
+
+len = sizeof("HTTP/1.1 " CRLF) - 1 + u->headers_in.status_line.len
+  + sizeof(CRLF) - 1;
+
+part = >headers_in.headers.part;
+header = part->elts;
+
+for (i = 0; /* void */; i++

[PATCH 12 of 14] Proxy: add HTTP/2 support

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora <piotrsik...@google.com>
# Date 1490769087 25200
#  Tue Mar 28 23:31:27 2017 -0700
# Node ID 7eb807b056da7abe9c679b59e94595d59a1406e6
# Parent  0637acdb51e29e1f68f5f3e762115c702cab4e72
Proxy: add HTTP/2 support.

Signed-off-by: Piotr Sikora <piotrsik...@google.com>

diff -r 0637acdb51e2 -r 7eb807b056da src/http/modules/ngx_http_proxy_module.c
--- a/src/http/modules/ngx_http_proxy_module.c
+++ b/src/http/modules/ngx_http_proxy_module.c
@@ -125,6 +125,9 @@ static ngx_int_t ngx_http_proxy_eval(ngx
 static ngx_int_t ngx_http_proxy_create_key(ngx_http_request_t *r);
 #endif
 static ngx_int_t ngx_http_proxy_create_request(ngx_http_request_t *r);
+#if (NGX_HTTP_V2)
+static ngx_int_t ngx_http_proxy_create_v2_request(ngx_http_request_t *r);
+#endif
 static ngx_int_t ngx_http_proxy_reinit_request(ngx_http_request_t *r);
 static ngx_int_t ngx_http_proxy_body_output_filter(void *data, ngx_chain_t 
*in);
 static ngx_int_t ngx_http_proxy_process_status_line(ngx_http_request_t *r);
@@ -149,6 +152,8 @@ static ngx_int_t ngx_http_proxy_port_var
 static ngx_int_t
 ngx_http_proxy_add_x_forwarded_for_variable(ngx_http_request_t *r,
 ngx_http_variable_value_t *v, uintptr_t data);
+static ngx_int_t ngx_http_proxy_internal_connection_variable(
+ngx_http_request_t *r, ngx_http_variable_value_t *v, uintptr_t data);
 static ngx_int_t
 ngx_http_proxy_internal_body_length_variable(ngx_http_request_t *r,
 ngx_http_variable_value_t *v, uintptr_t data);
@@ -245,6 +250,9 @@ static ngx_conf_bitmask_t  ngx_http_prox
 static ngx_conf_enum_t  ngx_http_proxy_http_version[] = {
 { ngx_string("1.0"), NGX_HTTP_VERSION_10 },
 { ngx_string("1.1"), NGX_HTTP_VERSION_11 },
+#if (NGX_HTTP_V2)
+{ ngx_string("2.0"), NGX_HTTP_VERSION_20 },
+#endif
 { ngx_null_string, 0 }
 };
 
@@ -765,7 +773,7 @@ static char  ngx_http_proxy_version_11[]
 
 static ngx_keyval_t  ngx_http_proxy_headers[] = {
 { ngx_string("Host"), ngx_string("$proxy_host") },
-{ ngx_string("Connection"), ngx_string("close") },
+{ ngx_string("Connection"), ngx_string("$proxy_internal_connection") },
 { ngx_string("Content-Length"), ngx_string("$proxy_internal_body_length") 
},
 { ngx_string("Transfer-Encoding"), ngx_string("$proxy_internal_chunked") },
 { ngx_string("TE"), ngx_string("") },
@@ -793,7 +801,7 @@ static ngx_str_t  ngx_http_proxy_hide_he
 
 static ngx_keyval_t  ngx_http_proxy_cache_headers[] = {
 { ngx_string("Host"), ngx_string("$proxy_host") },
-{ ngx_string("Connection"), ngx_string("close") },
+{ ngx_string("Connection"), ngx_string("$proxy_internal_connection") },
 { ngx_string("Content-Length"), ngx_string("$proxy_internal_body_length") 
},
 { ngx_string("Transfer-Encoding"), ngx_string("$proxy_internal_chunked") },
 { ngx_string("TE"), ngx_string("") },
@@ -828,6 +836,10 @@ static ngx_http_variable_t  ngx_http_pro
 { ngx_string("proxy_add_via"), NULL, NULL, 0, NGX_HTTP_VAR_NOHASH, 0 },
 #endif
 
+{ ngx_string("proxy_internal_connection"), NULL,
+  ngx_http_proxy_internal_connection_variable, 0,
+  NGX_HTTP_VAR_NOCACHEABLE|NGX_HTTP_VAR_NOHASH, 0 },
+
 { ngx_string("proxy_internal_body_length"), NULL,
   ngx_http_proxy_internal_body_length_variable, 0,
   NGX_HTTP_VAR_NOCACHEABLE|NGX_HTTP_VAR_NOHASH, 0 },
@@ -902,6 +914,18 @@ ngx_http_proxy_handler(ngx_http_request_
 u->finalize_request = ngx_http_proxy_finalize_request;
 r->state = 0;
 
+#if (NGX_HTTP_V2)
+
+if (plcf->http_version == NGX_HTTP_VERSION_20) {
+u->http2 = 1;
+
+u->create_request = ngx_http_proxy_create_v2_request;
+u->output.output_filter = ngx_http_v2_upstream_output_filter;
+u->output.filter_ctx = r;
+}
+
+#endif
+
 if (plcf->redirects) {
 u->rewrite_redirect = ngx_http_proxy_rewrite_redirect;
 }
@@ -929,7 +953,7 @@ ngx_http_proxy_handler(ngx_http_request_
 if (!plcf->upstream.request_buffering
 && plcf->body_values == NULL && plcf->upstream.pass_request_body
 && (!r->headers_in.chunked
-|| plcf->http_version == NGX_HTTP_VERSION_11))
+|| plcf->http_version >= NGX_HTTP_VERSION_11))
 {
 r->request_body_no_buffering = 1;
 }
@@ -1521,6 +1545,509 @@ ngx_http_proxy_create_request(ngx_http_r
 }
 
 
+#if (NGX_HTTP_V2)
+
+static ngx_int_t
+ngx_http_proxy_create_v2_request(ngx_http_request_t *r)
+{
+size_tlen, uri_len, loc_len, body_len;
+size_tkey_len, val_len, tmp_len;
+u_char

[PATCH 13 of 14] Proxy: add "proxy_pass_trailers" directive

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora <piotrsik...@google.com>
# Date 1490351854 25200
#  Fri Mar 24 03:37:34 2017 -0700
# Node ID cde1f42da7b26b7d2b788f916685e736b919138e
# Parent  7eb807b056da7abe9c679b59e94595d59a1406e6
Proxy: add "proxy_pass_trailers" directive.

Signed-off-by: Piotr Sikora <piotrsik...@google.com>

diff -r 7eb807b056da -r cde1f42da7b2 src/http/modules/ngx_http_fastcgi_module.c
--- a/src/http/modules/ngx_http_fastcgi_module.c
+++ b/src/http/modules/ngx_http_fastcgi_module.c
@@ -2788,10 +2788,10 @@ ngx_http_fastcgi_create_loc_conf(ngx_con
 
 conf->upstream.intercept_errors = NGX_CONF_UNSET;
 
-/* "fastcgi_cyclic_temp_file" is disabled */
+/* the hardcoded values */
 conf->upstream.cyclic_temp_file = 0;
-
 conf->upstream.change_buffering = 1;
+conf->upstream.pass_trailers = 0;
 
 conf->catch_stderr = NGX_CONF_UNSET_PTR;
 
diff -r 7eb807b056da -r cde1f42da7b2 
src/http/modules/ngx_http_memcached_module.c
--- a/src/http/modules/ngx_http_memcached_module.c
+++ b/src/http/modules/ngx_http_memcached_module.c
@@ -619,6 +619,7 @@ ngx_http_memcached_create_loc_conf(ngx_c
 conf->upstream.pass_request_headers = 0;
 conf->upstream.pass_request_body = 0;
 conf->upstream.force_ranges = 1;
+conf->upstream.pass_trailers = 0;
 
 conf->index = NGX_CONF_UNSET;
 conf->gzip_flag = NGX_CONF_UNSET_UINT;
diff -r 7eb807b056da -r cde1f42da7b2 src/http/modules/ngx_http_proxy_module.c
--- a/src/http/modules/ngx_http_proxy_module.c
+++ b/src/http/modules/ngx_http_proxy_module.c
@@ -630,6 +630,13 @@ static ngx_command_t  ngx_http_proxy_com
   offsetof(ngx_http_proxy_loc_conf_t, upstream.ignore_headers),
   _http_upstream_ignore_headers_masks },
 
+{ ngx_string("proxy_pass_trailers"),
+  NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_FLAG,
+  ngx_conf_set_flag_slot,
+  NGX_HTTP_LOC_CONF_OFFSET,
+  offsetof(ngx_http_proxy_loc_conf_t, upstream.pass_trailers),
+  NULL },
+
 { ngx_string("proxy_http_version"),
   NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_TAKE1,
   ngx_conf_set_enum_slot,
@@ -3483,6 +3490,8 @@ ngx_http_proxy_create_loc_conf(ngx_conf_
 conf->upstream.hide_headers = NGX_CONF_UNSET_PTR;
 conf->upstream.pass_headers = NGX_CONF_UNSET_PTR;
 
+conf->upstream.pass_trailers = NGX_CONF_UNSET;
+
 conf->upstream.intercept_errors = NGX_CONF_UNSET;
 
 #if (NGX_HTTP_SSL)
@@ -3494,11 +3503,11 @@ ngx_http_proxy_create_loc_conf(ngx_conf_
 conf->ssl_passwords = NGX_CONF_UNSET_PTR;
 #endif
 
-/* "proxy_cyclic_temp_file" is disabled */
+/* the hardcoded values */
 conf->upstream.cyclic_temp_file = 0;
+conf->upstream.change_buffering = 1;
 
 conf->redirect = NGX_CONF_UNSET;
-conf->upstream.change_buffering = 1;
 
 conf->cookie_domains = NGX_CONF_UNSET_PTR;
 conf->cookie_paths = NGX_CONF_UNSET_PTR;
@@ -3798,6 +3807,9 @@ ngx_http_proxy_merge_loc_conf(ngx_conf_t
 ngx_conf_merge_value(conf->upstream.pass_request_body,
   prev->upstream.pass_request_body, 1);
 
+ngx_conf_merge_value(conf->upstream.pass_trailers,
+  prev->upstream.pass_trailers, 0);
+
 ngx_conf_merge_value(conf->upstream.intercept_errors,
   prev->upstream.intercept_errors, 0);
 
@@ -4018,6 +4030,28 @@ ngx_http_proxy_merge_loc_conf(ngx_conf_t
 #endif
 }
 
+if (conf->upstream.pass_trailers) {
+
+if (conf->http_version != NGX_HTTP_VERSION_20) {
+ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
+   "\"proxy_pass_trailers\" requires "
+   "\"proxy_http_version 2.0\"");
+return NGX_CONF_ERROR;
+}
+
+#if (NGX_HTTP_CACHE)
+
+if (conf->upstream.cache) {
+ngx_conf_log_error(NGX_LOG_EMERG, cf, 0,
+   "\"proxy_pass_trailers\" doesn't work with "
+   "\"proxy_cache\"");
+return NGX_CONF_ERROR;
+}
+
+#endif
+
+}
+
 return NGX_CONF_OK;
 }
 
diff -r 7eb807b056da -r cde1f42da7b2 src/http/modules/ngx_http_scgi_module.c
--- a/src/http/modules/ngx_http_scgi_module.c
+++ b/src/http/modules/ngx_http_scgi_module.c
@@ -1236,10 +1236,10 @@ ngx_http_scgi_create_loc_conf(ngx_conf_t
 
 conf->upstream.intercept_errors = NGX_CONF_UNSET;
 
-/* "scgi_cyclic_temp_file" is disabled */
+/* the hardcoded values */
 conf->upstream.cyclic_temp_file = 0;
-
 conf->upstream.change_buffering = 1;
+conf->upstream.pass_trailers = 0;
 
 ngx_str_set(>upstream.module, "scgi");
 
diff -r 7eb807b056da -r cde1f42da7b2 src/http/modules/n

[PATCH 11 of 14] Proxy: split configured header names and values

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora <piotrsik...@google.com>
# Date 1489618535 25200
#  Wed Mar 15 15:55:35 2017 -0700
# Node ID 0637acdb51e29e1f68f5f3e762115c702cab4e72
# Parent  068381014f256ad6e2dc490bacc2529cebbb0462
Proxy: split configured header names and values.

Previously, each configured header was represented in one of two ways,
depending on whether or not its value included any variables.

If the value didn't include any variables, then it would be represented
as as a single script that contained complete header line with HTTP/1.1
delimiters, i.e.:

 "Header: value\r\n"

But if the value included any variables, then it would be represented
as a series of three scripts: first contained header name and the ": "
delimiter, second evaluated to header value, and third contained only
"\r\n", i.e.:

 "Header: "
 "$value"
 "\r\n"

This commit changes that, so that each configured header is represented
as a series of two scripts: first contains only header name, and second
contains (or evaluates to) only header value, i.e.:

"Header"
"$value"

or

"Header"
"value"

This not only makes things more consistent, but also allows header name
and value to be accessed separately.

Signed-off-by: Piotr Sikora <piotrsik...@google.com>

diff -r 068381014f25 -r 0637acdb51e2 src/http/modules/ngx_http_proxy_module.c
--- a/src/http/modules/ngx_http_proxy_module.c
+++ b/src/http/modules/ngx_http_proxy_module.c
@@ -1151,6 +1151,7 @@ static ngx_int_t
 ngx_http_proxy_create_request(ngx_http_request_t *r)
 {
 size_tlen, uri_len, loc_len, body_len;
+size_tkey_len, val_len;
 uintptr_t escape;
 ngx_buf_t*b;
 ngx_str_t method;
@@ -1265,11 +1266,20 @@ ngx_http_proxy_create_request(ngx_http_r
 le.flushed = 1;
 
 while (*(uintptr_t *) le.ip) {
-while (*(uintptr_t *) le.ip) {
+
+lcode = *(ngx_http_script_len_code_pt *) le.ip;
+key_len = lcode();
+
+for (val_len = 0; *(uintptr_t *) le.ip; val_len += lcode()) {
 lcode = *(ngx_http_script_len_code_pt *) le.ip;
-len += lcode();
 }
 le.ip += sizeof(uintptr_t);
+
+if (val_len == 0) {
+continue;
+}
+
+len += key_len + sizeof(": ") - 1 + val_len + sizeof(CRLF) - 1;
 }
 
 
@@ -1369,30 +1379,41 @@ ngx_http_proxy_create_request(ngx_http_r
 le.ip = headers->lengths->elts;
 
 while (*(uintptr_t *) le.ip) {
+
 lcode = *(ngx_http_script_len_code_pt *) le.ip;
-
-/* skip the header line name length */
 (void) lcode();
 
-if (*(ngx_http_script_len_code_pt *) le.ip) {
-
-for (len = 0; *(uintptr_t *) le.ip; len += lcode()) {
-lcode = *(ngx_http_script_len_code_pt *) le.ip;
+for (val_len = 0; *(uintptr_t *) le.ip; val_len += lcode()) {
+lcode = *(ngx_http_script_len_code_pt *) le.ip;
+}
+le.ip += sizeof(uintptr_t);
+
+if (val_len == 0) {
+e.skip = 1;
+
+while (*(uintptr_t *) e.ip) {
+code = *(ngx_http_script_code_pt *) e.ip;
+code((ngx_http_script_engine_t *) );
 }
-
-e.skip = (len == sizeof(CRLF) - 1) ? 1 : 0;
-
-} else {
+e.ip += sizeof(uintptr_t);
+
 e.skip = 0;
+
+continue;
 }
 
-le.ip += sizeof(uintptr_t);
+code = *(ngx_http_script_code_pt *) e.ip;
+code((ngx_http_script_engine_t *) );
+
+*e.pos++ = ':'; *e.pos++ = ' ';
 
 while (*(uintptr_t *) e.ip) {
 code = *(ngx_http_script_code_pt *) e.ip;
 code((ngx_http_script_engine_t *) );
 }
 e.ip += sizeof(uintptr_t);
+
+*e.pos++ = CR; *e.pos++ = LF;
 }
 
 b->last = e.pos;
@@ -3528,108 +3549,40 @@ ngx_http_proxy_init_headers(ngx_conf_t *
 continue;
 }
 
-if (ngx_http_script_variables_count([i].value) == 0) {
-copy = ngx_array_push_n(headers->lengths,
-sizeof(ngx_http_script_copy_code_t));
-if (copy == NULL) {
-return NGX_ERROR;
-}
-
-copy->code = (ngx_http_script_code_pt)
- ngx_http_script_copy_len_code;
-copy->len = src[i].key.len + sizeof(": ") - 1
-+ src[i].value.len + sizeof(CRLF) - 1;
-
-
-size = (sizeof(ngx_http_script_copy_code_t)
-   + src[i].key.len + sizeof(": ") - 1
-   + src[i].value.len + sizeof(CRLF) - 1
-   + sizeof(uintptr_t) - 1)
- 

[PATCH 10 of 14] Proxy: always emit "Host" header first

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora <piotrsik...@google.com>
# Date 1489618489 25200
#  Wed Mar 15 15:54:49 2017 -0700
# Node ID 068381014f256ad6e2dc490bacc2529cebbb0462
# Parent  96075d4cd2a6e8bd67caf1d7b78f8e87d757c48d
Proxy: always emit "Host" header first.

Signed-off-by: Piotr Sikora <piotrsik...@google.com>

diff -r 96075d4cd2a6 -r 068381014f25 src/http/modules/ngx_http_proxy_module.c
--- a/src/http/modules/ngx_http_proxy_module.c
+++ b/src/http/modules/ngx_http_proxy_module.c
@@ -3422,7 +3422,7 @@ ngx_http_proxy_init_headers(ngx_conf_t *
 uintptr_t*code;
 ngx_uint_ti;
 ngx_array_t   headers_names, headers_merged;
-ngx_keyval_t *src, *s, *h;
+ngx_keyval_t *host, *src, *s, *h;
 ngx_hash_key_t   *hk;
 ngx_hash_init_t   hash;
 ngx_http_script_compile_t sc;
@@ -3454,11 +3454,33 @@ ngx_http_proxy_init_headers(ngx_conf_t *
 return NGX_ERROR;
 }
 
+h = default_headers;
+
+if (h->key.len != sizeof("Host") - 1
+|| ngx_strcasecmp(h->key.data, (u_char *) "Host") != 0)
+{
+return NGX_ERROR;
+}
+
+host = ngx_array_push(_merged);
+if (host == NULL) {
+return NGX_ERROR;
+}
+
+*host = *h++;
+
 if (conf->headers_source) {
 
 src = conf->headers_source->elts;
 for (i = 0; i < conf->headers_source->nelts; i++) {
 
+if (src[i].key.len == sizeof("Host") - 1
+&& ngx_strcasecmp(src[i].key.data, (u_char *) "Host") == 0)
+{
+*host = src[i];
+continue;
+}
+
 s = ngx_array_push(_merged);
 if (s == NULL) {
 return NGX_ERROR;
@@ -3468,8 +3490,6 @@ ngx_http_proxy_init_headers(ngx_conf_t *
 }
 }
 
-h = default_headers;
-
 while (h->key.len) {
 
 src = headers_merged.elts;
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

[PATCH 06 of 14] HTTP/2: introduce stream->fake_connection

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora <piotrsik...@google.com>
# Date 1489111358 28800
#  Thu Mar 09 18:02:38 2017 -0800
# Node ID 64d12a65309eca3859055a04eb02cc14f3b3168d
# Parent  24b0f9f4ebfa560edd984146548ab07925dba73f
HTTP/2: introduce stream->fake_connection.

No functional changes.

Signed-off-by: Piotr Sikora <piotrsik...@google.com>

diff -r 24b0f9f4ebfa -r 64d12a65309e src/http/v2/ngx_http_v2.c
--- a/src/http/v2/ngx_http_v2.c
+++ b/src/http/v2/ngx_http_v2.c
@@ -1900,7 +1900,7 @@ ngx_http_v2_state_rst_stream(ngx_http_v2
 stream->in_closed = 1;
 stream->out_closed = 1;
 
-fc = stream->request->connection;
+fc = stream->fake_connection;
 fc->error = 1;
 
 switch (status) {
@@ -2293,7 +2293,7 @@ ngx_http_v2_state_window_update(ngx_http
 if (stream->exhausted) {
 stream->exhausted = 0;
 
-wev = stream->request->connection->write;
+wev = stream->fake_connection->write;
 
 wev->active = 0;
 wev->ready = 1;
@@ -2328,7 +2328,7 @@ ngx_http_v2_state_window_update(ngx_http
 
 stream->waiting = 0;
 
-wev = stream->request->connection->write;
+wev = stream->fake_connection->write;
 
 wev->active = 0;
 wev->ready = 1;
@@ -2444,17 +2444,18 @@ static u_char *
 ngx_http_v2_state_headers_save(ngx_http_v2_connection_t *h2c, u_char *pos,
 u_char *end, ngx_http_v2_handler_pt handler)
 {
-ngx_event_t   *rev;
-ngx_http_request_t*r;
+ngx_connection_t  *fc;
+ngx_http_v2_stream_t  *stream;
 ngx_http_core_srv_conf_t  *cscf;
 
 if (h2c->state.stream) {
-r = h2c->state.stream->request;
-rev = r->connection->read;
-
-if (!rev->timer_set) {
-cscf = ngx_http_get_module_srv_conf(r, ngx_http_core_module);
-ngx_add_timer(rev, cscf->client_header_timeout);
+stream = h2c->state.stream;
+fc = stream->fake_connection;
+
+if (!fc->read->timer_set) {
+cscf = ngx_http_get_module_srv_conf(stream->request,
+ngx_http_core_module);
+ngx_add_timer(fc->read, cscf->client_header_timeout);
 }
 }
 
@@ -2928,6 +2929,7 @@ ngx_http_v2_create_stream(ngx_http_v2_co
 
 stream->request = r;
 stream->connection = h2c;
+stream->fake_connection = fc;
 
 h2scf = ngx_http_get_module_srv_conf(r, ngx_http_v2_module);
 
@@ -3712,7 +3714,7 @@ ngx_http_v2_read_request_body(ngx_http_r
 }
 
 if (!buf) {
-ngx_add_timer(r->connection->read, clcf->client_body_timeout);
+ngx_add_timer(stream->fake_connection->read, 
clcf->client_body_timeout);
 }
 
 r->read_event_handler = ngx_http_v2_read_client_request_body_handler;
@@ -3732,7 +3734,7 @@ ngx_http_v2_process_request_body(ngx_htt
 ngx_http_request_body_t   *rb;
 ngx_http_core_loc_conf_t  *clcf;
 
-fc = r->connection;
+fc = r->stream->fake_connection;
 rb = r->request_body;
 buf = rb->buf;
 
@@ -3903,7 +3905,7 @@ ngx_http_v2_read_client_request_body_han
 {
 ngx_connection_t  *fc;
 
-fc = r->connection;
+fc = r->stream->fake_connection;
 
 ngx_log_debug0(NGX_LOG_DEBUG_HTTP, fc->log, 0,
"http2 read client request body handler");
@@ -3942,7 +3944,7 @@ ngx_http_v2_read_unbuffered_request_body
 ngx_http_core_loc_conf_t  *clcf;
 
 stream = r->stream;
-fc = r->connection;
+fc = stream->fake_connection;
 
 if (fc->read->timedout) {
 if (stream->recv_window) {
@@ -4042,7 +4044,7 @@ ngx_http_v2_terminate_stream(ngx_http_v2
 stream->rst_sent = 1;
 stream->skip_data = 1;
 
-fc = stream->request->connection;
+fc = stream->fake_connection;
 fc->error = 1;
 
 rev = fc->read;
@@ -4068,7 +4070,7 @@ ngx_http_v2_close_stream(ngx_http_v2_str
"http2 close stream %ui, queued %ui, processing %ui",
node->id, stream->queued, h2c->processing);
 
-fc = stream->request->connection;
+fc = stream->fake_connection;
 
 if (stream->queued) {
 fc->write->handler = ngx_http_v2_close_stream_handler;
@@ -4302,7 +4304,6 @@ ngx_http_v2_finalize_connection(ngx_http
 ngx_uint_t   i, size;
 ngx_event_t *ev;
 ngx_connection_t*c, *fc;
-ngx_http_request_t  *r;
 ngx_http_v2_node_t  *node;
 ngx_http_v2_stream_t*stream;
 ngx_http_v2_srv_conf_t  *h2scf;
@@ -4344,9 +4345,7 @@ ngx_http_v2_finalize_connection(ngx_http
 
 stream->waiting = 0;
 
-r = stream->request;
-fc = r->connection;
-
+fc = stream->fake_conn

[PATCH 09 of 14] Proxy: add "proxy_ssl_alpn" directive

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora <piotrsik...@google.com>
# Date 1489621682 25200
#  Wed Mar 15 16:48:02 2017 -0700
# Node ID 96075d4cd2a6e8bd67caf1d7b78f8e87d757c48d
# Parent  154ca6c5e62a1931a616e9f2b99ef2553b7c2c8b
Proxy: add "proxy_ssl_alpn" directive.

ALPN is used here only to indicate which version of the HTTP protocol
is going to be used and we doesn't verify that upstream agreed to it.

Please note that upstream is allowed to reject SSL connection with a
fatal "no_application_protocol" alert if it doesn't support it.

Signed-off-by: Piotr Sikora <piotrsik...@google.com>

diff -r 154ca6c5e62a -r 96075d4cd2a6 src/event/ngx_event_openssl.c
--- a/src/event/ngx_event_openssl.c
+++ b/src/event/ngx_event_openssl.c
@@ -654,6 +654,29 @@ ngx_ssl_ciphers(ngx_conf_t *cf, ngx_ssl_
 
 
 ngx_int_t
+ngx_ssl_alpn_protos(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_str_t *protos)
+{
+#ifdef TLSEXT_TYPE_application_layer_protocol_negotiation
+
+if (SSL_CTX_set_alpn_protos(ssl->ctx, protos->data, protos->len) != 0) {
+ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0,
+  "SSL_CTX_set_alpn_protos() failed");
+return NGX_ERROR;
+}
+
+return NGX_OK;
+
+#else
+
+ngx_log_error(NGX_LOG_EMERG, cf->log, 0,
+  "nginx was built with OpenSSL that lacks ALPN support");
+return NGX_ERROR;
+
+#endif
+}
+
+
+ngx_int_t
 ngx_ssl_client_certificate(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_str_t *cert,
 ngx_int_t depth)
 {
diff -r 154ca6c5e62a -r 96075d4cd2a6 src/event/ngx_event_openssl.h
--- a/src/event/ngx_event_openssl.h
+++ b/src/event/ngx_event_openssl.h
@@ -153,6 +153,8 @@ ngx_int_t ngx_ssl_certificate(ngx_conf_t
 ngx_str_t *cert, ngx_str_t *key, ngx_array_t *passwords);
 ngx_int_t ngx_ssl_ciphers(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_str_t *ciphers,
 ngx_uint_t prefer_server_ciphers);
+ngx_int_t ngx_ssl_alpn_protos(ngx_conf_t *cf, ngx_ssl_t *ssl,
+ngx_str_t *protos);
 ngx_int_t ngx_ssl_client_certificate(ngx_conf_t *cf, ngx_ssl_t *ssl,
 ngx_str_t *cert, ngx_int_t depth);
 ngx_int_t ngx_ssl_trusted_certificate(ngx_conf_t *cf, ngx_ssl_t *ssl,
diff -r 154ca6c5e62a -r 96075d4cd2a6 src/http/modules/ngx_http_proxy_module.c
--- a/src/http/modules/ngx_http_proxy_module.c
+++ b/src/http/modules/ngx_http_proxy_module.c
@@ -652,6 +652,13 @@ static ngx_command_t  ngx_http_proxy_com
   offsetof(ngx_http_proxy_loc_conf_t, ssl_ciphers),
   NULL },
 
+{ ngx_string("proxy_ssl_alpn"),
+  NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_FLAG,
+  ngx_conf_set_flag_slot,
+  NGX_HTTP_LOC_CONF_OFFSET,
+  offsetof(ngx_http_proxy_loc_conf_t, upstream.ssl_alpn),
+  NULL },
+
 { ngx_string("proxy_ssl_name"),
   NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_TAKE1,
   ngx_http_set_complex_value_slot,
@@ -2882,6 +2889,7 @@ ngx_http_proxy_create_loc_conf(ngx_conf_
 conf->upstream.intercept_errors = NGX_CONF_UNSET;
 
 #if (NGX_HTTP_SSL)
+conf->upstream.ssl_alpn = NGX_CONF_UNSET;
 conf->upstream.ssl_session_reuse = NGX_CONF_UNSET;
 conf->upstream.ssl_server_name = NGX_CONF_UNSET;
 conf->upstream.ssl_verify = NGX_CONF_UNSET;
@@ -3212,6 +3220,8 @@ ngx_http_proxy_merge_loc_conf(ngx_conf_t
 conf->upstream.ssl_name = prev->upstream.ssl_name;
 }
 
+ngx_conf_merge_value(conf->upstream.ssl_alpn,
+  prev->upstream.ssl_alpn, 0);
 ngx_conf_merge_value(conf->upstream.ssl_server_name,
   prev->upstream.ssl_server_name, 0);
 ngx_conf_merge_value(conf->upstream.ssl_verify,
@@ -4320,6 +4330,7 @@ ngx_http_proxy_lowat_check(ngx_conf_t *c
 static ngx_int_t
 ngx_http_proxy_set_ssl(ngx_conf_t *cf, ngx_http_proxy_loc_conf_t *plcf)
 {
+ngx_str_talpn;
 ngx_pool_cleanup_t  *cln;
 
 plcf->upstream.ssl = ngx_pcalloc(cf->pool, sizeof(ngx_ssl_t));
@@ -4366,6 +4377,24 @@ ngx_http_proxy_set_ssl(ngx_conf_t *cf, n
 return NGX_ERROR;
 }
 
+if (plcf->upstream.ssl_alpn) {
+
+switch (plcf->http_version) {
+
+case NGX_HTTP_VERSION_10:
+ngx_str_set(, NGX_HTTP_10_ALPN_ADVERTISE);
+break;
+
+case NGX_HTTP_VERSION_11:
+ngx_str_set(, NGX_HTTP_11_ALPN_ADVERTISE);
+break;
+}
+
+if (ngx_ssl_alpn_protos(cf, plcf->upstream.ssl, ) != NGX_OK) {
+return NGX_ERROR;
+}
+}
+
 if (plcf->upstream.ssl_verify) {
 if (plcf->ssl_trusted_certificate.len == 0) {
 ngx_log_error(NGX_LOG_EMERG, cf->log, 0,
diff -r 154ca6c5e62a -r 96075d4cd2a6 src/http/modules/ngx_http_ssl_module.c
--- a/src/http/modules/ngx_http_ssl_module.c
+++ b/src/http/modules/ngx_http_ssl_module.c
@@ -17,8 +17,6 @@ typedef ngx_int_t (*ngx_ssl_variable_han
 #defi

[PATCH 08 of 14] HTTP/2: add HTTP/2 to upstreams

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora <piotrsik...@google.com>
# Date 1490767180 25200
#  Tue Mar 28 22:59:40 2017 -0700
# Node ID 154ca6c5e62a1931a616e9f2b99ef2553b7c2c8b
# Parent  00bfd879eaf03f32373ab27110dd8f77c2b722a0
HTTP/2: add HTTP/2 to upstreams.

Signed-off-by: Piotr Sikora <piotrsik...@google.com>

diff -r 00bfd879eaf0 -r 154ca6c5e62a auto/modules
--- a/auto/modules
+++ b/auto/modules
@@ -429,7 +429,8 @@ if [ $HTTP = YES ]; then
  src/http/v2/ngx_http_v2_table.c \
  src/http/v2/ngx_http_v2_huff_decode.c \
  src/http/v2/ngx_http_v2_huff_encode.c \
- src/http/v2/ngx_http_v2_module.c"
+ src/http/v2/ngx_http_v2_module.c \
+ src/http/v2/ngx_http_v2_upstream.c"
 ngx_module_libs=
 ngx_module_link=$HTTP_V2
 
diff -r 00bfd879eaf0 -r 154ca6c5e62a src/core/ngx_connection.h
--- a/src/core/ngx_connection.h
+++ b/src/core/ngx_connection.h
@@ -175,6 +175,10 @@ struct ngx_connection_s {
 unsignedclose:1;
 unsignedshared:1;
 
+#if (NGX_HTTP_V2 || NGX_COMPAT)
+unsignedhttp2:1;
+#endif
+
 unsignedsendfile:1;
 unsignedsndlowat:1;
 unsignedtcp_nodelay:2;   /* ngx_connection_tcp_nodelay_e */
diff -r 00bfd879eaf0 -r 154ca6c5e62a src/http/ngx_http_upstream.c
--- a/src/http/ngx_http_upstream.c
+++ b/src/http/ngx_http_upstream.c
@@ -52,7 +52,8 @@ static ngx_int_t ngx_http_upstream_test_
 ngx_http_upstream_t *u);
 static ngx_int_t ngx_http_upstream_intercept_errors(ngx_http_request_t *r,
 ngx_http_upstream_t *u);
-static ngx_int_t ngx_http_upstream_test_connect(ngx_connection_t *c);
+static ngx_int_t ngx_http_upstream_test_connect(ngx_http_upstream_t *u,
+ngx_connection_t *c);
 static ngx_int_t ngx_http_upstream_process_headers(ngx_http_request_t *r,
 ngx_http_upstream_t *u);
 static void ngx_http_upstream_process_body_in_memory(ngx_http_request_t *r,
@@ -187,6 +188,11 @@ static ngx_int_t ngx_http_upstream_ssl_n
 ngx_http_upstream_t *u, ngx_connection_t *c);
 #endif
 
+#if (NGX_HTTP_V2)
+static ngx_int_t ngx_http_upstream_v2_init_connection(ngx_http_request_t *,
+ngx_http_upstream_t *u, ngx_connection_t *c);
+#endif
+
 
 static ngx_http_upstream_header_t  ngx_http_upstream_headers_in[] = {
 
@@ -1510,6 +1516,18 @@ ngx_http_upstream_connect(ngx_http_reque
 
 c = u->peer.connection;
 
+#if (NGX_HTTP_V2)
+
+if (u->http2 && c->http2) {
+if (ngx_http_upstream_v2_init_connection(r, u, c) != NGX_OK) {
+return;
+}
+
+c = u->peer.connection;
+}
+
+#endif
+
 c->data = r;
 
 c->write->handler = ngx_http_upstream_handler;
@@ -1533,10 +1551,15 @@ ngx_http_upstream_connect(ngx_http_reque
 }
 }
 
-c->log = r->connection->log;
-c->pool->log = c->log;
-c->read->log = c->log;
-c->write->log = c->log;
+#if (NGX_HTTP_V2)
+if (u->stream == NULL)
+#endif
+{
+c->log = r->connection->log;
+c->pool->log = c->log;
+c->read->log = c->log;
+c->write->log = c->log;
+}
 
 /* init or reinit the ngx_output_chain() and ngx_chain_writer() contexts */
 
@@ -1596,6 +1619,16 @@ ngx_http_upstream_connect(ngx_http_reque
 
 #endif
 
+#if (NGX_HTTP_V2)
+
+if (u->http2 && u->stream == NULL) {
+if (ngx_http_upstream_v2_init_connection(r, u, c) != NGX_OK) {
+return;
+}
+}
+
+#endif
+
 ngx_http_upstream_send_request(r, u, 1);
 }
 
@@ -1609,7 +1642,7 @@ ngx_http_upstream_ssl_init_connection(ng
 ngx_int_t  rc;
 ngx_http_core_loc_conf_t  *clcf;
 
-if (ngx_http_upstream_test_connect(c) != NGX_OK) {
+if (ngx_http_upstream_test_connect(u, c) != NGX_OK) {
 ngx_http_upstream_next(r, u, NGX_HTTP_UPSTREAM_FT_ERROR);
 return;
 }
@@ -1709,6 +1742,16 @@ ngx_http_upstream_ssl_handshake(ngx_conn
 c->write->handler = ngx_http_upstream_handler;
 c->read->handler = ngx_http_upstream_handler;
 
+#if (NGX_HTTP_V2)
+
+if (u->http2 && u->stream == NULL) {
+if (ngx_http_upstream_v2_init_connection(r, u, c) != NGX_OK) {
+return;
+}
+}
+
+#endif
+
 c = r->connection;
 
 ngx_http_upstream_send_request(r, u, 1);
@@ -1830,6 +1873,51 @@ done:
 #endif
 
 
+#if (NGX_HTTP_V2)
+
+static ngx_int_t
+ngx_http_upstream_v2_init_connection(ngx_http_request_t *r,
+ngx_http_upstream_t *u, ngx_connection_t *c)
+{
+ngx_connection_t  *fc;
+ngx_http_v2_connection_t  *h2c;
+
+if (ngx_http_upstream_test_connect(u, c) != NGX_OK) {
+ngx_http_upstream_next(r, u, NGX_HTTP_UPSTREAM_FT_ERROR);
+ 

[PATCH 05 of 14] HTTP/2: introduce h2c->conf_ctx

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora <piotrsik...@google.com>
# Date 1489106035 25200
#  Thu Mar 09 17:33:55 2017 -0700
# Node ID 24b0f9f4ebfa560edd984146548ab07925dba73f
# Parent  912d9cf36783146e61a68d554253e70956ea9125
HTTP/2: introduce h2c->conf_ctx.

No functional changes.

Signed-off-by: Piotr Sikora <piotrsik...@google.com>

diff -r 912d9cf36783 -r 24b0f9f4ebfa src/http/v2/ngx_http_v2.c
--- a/src/http/v2/ngx_http_v2.c
+++ b/src/http/v2/ngx_http_v2.c
@@ -239,6 +239,7 @@ ngx_http_v2_init(ngx_event_t *rev)
 
 h2c->connection = c;
 h2c->http_connection = hc;
+h2c->conf_ctx = hc->conf_ctx;
 
 h2c->send_window = NGX_HTTP_V2_DEFAULT_WINDOW;
 h2c->recv_window = NGX_HTTP_V2_MAX_WINDOW;
@@ -247,7 +248,7 @@ ngx_http_v2_init(ngx_event_t *rev)
 
 h2c->frame_size = NGX_HTTP_V2_DEFAULT_FRAME_SIZE;
 
-h2scf = ngx_http_get_module_srv_conf(hc->conf_ctx, ngx_http_v2_module);
+h2scf = ngx_http_get_module_srv_conf(h2c->conf_ctx, ngx_http_v2_module);
 
 h2c->pool = ngx_create_pool(h2scf->pool_size, h2c->connection->log);
 if (h2c->pool == NULL) {
@@ -349,8 +350,7 @@ ngx_http_v2_read_handler(ngx_event_t *re
 return;
 }
 
-h2mcf = ngx_http_get_module_main_conf(h2c->http_connection->conf_ctx,
-  ngx_http_v2_module);
+h2mcf = ngx_http_get_module_main_conf(h2c->conf_ctx, ngx_http_v2_module);
 
 available = h2mcf->recv_buffer_size - 2 * NGX_HTTP_V2_STATE_BUFFER_SIZE;
 
@@ -511,8 +511,7 @@ ngx_http_v2_send_output_queue(ngx_http_v
 goto error;
 }
 
-clcf = ngx_http_get_module_loc_conf(h2c->http_connection->conf_ctx,
-ngx_http_core_module);
+clcf = ngx_http_get_module_loc_conf(h2c->conf_ctx, ngx_http_core_module);
 
 if (ngx_handle_write_event(wev, clcf->send_lowat) != NGX_OK) {
 goto error;
@@ -624,8 +623,7 @@ ngx_http_v2_handle_connection(ngx_http_v
 return;
 }
 
-h2scf = ngx_http_get_module_srv_conf(h2c->http_connection->conf_ctx,
- ngx_http_v2_module);
+h2scf = ngx_http_get_module_srv_conf(h2c->conf_ctx, ngx_http_v2_module);
 if (h2c->state.incomplete) {
 ngx_add_timer(c->read, h2scf->recv_timeout);
 return;
@@ -1086,8 +1084,7 @@ ngx_http_v2_state_headers(ngx_http_v2_co
 goto rst_stream;
 }
 
-h2scf = ngx_http_get_module_srv_conf(h2c->http_connection->conf_ctx,
- ngx_http_v2_module);
+h2scf = ngx_http_get_module_srv_conf(h2c->conf_ctx, ngx_http_v2_module);
 
 h2c->state.header_limit = h2scf->max_header_size;
 
@@ -1319,8 +1316,7 @@ ngx_http_v2_state_field_len(ngx_http_v2_
"http2 hpack %s string length: %i",
huff ? "encoded" : "raw", len);
 
-h2scf = ngx_http_get_module_srv_conf(h2c->http_connection->conf_ctx,
- ngx_http_v2_module);
+h2scf = ngx_http_get_module_srv_conf(h2c->conf_ctx, ngx_http_v2_module);
 
 if ((size_t) len > h2scf->max_field_size) {
 ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0,
@@ -2590,8 +2586,7 @@ ngx_http_v2_send_settings(ngx_http_v2_co
 
 buf->last = ngx_http_v2_write_sid(buf->last, 0);
 
-h2scf = ngx_http_get_module_srv_conf(h2c->http_connection->conf_ctx,
- ngx_http_v2_module);
+h2scf = ngx_http_get_module_srv_conf(h2c->conf_ctx, ngx_http_v2_module);
 
 ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0,
"http2 send SETTINGS param MAX_CONCURRENT_STREAMS:%ui",
@@ -2953,8 +2948,7 @@ ngx_http_v2_get_node_by_id(ngx_http_v2_c
 ngx_http_v2_node_t  *node;
 ngx_http_v2_srv_conf_t  *h2scf;
 
-h2scf = ngx_http_get_module_srv_conf(h2c->http_connection->conf_ctx,
- ngx_http_v2_module);
+h2scf = ngx_http_get_module_srv_conf(h2c->conf_ctx, ngx_http_v2_module);
 
 index = ngx_http_v2_index(h2scf, sid);
 
@@ -2998,8 +2992,7 @@ ngx_http_v2_get_closed_node(ngx_http_v2_
 ngx_http_v2_node_t  *node, **next, *n, *parent, *child;
 ngx_http_v2_srv_conf_t  *h2scf;
 
-h2scf = ngx_http_get_module_srv_conf(h2c->http_connection->conf_ctx,
- ngx_http_v2_module);
+h2scf = ngx_http_get_module_srv_conf(h2c->conf_ctx, ngx_http_v2_module);
 
 h2c->closed_nodes--;
 
@@ -4287,8 +4280,7 @@ ngx_http_v2_idle_handler(ngx_event_t *re
 c->destroyed = 0;
 ngx_reusable_connection(c, 0);
 
-h2scf = ngx_http_get_module_srv_conf(h2c->http_connection->conf_ctx,
- ngx_http_v2_module);
+h2scf = ngx_http_get_module_srv_conf(h2c-&g

[PATCH 03 of 14] HTTP/2: add debug logging of control frames

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora <piotrsik...@google.com>
# Date 1490516711 25200
#  Sun Mar 26 01:25:11 2017 -0700
# Node ID 22d178a11e30c4a8576c3ce28859dfe1cc8adec0
# Parent  a147dd50ee3fb8628b79f4482c552c7c2852a732
HTTP/2: add debug logging of control frames.

Signed-off-by: Piotr Sikora <piotrsik...@google.com>

diff -r a147dd50ee3f -r 22d178a11e30 src/http/v2/ngx_http_v2.c
--- a/src/http/v2/ngx_http_v2.c
+++ b/src/http/v2/ngx_http_v2.c
@@ -41,9 +41,11 @@
 
 /* settings fields */
 #define NGX_HTTP_V2_HEADER_TABLE_SIZE_SETTING0x1
+#define NGX_HTTP_V2_ENABLE_PUSH_SETTING  0x2
 #define NGX_HTTP_V2_MAX_STREAMS_SETTING  0x3
 #define NGX_HTTP_V2_INIT_WINDOW_SIZE_SETTING 0x4
 #define NGX_HTTP_V2_MAX_FRAME_SIZE_SETTING   0x5
+#define NGX_HTTP_V2_HEADER_LIST_SIZE_SETTING 0x6
 
 #define NGX_HTTP_V2_FRAME_BUFFER_SIZE24
 
@@ -1946,6 +1948,9 @@ ngx_http_v2_state_settings(ngx_http_v2_c
 return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_SIZE_ERROR);
 }
 
+ngx_log_debug0(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0,
+   "http2 SETTINGS frame ack:1");
+
 h2c->settings_ack = 1;
 
 return ngx_http_v2_state_complete(h2c, pos, end);
@@ -1959,6 +1964,10 @@ ngx_http_v2_state_settings(ngx_http_v2_c
 return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_SIZE_ERROR);
 }
 
+ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0,
+   "http2 SETTINGS frame params:%uz",
+   h2c->state.length / NGX_HTTP_V2_SETTINGS_PARAM_SIZE);
+
 return ngx_http_v2_state_settings_params(h2c, pos, end);
 }
 
@@ -1986,6 +1995,27 @@ ngx_http_v2_state_settings_params(ngx_ht
 
 switch (id) {
 
+case NGX_HTTP_V2_HEADER_TABLE_SIZE_SETTING:
+
+ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0,
+   "http2 SETTINGS param HEADER_TABLE_SIZE:%ui "
+   "(ignored)", value);
+break;
+
+case NGX_HTTP_V2_ENABLE_PUSH_SETTING:
+
+ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0,
+   "http2 SETTINGS param ENABLE_PUSH:%ui "
+   "(ignored)", value);
+break;
+
+case NGX_HTTP_V2_MAX_STREAMS_SETTING:
+
+ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0,
+   "http2 SETTINGS param MAX_CONCURRENT_STREAMS:%ui "
+   "(ignored)", value);
+break;
+
 case NGX_HTTP_V2_INIT_WINDOW_SIZE_SETTING:
 
 if (value > NGX_HTTP_V2_MAX_WINDOW) {
@@ -1997,6 +2027,10 @@ ngx_http_v2_state_settings_params(ngx_ht
   NGX_HTTP_V2_FLOW_CTRL_ERROR);
 }
 
+ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0,
+   "http2 SETTINGS param INITIAL_WINDOW_SIZE:%ui",
+   value);
+
 window_delta = value - h2c->init_window;
 
 h2c->init_window = value;
@@ -2015,16 +2049,34 @@ ngx_http_v2_state_settings_params(ngx_ht
 
NGX_HTTP_V2_PROTOCOL_ERROR);
 }
 
+ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0,
+   "http2 SETTINGS param MAX_FRAME_SIZE:%ui",
+   value);
+
 h2c->frame_size = value;
 break;
 
+case NGX_HTTP_V2_HEADER_LIST_SIZE_SETTING:
+
+ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0,
+   "http2 SETTINGS param MAX_HEADER_LIST_SIZE:%ui "
+   "(ignored)", value);
+break;
+
 default:
+
+ngx_log_debug2(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0,
+   "http2 SETTINGS param 0x%Xi:%ui "
+   "(ignored)", id, value);
 break;
 }
 
 pos += NGX_HTTP_V2_SETTINGS_PARAM_SIZE;
 }
 
+ngx_log_debug0(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0,
+   "http2 send SETTINGS frame ack:1");
+
 frame = ngx_http_v2_get_frame(h2c, NGX_HTTP_V2_SETTINGS_ACK_SIZE,
   NGX_HTTP_V2_SETTINGS_FRAME,
   NGX_HTTP_V2_ACK_FLAG, 0);
@@ -2075,12 +2127,16 @@ ngx_http_v2_state_ping(ngx_http_v2_conne
 }
 
 ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0,
-   "http2 PING frame, flags: %ud", h2c->state.flags);
+   "http2 PING frame ack:%ud",
+   h2c->state.flags & NG

[PATCH 02 of 14] Upstream keepalive: preserve c->data

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora <piotrsik...@google.com>
# Date 1491886301 25200
#  Mon Apr 10 21:51:41 2017 -0700
# Node ID a147dd50ee3fb8628b79f4482c552c7c2852a732
# Parent  5f5d70428655db0889a2111d17d912a7383df152
Upstream keepalive: preserve c->data.

Signed-off-by: Piotr Sikora <piotrsik...@google.com>

diff -r 5f5d70428655 -r a147dd50ee3f 
src/http/modules/ngx_http_upstream_keepalive_module.c
--- a/src/http/modules/ngx_http_upstream_keepalive_module.c
+++ b/src/http/modules/ngx_http_upstream_keepalive_module.c
@@ -27,6 +27,7 @@ typedef struct {
 
 ngx_queue_tqueue;
 ngx_connection_t  *connection;
+void  *data;
 
 socklen_t  socklen;
 ngx_sockaddr_t sockaddr;
@@ -254,6 +255,7 @@ found:
 ngx_log_debug1(NGX_LOG_DEBUG_HTTP, pc->log, 0,
"get keepalive peer: using connection %p", c);
 
+c->data = item->data;
 c->idle = 0;
 c->sent = 0;
 c->log = pc->log;
@@ -336,6 +338,7 @@ ngx_http_upstream_free_keepalive_peer(ng
 ngx_queue_insert_head(>conf->cache, q);
 
 item->connection = c;
+item->data = c->data;
 
 pc->connection = NULL;
 
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

[PATCH 04 of 14] HTTP/2: s/client/peer/

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora <piotrsik...@google.com>
# Date 1485917964 28800
#  Tue Jan 31 18:59:24 2017 -0800
# Node ID 912d9cf36783146e61a68d554253e70956ea9125
# Parent  22d178a11e30c4a8576c3ce28859dfe1cc8adec0
HTTP/2: s/client/peer/.

No functional changes.

Signed-off-by: Piotr Sikora <piotrsik...@google.com>

diff -r 22d178a11e30 -r 912d9cf36783 src/http/v2/ngx_http_v2.c
--- a/src/http/v2/ngx_http_v2.c
+++ b/src/http/v2/ngx_http_v2.c
@@ -316,7 +316,7 @@ ngx_http_v2_read_handler(ngx_event_t *re
 h2c = c->data;
 
 if (rev->timedout) {
-ngx_log_error(NGX_LOG_INFO, c->log, NGX_ETIMEDOUT, "client timed out");
+ngx_log_error(NGX_LOG_INFO, c->log, NGX_ETIMEDOUT, "peer timed out");
 ngx_http_v2_finalize_connection(h2c, NGX_HTTP_V2_PROTOCOL_ERROR);
 return;
 }
@@ -368,7 +368,7 @@ ngx_http_v2_read_handler(ngx_event_t *re
 
 if (n == 0 && (h2c->state.incomplete || h2c->processing)) {
 ngx_log_error(NGX_LOG_INFO, c->log, 0,
-  "client prematurely closed connection");
+  "peer prematurely closed connection");
 }
 
 if (n == 0 || n == NGX_ERROR) {
@@ -774,7 +774,7 @@ ngx_http_v2_state_data(ngx_http_v2_conne
 
 if (h2c->state.length == 0) {
 ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0,
-  "client sent padded DATA frame "
+  "peer sent padded DATA frame "
   "with incorrect length: 0");
 
 return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_SIZE_ERROR);
@@ -789,7 +789,7 @@ ngx_http_v2_state_data(ngx_http_v2_conne
 
 if (h2c->state.padding >= size) {
 ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0,
-  "client sent padded DATA frame "
+  "peer sent padded DATA frame "
   "with incorrect length: %uz, padding: %uz",
   size, h2c->state.padding);
 
@@ -805,7 +805,7 @@ ngx_http_v2_state_data(ngx_http_v2_conne
 
 if (size > h2c->recv_window) {
 ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0,
-  "client violated connection flow control: "
+  "peer violated connection flow control: "
   "received DATA frame length %uz, available window %uz",
   size, h2c->recv_window);
 
@@ -840,7 +840,7 @@ ngx_http_v2_state_data(ngx_http_v2_conne
 
 if (size > stream->recv_window) {
 ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0,
-  "client violated flow control for stream %ui: "
+  "peer violated flow control for stream %ui: "
   "received DATA frame length %uz, available window %uz",
   node->id, size, stream->recv_window);
 
@@ -874,7 +874,7 @@ ngx_http_v2_state_data(ngx_http_v2_conne
 
 if (stream->in_closed) {
 ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0,
-  "client sent DATA frame for half-closed stream %ui",
+  "peer sent DATA frame for half-closed stream %ui",
   node->id);
 
 if (ngx_http_v2_terminate_stream(h2c, stream,
@@ -1002,7 +1002,7 @@ ngx_http_v2_state_headers(ngx_http_v2_co
 
 if (h2c->state.length < size) {
 ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0,
-  "client sent HEADERS frame with incorrect length %uz",
+  "peer sent HEADERS frame with incorrect length %uz",
   h2c->state.length);
 
 return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_SIZE_ERROR);
@@ -1010,7 +1010,7 @@ ngx_http_v2_state_headers(ngx_http_v2_co
 
 if (h2c->state.length == size) {
 ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0,
-  "client sent HEADERS frame with empty header block");
+  "peer sent HEADERS frame with empty header block");
 
 return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_SIZE_ERROR);
 }
@@ -1033,7 +1033,7 @@ ngx_http_v2_state_headers(ngx_http_v2_co
 
 if (h2c->state.padding > h2c->state.length) {
 ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0,
-  "client sent padded HEADERS frame "
+  "peer sent padded HEADERS frame "
   "with incorrect length: %uz, padding: %uz",
   h2c->state.length, h2c->state.padding);

[PATCH 01 of 14] Output chain: propagate last_buf flag to c->send_chain()

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora <piotrsik...@google.com>
# Date 1491708381 25200
#  Sat Apr 08 20:26:21 2017 -0700
# Node ID 5f5d70428655db0889a2111d17d912a7383df152
# Parent  a39bc74873faf9e5bea616561b43f6ecc55229f9
Output chain: propagate last_buf flag to c->send_chain().

Signed-off-by: Piotr Sikora <piotrsik...@google.com>

diff -r a39bc74873fa -r 5f5d70428655 src/core/ngx_output_chain.c
--- a/src/core/ngx_output_chain.c
+++ b/src/core/ngx_output_chain.c
@@ -658,6 +658,7 @@ ngx_chain_writer(void *data, ngx_chain_t
 ngx_chain_writer_ctx_t *ctx = data;
 
 off_t  size;
+ngx_uint_t last;
 ngx_chain_t   *cl, *ln, *chain;
 ngx_connection_t  *c;
 
@@ -689,9 +690,10 @@ ngx_chain_writer(void *data, ngx_chain_t
 
 size += ngx_buf_size(in->buf);
 
-ngx_log_debug2(NGX_LOG_DEBUG_CORE, c->log, 0,
-   "chain writer buf fl:%d s:%uO",
-   in->buf->flush, ngx_buf_size(in->buf));
+ngx_log_debug3(NGX_LOG_DEBUG_CORE, c->log, 0,
+   "chain writer buf fl:%d l:%d s:%uO",
+   in->buf->flush, in->buf->last_buf,
+   ngx_buf_size(in->buf));
 
 cl = ngx_alloc_chain_link(ctx->pool);
 if (cl == NULL) {
@@ -707,6 +709,8 @@ ngx_chain_writer(void *data, ngx_chain_t
 ngx_log_debug1(NGX_LOG_DEBUG_CORE, c->log, 0,
"chain writer in: %p", ctx->out);
 
+last = 0;
+
 for (cl = ctx->out; cl; cl = cl->next) {
 
 #if 1
@@ -732,9 +736,16 @@ ngx_chain_writer(void *data, ngx_chain_t
 #endif
 
 size += ngx_buf_size(cl->buf);
+
+if (cl->buf->last_buf) {
+last = 1;
+}
 }
 
-if (size == 0 && !c->buffered) {
+if (size == 0
+&& !c->buffered
+&& !(last && c->need_last_buf))
+{
 return NGX_OK;
 }
 
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

Re: [PATCH] Proxy: split configured header names and values

[Piotr Sikora via nginx-devel]

Hey Maxim,

> Full patch with the above comments below:

Applied, thanks!

Best regards,
Piotr Sikora
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

[PATCH] HTTP/2: add debug logging of control frames

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora <piotrsik...@google.com>
# Date 1490516711 25200
#  Sun Mar 26 01:25:11 2017 -0700
# Node ID 1f1549823fba355a0dd1af49108be4b4898bf331
# Parent  d1816a2696de8c2faa1cd913a151e5f62a8620f3
HTTP/2: add debug logging of control frames.

Signed-off-by: Piotr Sikora <piotrsik...@google.com>

diff -r d1816a2696de -r 1f1549823fba src/http/v2/ngx_http_v2.c
--- a/src/http/v2/ngx_http_v2.c
+++ b/src/http/v2/ngx_http_v2.c
@@ -41,9 +41,11 @@
 
 /* settings fields */
 #define NGX_HTTP_V2_HEADER_TABLE_SIZE_SETTING0x1
+#define NGX_HTTP_V2_ENABLE_PUSH_SETTING  0x2
 #define NGX_HTTP_V2_MAX_STREAMS_SETTING  0x3
 #define NGX_HTTP_V2_INIT_WINDOW_SIZE_SETTING 0x4
 #define NGX_HTTP_V2_MAX_FRAME_SIZE_SETTING   0x5
+#define NGX_HTTP_V2_HEADER_LIST_SIZE_SETTING 0x6
 
 #define NGX_HTTP_V2_FRAME_BUFFER_SIZE24
 
@@ -1946,6 +1948,9 @@ ngx_http_v2_state_settings(ngx_http_v2_c
 return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_SIZE_ERROR);
 }
 
+ngx_log_debug0(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0,
+   "http2 SETTINGS frame ack:1");
+
 h2c->settings_ack = 1;
 
 return ngx_http_v2_state_complete(h2c, pos, end);
@@ -1959,6 +1964,10 @@ ngx_http_v2_state_settings(ngx_http_v2_c
 return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_SIZE_ERROR);
 }
 
+ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0,
+   "http2 SETTINGS frame params:%uz",
+   h2c->state.length / NGX_HTTP_V2_SETTINGS_PARAM_SIZE);
+
 return ngx_http_v2_state_settings_params(h2c, pos, end);
 }
 
@@ -1986,6 +1995,27 @@ ngx_http_v2_state_settings_params(ngx_ht
 
 switch (id) {
 
+case NGX_HTTP_V2_HEADER_TABLE_SIZE_SETTING:
+
+ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0,
+   "http2 SETTINGS param HEADER_TABLE_SIZE:%ui "
+   "(ignored)", value);
+break;
+
+case NGX_HTTP_V2_ENABLE_PUSH_SETTING:
+
+ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0,
+   "http2 SETTINGS param ENABLE_PUSH:%ui "
+   "(ignored)", value);
+break;
+
+case NGX_HTTP_V2_MAX_STREAMS_SETTING:
+
+ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0,
+   "http2 SETTINGS param MAX_CONCURRENT_STREAMS:%ui "
+   "(ignored)", value);
+break;
+
 case NGX_HTTP_V2_INIT_WINDOW_SIZE_SETTING:
 
 if (value > NGX_HTTP_V2_MAX_WINDOW) {
@@ -1997,6 +2027,10 @@ ngx_http_v2_state_settings_params(ngx_ht
   NGX_HTTP_V2_FLOW_CTRL_ERROR);
 }
 
+ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0,
+   "http2 SETTINGS param INITIAL_WINDOW_SIZE:%ui",
+   value);
+
 window_delta = value - h2c->init_window;
 
 h2c->init_window = value;
@@ -2015,16 +2049,34 @@ ngx_http_v2_state_settings_params(ngx_ht
 
NGX_HTTP_V2_PROTOCOL_ERROR);
 }
 
+ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0,
+   "http2 SETTINGS param MAX_FRAME_SIZE:%ui",
+   value);
+
 h2c->frame_size = value;
 break;
 
+case NGX_HTTP_V2_HEADER_LIST_SIZE_SETTING:
+
+ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0,
+   "http2 SETTINGS param MAX_HEADER_LIST_SIZE:%ui "
+   "(ignored)", value);
+break;
+
 default:
+
+ngx_log_debug2(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0,
+   "http2 SETTINGS param 0x%Xi:%ui "
+   "(ignored)", id, value);
 break;
 }
 
 pos += NGX_HTTP_V2_SETTINGS_PARAM_SIZE;
 }
 
+ngx_log_debug0(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0,
+   "http2 send SETTINGS frame ack:1");
+
 frame = ngx_http_v2_get_frame(h2c, NGX_HTTP_V2_SETTINGS_ACK_SIZE,
   NGX_HTTP_V2_SETTINGS_FRAME,
   NGX_HTTP_V2_ACK_FLAG, 0);
@@ -2075,12 +2127,16 @@ ngx_http_v2_state_ping(ngx_http_v2_conne
 }
 
 ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0,
-   "http2 PING frame, flags: %ud", h2c->state.flags);
+   "http2 PING frame ack:%ud",
+   h2c->state.flags & NG

Re: [PATCH] HTTP/2: add debug logging of control frames

[Piotr Sikora via nginx-devel]

Hey Valentin,

> Ok, I've already resigned myself to multiline output, but don't let it
> look like an another SETTINGS frame.
>
> IMHO, something like that will be good enough:
>
>http2 send SETTINGS frame
>http2 SETTINGS param MAX_CONCURRENT_STREAMS: 100
>http2 SETTINGS param INITIAL_WINDOW_SIZE: 65536
>http2 SETTINGS param MAX_FRAME_SIZE: 16777215

Done, with retained "send " prefix to differentiate params that we
send and receive.

I've also re-added params counter to the "send SETTINGS frame", by
setting "len" value a bit sooner, so that the number of params is
calculated and cannot be forgotten in subsequent commits.

Best regards,
Piotr Sikora
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

[PATCH] Output chain: propagate last_buf flag to c->send_chain()

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora <piotrsik...@google.com>
# Date 1491708381 25200
#  Sat Apr 08 20:26:21 2017 -0700
# Node ID 3363bdf821c7110b577437bd59c962653f3a144f
# Parent  d1816a2696de8c2faa1cd913a151e5f62a8620f3
Output chain: propagate last_buf flag to c->send_chain().

Signed-off-by: Piotr Sikora <piotrsik...@google.com>

diff -r d1816a2696de -r 3363bdf821c7 src/core/ngx_output_chain.c
--- a/src/core/ngx_output_chain.c
+++ b/src/core/ngx_output_chain.c
@@ -658,6 +658,7 @@ ngx_chain_writer(void *data, ngx_chain_t
 ngx_chain_writer_ctx_t *ctx = data;
 
 off_t  size;
+ngx_uint_t last;
 ngx_chain_t   *cl, *ln, *chain;
 ngx_connection_t  *c;
 
@@ -689,9 +690,10 @@ ngx_chain_writer(void *data, ngx_chain_t
 
 size += ngx_buf_size(in->buf);
 
-ngx_log_debug2(NGX_LOG_DEBUG_CORE, c->log, 0,
-   "chain writer buf fl:%d s:%uO",
-   in->buf->flush, ngx_buf_size(in->buf));
+ngx_log_debug3(NGX_LOG_DEBUG_CORE, c->log, 0,
+   "chain writer buf fl:%d l:%d s:%uO",
+   in->buf->flush, in->buf->last_buf,
+   ngx_buf_size(in->buf));
 
 cl = ngx_alloc_chain_link(ctx->pool);
 if (cl == NULL) {
@@ -707,6 +709,8 @@ ngx_chain_writer(void *data, ngx_chain_t
 ngx_log_debug1(NGX_LOG_DEBUG_CORE, c->log, 0,
"chain writer in: %p", ctx->out);
 
+last = 0;
+
 for (cl = ctx->out; cl; cl = cl->next) {
 
 #if 1
@@ -732,9 +736,16 @@ ngx_chain_writer(void *data, ngx_chain_t
 #endif
 
 size += ngx_buf_size(cl->buf);
+
+if (cl->buf->last_buf) {
+last = 1;
+}
 }
 
-if (size == 0 && !c->buffered) {
+if (size == 0
+&& !c->buffered
+&& !(last && c->need_last_buf))
+{
 return NGX_OK;
 }
 
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

Re: [PATCH] Output chain: propagate flush and last_buf flags to send_chain()

[Piotr Sikora via nginx-devel]

Hey Maxim,

> Note well that in HTTP/2-related code the special flag
> c->need_last_buf is used to indicate that a (fake) connection
> needs an information about last_buf, thus allowing HTTP/2
> c->send_chain() wrapper to add its own framing.  If the goal is
> the same, please consider using the same approach.

Yes, it's exactly the same use case.

I originally used c->need_last_buf, but then decided against it, since
c->send_chain() is "pluggable" and therefore we shouldn't be assuming
it's behavior, but I don't mind either way, as both solutions for work
my use case.

Best regards,
Piotr Sikora
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

Re: [PATCH 4 of 4] HTTP/2: reject HTTP/2 requests with connection-specific headers

[Piotr Sikora via nginx-devel]

Hey Maxim,

> I'm highly sceptical about the whole series in general, and this
> patch specifically.
>
> In particular, the "Proxy-Connection" header is not something even
> defined by any standard, and even in its non-standard [broken]
> meaning never expected to be used in connections to nginx.  Not to
> mention that Proxy-Authorization, a standard-defined hop-by-hop
> (connection-specific in terms of HTTP/2) header, is not checked
> anywhere.

Proxy-Connection is mentioned (and discouraged) in RFC7230.

> Additionally, I really think that disabling upgrades is one of the
> big mistakes of HTTP/2.  It would be much more logical to
> interpret a HTTP/2 stream as a connection to upgrade, and allow to
> multiplex arbitrary protocols via a single HTTP/2 connection.

Unfortunately, I have to agree.

> Unless there are practical reasons for these changes, I would
> rather reject the series.

The practical reason is that other implementations (e.g. nghttp2)
reject requests with those headers, which leads to a weird behavior
where NGINX accepts requests and proxies them to a HTTP/2 upstream
which rejects them because they contain one of those headers.

We could clear those headers in proxy module (I'm already doing that
for most of the headers, anyway), but it feels like a workaround for
broken clients.

Having said that, I'm fine with dropping the whole patchset.

Best regards,
Piotr Sikora
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

Re: [PATCH 2 of 3] HTTP/2: added support for trailers in HTTP responses

[Piotr Sikora via nginx-devel]

Hey Valentin,

> I've overlooked this while doing previous review, but it looks strange.
>
> Why do you use NGX_LOG_WARN for trailers headers?  It results in
> finalizing request with an error (in case of HTTP/2 it means RST_STREAM).
>
> For main headers the NGX_LOG_CRIT level is used.  It looks too serious,
> but the WARN level is too low.

Good catch, thanks!

This was left-over from the initial version, which skipped too long
trailers, instead of resetting stream.

Best regards,
Piotr Sikora
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

Re: [PATCH 2 of 3] HTTP/2: added support for trailers in HTTP responses

[Piotr Sikora via nginx-devel]

Hey Maxim,

> Series committed with the above change.
> Thanks to all involved.

Thanks! :)

Best regards,
Piotr Sikora
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

Re: [PATCH] HTTP/2: reject HTTP/2 requests with "Connection" header

[Piotr Sikora via nginx-devel]

Hey Valentin,

> Since HTTP/2 is a separate protocol and not just GET / HTTP/2.0, so
> the r->stream pointer should be tested instead (like in many other
> places).

Done. I've also sent few more patches rejecting remaining invalid
headers. Feel free to squash them together if you prefer.

Best regards,
Piotr Sikora
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

Re: [PATCH 2 of 3] HTTP/2: added support for trailers in HTTP responses

[Piotr Sikora via nginx-devel]

Hey Valentin,

> It's better to keep return values consistent with
> ngx_http_v2_create_headers_frame() and introduce
> NGX_HTTP_V2_NO_TRAILERS instead of NGX_HTTP_V2_FRAME_ERROR.

NGX_HTTP_V2_NO_TRAILERS feels a bit weird, but whatever works for you is fine.

> There's no reason to check "trailers" on each iteration.
> I think you can put it inside the "if (in == NULL)" condition.

Good catch, thanks!

> Please consider the changes below.

Applied (with removed empty lines between error message and "return NULL").

Thanks!

Best regards,
Piotr Sikora
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

Re: [PATCH 1 of 3] Added support for trailers in HTTP responses

[Piotr Sikora via nginx-devel]

Hey Maxim,

> I've tried this as well, and decided that "if (len ==
> sizeof(...))" is slightly more readable, and also produces smaller
> patch to your code.   No strict preference though, feel free to
> use any variant you think is better.

I've ended up using "if (len == 0) { ... }" in the end, but applied
rest of you changes. Thanks!

Best regards,
Piotr Sikora
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

[PATCH 4 of 4] HTTP/2: reject HTTP/2 requests with connection-specific headers

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora <piotrsik...@google.com>
# Date 1490516709 25200
#  Sun Mar 26 01:25:09 2017 -0700
# Node ID e2abc3bc3fc12b788d2631d3c47215acdc4ebbe6
# Parent  6263d68cb96042d8f8974a4a3945226227ce13b9
HTTP/2: reject HTTP/2 requests with connection-specific headers.

Signed-off-by: Piotr Sikora <piotrsik...@google.com>

diff -r 6263d68cb960 -r e2abc3bc3fc1 src/http/ngx_http_request.c
--- a/src/http/ngx_http_request.c
+++ b/src/http/ngx_http_request.c
@@ -19,6 +19,8 @@ static ngx_int_t ngx_http_alloc_large_he
 
 static ngx_int_t ngx_http_process_header_line(ngx_http_request_t *r,
 ngx_table_elt_t *h, ngx_uint_t offset);
+static ngx_int_t ngx_http_process_http1_header_line(ngx_http_request_t *r,
+ngx_table_elt_t *h, ngx_uint_t offset);
 static ngx_int_t ngx_http_process_unique_header_line(ngx_http_request_t *r,
 ngx_table_elt_t *h, ngx_uint_t offset);
 static ngx_int_t ngx_http_process_multi_header_lines(ngx_http_request_t *r,
@@ -146,7 +148,7 @@ ngx_http_header_t  ngx_http_headers_in[]
 
 { ngx_string("Upgrade"),
  offsetof(ngx_http_headers_in_t, upgrade),
- ngx_http_process_header_line },
+ ngx_http_process_http1_header_line },
 
 #if (NGX_HTTP_GZIP)
 { ngx_string("Accept-Encoding"),
@@ -161,8 +163,13 @@ ngx_http_header_t  ngx_http_headers_in[]
  offsetof(ngx_http_headers_in_t, authorization),
  ngx_http_process_unique_header_line },
 
-{ ngx_string("Keep-Alive"), offsetof(ngx_http_headers_in_t, keep_alive),
- ngx_http_process_header_line },
+{ ngx_string("Keep-Alive"),
+ offsetof(ngx_http_headers_in_t, keep_alive),
+ ngx_http_process_http1_header_line },
+
+{ ngx_string("Proxy-Connection"),
+ offsetof(ngx_http_headers_in_t, proxy_connection),
+ ngx_http_process_http1_header_line },
 
 #if (NGX_HTTP_X_FORWARDED_FOR)
 { ngx_string("X-Forwarded-For"),
@@ -1618,6 +1625,35 @@ ngx_http_process_header_line(ngx_http_re
 
 
 static ngx_int_t
+ngx_http_process_http1_header_line(ngx_http_request_t *r, ngx_table_elt_t *h,
+ngx_uint_t offset)
+{
+ngx_table_elt_t  **ph;
+
+ph = (ngx_table_elt_t **) ((char *) >headers_in + offset);
+
+if (*ph == NULL) {
+*ph = h;
+}
+
+#if (NGX_HTTP_V2)
+
+if (r->stream) {
+ngx_log_error(NGX_LOG_INFO, r->connection->log, 0,
+  "client sent HTTP/2 request with \"%V\" header",
+  >key);
+
+ngx_http_finalize_request(r, NGX_HTTP_BAD_REQUEST);
+return NGX_ERROR;
+}
+
+#endif
+
+return NGX_OK;
+}
+
+
+static ngx_int_t
 ngx_http_process_unique_header_line(ngx_http_request_t *r, ngx_table_elt_t *h,
 ngx_uint_t offset)
 {
diff -r 6263d68cb960 -r e2abc3bc3fc1 src/http/ngx_http_request.h
--- a/src/http/ngx_http_request.h
+++ b/src/http/ngx_http_request.h
@@ -209,6 +209,7 @@ typedef struct {
 ngx_table_elt_t  *authorization;
 
 ngx_table_elt_t  *keep_alive;
+ngx_table_elt_t  *proxy_connection;
 
 #if (NGX_HTTP_X_FORWARDED_FOR)
 ngx_array_t   x_forwarded_for;
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

[PATCH 3 of 4] HTTP/2: reject HTTP/2 requests with "Transfer-Encoding" header

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora <piotrsik...@google.com>
# Date 1490516709 25200
#  Sun Mar 26 01:25:09 2017 -0700
# Node ID 6263d68cb96042d8f8974a4a3945226227ce13b9
# Parent  349648a6f91f9bd5cc80d22390b95c2239a8bfb3
HTTP/2: reject HTTP/2 requests with "Transfer-Encoding" header.

Signed-off-by: Piotr Sikora <piotrsik...@google.com>

diff -r 349648a6f91f -r 6263d68cb960 src/http/ngx_http_request.c
--- a/src/http/ngx_http_request.c
+++ b/src/http/ngx_http_request.c
@@ -29,6 +29,8 @@ static ngx_int_t ngx_http_process_connec
 ngx_table_elt_t *h, ngx_uint_t offset);
 static ngx_int_t ngx_http_process_te(ngx_http_request_t *r,
 ngx_table_elt_t *h, ngx_uint_t offset);
+static ngx_int_t ngx_http_process_transfer_encoding(ngx_http_request_t *r,
+ngx_table_elt_t *h, ngx_uint_t offset);
 static ngx_int_t ngx_http_process_user_agent(ngx_http_request_t *r,
 ngx_table_elt_t *h, ngx_uint_t offset);
 
@@ -136,7 +138,7 @@ ngx_http_header_t  ngx_http_headers_in[]
 
 { ngx_string("Transfer-Encoding"),
  offsetof(ngx_http_headers_in_t, transfer_encoding),
- ngx_http_process_header_line },
+ ngx_http_process_transfer_encoding },
 
 { ngx_string("Expect"),
  offsetof(ngx_http_headers_in_t, expect),
@@ -1743,6 +1745,49 @@ ngx_http_process_te(ngx_http_request_t *
 
 
 static ngx_int_t
+ngx_http_process_transfer_encoding(ngx_http_request_t *r, ngx_table_elt_t *h,
+ngx_uint_t offset)
+{
+if (r->headers_in.transfer_encoding == NULL) {
+r->headers_in.transfer_encoding = h;
+}
+
+#if (NGX_HTTP_V2)
+
+if (r->stream) {
+ngx_log_error(NGX_LOG_INFO, r->connection->log, 0,
+  "client sent HTTP/2 request with \"Transfer-Encoding\" "
+  "header");
+
+ngx_http_finalize_request(r, NGX_HTTP_BAD_REQUEST);
+return NGX_ERROR;
+}
+
+#endif
+
+if (h->value.len == 7
+&& ngx_strncasecmp(h->value.data, (u_char *) "chunked", 7) == 0)
+{
+r->headers_in.chunked = 1;
+return NGX_OK;
+}
+
+if (h->value.len != 8
+|| ngx_strncasecmp(h->value.data, (u_char *) "identity", 8) != 0)
+{
+ngx_log_error(NGX_LOG_INFO, r->connection->log, 0,
+  "client sent unknown \"Transfer-Encoding: %V\" header",
+  >value);
+
+ngx_http_finalize_request(r, NGX_HTTP_NOT_IMPLEMENTED);
+return NGX_ERROR;
+}
+
+return NGX_OK;
+}
+
+
+static ngx_int_t
 ngx_http_process_user_agent(ngx_http_request_t *r, ngx_table_elt_t *h,
 ngx_uint_t offset)
 {
@@ -1881,25 +1926,9 @@ ngx_http_process_request_header(ngx_http
 return NGX_ERROR;
 }
 
-if (r->headers_in.transfer_encoding) {
-if (r->headers_in.transfer_encoding->value.len == 7
-&& ngx_strncasecmp(r->headers_in.transfer_encoding->value.data,
-   (u_char *) "chunked", 7) == 0)
-{
-r->headers_in.content_length = NULL;
-r->headers_in.content_length_n = -1;
-r->headers_in.chunked = 1;
-
-} else if (r->headers_in.transfer_encoding->value.len != 8
-|| ngx_strncasecmp(r->headers_in.transfer_encoding->value.data,
-   (u_char *) "identity", 8) != 0)
-{
-ngx_log_error(NGX_LOG_INFO, r->connection->log, 0,
-  "client sent unknown \"Transfer-Encoding\": \"%V\"",
-  >headers_in.transfer_encoding->value);
-ngx_http_finalize_request(r, NGX_HTTP_NOT_IMPLEMENTED);
-return NGX_ERROR;
-}
+if (r->headers_in.chunked) {
+r->headers_in.content_length = NULL;
+r->headers_in.content_length_n = -1;
 }
 
 if (r->headers_in.connection_type == NGX_HTTP_CONNECTION_KEEP_ALIVE) {
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

[PATCH 3 of 3] Headers filter: added "add_trailer" directive

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora <piotrsik...@google.com>
# Date 1490351854 25200
#  Fri Mar 24 03:37:34 2017 -0700
# Node ID 46150fb672e7b92cfbe678bad71df187fcb25ae6
# Parent  73f67e06ab103e0368d1810c6f8cac5c70c4e246
Headers filter: added "add_trailer" directive.

Trailers added using this directive are evaluated after response body
is processed by output filters (but before it's written to the wire),
so it's possible to use variables calculated from the response body
as the trailer value.

Signed-off-by: Piotr Sikora <piotrsik...@google.com>

diff -r 73f67e06ab10 -r 46150fb672e7 
src/http/modules/ngx_http_headers_filter_module.c
--- a/src/http/modules/ngx_http_headers_filter_module.c
+++ b/src/http/modules/ngx_http_headers_filter_module.c
@@ -48,6 +48,7 @@ typedef struct {
 time_t expires_time;
 ngx_http_complex_value_t  *expires_value;
 ngx_array_t   *headers;
+ngx_array_t   *trailers;
 } ngx_http_headers_conf_t;
 
 
@@ -105,7 +106,15 @@ static ngx_command_t  ngx_http_headers_f
 |NGX_CONF_TAKE23,
   ngx_http_headers_add,
   NGX_HTTP_LOC_CONF_OFFSET,
-  0,
+  offsetof(ngx_http_headers_conf_t, headers),
+  NULL },
+
+{ ngx_string("add_trailer"),
+  NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_HTTP_LIF_CONF
+|NGX_CONF_TAKE23,
+  ngx_http_headers_add,
+  NGX_HTTP_LOC_CONF_OFFSET,
+  offsetof(ngx_http_headers_conf_t, trailers),
   NULL },
 
   ngx_null_command
@@ -144,6 +153,7 @@ ngx_module_t  ngx_http_headers_filter_mo
 
 
 static ngx_http_output_header_filter_pt  ngx_http_next_header_filter;
+static ngx_http_output_body_filter_ptngx_http_next_body_filter;
 
 
 static ngx_int_t
@@ -154,10 +164,15 @@ ngx_http_headers_filter(ngx_http_request
 ngx_http_header_val_t*h;
 ngx_http_headers_conf_t  *conf;
 
+if (r != r->main) {
+return ngx_http_next_header_filter(r);
+}
+
 conf = ngx_http_get_module_loc_conf(r, ngx_http_headers_filter_module);
 
-if ((conf->expires == NGX_HTTP_EXPIRES_OFF && conf->headers == NULL)
-|| r != r->main)
+if (conf->expires == NGX_HTTP_EXPIRES_OFF
+&& conf->headers == NULL
+&& conf->trailers == NULL)
 {
 return ngx_http_next_header_filter(r);
 }
@@ -206,11 +221,101 @@ ngx_http_headers_filter(ngx_http_request
 }
 }
 
+if (conf->trailers) {
+h = conf->trailers->elts;
+for (i = 0; i < conf->trailers->nelts; i++) {
+
+if (!safe_status && !h[i].always) {
+continue;
+}
+
+r->expect_trailers = 1;
+break;
+}
+}
+
 return ngx_http_next_header_filter(r);
 }
 
 
 static ngx_int_t
+ngx_http_trailers_filter(ngx_http_request_t *r, ngx_chain_t *in)
+{
+ngx_str_t value;
+ngx_uint_ti, safe_status;
+ngx_chain_t  *cl;
+ngx_table_elt_t  *t;
+ngx_http_header_val_t*h;
+ngx_http_headers_conf_t  *conf;
+
+conf = ngx_http_get_module_loc_conf(r, ngx_http_headers_filter_module);
+
+if (in == NULL
+|| conf->trailers == NULL
+|| !r->expect_trailers
+|| r->header_only)
+{
+return ngx_http_next_body_filter(r, in);
+}
+
+for (cl = in; cl; cl = cl->next) {
+if (cl->buf->last_buf) {
+break;
+}
+}
+
+if (cl == NULL) {
+return ngx_http_next_body_filter(r, in);
+}
+
+switch (r->headers_out.status) {
+
+case NGX_HTTP_OK:
+case NGX_HTTP_CREATED:
+case NGX_HTTP_NO_CONTENT:
+case NGX_HTTP_PARTIAL_CONTENT:
+case NGX_HTTP_MOVED_PERMANENTLY:
+case NGX_HTTP_MOVED_TEMPORARILY:
+case NGX_HTTP_SEE_OTHER:
+case NGX_HTTP_NOT_MODIFIED:
+case NGX_HTTP_TEMPORARY_REDIRECT:
+case NGX_HTTP_PERMANENT_REDIRECT:
+safe_status = 1;
+break;
+
+default:
+safe_status = 0;
+break;
+}
+
+h = conf->trailers->elts;
+for (i = 0; i < conf->trailers->nelts; i++) {
+
+if (!safe_status && !h[i].always) {
+continue;
+}
+
+if (ngx_http_complex_value(r, [i].value, ) != NGX_OK) {
+return NGX_ERROR;
+}
+
+if (value.len) {
+t = ngx_list_push(>headers_out.trailers);
+if (t == NULL) {
+return NGX_ERROR;
+}
+
+t->key = h[i].key;
+t->value = value;
+t->hash = 1;
+}
+}
+
+return ngx_http_next_body_filter(r, in);
+}
+
+
+static ngx_int_t
 ngx_http_set_expires(ngx_http_request_t *r, ngx_http_headers_conf_t *conf)
 {
 char*err;
@@ -557,6 +662,7 @@ ngx_http_headers_create_conf(ngx_conf_

[PATCH 1 of 4] HTTP/2: reject HTTP/2 requests with "Connection" header

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora <piotrsik...@google.com>
# Date 1490516709 25200
#  Sun Mar 26 01:25:09 2017 -0700
# Node ID 10c3f4c37f96ef496eff859b6f6815817e79455a
# Parent  e6f399a176e7cae0fa08f1183d31315bce3b9ecb
HTTP/2: reject HTTP/2 requests with "Connection" header.

While there, populate r->headers_in.connection.

Signed-off-by: Piotr Sikora <piotrsik...@google.com>

diff -r e6f399a176e7 -r 10c3f4c37f96 src/http/ngx_http_request.c
--- a/src/http/ngx_http_request.c
+++ b/src/http/ngx_http_request.c
@@ -1678,6 +1678,22 @@ static ngx_int_t
 ngx_http_process_connection(ngx_http_request_t *r, ngx_table_elt_t *h,
 ngx_uint_t offset)
 {
+if (r->headers_in.connection == NULL) {
+r->headers_in.connection = h;
+}
+
+#if (NGX_HTTP_V2)
+
+if (r->stream) {
+ngx_log_error(NGX_LOG_INFO, r->connection->log, 0,
+  "client sent HTTP/2 request with \"Connection\" header");
+
+ngx_http_finalize_request(r, NGX_HTTP_BAD_REQUEST);
+return NGX_ERROR;
+}
+
+#endif
+
 if (ngx_strcasestrn(h->value.data, "close", 5 - 1)) {
 r->headers_in.connection_type = NGX_HTTP_CONNECTION_CLOSE;
 
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

[PATCH 2 of 3] HTTP/2: added support for trailers in HTTP responses

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora <piotrsik...@google.com>
# Date 1490351854 25200
#  Fri Mar 24 03:37:34 2017 -0700
# Node ID 73f67e06ab103e0368d1810c6f8cac5c70c4e246
# Parent  07a5d26b49f04425ff54cc998f885aa987b7823f
HTTP/2: added support for trailers in HTTP responses.

Signed-off-by: Piotr Sikora <piotrsik...@google.com>

diff -r 07a5d26b49f0 -r 73f67e06ab10 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
@@ -50,13 +50,17 @@
 #define NGX_HTTP_V2_SERVER_INDEX  54
 #define NGX_HTTP_V2_VARY_INDEX59
 
+#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_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(
+ngx_http_request_t *r);
 
 static ngx_chain_t *ngx_http_v2_send_chain(ngx_connection_t *fc,
 ngx_chain_t *in, off_t limit);
@@ -612,7 +616,7 @@ ngx_http_v2_header_filter(ngx_http_reque
   header[i].value.len, tmp);
 }
 
-frame = ngx_http_v2_create_headers_frame(r, start, pos);
+frame = ngx_http_v2_create_headers_frame(r, start, pos, r->header_only);
 if (frame == NULL) {
 return NGX_ERROR;
 }
@@ -636,6 +640,118 @@ ngx_http_v2_header_filter(ngx_http_reque
 }
 
 
+static ngx_http_v2_out_frame_t *
+ngx_http_v2_create_trailers_frame(ngx_http_request_t *r)
+{
+u_char   *pos, *start, *tmp;
+size_tlen, tmp_len;
+ngx_uint_ti;
+ngx_list_part_t  *part;
+ngx_table_elt_t  *header;
+
+len = 0;
+tmp_len = 0;
+
+part = >headers_out.trailers.part;
+header = part->elts;
+
+for (i = 0; /* void */; i++) {
+
+if (i >= part->nelts) {
+if (part->next == NULL) {
+break;
+}
+
+part = part->next;
+header = part->elts;
+i = 0;
+}
+
+if (header[i].hash == 0) {
+continue;
+}
+
+if (header[i].key.len > NGX_HTTP_V2_MAX_FIELD) {
+ngx_log_error(NGX_LOG_WARN, r->connection->log, 0,
+  "too long response trailer name: \"%V\"",
+  [i].key);
+return NULL;
+}
+
+if (header[i].value.len > NGX_HTTP_V2_MAX_FIELD) {
+ngx_log_error(NGX_LOG_WARN, r->connection->log, 0,
+  "too long response trailer value: \"%V: %V\"",
+  [i].key, [i].value);
+return NULL;
+}
+
+len += 1 + NGX_HTTP_V2_INT_OCTETS + header[i].key.len
+ + NGX_HTTP_V2_INT_OCTETS + header[i].value.len;
+
+if (header[i].key.len > tmp_len) {
+tmp_len = header[i].key.len;
+}
+
+if (header[i].value.len > tmp_len) {
+tmp_len = header[i].value.len;
+}
+}
+
+if (len == 0) {
+return NGX_HTTP_V2_NO_TRAILERS;
+}
+
+tmp = ngx_palloc(r->pool, tmp_len);
+pos = ngx_pnalloc(r->pool, len);
+
+if (pos == NULL || tmp == NULL) {
+return NULL;
+}
+
+start = pos;
+
+part = >headers_out.trailers.part;
+header = part->elts;
+
+for (i = 0; /* void */; i++) {
+
+if (i >= part->nelts) {
+if (part->next == NULL) {
+break;
+}
+
+part = part->next;
+header = part->elts;
+i = 0;
+}
+
+if (header[i].hash == 0) {
+continue;
+}
+
+#if (NGX_DEBUG)
+if (r->connection->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, r->connection->log, 0,
+   "http2 output trailer: \"%*s: %V\"",
+   header[i].key.len, tmp, [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);
+}
+
+return ngx_http_v2_create_headers_frame(r, start, pos, 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)
@@ -686,7 +802,7 @@ ngx_http_v2_write_int(u_char *p

[PATCH 2 of 4] HTTP/2: reject HTTP/2 requests with invalid "TE" header value

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora <piotrsik...@google.com>
# Date 1490516709 25200
#  Sun Mar 26 01:25:09 2017 -0700
# Node ID 349648a6f91f9bd5cc80d22390b95c2239a8bfb3
# Parent  10c3f4c37f96ef496eff859b6f6815817e79455a
HTTP/2: reject HTTP/2 requests with invalid "TE" header value.

Signed-off-by: Piotr Sikora <piotrsik...@google.com>

diff -r 10c3f4c37f96 -r 349648a6f91f src/http/ngx_http_request.c
--- a/src/http/ngx_http_request.c
+++ b/src/http/ngx_http_request.c
@@ -27,6 +27,8 @@ static ngx_int_t ngx_http_process_host(n
 ngx_table_elt_t *h, ngx_uint_t offset);
 static ngx_int_t ngx_http_process_connection(ngx_http_request_t *r,
 ngx_table_elt_t *h, ngx_uint_t offset);
+static ngx_int_t ngx_http_process_te(ngx_http_request_t *r,
+ngx_table_elt_t *h, ngx_uint_t offset);
 static ngx_int_t ngx_http_process_user_agent(ngx_http_request_t *r,
 ngx_table_elt_t *h, ngx_uint_t offset);
 
@@ -128,6 +130,10 @@ ngx_http_header_t  ngx_http_headers_in[]
  offsetof(ngx_http_headers_in_t, if_range),
  ngx_http_process_unique_header_line },
 
+{ ngx_string("TE"),
+ offsetof(ngx_http_headers_in_t, te),
+ ngx_http_process_te },
+
 { ngx_string("Transfer-Encoding"),
  offsetof(ngx_http_headers_in_t, transfer_encoding),
  ngx_http_process_header_line },
@@ -1706,6 +1712,37 @@ ngx_http_process_connection(ngx_http_req
 
 
 static ngx_int_t
+ngx_http_process_te(ngx_http_request_t *r, ngx_table_elt_t *h,
+ngx_uint_t offset)
+{
+if (r->headers_in.te == NULL) {
+r->headers_in.te = h;
+}
+
+if (h->value.len == sizeof("trailers") - 1
+&& ngx_memcmp(h->value.data, "trailers", sizeof("trailers") - 1) == 0)
+{
+return NGX_OK;
+}
+
+#if (NGX_HTTP_V2)
+
+if (r->stream) {
+ngx_log_error(NGX_LOG_INFO, r->connection->log, 0,
+  "client sent HTTP/2 request with invalid header value: "
+  "\"TE: %V\"", >value);
+
+ngx_http_finalize_request(r, NGX_HTTP_BAD_REQUEST);
+return NGX_ERROR;
+}
+
+#endif
+
+return NGX_OK;
+}
+
+
+static ngx_int_t
 ngx_http_process_user_agent(ngx_http_request_t *r, ngx_table_elt_t *h,
 ngx_uint_t offset)
 {
diff -r 10c3f4c37f96 -r 349648a6f91f src/http/ngx_http_request.h
--- a/src/http/ngx_http_request.h
+++ b/src/http/ngx_http_request.h
@@ -196,6 +196,7 @@ typedef struct {
 ngx_table_elt_t  *range;
 ngx_table_elt_t  *if_range;
 
+ngx_table_elt_t  *te;
 ngx_table_elt_t  *transfer_encoding;
 ngx_table_elt_t  *expect;
 ngx_table_elt_t  *upgrade;
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

[PATCH 1 of 3] Added support for trailers in HTTP responses

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora <piotrsik...@google.com>
# Date 1490351854 25200
#  Fri Mar 24 03:37:34 2017 -0700
# Node ID 07a5d26b49f04425ff54cc998f885aa987b7823f
# Parent  e6f399a176e7cae0fa08f1183d31315bce3b9ecb
Added support for trailers in HTTP responses.

Example:

   ngx_table_elt_t  *h;

   h = ngx_list_push(>headers_out.trailers);
   if (h == NULL) {
   return NGX_ERROR;
   }

   ngx_str_set(>key, "Fun");
   ngx_str_set(>value, "with trailers");
   h->hash = ngx_hash_key_lc(h->key.data, h->key.len);

The code above adds "Fun: with trailers" trailer to the response.

Modules that want to emit trailers must set r->expect_trailers = 1
in header filter, otherwise they might not be emitted for HTTP/1.1
responses that aren't already chunked.

This change also adds $sent_trailer_* variables.

Signed-off-by: Piotr Sikora <piotrsik...@google.com>

diff -r e6f399a176e7 -r 07a5d26b49f0 
src/http/modules/ngx_http_chunked_filter_module.c
--- a/src/http/modules/ngx_http_chunked_filter_module.c
+++ b/src/http/modules/ngx_http_chunked_filter_module.c
@@ -17,6 +17,8 @@ typedef struct {
 
 
 static ngx_int_t ngx_http_chunked_filter_init(ngx_conf_t *cf);
+static ngx_chain_t *ngx_http_chunked_create_trailers(ngx_http_request_t *r,
+ngx_http_chunked_filter_ctx_t *ctx);
 
 
 static ngx_http_module_t  ngx_http_chunked_filter_module_ctx = {
@@ -69,27 +71,29 @@ ngx_http_chunked_header_filter(ngx_http_
 return ngx_http_next_header_filter(r);
 }
 
-if (r->headers_out.content_length_n == -1) {
-if (r->http_version < NGX_HTTP_VERSION_11) {
+if (r->headers_out.content_length_n == -1
+|| r->expect_trailers)
+{
+clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module);
+
+if (r->http_version >= NGX_HTTP_VERSION_11
+&& clcf->chunked_transfer_encoding)
+{
+if (r->expect_trailers) {
+ngx_http_clear_content_length(r);
+}
+
+r->chunked = 1;
+
+ctx = ngx_pcalloc(r->pool, sizeof(ngx_http_chunked_filter_ctx_t));
+if (ctx == NULL) {
+return NGX_ERROR;
+}
+
+ngx_http_set_ctx(r, ctx, ngx_http_chunked_filter_module);
+
+} else if (r->headers_out.content_length_n == -1) {
 r->keepalive = 0;
-
-} else {
-clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module);
-
-if (clcf->chunked_transfer_encoding) {
-r->chunked = 1;
-
-ctx = ngx_pcalloc(r->pool,
-  sizeof(ngx_http_chunked_filter_ctx_t));
-if (ctx == NULL) {
-return NGX_ERROR;
-}
-
-ngx_http_set_ctx(r, ctx, ngx_http_chunked_filter_module);
-
-} else {
-r->keepalive = 0;
-}
 }
 }
 
@@ -179,26 +183,17 @@ ngx_http_chunked_body_filter(ngx_http_re
 }
 
 if (cl->buf->last_buf) {
-tl = ngx_chain_get_free_buf(r->pool, >free);
+tl = ngx_http_chunked_create_trailers(r, ctx);
 if (tl == NULL) {
 return NGX_ERROR;
 }
 
-b = tl->buf;
-
-b->tag = (ngx_buf_tag_t) _http_chunked_filter_module;
-b->temporary = 0;
-b->memory = 1;
-b->last_buf = 1;
-b->pos = (u_char *) CRLF "0" CRLF CRLF;
-b->last = b->pos + 7;
-
 cl->buf->last_buf = 0;
 
 *ll = tl;
 
 if (size == 0) {
-b->pos += 2;
+tl->buf->pos += 2;
 }
 
 } else if (size > 0) {
@@ -230,6 +225,109 @@ ngx_http_chunked_body_filter(ngx_http_re
 }
 
 
+static ngx_chain_t *
+ngx_http_chunked_create_trailers(ngx_http_request_t *r,
+ngx_http_chunked_filter_ctx_t *ctx)
+{
+size_tlen;
+ngx_buf_t*b;
+ngx_uint_ti;
+ngx_chain_t  *cl;
+ngx_list_part_t  *part;
+ngx_table_elt_t  *header;
+
+len = 0;
+
+part = >headers_out.trailers.part;
+header = part->elts;
+
+for (i = 0; /* void */; i++) {
+
+if (i >= part->nelts) {
+if (part->next == NULL) {
+break;
+}
+
+part = part->next;
+header = part->elts;
+i = 0;
+}
+
+if (header[i].hash == 0) {
+continue;
+}
+
+len += header[i].key.len + sizeof(": ") - 1
+   + header[i].value.len + sizeof(CRLF) - 1;
+}
+
+cl = ngx_chain_get_free_buf(r->pool, >free);
+if (cl == NULL) {
+return NULL;
+}
+
+b = cl->buf;
+
+b->tag = (ngx_buf_tag_t) _http_chunked_filter_module;
+b->temporary = 0;
+b->memory = 1;
+b->last_buf = 1;
+
+   

Re: [PATCH 3 of 3] Headers filter: added "add_trailer" directive

[Piotr Sikora via nginx-devel]

Hey Maxim,

> It doesn't look like "if (h[i].value.value.len)" is needed here.
> It is either true, or the "add_trailer" directive is nop and we
> already know this while parsing the configuration.
>
> -if (h[i].value.value.len) {
> -r->expect_trailers = 1;
> -break;
> -}
> +r->expect_trailers = 1;
> +break;

Well, both "add_header" and "add_trailer" allow setting something like:

add_trailer Empty "";

which will get added to headers / trailers list.

I've added this extra check to avoid forcing chunked encoding with
such configuration.

Maybe we should reject it during configuration instead, or ignore this
case and let it force chunked encoding? Which one do you prefer?

Best regards,
Piotr Sikora
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

Re: [PATCH 1 of 3] Added support for trailers in HTTP responses

[Piotr Sikora via nginx-devel]

Hey Maxim,

> I would prefer to preserve the typical code path (when there are no
> trailers) without an extra allocation.  It looks like it would be
> as trivail as:
>
> @@ -273,14 +273,18 @@ ngx_http_chunked_create_trailers(ngx_htt
>  b->memory = 1;
>  b->last_buf = 1;
>
> +if (len == sizeof(CRLF "0" CRLF CRLF) - 1) {
> +b->pos = (u_char *) CRLF "0" CRLF CRLF;
> +b->last = b->pos + sizeof(CRLF "0" CRLF CRLF) - 1;
> +return cl;
> +}

Sounds good, but the if statement reads a bit weird.

What about this instead, even though it might be a bit more expensive?

@@ -236,7 +236,7 @@ ngx_http_chunked_create_trailers(ngx_http_request_t *r,
 ngx_list_part_t  *part;
 ngx_table_elt_t  *header;

-len = sizeof(CRLF "0" CRLF CRLF) - 1;
+len = 0;

 part = >headers_out.trailers.part;
 header = part->elts;
@@ -273,12 +273,14 @@ ngx_http_chunked_create_trailers(ngx_http_request_t *r,
 b->memory = 1;
 b->last_buf = 1;

-if (len == sizeof(CRLF "0" CRLF CRLF) - 1) {
+if (len == 0) {
 b->pos = (u_char *) CRLF "0" CRLF CRLF;
 b->last = b->pos + sizeof(CRLF "0" CRLF CRLF) - 1;
 return cl;
 }

+len += sizeof(CRLF "0" CRLF CRLF) - 1;
+
 b->pos = ngx_palloc(r->pool, len);
 if (b->pos == NULL) {
 return NULL;

Best regards,
Piotr Sikora
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

[PATCH] Upstream: ignore read-readiness if request wasn't sent

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora <piotrsik...@google.com>
# Date 1491296505 25200
#  Tue Apr 04 02:01:45 2017 -0700
# Node ID bff5ac3da350d8d9225d4204d8aded90fb670f3f
# Parent  716852cce9136d977b81a2d1b8b6f9fbca0dce49
Upstream: ignore read-readiness if request wasn't sent.

Signed-off-by: Piotr Sikora <piotrsik...@google.com>

diff -r 716852cce913 -r bff5ac3da350 src/http/ngx_http_upstream.c
--- a/src/http/ngx_http_upstream.c
+++ b/src/http/ngx_http_upstream.c
@@ -2179,8 +2179,12 @@ ngx_http_upstream_process_header(ngx_htt
 return;
 }
 
-if (!u->request_sent && ngx_http_upstream_test_connect(c) != NGX_OK) {
-ngx_http_upstream_next(r, u, NGX_HTTP_UPSTREAM_FT_ERROR);
+if (!u->request_sent) {
+if (ngx_http_upstream_test_connect(c) != NGX_OK) {
+ngx_http_upstream_next(r, u, NGX_HTTP_UPSTREAM_FT_ERROR);
+return;
+}
+
 return;
 }
 
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

[PATCH] Proxy: add "proxy_ssl_alpn" directive

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora <piotrsik...@google.com>
# Date 1489621682 25200
#  Wed Mar 15 16:48:02 2017 -0700
# Node ID 7733d946e2651a2486a53d912703e2dfaea30421
# Parent  716852cce9136d977b81a2d1b8b6f9fbca0dce49
Proxy: add "proxy_ssl_alpn" directive.

ALPN is used here only to indicate which version of the HTTP protocol
is going to be used and we doesn't verify that upstream agreed to it.

Please note that upstream is allowed to reject SSL connection with a
fatal "no_application_protocol" alert if it doesn't support it.

Signed-off-by: Piotr Sikora <piotrsik...@google.com>

diff -r 716852cce913 -r 7733d946e265 src/event/ngx_event_openssl.c
--- a/src/event/ngx_event_openssl.c
+++ b/src/event/ngx_event_openssl.c
@@ -654,6 +654,29 @@ ngx_ssl_ciphers(ngx_conf_t *cf, ngx_ssl_
 
 
 ngx_int_t
+ngx_ssl_alpn_protos(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_str_t *protos)
+{
+#ifdef TLSEXT_TYPE_application_layer_protocol_negotiation
+
+if (SSL_CTX_set_alpn_protos(ssl->ctx, protos->data, protos->len) != 0) {
+ngx_ssl_error(NGX_LOG_EMERG, ssl->log, 0,
+  "SSL_CTX_set_alpn_protos() failed");
+return NGX_ERROR;
+}
+
+return NGX_OK;
+
+#else
+
+ngx_log_error(NGX_LOG_EMERG, cf->log, 0,
+  "nginx was built with OpenSSL that lacks ALPN support");
+return NGX_ERROR;
+
+#endif
+}
+
+
+ngx_int_t
 ngx_ssl_client_certificate(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_str_t *cert,
 ngx_int_t depth)
 {
diff -r 716852cce913 -r 7733d946e265 src/event/ngx_event_openssl.h
--- a/src/event/ngx_event_openssl.h
+++ b/src/event/ngx_event_openssl.h
@@ -153,6 +153,8 @@ ngx_int_t ngx_ssl_certificate(ngx_conf_t
 ngx_str_t *cert, ngx_str_t *key, ngx_array_t *passwords);
 ngx_int_t ngx_ssl_ciphers(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_str_t *ciphers,
 ngx_uint_t prefer_server_ciphers);
+ngx_int_t ngx_ssl_alpn_protos(ngx_conf_t *cf, ngx_ssl_t *ssl,
+ngx_str_t *protos);
 ngx_int_t ngx_ssl_client_certificate(ngx_conf_t *cf, ngx_ssl_t *ssl,
 ngx_str_t *cert, ngx_int_t depth);
 ngx_int_t ngx_ssl_trusted_certificate(ngx_conf_t *cf, ngx_ssl_t *ssl,
diff -r 716852cce913 -r 7733d946e265 src/http/modules/ngx_http_proxy_module.c
--- a/src/http/modules/ngx_http_proxy_module.c
+++ b/src/http/modules/ngx_http_proxy_module.c
@@ -652,6 +652,13 @@ static ngx_command_t  ngx_http_proxy_com
   offsetof(ngx_http_proxy_loc_conf_t, ssl_ciphers),
   NULL },
 
+{ ngx_string("proxy_ssl_alpn"),
+  NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_FLAG,
+  ngx_conf_set_flag_slot,
+  NGX_HTTP_LOC_CONF_OFFSET,
+  offsetof(ngx_http_proxy_loc_conf_t, upstream.ssl_alpn),
+  NULL },
+
 { ngx_string("proxy_ssl_name"),
   NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_TAKE1,
   ngx_http_set_complex_value_slot,
@@ -2882,6 +2889,7 @@ ngx_http_proxy_create_loc_conf(ngx_conf_
 conf->upstream.intercept_errors = NGX_CONF_UNSET;
 
 #if (NGX_HTTP_SSL)
+conf->upstream.ssl_alpn = NGX_CONF_UNSET;
 conf->upstream.ssl_session_reuse = NGX_CONF_UNSET;
 conf->upstream.ssl_server_name = NGX_CONF_UNSET;
 conf->upstream.ssl_verify = NGX_CONF_UNSET;
@@ -3212,6 +3220,8 @@ ngx_http_proxy_merge_loc_conf(ngx_conf_t
 conf->upstream.ssl_name = prev->upstream.ssl_name;
 }
 
+ngx_conf_merge_value(conf->upstream.ssl_alpn,
+  prev->upstream.ssl_alpn, 0);
 ngx_conf_merge_value(conf->upstream.ssl_server_name,
   prev->upstream.ssl_server_name, 0);
 ngx_conf_merge_value(conf->upstream.ssl_verify,
@@ -4320,6 +4330,7 @@ ngx_http_proxy_lowat_check(ngx_conf_t *c
 static ngx_int_t
 ngx_http_proxy_set_ssl(ngx_conf_t *cf, ngx_http_proxy_loc_conf_t *plcf)
 {
+ngx_str_talpn;
 ngx_pool_cleanup_t  *cln;
 
 plcf->upstream.ssl = ngx_pcalloc(cf->pool, sizeof(ngx_ssl_t));
@@ -4366,6 +4377,24 @@ ngx_http_proxy_set_ssl(ngx_conf_t *cf, n
 return NGX_ERROR;
 }
 
+if (plcf->upstream.ssl_alpn) {
+
+switch (plcf->http_version) {
+
+case NGX_HTTP_VERSION_10:
+ngx_str_set(, NGX_HTTP_10_ALPN_ADVERTISE);
+break;
+
+case NGX_HTTP_VERSION_11:
+ngx_str_set(, NGX_HTTP_11_ALPN_ADVERTISE);
+break;
+}
+
+if (ngx_ssl_alpn_protos(cf, plcf->upstream.ssl, ) != NGX_OK) {
+return NGX_ERROR;
+}
+}
+
 if (plcf->upstream.ssl_verify) {
 if (plcf->ssl_trusted_certificate.len == 0) {
 ngx_log_error(NGX_LOG_EMERG, cf->log, 0,
diff -r 716852cce913 -r 7733d946e265 src/http/modules/ngx_http_ssl_module.c
--- a/src/http/modules/ngx_http_ssl_module.c
+++ b/src/http/modules/ngx_http_ssl_module.c
@@ -17,8 +17,6 @@ typedef ngx_int_t (*ngx_ssl_variable_han
 #defi

[PATCH] Output chain: propagate flush and last_buf flags to send_chain()

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora <piotrsik...@google.com>
# Date 1491708381 25200
#  Sat Apr 08 20:26:21 2017 -0700
# Node ID 2a48b9b6e67d91594c1787ebf721daebf5f88c91
# Parent  716852cce9136d977b81a2d1b8b6f9fbca0dce49
Output chain: propagate flush and last_buf flags to send_chain().

Signed-off-by: Piotr Sikora <piotrsik...@google.com>

diff -r 716852cce913 -r 2a48b9b6e67d src/core/ngx_output_chain.c
--- a/src/core/ngx_output_chain.c
+++ b/src/core/ngx_output_chain.c
@@ -658,6 +658,7 @@ ngx_chain_writer(void *data, ngx_chain_t
 ngx_chain_writer_ctx_t *ctx = data;
 
 off_t  size;
+ngx_uint_t flush;
 ngx_chain_t   *cl, *ln, *chain;
 ngx_connection_t  *c;
 
@@ -689,9 +690,10 @@ ngx_chain_writer(void *data, ngx_chain_t
 
 size += ngx_buf_size(in->buf);
 
-ngx_log_debug2(NGX_LOG_DEBUG_CORE, c->log, 0,
-   "chain writer buf fl:%d s:%uO",
-   in->buf->flush, ngx_buf_size(in->buf));
+ngx_log_debug3(NGX_LOG_DEBUG_CORE, c->log, 0,
+   "chain writer buf fl:%d l:%d s:%uO",
+   in->buf->flush, in->buf->last_buf,
+   ngx_buf_size(in->buf));
 
 cl = ngx_alloc_chain_link(ctx->pool);
 if (cl == NULL) {
@@ -707,6 +709,8 @@ ngx_chain_writer(void *data, ngx_chain_t
 ngx_log_debug1(NGX_LOG_DEBUG_CORE, c->log, 0,
"chain writer in: %p", ctx->out);
 
+flush = 0;
+
 for (cl = ctx->out; cl; cl = cl->next) {
 
 #if 1
@@ -732,9 +736,13 @@ ngx_chain_writer(void *data, ngx_chain_t
 #endif
 
 size += ngx_buf_size(cl->buf);
+
+if (cl->buf->flush || cl->buf->last_buf) {
+flush = 1;
+}
 }
 
-if (size == 0 && !c->buffered) {
+if (size == 0 && !flush && !c->buffered) {
 return NGX_OK;
 }
 
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

[PATCH] Proxy: split configured header names and values

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora <piotrsik...@google.com>
# Date 1489618535 25200
#  Wed Mar 15 15:55:35 2017 -0700
# Node ID ff79d6887fc92d0344eac3e87339583265241e36
# Parent  716852cce9136d977b81a2d1b8b6f9fbca0dce49
Proxy: split configured header names and values.

Previously, each configured header was represented in one of two ways,
depending on whether or not its value included any variables.

If the value didn't include any variables, then it would be represented
as as a single script that contained complete header line with HTTP/1.1
delimiters, i.e.:

 "Header: value\r\n"

But if the value included any variables, then it would be represented
as a series of three scripts: first contained header name and the ":"
delimiter, second evaluated to header value, and third contained only
"\r\n", i.e.:

 "Header:"
 "$value"
 "\r\n"

This commit changes that, so that each configured header is represented
as a series of two scripts: first contains only header name, and second
contains (or evaluates to) only header value, i.e.:

"Header"
"$value"

or

"Header"
"value"

This not only makes things more consistent, but also allows header name
and value to be accessed separately.

Signed-off-by: Piotr Sikora <piotrsik...@google.com>

diff -r 716852cce913 -r ff79d6887fc9 src/http/modules/ngx_http_proxy_module.c
--- a/src/http/modules/ngx_http_proxy_module.c
+++ b/src/http/modules/ngx_http_proxy_module.c
@@ -1144,6 +1144,7 @@ static ngx_int_t
 ngx_http_proxy_create_request(ngx_http_request_t *r)
 {
 size_tlen, uri_len, loc_len, body_len;
+size_tkey_len, val_len;
 uintptr_t escape;
 ngx_buf_t*b;
 ngx_str_t method;
@@ -1258,10 +1259,17 @@ ngx_http_proxy_create_request(ngx_http_r
 le.flushed = 1;
 
 while (*(uintptr_t *) le.ip) {
-while (*(uintptr_t *) le.ip) {
+lcode = *(ngx_http_script_len_code_pt *) le.ip;
+key_len = lcode();
+
+for (val_len = 0; *(uintptr_t *) le.ip; val_len += lcode()) {
 lcode = *(ngx_http_script_len_code_pt *) le.ip;
-len += lcode();
 }
+
+if (val_len) {
+len += key_len + sizeof(": ") - 1 + val_len + sizeof(CRLF) - 1;
+}
+
 le.ip += sizeof(uintptr_t);
 }
 
@@ -1363,28 +1371,32 @@ ngx_http_proxy_create_request(ngx_http_r
 
 while (*(uintptr_t *) le.ip) {
 lcode = *(ngx_http_script_len_code_pt *) le.ip;
-
-/* skip the header line name length */
 (void) lcode();
 
-if (*(ngx_http_script_len_code_pt *) le.ip) {
-
-for (len = 0; *(uintptr_t *) le.ip; len += lcode()) {
-lcode = *(ngx_http_script_len_code_pt *) le.ip;
-}
-
-e.skip = (len == sizeof(CRLF) - 1) ? 1 : 0;
-
-} else {
-e.skip = 0;
+for (val_len = 0; *(uintptr_t *) le.ip; val_len += lcode()) {
+lcode = *(ngx_http_script_len_code_pt *) le.ip;
 }
 
 le.ip += sizeof(uintptr_t);
 
+e.skip = (val_len == 0) ? 1 : 0;
+
+code = *(ngx_http_script_code_pt *) e.ip;
+code((ngx_http_script_engine_t *) );
+
+if (!e.skip) {
+*e.pos++ = ':'; *e.pos++ = ' ';
+}
+
 while (*(uintptr_t *) e.ip) {
 code = *(ngx_http_script_code_pt *) e.ip;
 code((ngx_http_script_engine_t *) );
 }
+
+if (!e.skip) {
+*e.pos++ = CR; *e.pos++ = LF;
+}
+
 e.ip += sizeof(uintptr_t);
 }
 
@@ -3498,6 +3510,30 @@ ngx_http_proxy_init_headers(ngx_conf_t *
 continue;
 }
 
+copy = ngx_array_push_n(headers->lengths,
+sizeof(ngx_http_script_copy_code_t));
+if (copy == NULL) {
+return NGX_ERROR;
+}
+
+copy->code = (ngx_http_script_code_pt) ngx_http_script_copy_len_code;
+copy->len = src[i].key.len;
+
+size = (sizeof(ngx_http_script_copy_code_t)
++ src[i].key.len + sizeof(uintptr_t) - 1)
+& ~(sizeof(uintptr_t) - 1);
+
+copy = ngx_array_push_n(headers->values, size);
+if (copy == NULL) {
+return NGX_ERROR;
+}
+
+copy->code = ngx_http_script_copy_code;
+copy->len = src[i].key.len;
+
+p = (u_char *) copy + sizeof(ngx_http_script_copy_code_t);
+ngx_memcpy(p, src[i].key.data, src[i].key.len);
+
 if (ngx_http_script_variables_count([i].value) == 0) {
 copy = ngx_array_push_n(headers->lengths,
 sizeof(ngx_http_script_copy_code_t));
@@ -3507,14 +3543,10 @@ ngx_http_proxy_init_headers(ngx_conf_t *
 
 copy->code = (ngx_http_scrip

[PATCH] Proxy: always emit "Host" header first

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora <piotrsik...@google.com>
# Date 1489618489 25200
#  Wed Mar 15 15:54:49 2017 -0700
# Node ID e472b23fdc387943ea90fb2f0ae415d9d104edc7
# Parent  716852cce9136d977b81a2d1b8b6f9fbca0dce49
Proxy: always emit "Host" header first.

Signed-off-by: Piotr Sikora <piotrsik...@google.com>

diff -r 716852cce913 -r e472b23fdc38 src/http/modules/ngx_http_proxy_module.c
--- a/src/http/modules/ngx_http_proxy_module.c
+++ b/src/http/modules/ngx_http_proxy_module.c
@@ -3412,7 +3412,7 @@ ngx_http_proxy_init_headers(ngx_conf_t *
 uintptr_t*code;
 ngx_uint_ti;
 ngx_array_t   headers_names, headers_merged;
-ngx_keyval_t *src, *s, *h;
+ngx_keyval_t *host, *src, *s, *h;
 ngx_hash_key_t   *hk;
 ngx_hash_init_t   hash;
 ngx_http_script_compile_t sc;
@@ -3444,11 +3444,33 @@ ngx_http_proxy_init_headers(ngx_conf_t *
 return NGX_ERROR;
 }
 
+h = default_headers;
+
+if (h->key.len != sizeof("Host") - 1
+|| ngx_strcasecmp(h->key.data, (u_char *) "Host") != 0)
+{
+return NGX_ERROR;
+}
+
+host = ngx_array_push(_merged);
+if (host == NULL) {
+return NGX_ERROR;
+}
+
+*host = *h++;
+
 if (conf->headers_source) {
 
 src = conf->headers_source->elts;
 for (i = 0; i < conf->headers_source->nelts; i++) {
 
+if (src[i].key.len == sizeof("Host") - 1
+&& ngx_strcasecmp(src[i].key.data, (u_char *) "Host") == 0)
+{
+*host = src[i];
+continue;
+}
+
 s = ngx_array_push(_merged);
 if (s == NULL) {
 return NGX_ERROR;
@@ -3458,8 +3480,6 @@ ngx_http_proxy_init_headers(ngx_conf_t *
 }
 }
 
-h = default_headers;
-
 while (h->key.len) {
 
 src = headers_merged.elts;
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

[PATCH 3 of 3] Headers filter: added "add_trailer" directive

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora <piotrsik...@google.com>
# Date 1490351854 25200
#  Fri Mar 24 03:37:34 2017 -0700
# Node ID acdc80c0d4ef8aa2519e2882ff1a3bd4a316ad81
# Parent  8d74ff6c2015180f5c1f399f492214d7d0a52b3f
Headers filter: added "add_trailer" directive.

Trailers added using this directive are evaluated after response body
is processed by output filters (but before it's written to the wire),
so it's possible to use variables calculated from the response body
as the trailer value.

Signed-off-by: Piotr Sikora <piotrsik...@google.com>

diff -r 8d74ff6c2015 -r acdc80c0d4ef 
src/http/modules/ngx_http_headers_filter_module.c
--- a/src/http/modules/ngx_http_headers_filter_module.c
+++ b/src/http/modules/ngx_http_headers_filter_module.c
@@ -48,6 +48,7 @@ typedef struct {
 time_t expires_time;
 ngx_http_complex_value_t  *expires_value;
 ngx_array_t   *headers;
+ngx_array_t   *trailers;
 } ngx_http_headers_conf_t;
 
 
@@ -72,6 +73,8 @@ static char *ngx_http_headers_expires(ng
 void *conf);
 static char *ngx_http_headers_add(ngx_conf_t *cf, ngx_command_t *cmd,
 void *conf);
+static char *ngx_http_headers_add_trailer(ngx_conf_t *cf, ngx_command_t *cmd,
+void *conf);
 
 
 static ngx_http_set_header_t  ngx_http_set_headers[] = {
@@ -108,6 +111,14 @@ static ngx_command_t  ngx_http_headers_f
   0,
   NULL },
 
+{ ngx_string("add_trailer"),
+  NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_HTTP_LIF_CONF
+|NGX_CONF_TAKE23,
+  ngx_http_headers_add_trailer,
+  NGX_HTTP_LOC_CONF_OFFSET,
+  0,
+  NULL },
+
   ngx_null_command
 };
 
@@ -144,6 +155,7 @@ ngx_module_t  ngx_http_headers_filter_mo
 
 
 static ngx_http_output_header_filter_pt  ngx_http_next_header_filter;
+static ngx_http_output_body_filter_ptngx_http_next_body_filter;
 
 
 static ngx_int_t
@@ -154,10 +166,15 @@ ngx_http_headers_filter(ngx_http_request
 ngx_http_header_val_t*h;
 ngx_http_headers_conf_t  *conf;
 
+if (r != r->main) {
+return ngx_http_next_header_filter(r);
+}
+
 conf = ngx_http_get_module_loc_conf(r, ngx_http_headers_filter_module);
 
-if ((conf->expires == NGX_HTTP_EXPIRES_OFF && conf->headers == NULL)
-|| r != r->main)
+if (conf->expires == NGX_HTTP_EXPIRES_OFF
+&& conf->headers == NULL
+&& conf->trailers == NULL)
 {
 return ngx_http_next_header_filter(r);
 }
@@ -206,11 +223,103 @@ ngx_http_headers_filter(ngx_http_request
 }
 }
 
+if (conf->trailers) {
+h = conf->trailers->elts;
+for (i = 0; i < conf->trailers->nelts; i++) {
+
+if (!safe_status && !h[i].always) {
+continue;
+}
+
+if (h[i].value.value.len) {
+r->expect_trailers = 1;
+break;
+}
+}
+}
+
 return ngx_http_next_header_filter(r);
 }
 
 
 static ngx_int_t
+ngx_http_trailers_filter(ngx_http_request_t *r, ngx_chain_t *in)
+{
+ngx_str_t value;
+ngx_uint_ti, safe_status;
+ngx_chain_t  *cl;
+ngx_table_elt_t  *t;
+ngx_http_header_val_t*h;
+ngx_http_headers_conf_t  *conf;
+
+conf = ngx_http_get_module_loc_conf(r, ngx_http_headers_filter_module);
+
+if (in == NULL
+|| conf->trailers == NULL
+|| !r->expect_trailers
+|| r->header_only)
+{
+return ngx_http_next_body_filter(r, in);
+}
+
+for (cl = in; cl; cl = cl->next) {
+if (cl->buf->last_buf) {
+break;
+}
+}
+
+if (cl == NULL) {
+return ngx_http_next_body_filter(r, in);
+}
+
+switch (r->headers_out.status) {
+
+case NGX_HTTP_OK:
+case NGX_HTTP_CREATED:
+case NGX_HTTP_NO_CONTENT:
+case NGX_HTTP_PARTIAL_CONTENT:
+case NGX_HTTP_MOVED_PERMANENTLY:
+case NGX_HTTP_MOVED_TEMPORARILY:
+case NGX_HTTP_SEE_OTHER:
+case NGX_HTTP_NOT_MODIFIED:
+case NGX_HTTP_TEMPORARY_REDIRECT:
+case NGX_HTTP_PERMANENT_REDIRECT:
+safe_status = 1;
+break;
+
+default:
+safe_status = 0;
+break;
+}
+
+h = conf->trailers->elts;
+for (i = 0; i < conf->trailers->nelts; i++) {
+
+if (!safe_status && !h[i].always) {
+continue;
+}
+
+if (ngx_http_complex_value(r, [i].value, ) != NGX_OK) {
+return NGX_ERROR;
+}
+
+if (value.len) {
+t = ngx_list_push(>headers_out.trailers);
+if (t == NULL) {
+return NGX_ERROR;
+}
+
+t->key = h[i].key;
+t->value = value;
+t->hash = 1;
+}
+}
+
+return ngx_http_next_body_filt

[PATCH 2 of 3] HTTP/2: added support for trailers in HTTP responses

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora <piotrsik...@google.com>
# Date 1493191954 25200
#  Wed Apr 26 00:32:34 2017 -0700
# Node ID 8d74ff6c2015180f5c1f399f492214d7d0a52b3f
# Parent  41c09a2fd90410e25ad8515793bd48028001c954
HTTP/2: added support for trailers in HTTP responses.

Signed-off-by: Piotr Sikora <piotrsik...@google.com>

diff -r 41c09a2fd904 -r 8d74ff6c2015 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
@@ -50,13 +50,17 @@
 #define NGX_HTTP_V2_SERVER_INDEX  54
 #define NGX_HTTP_V2_VARY_INDEX59
 
+#define NGX_HTTP_V2_FRAME_ERROR   (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_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(
+ngx_http_request_t *r);
 
 static ngx_chain_t *ngx_http_v2_send_chain(ngx_connection_t *fc,
 ngx_chain_t *in, off_t limit);
@@ -612,7 +616,7 @@ ngx_http_v2_header_filter(ngx_http_reque
   header[i].value.len, tmp);
 }
 
-frame = ngx_http_v2_create_headers_frame(r, start, pos);
+frame = ngx_http_v2_create_headers_frame(r, start, pos, r->header_only);
 if (frame == NULL) {
 return NGX_ERROR;
 }
@@ -636,6 +640,126 @@ ngx_http_v2_header_filter(ngx_http_reque
 }
 
 
+static ngx_http_v2_out_frame_t *
+ngx_http_v2_create_trailers_frame(ngx_http_request_t *r)
+{
+u_char   *pos, *start, *tmp;
+size_tlen, tmp_len;
+ngx_uint_ti;
+ngx_list_part_t  *part;
+ngx_table_elt_t  *header;
+ngx_http_v2_out_frame_t  *frame;
+
+len = 0;
+tmp_len = 0;
+
+part = >headers_out.trailers.part;
+header = part->elts;
+
+for (i = 0; /* void */; i++) {
+
+if (i >= part->nelts) {
+if (part->next == NULL) {
+break;
+}
+
+part = part->next;
+header = part->elts;
+i = 0;
+}
+
+if (header[i].hash == 0) {
+continue;
+}
+
+if (header[i].key.len > NGX_HTTP_V2_MAX_FIELD) {
+ngx_log_error(NGX_LOG_WARN, r->connection->log, 0,
+  "too long response trailer name: \"%V\"",
+  [i].key);
+
+return NGX_HTTP_V2_FRAME_ERROR;
+}
+
+if (header[i].value.len > NGX_HTTP_V2_MAX_FIELD) {
+ngx_log_error(NGX_LOG_WARN, r->connection->log, 0,
+  "too long response trailer value: \"%V: %V\"",
+  [i].key, [i].value);
+
+return NGX_HTTP_V2_FRAME_ERROR;
+}
+
+len += 1 + NGX_HTTP_V2_INT_OCTETS + header[i].key.len
+ + NGX_HTTP_V2_INT_OCTETS + header[i].value.len;
+
+if (header[i].key.len > tmp_len) {
+tmp_len = header[i].key.len;
+}
+
+if (header[i].value.len > tmp_len) {
+tmp_len = header[i].value.len;
+}
+}
+
+if (len == 0) {
+return NULL;
+}
+
+tmp = ngx_palloc(r->pool, tmp_len);
+pos = ngx_pnalloc(r->pool, len);
+
+if (pos == NULL || tmp == NULL) {
+return NGX_HTTP_V2_FRAME_ERROR;
+}
+
+start = pos;
+
+part = >headers_out.trailers.part;
+header = part->elts;
+
+for (i = 0; /* void */; i++) {
+
+if (i >= part->nelts) {
+if (part->next == NULL) {
+break;
+}
+
+part = part->next;
+header = part->elts;
+i = 0;
+}
+
+if (header[i].hash == 0) {
+continue;
+}
+
+#if (NGX_DEBUG)
+if (r->connection->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, r->connection->log, 0,
+   "http2 output trailer: \"%*s: %V\"",
+   header[i].key.len, tmp, [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, 1);
+if (frame == NULL) {
+return NGX_HTTP_V

Re: [PATCH 1 of 3] Added support for trailers in HTTP responses

[Piotr Sikora via nginx-devel]

Hey Maxim,

> Note: the "TE: trailers" requirement is no longer present in the
> code.

Good catch, thanks!

> This code results in using chunked encoding for HTTP/1.0 when
> trailers are expected.  Such behaviour is explicitly forbidden by
> the HTTP/1.1 specification, and will very likely result in
> problems (we've seen lots of such problems with broken backends
> when there were no HTTP/1.1 support in the proxy module).

Oops, this regression is a result of removal of r->accept_trailers,
which previously disallowed trailers in HTTP/1.0 requests.

> Something like this should be a better solution:
>
> if (r->headers_out.content_length_n == -1
> || r->expect_trailers)
> {
> clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module);
>
> if (r->http_version >= NGX_HTTP_VERSION_11
> && clcf->chunked_transfer_encoding)
> {
> if (r->expect_trailers) {
> ngx_http_clear_content_length(r);
> }
>
> r->chunked = 1;
>
> ctx = ngx_pcalloc(r->pool,
>   sizeof(ngx_http_chunked_filter_ctx_t));
> if (ctx == NULL) {
> return NGX_ERROR;
> }
>
> ngx_http_set_ctx(r, ctx, ngx_http_chunked_filter_module);
>
> } else if (r->headers_out.content_length_n == -1) {
> r->keepalive = 0;
> }
> }

Applied with small style changes.

> Instead of providing two separate code paths for "with trailer
> headers" and "without trailer headers", it might be better and
> more readable to generate last-chunk in one function regardless of
> whether trailer headers are present or not.
>
> It will also make error handling better: as of now, an allocation
> error in ngx_http_chunked_create_trailers() will result in "no
> trailers" code path being tried instead of returning an
> unconditional error.

Done.

> There is no need to write sizeof() so many times, just
>
> len += sizeof(CRLF "0" CRLF CRLF) - 1;
>
> would be enough.

Done.

Best regards,
Piotr Sikora
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

[PATCH 1 of 3] Added support for trailers in HTTP responses

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora <piotrsik...@google.com>
# Date 1490351854 25200
#  Fri Mar 24 03:37:34 2017 -0700
# Node ID 41c09a2fd90410e25ad8515793bd48028001c954
# Parent  716852cce9136d977b81a2d1b8b6f9fbca0dce49
Added support for trailers in HTTP responses.

Example:

   ngx_table_elt_t  *h;

   h = ngx_list_push(>headers_out.trailers);
   if (h == NULL) {
   return NGX_ERROR;
   }

   ngx_str_set(>key, "Fun");
   ngx_str_set(>value, "with trailers");
   h->hash = ngx_hash_key_lc(h->key.data, h->key.len);

The code above adds "Fun: with trailers" trailer to the response.

Modules that want to emit trailers must set r->expect_trailers = 1
in header filter, otherwise they might not be emitted for HTTP/1.1
responses that aren't already chunked.

This change also adds $sent_trailer_* variables.

Signed-off-by: Piotr Sikora <piotrsik...@google.com>

diff -r 716852cce913 -r 41c09a2fd904 
src/http/modules/ngx_http_chunked_filter_module.c
--- a/src/http/modules/ngx_http_chunked_filter_module.c
+++ b/src/http/modules/ngx_http_chunked_filter_module.c
@@ -17,6 +17,7 @@ typedef struct {
 
 
 static ngx_int_t ngx_http_chunked_filter_init(ngx_conf_t *cf);
+static ngx_chain_t *ngx_http_chunked_create_trailers(ngx_http_request_t *r);
 
 
 static ngx_http_module_t  ngx_http_chunked_filter_module_ctx = {
@@ -69,27 +70,28 @@ ngx_http_chunked_header_filter(ngx_http_
 return ngx_http_next_header_filter(r);
 }
 
-if (r->headers_out.content_length_n == -1) {
-if (r->http_version < NGX_HTTP_VERSION_11) {
+if (r->headers_out.content_length_n == -1 || r->expect_trailers) {
+
+clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module);
+
+if (r->http_version >= NGX_HTTP_VERSION_11
+&& clcf->chunked_transfer_encoding)
+{
+if (r->expect_trailers) {
+ngx_http_clear_content_length(r);
+}
+
+r->chunked = 1;
+
+ctx = ngx_pcalloc(r->pool, sizeof(ngx_http_chunked_filter_ctx_t));
+if (ctx == NULL) {
+return NGX_ERROR;
+}
+
+ngx_http_set_ctx(r, ctx, ngx_http_chunked_filter_module);
+
+} else if (r->headers_out.content_length_n == -1) {
 r->keepalive = 0;
-
-} else {
-clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module);
-
-if (clcf->chunked_transfer_encoding) {
-r->chunked = 1;
-
-ctx = ngx_pcalloc(r->pool,
-  sizeof(ngx_http_chunked_filter_ctx_t));
-if (ctx == NULL) {
-return NGX_ERROR;
-}
-
-ngx_http_set_ctx(r, ctx, ngx_http_chunked_filter_module);
-
-} else {
-r->keepalive = 0;
-}
 }
 }
 
@@ -179,26 +181,17 @@ ngx_http_chunked_body_filter(ngx_http_re
 }
 
 if (cl->buf->last_buf) {
-tl = ngx_chain_get_free_buf(r->pool, >free);
+tl = ngx_http_chunked_create_trailers(r);
 if (tl == NULL) {
 return NGX_ERROR;
 }
 
-b = tl->buf;
-
-b->tag = (ngx_buf_tag_t) _http_chunked_filter_module;
-b->temporary = 0;
-b->memory = 1;
-b->last_buf = 1;
-b->pos = (u_char *) CRLF "0" CRLF CRLF;
-b->last = b->pos + 7;
-
 cl->buf->last_buf = 0;
 
 *ll = tl;
 
 if (size == 0) {
-b->pos += 2;
+tl->buf->pos += 2;
 }
 
 } else if (size > 0) {
@@ -230,6 +223,105 @@ ngx_http_chunked_body_filter(ngx_http_re
 }
 
 
+static ngx_chain_t *
+ngx_http_chunked_create_trailers(ngx_http_request_t *r)
+{
+size_t  len;
+ngx_buf_t  *b;
+ngx_uint_t  i;
+ngx_chain_t*cl;
+ngx_list_part_t*part;
+ngx_table_elt_t*header;
+ngx_http_chunked_filter_ctx_t  *ctx;
+
+len = sizeof(CRLF "0" CRLF CRLF) - 1;
+
+part = >headers_out.trailers.part;
+header = part->elts;
+
+for (i = 0; /* void */; i++) {
+
+if (i >= part->nelts) {
+if (part->next == NULL) {
+break;
+}
+
+part = part->next;
+header = part->elts;
+i = 0;
+}
+
+if (header[i].hash == 0) {
+continue;
+}
+
+len += header[i].key.len + sizeof(": ") - 1
+   + header[i].value.len + sizeof(CRLF) - 1;
+}
+
+ctx = ngx_http_get_module_ctx(r, ngx_http_chunked_filter_module);
+
+cl = ngx_chain_get_free_buf(r->pool, >free);
+if (cl == NULL) {
+return NULL;
+}
+
+b = cl->buf;
+
+b->ta

Re: [PATCH 1 of 3] HTTP: add support for trailers in HTTP responses

[Piotr Sikora via nginx-devel]

Hey Maxim,

> I see two problems here:
>
> a. There may be use cases when forcing chunked encoding is not
> desired, but emitting trailers if it is used still makes sense.

Like what, exactly?

Also, gzip module forces chunked encoding and it works just fine. I
don't see why are you making this such a big deal out of this.

> b. Nothing stops modules from changing r->expect_trailers when the
> response header was already sent and it is already too late to
> switch to chunked transfer encoding.  Moreover, this will
> naturally happen with any module which is simply following the
> requirement to set r->expect_trailers to 1 as in your commit log.

Same is true for majority of ngx_http_request_t fields, i.e. bad
things can happen if some module misuses them.

> So (a) makes (2) excessively limiting, and (b) makes it useless.

I disagree. Removing this check results in less consistent behavior
and doesn't solve any real problems.

Having said that, I'm going to remove it, since I don't want to spend
another few months arguing about this...

Best regards,
Piotr Sikora
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

Re: [PATCH 3 of 3] Upstream: add support for trailers in HTTP responses

[Piotr Sikora via nginx-devel]

Hey Maxim,

> Overral, this patch looks at most half-ready, as it doesn't even
> try to implement sending trailers (r->expect_trailers is never
> set), lacks any support for trailers in the cache, and so on.

Actually, it's pretty much ready... r->expect_trailers is supposed to
be set by upstream modules (i.e. proxy or 3rd-party upstream module),
and not by the upstream module itself.

But I can see how it can be hard to review only part of the feature,
so I'm going to drop this for now (mostly to speed up the turnaround
of code reviews), and I'll send this later, along HTTP/2 patchset that
uses those changes.

Best regards,
Piotr Sikora
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

Re: [PATCH 2 of 3] Headers filter: add "add_trailer" directive

[Piotr Sikora via nginx-devel]

Hey Maxim,

> This introduces a layering violation between the headers filter
> and the chunked filter.

I moved trailer generation to headers filter, let me know if that works for you.

Unfortunately, because of that change, trailers are evaluated earlier
in the process, and some variables from body filters that run after
headers filter (like $gzip_ratio) are not usable anymore.

> There should be a space between "NULL" and "}".  It might worth
> fixing the style in previously exiting directives in a separate
> commit.

Done.

> Predicting conditions which will result in header-only response
> looks really fragile.  As well as checks for chunked transfer
> encoding.

Sending "Trailer" header in responses that cannot have response body
is known to result in interoperability issues, so I'd rather leave it,
see:
https://github.com/nodejs/node/issues/2842

> Note: this can result in an access to uninitialized memory if
> ngx_pnalloc() fails and $sent_http_trailer used in logs.

Fixed.

> Note that at this point we do not know if the trailer in question
> is going to be present in the particular response or not, as
> trailers with empty values are not returned (and this is often
> used to only return some headers in some responses, at least with
> add_header).

But we know that it might be present.

"Trailer" header is an indicator.

> It might be a better idea to avoid generating the
> "Trailer" header automatically and let users to control this with
> add_header instead, and/or introduce an option to control this.

That sounds like a terrible idea, which will result in always missing
"Trailer" header.

> Note well that with the approach taken it will be very
> inconvenient for other modules to add their own trailers, due to
> both a) layering violation introduced, which makes headers filter
> special, and b) no easy way to add anything to the Trailer header
> generated.

This is well-formatted header, so other modules can emit their own
"Trailer" headers, or append to it.

Also, I'm don't see how this is different from virtually any other
header in NGINX.

> There is no need to set hash for output headers, just t->hash = 1
> is good enough.

Actually, it's not "good enough". Using "hash = 1" has absolutely no
performance benefit and it results in full search instead of simple
hash comparison during header lookup, because some headers won't have
hash set to proper value.

Anyway, I'm going set it to "1" to avoid side-tracking this discussion.

Best regards,
Piotr Sikora
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

Re: [PATCH 1 of 3] HTTP: add support for trailers in HTTP responses

[Piotr Sikora via nginx-devel]

Hey Maxim,

> In your patch, you test r->expect_trailers in two places in
> chunked filter:
>
> 1. when you decide whether to use chunked encoding or not, in
>ngx_http_chunked_header_filter();
>
> 2. when you generate trailer, in ngx_http_chunked_body_filter().
>
> I mostly agree with (1) (I would like see it configurable too, but
> it's probably up to a module which adds trailers to decide, so
> don't belong to this particular patch), but don't see any reasons
> for (2).

This is done to avoid unexpected behavior when r->expect_trailers
isn't set, i.e. if module added trailers, but didn't set
r->expect_trailers, then without (2) trailers would be emitted only if
Content-Encoding was already chunked, but not otherwise.

Testing r->expect_trailers in both (1) and (2) ensures that Trailers
are emitted in consistent manner, without depending on unrelated
factors, like gzip support by the client, etc.

> Certainly merging is a bad idea either.  May be the solution would
> be avoid sending trailers for header-only requests consistently
> for both HTTP/1.1 and HTTP/2.

Done, I removed trailers from headers-only HTTP/2 responses.

As mentioned earlier, I also removed "TE: trailers" requirement and
merged whole trailer-part into one buffer, in case of trailers.

Best regards,
Piotr Sikora
___
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel

[PATCH 3 of 3] Headers filter: added "add_trailer" directive

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora <piotrsik...@google.com>
# Date 1490351854 25200
#  Fri Mar 24 03:37:34 2017 -0700
# Node ID 52201fff87e8c5a096287cf206cdcc14c0d81ce7
# Parent  e84aa49c5bc7a3250d4844b581e4bf3ed42db5f5
Headers filter: added "add_trailer" directive.

Trailers added using this directive are evaluated after response body
is processed by output filters (but before it's written to the wire),
so it's possible to use variables calculated from the response body
as the trailer value.

Signed-off-by: Piotr Sikora <piotrsik...@google.com>

diff -r e84aa49c5bc7 -r 52201fff87e8 
src/http/modules/ngx_http_headers_filter_module.c
--- a/src/http/modules/ngx_http_headers_filter_module.c
+++ b/src/http/modules/ngx_http_headers_filter_module.c
@@ -48,6 +48,7 @@ typedef struct {
 time_t expires_time;
 ngx_http_complex_value_t  *expires_value;
 ngx_array_t   *headers;
+ngx_array_t   *trailers;
 } ngx_http_headers_conf_t;
 
 
@@ -72,6 +73,8 @@ static char *ngx_http_headers_expires(ng
 void *conf);
 static char *ngx_http_headers_add(ngx_conf_t *cf, ngx_command_t *cmd,
 void *conf);
+static char *ngx_http_headers_add_trailer(ngx_conf_t *cf, ngx_command_t *cmd,
+void *conf);
 
 
 static ngx_http_set_header_t  ngx_http_set_headers[] = {
@@ -108,6 +111,14 @@ static ngx_command_t  ngx_http_headers_f
   0,
   NULL },
 
+{ ngx_string("add_trailer"),
+  NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_HTTP_LIF_CONF
+|NGX_CONF_TAKE23,
+  ngx_http_headers_add_trailer,
+  NGX_HTTP_LOC_CONF_OFFSET,
+  0,
+  NULL },
+
   ngx_null_command
 };
 
@@ -144,20 +155,30 @@ ngx_module_t  ngx_http_headers_filter_mo
 
 
 static ngx_http_output_header_filter_pt  ngx_http_next_header_filter;
+static ngx_http_output_body_filter_ptngx_http_next_body_filter;
 
 
 static ngx_int_t
 ngx_http_headers_filter(ngx_http_request_t *r)
 {
-ngx_str_t value;
-ngx_uint_ti, safe_status;
-ngx_http_header_val_t*h;
-ngx_http_headers_conf_t  *conf;
+u_char*p, *data;
+size_t len;
+ngx_str_t  value;
+ngx_uint_t i, safe_status;
+ngx_table_elt_t   *t;
+ngx_http_header_val_t *h;
+ngx_http_headers_conf_t   *conf;
+ngx_http_core_loc_conf_t  *clcf;
+
+if (r != r->main) {
+return ngx_http_next_header_filter(r);
+}
 
 conf = ngx_http_get_module_loc_conf(r, ngx_http_headers_filter_module);
 
-if ((conf->expires == NGX_HTTP_EXPIRES_OFF && conf->headers == NULL)
-|| r != r->main)
+if (conf->expires == NGX_HTTP_EXPIRES_OFF
+&& conf->headers == NULL
+&& conf->trailers == NULL)
 {
 return ngx_http_next_header_filter(r);
 }
@@ -206,11 +227,162 @@ ngx_http_headers_filter(ngx_http_request
 }
 }
 
+if (conf->trailers) {
+
+if (r->header_only
+|| r->headers_out.status == NGX_HTTP_NOT_MODIFIED
+|| r->headers_out.status == NGX_HTTP_NO_CONTENT
+|| r->headers_out.status < NGX_HTTP_OK
+|| r->method == NGX_HTTP_HEAD)
+{
+   return ngx_http_next_header_filter(r);
+}
+
+if (r->http_version < NGX_HTTP_VERSION_20) {
+clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module);
+
+if (!clcf->chunked_transfer_encoding) {
+return ngx_http_next_header_filter(r);
+}
+}
+
+len = 0;
+
+h = conf->trailers->elts;
+for (i = 0; i < conf->trailers->nelts; i++) {
+
+if (!safe_status && !h[i].always) {
+continue;
+}
+
+if (h[i].value.value.len) {
+len += h[i].key.len + sizeof(", ") - 1;
+}
+}
+
+if (len == 0) {
+return ngx_http_next_header_filter(r);
+}
+
+len -= sizeof(", ") - 1;
+
+data = ngx_pnalloc(r->pool, len);
+if (data == NULL) {
+return NGX_ERROR;
+}
+
+t = ngx_list_push(>headers_out.headers);
+if (t == NULL) {
+return NGX_ERROR;
+}
+
+p = data;
+
+h = conf->trailers->elts;
+for (i = 0; i < conf->trailers->nelts; i++) {
+
+if (!safe_status && !h[i].always) {
+continue;
+}
+
+if (h[i].value.value.len) {
+p = ngx_copy(p, h[i].key.data, h[i].key.len);
+
+if (p == data + len) {
+break;
+}
+
+*p++ = ','; *p++ = ' ';
+}
+}
+
+ngx_str_set(>key, "Trailer");
+ 

[PATCH 1 of 3] Added support for trailers in HTTP responses

[Piotr Sikora via nginx-devel]

# HG changeset patch
# User Piotr Sikora <piotrsik...@google.com>
# Date 1490351854 25200
#  Fri Mar 24 03:37:34 2017 -0700
# Node ID b0a910ad494158427ba102bdac71ce01d0667f72
# Parent  716852cce9136d977b81a2d1b8b6f9fbca0dce49
Added support for trailers in HTTP responses.

Example:

   ngx_table_elt_t  *h;

   h = ngx_list_push(>headers_out.trailers);
   if (h == NULL) {
   return NGX_ERROR;
   }

   ngx_str_set(>key, "Fun");
   ngx_str_set(>value, "with trailers");
   h->hash = ngx_hash_key_lc(h->key.data, h->key.len);

The code above adds "Fun: with trailers" trailer to the response to
the request with "TE: trailers" header (which indicates support for
trailers).

Modules that want to emit trailers must set r->expect_trailers = 1,
otherwise they are going to be ignored.

This change also adds $sent_trailer_* variables.

Signed-off-by: Piotr Sikora <piotrsik...@google.com>

diff -r 716852cce913 -r b0a910ad4941 
src/http/modules/ngx_http_chunked_filter_module.c
--- a/src/http/modules/ngx_http_chunked_filter_module.c
+++ b/src/http/modules/ngx_http_chunked_filter_module.c
@@ -17,6 +17,8 @@ typedef struct {
 
 
 static ngx_int_t ngx_http_chunked_filter_init(ngx_conf_t *cf);
+static ngx_chain_t *ngx_http_chunked_create_trailers(ngx_http_request_t *r,
+ngx_uint_t emit_crlf);
 
 
 static ngx_http_module_t  ngx_http_chunked_filter_module_ctx = {
@@ -69,28 +71,31 @@ ngx_http_chunked_header_filter(ngx_http_
 return ngx_http_next_header_filter(r);
 }
 
-if (r->headers_out.content_length_n == -1) {
+clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module);
+
+if (clcf->chunked_transfer_encoding && r->expect_trailers) {
+ngx_http_clear_content_length(r);
+r->chunked = 1;
+
+} else if (r->headers_out.content_length_n == -1) {
 if (r->http_version < NGX_HTTP_VERSION_11) {
 r->keepalive = 0;
 
+} else if (clcf->chunked_transfer_encoding) {
+r->chunked = 1;
+
 } else {
-clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module);
+r->keepalive = 0;
+}
+}
 
-if (clcf->chunked_transfer_encoding) {
-r->chunked = 1;
+if (r->chunked) {
+ctx = ngx_pcalloc(r->pool, sizeof(ngx_http_chunked_filter_ctx_t));
+if (ctx == NULL) {
+return NGX_ERROR;
+}
 
-ctx = ngx_pcalloc(r->pool,
-  sizeof(ngx_http_chunked_filter_ctx_t));
-if (ctx == NULL) {
-return NGX_ERROR;
-}
-
-ngx_http_set_ctx(r, ctx, ngx_http_chunked_filter_module);
-
-} else {
-r->keepalive = 0;
-}
-}
+ngx_http_set_ctx(r, ctx, ngx_http_chunked_filter_module);
 }
 
 return ngx_http_next_header_filter(r);
@@ -179,28 +184,38 @@ ngx_http_chunked_body_filter(ngx_http_re
 }
 
 if (cl->buf->last_buf) {
-tl = ngx_chain_get_free_buf(r->pool, >free);
-if (tl == NULL) {
-return NGX_ERROR;
+
+if (r->expect_trailers) {
+tl = ngx_http_chunked_create_trailers(r, size ? 1 : 0);
+
+} else {
+tl = NULL;
 }
 
-b = tl->buf;
+if (tl == NULL) {
+tl = ngx_chain_get_free_buf(r->pool, >free);
+if (tl == NULL) {
+return NGX_ERROR;
+}
 
-b->tag = (ngx_buf_tag_t) _http_chunked_filter_module;
-b->temporary = 0;
-b->memory = 1;
-b->last_buf = 1;
-b->pos = (u_char *) CRLF "0" CRLF CRLF;
-b->last = b->pos + 7;
+b = tl->buf;
 
-cl->buf->last_buf = 0;
+b->tag = (ngx_buf_tag_t) _http_chunked_filter_module;
+b->temporary = 0;
+b->memory = 1;
+b->last_buf = 1;
+b->pos = (u_char *) CRLF "0" CRLF CRLF;
+b->last = b->pos + 7;
+
+cl->buf->last_buf = 0;
+
+if (size == 0) {
+b->pos += 2;
+}
+}
 
 *ll = tl;
 
-if (size == 0) {
-b->pos += 2;
-}
-
 } else if (size > 0) {
 tl = ngx_chain_get_free_buf(r->pool, >free);
 if (tl == NULL) {
@@ -230,6 +245,118 @@ ngx_http_chunked_body_filter(ngx_http_re
 }
 
 
+static ngx_chain_t *
+ngx_http_chunked_create_trailers(ngx_http_request_t *r, ngx_uint_t emit_crlf)
+{
+size_t  len;
+ngx_buf_t  *b;
+ngx_uint_t  i;
+ngx_chain_t*cl;
+ngx_list_part_t*part;
+ngx_table_elt_t*header;
+ngx_htt