Discussion: websocket over http2 / http3?
Hi, websocket over http2 is defined in RFC8441, chrome and firefox support it. https://chromestatus.com/feature/6251293127475200 websocket over http3 is defined in RFC9220, neither of chrome nor firefox support it. https://chromestatus.com/feature/5080537855688704 Is there any plan to support them in nginx? I'm implementing them, and would nginx accept them? Regards, Liu Dongmiao ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] Documented the http2 directive
> On 13 Jun 2023, at 15:46, Maxim Dounin wrote: [...] >> + >> +Enables >> +the https://datatracker.ietf.org/doc/html/rfc9113;>HTTP/2 >> +protocol. >> + > > This probably needs some more details. > > In particular: > > - For SNI-based virtual servers, this only fully works with > OpenSSL 1.0.2h and up. For older OpenSSL versions (1.0.2 - 1.0.2g), > HTTP/2 needs to be enabled in the default virtual server as well. > > - For non-SSL listening sockets, HTTP/2 needs to be enabled in > the default virtual server as well. > > Do not insist on adding it right now though, a separate patch > clarifying the details should be good enough. > > [...] > Thank you, the patch was updated with Sergey comments, and I'll create a separate patch explaining these details. # HG changeset patch # User Yaroslav Zhuravlev # Date 1686668685 -3600 # Tue Jun 13 16:04:45 2023 +0100 # Node ID b42c78a25477b40c78c048cd3d8f44421e814fca # Parent 2fa6471cd138071038f055031a7a379a7e9ab108 Documented the http2 directive. diff --git a/xml/en/docs/http/ngx_http_core_module.xml b/xml/en/docs/http/ngx_http_core_module.xml --- a/xml/en/docs/http/ngx_http_core_module.xml +++ b/xml/en/docs/http/ngx_http_core_module.xml @@ -10,7 +10,7 @@ +rev="106"> @@ -1384,6 +1384,11 @@ Normally, for this to work the ssl parameter should be specified as well, but nginx can also be configured to accept HTTP/2 connections without SSL. + +The parameter is deprecated, +the http2 directive +should be used instead. + diff --git a/xml/en/docs/http/ngx_http_grpc_module.xml b/xml/en/docs/http/ngx_http_grpc_module.xml --- a/xml/en/docs/http/ngx_http_grpc_module.xml +++ b/xml/en/docs/http/ngx_http_grpc_module.xml @@ -10,7 +10,7 @@ + rev="10"> @@ -29,7 +29,9 @@ server { -listen 9000 http2; +listen 9000; + +http2 on; location / { grpc_pass 127.0.0.1:9000; diff --git a/xml/en/docs/http/ngx_http_v2_module.xml b/xml/en/docs/http/ngx_http_v2_module.xml --- a/xml/en/docs/http/ngx_http_v2_module.xml +++ b/xml/en/docs/http/ngx_http_v2_module.xml @@ -9,7 +9,7 @@ + rev="16"> @@ -55,7 +55,9 @@ server { -listen 443 ssl http2; +listen 443 ssl; + +http2 on; ssl_certificate server.crt; ssl_certificate_key server.key; @@ -82,6 +84,22 @@ + +on | off +off +http +server +1.25.1 + + +Enables +the https://datatracker.ietf.org/doc/html/rfc9113;>HTTP/2 +protocol. + + + + + size 64k diff --git a/xml/ru/docs/http/ngx_http_core_module.xml b/xml/ru/docs/http/ngx_http_core_module.xml --- a/xml/ru/docs/http/ngx_http_core_module.xml +++ b/xml/ru/docs/http/ngx_http_core_module.xml @@ -10,7 +10,7 @@ +rev="106"> @@ -1376,6 +1376,10 @@ Обычно, чтобы это работало, следует также указать параметр ssl, однако nginx можно также настроить и на приём HTTP/2-соединений без SSL. + +Параметр устарел, вместо него следует использовать +директиву http2. + diff --git a/xml/ru/docs/http/ngx_http_grpc_module.xml b/xml/ru/docs/http/ngx_http_grpc_module.xml --- a/xml/ru/docs/http/ngx_http_grpc_module.xml +++ b/xml/ru/docs/http/ngx_http_grpc_module.xml @@ -10,7 +10,7 @@ +rev="10"> @@ -29,7 +29,9 @@ server { -listen 9000 http2; +listen 9000; + +http2 on; location / { grpc_pass 127.0.0.1:9000; diff --git a/xml/ru/docs/http/ngx_http_v2_module.xml b/xml/ru/docs/http/ngx_http_v2_module.xml --- a/xml/ru/docs/http/ngx_http_v2_module.xml +++ b/xml/ru/docs/http/ngx_http_v2_module.xml @@ -9,7 +9,7 @@ +rev="16"> @@ -55,7 +55,9 @@ server { -listen 443 ssl http2; +listen 443 ssl; + +http2 on; ssl_certificate server.crt; ssl_certificate_key server.key; @@ -82,6 +84,22 @@ + +on | off +off +http +server +1.25.1 + + +Разрешает +протокол +https://datatracker.ietf.org/doc/html/rfc9113;>HTTP/2. + + + + + размер 64k [...] ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] Documented the http2 directive
Hello! On Tue, Jun 13, 2023 at 02:56:50PM +0100, Yaroslav Zhuravlev wrote: > @@ -55,7 +55,9 @@ > > > server { > -listen 443 ssl http2; > +listen 443 ssl; > + > +http2 on; > > ssl_certificate server.crt; > ssl_certificate_key server.key; > @@ -82,6 +84,22 @@ > > > > + > +on | off > +off > +http > +server > +1.25.1 > + > + > +Enables > +the https://datatracker.ietf.org/doc/html/rfc9113;>HTTP/2 > +protocol. > + This probably needs some more details. In particular: - For SNI-based virtual servers, this only fully works with OpenSSL 1.0.2h and up. For older OpenSSL versions (1.0.2 - 1.0.2g), HTTP/2 needs to be enabled in the default virtual server as well. - For non-SSL listening sockets, HTTP/2 needs to be enabled in the default virtual server as well. Do not insist on adding it right now though, a separate patch clarifying the details should be good enough. [...] -- Maxim Dounin http://mdounin.ru/ ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] Documented the http2 directive
> On 13 Jun 2023, at 17:56, Yaroslav Zhuravlev wrote: > > xml/en/docs/http/ngx_http_core_module.xml | 13 + > xml/en/docs/http/ngx_http_grpc_module.xml | 6 -- > xml/en/docs/http/ngx_http_v2_module.xml | 22 -- > xml/ru/docs/http/ngx_http_core_module.xml | 8 ++-- > xml/ru/docs/http/ngx_http_grpc_module.xml | 6 -- > xml/ru/docs/http/ngx_http_v2_module.xml | 22 -- > 6 files changed, 63 insertions(+), 14 deletions(-) > > > # HG changeset patch > # User Yaroslav Zhuravlev > # Date 1686664161 -3600 > # Tue Jun 13 14:49:21 2023 +0100 > # Node ID d0b3b7889bfa45ce03845acdc51919463f455874 > # Parent 2fa6471cd138071038f055031a7a379a7e9ab108 > Documented the http2 directive. > > diff --git a/xml/en/docs/http/ngx_http_core_module.xml > b/xml/en/docs/http/ngx_http_core_module.xml > --- a/xml/en/docs/http/ngx_http_core_module.xml > +++ b/xml/en/docs/http/ngx_http_core_module.xml > @@ -10,7 +10,7 @@ > link="/en/docs/http/ngx_http_core_module.html" > lang="en" > -rev="105"> > +rev="106"> > > > > @@ -1378,12 +1378,17 @@ > handles both HTTP and HTTPS requests. > > > - > -The http2 parameter (1.9.5) configures the port to accept > -HTTP/2 connections. > + this anchor looks unused > +The http2 parameter (1.9.5) configures the port > +to accept HTTP/2 connections. unnecessary line re-wrap > Normally, for this to work the ssl parameter should be > specified as well, but nginx can also be configured to accept HTTP/2 > connections without SSL. > + > +The parameter is deprecated, > +the http2 directive > +should be used instead. > + > [..] -- Sergey Kandaurov ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] Documented the http2 directive
xml/en/docs/http/ngx_http_core_module.xml | 13 + xml/en/docs/http/ngx_http_grpc_module.xml | 6 -- xml/en/docs/http/ngx_http_v2_module.xml | 22 -- xml/ru/docs/http/ngx_http_core_module.xml | 8 ++-- xml/ru/docs/http/ngx_http_grpc_module.xml | 6 -- xml/ru/docs/http/ngx_http_v2_module.xml | 22 -- 6 files changed, 63 insertions(+), 14 deletions(-) # HG changeset patch # User Yaroslav Zhuravlev # Date 1686664161 -3600 # Tue Jun 13 14:49:21 2023 +0100 # Node ID d0b3b7889bfa45ce03845acdc51919463f455874 # Parent 2fa6471cd138071038f055031a7a379a7e9ab108 Documented the http2 directive. diff --git a/xml/en/docs/http/ngx_http_core_module.xml b/xml/en/docs/http/ngx_http_core_module.xml --- a/xml/en/docs/http/ngx_http_core_module.xml +++ b/xml/en/docs/http/ngx_http_core_module.xml @@ -10,7 +10,7 @@ +rev="106"> @@ -1378,12 +1378,17 @@ handles both HTTP and HTTPS requests. - -The http2 parameter (1.9.5) configures the port to accept -HTTP/2 connections. + +The http2 parameter (1.9.5) configures the port +to accept HTTP/2 connections. Normally, for this to work the ssl parameter should be specified as well, but nginx can also be configured to accept HTTP/2 connections without SSL. + +The parameter is deprecated, +the http2 directive +should be used instead. + diff --git a/xml/en/docs/http/ngx_http_grpc_module.xml b/xml/en/docs/http/ngx_http_grpc_module.xml --- a/xml/en/docs/http/ngx_http_grpc_module.xml +++ b/xml/en/docs/http/ngx_http_grpc_module.xml @@ -10,7 +10,7 @@ +rev="10"> @@ -29,7 +29,9 @@ server { -listen 9000 http2; + listen 9000; + +http2 on; location / { grpc_pass 127.0.0.1:9000; diff --git a/xml/en/docs/http/ngx_http_v2_module.xml b/xml/en/docs/http/ngx_http_v2_module.xml --- a/xml/en/docs/http/ngx_http_v2_module.xml +++ b/xml/en/docs/http/ngx_http_v2_module.xml @@ -9,7 +9,7 @@ +rev="16"> @@ -55,7 +55,9 @@ server { -listen 443 ssl http2; +listen 443 ssl; + +http2 on; ssl_certificate server.crt; ssl_certificate_key server.key; @@ -82,6 +84,22 @@ + +on | off +off +http +server +1.25.1 + + +Enables +the https://datatracker.ietf.org/doc/html/rfc9113;>HTTP/2 +protocol. + + + + + size 64k diff --git a/xml/ru/docs/http/ngx_http_core_module.xml b/xml/ru/docs/http/ngx_http_core_module.xml --- a/xml/ru/docs/http/ngx_http_core_module.xml +++ b/xml/ru/docs/http/ngx_http_core_module.xml @@ -10,7 +10,7 @@ +rev="106"> @@ -1370,12 +1370,16 @@ работающего сразу в двух режимах — HTTP и HTTPS. - + Параметр http2 (1.9.5) позволяет принимать на этом порту HTTP/2-соединения. Обычно, чтобы это работало, следует также указать параметр ssl, однако nginx можно также настроить и на приём HTTP/2-соединений без SSL. + +Параметр устарел, вместо него следует использовать +директиву http2. + diff --git a/xml/ru/docs/http/ngx_http_grpc_module.xml b/xml/ru/docs/http/ngx_http_grpc_module.xml --- a/xml/ru/docs/http/ngx_http_grpc_module.xml +++ b/xml/ru/docs/http/ngx_http_grpc_module.xml @@ -10,7 +10,7 @@ +rev="10"> @@ -29,7 +29,9 @@ server { -listen 9000 http2; +listen 9000; + +http2 on; location / { grpc_pass 127.0.0.1:9000; diff --git a/xml/ru/docs/http/ngx_http_v2_module.xml b/xml/ru/docs/http/ngx_http_v2_module.xml --- a/xml/ru/docs/http/ngx_http_v2_module.xml +++ b/xml/ru/docs/http/ngx_http_v2_module.xml @@ -9,7 +9,7 @@ +rev="16"> @@ -55,7 +55,9 @@ server { -listen 443 ssl http2; +listen 443 ssl; + +http2 on; ssl_certificate server.crt; ssl_certificate_key server.key; @@ -82,6 +84,22 @@ + +on | off +off +http +server +1.25.1 + + +Разрешает +протокол +https://datatracker.ietf.org/doc/html/rfc9113;>HTTP/2. + + + + + размер 64k ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
[nginx] HTTP/2: "http2" directive.
details: https://hg.nginx.org/nginx/rev/08ef02ad5c54 branches: changeset: 9119:08ef02ad5c54 user: Roman Arutyunyan date: Tue May 16 16:30:08 2023 +0400 description: HTTP/2: "http2" directive. The directive enables HTTP/2 in the current server. The previous way to enable HTTP/2 via "listen ... http2" is now deprecated. The new approach allows to share HTTP/2 and HTTP/0.9-1.1 on the same port. For SSL connections, HTTP/2 is now selected by ALPN callback based on whether the protocol is enabled in the virtual server chosen by SNI. This however only works since OpenSSL 1.0.2h, where ALPN callback is invoked after SNI callback. For older versions of OpenSSL, HTTP/2 is enabled based on the default virtual server configuration. For plain TCP connections, HTTP/2 is now auto-detected by HTTP/2 preface, if HTTP/2 is enabled in the default virtual server. If preface is not matched, HTTP/0.9-1.1 is assumed. diffstat: src/http/modules/ngx_http_ssl_module.c | 24 src/http/ngx_http_core_module.c| 5 ++ src/http/ngx_http_request.c| 61 + src/http/v2/ngx_http_v2.c | 60 ++--- src/http/v2/ngx_http_v2.h | 18 ++ src/http/v2/ngx_http_v2_module.c | 11 ++ src/http/v2/ngx_http_v2_module.h | 12 -- 7 files changed, 128 insertions(+), 63 deletions(-) diffs (391 lines): diff -r b4a57278bf24 -r 08ef02ad5c54 src/http/modules/ngx_http_ssl_module.c --- a/src/http/modules/ngx_http_ssl_module.cSun May 28 11:17:07 2023 +0400 +++ b/src/http/modules/ngx_http_ssl_module.cTue May 16 16:30:08 2023 +0400 @@ -435,6 +435,9 @@ ngx_http_ssl_alpn_select(ngx_ssl_conn_t #if (NGX_HTTP_V2 || NGX_HTTP_V3) ngx_http_connection_t *hc; #endif +#if (NGX_HTTP_V2) +ngx_http_v2_srv_conf_t *h2scf; +#endif #if (NGX_HTTP_V3) ngx_http_v3_srv_conf_t *h3scf; #endif @@ -456,12 +459,6 @@ ngx_http_ssl_alpn_select(ngx_ssl_conn_t hc = c->data; #endif -#if (NGX_HTTP_V2) -if (hc->addr_conf->http2) { -srv = (unsigned char *) NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS; -srvlen = sizeof(NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS) - 1; -} else -#endif #if (NGX_HTTP_V3) if (hc->addr_conf->quic) { @@ -488,8 +485,19 @@ ngx_http_ssl_alpn_select(ngx_ssl_conn_t } else #endif { -srv = (unsigned char *) NGX_HTTP_ALPN_PROTOS; -srvlen = sizeof(NGX_HTTP_ALPN_PROTOS) - 1; +#if (NGX_HTTP_V2) +h2scf = ngx_http_get_module_srv_conf(hc->conf_ctx, ngx_http_v2_module); + + if (h2scf->enable || hc->addr_conf->http2) { +srv = (unsigned char *) NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS; +srvlen = sizeof(NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS) - 1; + +} else +#endif +{ +srv = (unsigned char *) NGX_HTTP_ALPN_PROTOS; +srvlen = sizeof(NGX_HTTP_ALPN_PROTOS) - 1; +} } if (SSL_select_next_proto((unsigned char **) out, outlen, srv, srvlen, diff -r b4a57278bf24 -r 08ef02ad5c54 src/http/ngx_http_core_module.c --- a/src/http/ngx_http_core_module.c Sun May 28 11:17:07 2023 +0400 +++ b/src/http/ngx_http_core_module.c Tue May 16 16:30:08 2023 +0400 @@ -4176,6 +4176,11 @@ ngx_http_core_listen(ngx_conf_t *cf, ngx if (ngx_strcmp(value[n].data, "http2") == 0) { #if (NGX_HTTP_V2) +ngx_conf_log_error(NGX_LOG_WARN, cf, 0, + "the \"listen ... http2\" directive " + "is deprecated, use " + "the \"http2\" directive instead"); + lsopt.http2 = 1; continue; #else diff -r b4a57278bf24 -r 08ef02ad5c54 src/http/ngx_http_request.c --- a/src/http/ngx_http_request.c Sun May 28 11:17:07 2023 +0400 +++ b/src/http/ngx_http_request.c Tue May 16 16:30:08 2023 +0400 @@ -318,12 +318,6 @@ ngx_http_init_connection(ngx_connection_ rev->handler = ngx_http_wait_request_handler; c->write->handler = ngx_http_empty_handler; -#if (NGX_HTTP_V2) -if (hc->addr_conf->http2) { -rev->handler = ngx_http_v2_init; -} -#endif - #if (NGX_HTTP_V3) if (hc->addr_conf->quic) { ngx_http_v3_init_stream(c); @@ -383,6 +377,9 @@ ngx_http_wait_request_handler(ngx_event_ ngx_buf_t *b; ngx_connection_t *c; ngx_http_connection_t *hc; +#if (NGX_HTTP_V2) +ngx_http_v2_srv_conf_t*h2scf; +#endif ngx_http_core_srv_conf_t *cscf; c = rev->data; @@ -429,6 +426,8 @@ ngx_http_wait_request_handler(ngx_event_ b->end = b->last + size; } +size = b->end - b->last; + n = c->recv(c, b->last, size); if (n == NGX_AGAIN) { @@ -443,12 +442,16 @@ ngx_http_wait_
Re: [PATCH 2 of 3] HTTP/2: "http2" directive
Hello! On Mon, Jun 05, 2023 at 06:19:51PM +0400, Roman Arutyunyan wrote: > On Sat, Jun 03, 2023 at 12:37:03AM +0300, Maxim Dounin wrote: > > Hello! > > > > On Tue, May 16, 2023 at 04:39:38PM +0400, Roman Arutyunyan wrote: > > > > > On Thu, Feb 09, 2023 at 07:56:55PM +0400, Sergey Kandaurov wrote: > > > > > > > > > On 9 Feb 2023, at 16:33, Roman Arutyunyan wrote: > > > > > > > > > > Hi, > > > > > > > > > > On Thu, Feb 09, 2023 at 04:02:34PM +0400, Roman Arutyunyan wrote: > > > > > > > > > > [..] > > > > > > > > > >> diff --git a/src/http/ngx_http_request.c > > > > >> b/src/http/ngx_http_request.c > > > > >> --- a/src/http/ngx_http_request.c > > > > >> +++ b/src/http/ngx_http_request.c > > > > >> @@ -318,12 +318,6 @@ ngx_http_init_connection(ngx_connection_ > > > > >> rev->handler = ngx_http_wait_request_handler; > > > > >> c->write->handler = ngx_http_empty_handler; > > > > >> > > > > >> -#if (NGX_HTTP_V2) > > > > >> -if (hc->addr_conf->http2) { > > > > >> -rev->handler = ngx_http_v2_init; > > > > >> -} > > > > >> -#endif > > > > >> - > > > > >> #if (NGX_HTTP_V3) > > > > >> if (hc->addr_conf->quic) { > > > > >> ngx_http_v3_init_stream(c); > > > > >> @@ -383,6 +377,9 @@ ngx_http_wait_request_handler(ngx_event_ > > > > >> ngx_buf_t *b; > > > > >> ngx_connection_t *c; > > > > >> ngx_http_connection_t *hc; > > > > >> +#if (NGX_HTTP_V2) > > > > >> +ngx_http_v2_srv_conf_t*h2scf; > > > > >> +#endif > > > > >> ngx_http_core_srv_conf_t *cscf; > > > > >> > > > > >> c = rev->data; > > > > >> @@ -429,6 +426,8 @@ ngx_http_wait_request_handler(ngx_event_ > > > > >> b->end = b->last + size; > > > > >> } > > > > >> > > > > >> +size = b->end - b->last; > > > > >> + > > > > >> n = c->recv(c, b->last, size); > > > > >> > > > > >> if (n == NGX_AGAIN) { > > > > >> @@ -443,12 +442,16 @@ ngx_http_wait_request_handler(ngx_event_ > > > > >> return; > > > > >> } > > > > >> > > > > >> -/* > > > > >> - * We are trying to not hold c->buffer's memory for an idle > > > > >> connection. > > > > >> - */ > > > > >> - > > > > >> -if (ngx_pfree(c->pool, b->start) == NGX_OK) { > > > > >> -b->start = NULL; > > > > >> +if (b->pos == b->last) { > > > > >> + > > > > >> +/* > > > > >> + * We are trying to not hold c->buffer's memory for an > > > > >> + * idle connection. > > > > >> + */ > > > > >> + > > > > >> +if (ngx_pfree(c->pool, b->start) == NGX_OK) { > > > > >> +b->start = NULL; > > > > >> +} > > > > >> } > > > > >> > > > > >> return; > > > > >> @@ -489,10 +492,34 @@ ngx_http_wait_request_handler(ngx_event_ > > > > >> } > > > > >> } > > > > >> > > > > >> +ngx_reusable_connection(c, 0); > > > > >> + > > > > >> +#if (NGX_HTTP_V2) > > > > >> + > > > > >> +h2scf = ngx_http_get_module_srv_conf(hc->conf_ctx, > > > > >> ngx_http_v2_module); > > > > >> + > > > > >> +if (!c->ssl && (h2scf->enable || hc->addr_conf->http2)) { > > > > > > > > > > And one more fix for compilation with HTTP/2, but without SSL: > > > > > > > > > > diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c > > > > > ---
Re: [PATCH 2 of 3] HTTP/2: "http2" directive
Hi, On Sat, Jun 03, 2023 at 12:37:03AM +0300, Maxim Dounin wrote: > Hello! > > On Tue, May 16, 2023 at 04:39:38PM +0400, Roman Arutyunyan wrote: > > > On Thu, Feb 09, 2023 at 07:56:55PM +0400, Sergey Kandaurov wrote: > > > > > > > On 9 Feb 2023, at 16:33, Roman Arutyunyan wrote: > > > > > > > > Hi, > > > > > > > > On Thu, Feb 09, 2023 at 04:02:34PM +0400, Roman Arutyunyan wrote: > > > > > > > > [..] > > > > > > > >> diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c > > > >> --- a/src/http/ngx_http_request.c > > > >> +++ b/src/http/ngx_http_request.c > > > >> @@ -318,12 +318,6 @@ ngx_http_init_connection(ngx_connection_ > > > >> rev->handler = ngx_http_wait_request_handler; > > > >> c->write->handler = ngx_http_empty_handler; > > > >> > > > >> -#if (NGX_HTTP_V2) > > > >> -if (hc->addr_conf->http2) { > > > >> -rev->handler = ngx_http_v2_init; > > > >> -} > > > >> -#endif > > > >> - > > > >> #if (NGX_HTTP_V3) > > > >> if (hc->addr_conf->quic) { > > > >> ngx_http_v3_init_stream(c); > > > >> @@ -383,6 +377,9 @@ ngx_http_wait_request_handler(ngx_event_ > > > >> ngx_buf_t *b; > > > >> ngx_connection_t *c; > > > >> ngx_http_connection_t *hc; > > > >> +#if (NGX_HTTP_V2) > > > >> +ngx_http_v2_srv_conf_t*h2scf; > > > >> +#endif > > > >> ngx_http_core_srv_conf_t *cscf; > > > >> > > > >> c = rev->data; > > > >> @@ -429,6 +426,8 @@ ngx_http_wait_request_handler(ngx_event_ > > > >> b->end = b->last + size; > > > >> } > > > >> > > > >> +size = b->end - b->last; > > > >> + > > > >> n = c->recv(c, b->last, size); > > > >> > > > >> if (n == NGX_AGAIN) { > > > >> @@ -443,12 +442,16 @@ ngx_http_wait_request_handler(ngx_event_ > > > >> return; > > > >> } > > > >> > > > >> -/* > > > >> - * We are trying to not hold c->buffer's memory for an idle > > > >> connection. > > > >> - */ > > > >> - > > > >> -if (ngx_pfree(c->pool, b->start) == NGX_OK) { > > > >> -b->start = NULL; > > > >> +if (b->pos == b->last) { > > > >> + > > > >> +/* > > > >> + * We are trying to not hold c->buffer's memory for an > > > >> + * idle connection. > > > >> + */ > > > >> + > > > >> +if (ngx_pfree(c->pool, b->start) == NGX_OK) { > > > >> +b->start = NULL; > > > >> +} > > > >> } > > > >> > > > >> return; > > > >> @@ -489,10 +492,34 @@ ngx_http_wait_request_handler(ngx_event_ > > > >> } > > > >> } > > > >> > > > >> +ngx_reusable_connection(c, 0); > > > >> + > > > >> +#if (NGX_HTTP_V2) > > > >> + > > > >> +h2scf = ngx_http_get_module_srv_conf(hc->conf_ctx, > > > >> ngx_http_v2_module); > > > >> + > > > >> +if (!c->ssl && (h2scf->enable || hc->addr_conf->http2)) { > > > > > > > > And one more fix for compilation with HTTP/2, but without SSL: > > > > > > > > diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c > > > > --- a/src/http/ngx_http_request.c > > > > +++ b/src/http/ngx_http_request.c > > > > @@ -498,8 +498,12 @@ ngx_http_wait_request_handler(ngx_event_ > > > > > > > > h2scf = ngx_http_get_module_srv_conf(hc->conf_ctx, > > > > ngx_http_v2_module); > > > > > > > > -if (!c->ssl && (h2scf->enable || hc->addr_conf->http2)) { > > > > - > > > > +if ((h2scf->enable || hc->addr_conf->http2) > > > > +#if
Re: [PATCH 2 of 3] HTTP/2: "http2" directive
Hello! On Tue, May 16, 2023 at 04:39:38PM +0400, Roman Arutyunyan wrote: > On Thu, Feb 09, 2023 at 07:56:55PM +0400, Sergey Kandaurov wrote: > > > > > On 9 Feb 2023, at 16:33, Roman Arutyunyan wrote: > > > > > > Hi, > > > > > > On Thu, Feb 09, 2023 at 04:02:34PM +0400, Roman Arutyunyan wrote: > > > > > > [..] > > > > > >> diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c > > >> --- a/src/http/ngx_http_request.c > > >> +++ b/src/http/ngx_http_request.c > > >> @@ -318,12 +318,6 @@ ngx_http_init_connection(ngx_connection_ > > >> rev->handler = ngx_http_wait_request_handler; > > >> c->write->handler = ngx_http_empty_handler; > > >> > > >> -#if (NGX_HTTP_V2) > > >> -if (hc->addr_conf->http2) { > > >> -rev->handler = ngx_http_v2_init; > > >> -} > > >> -#endif > > >> - > > >> #if (NGX_HTTP_V3) > > >> if (hc->addr_conf->quic) { > > >> ngx_http_v3_init_stream(c); > > >> @@ -383,6 +377,9 @@ ngx_http_wait_request_handler(ngx_event_ > > >> ngx_buf_t *b; > > >> ngx_connection_t *c; > > >> ngx_http_connection_t *hc; > > >> +#if (NGX_HTTP_V2) > > >> +ngx_http_v2_srv_conf_t*h2scf; > > >> +#endif > > >> ngx_http_core_srv_conf_t *cscf; > > >> > > >> c = rev->data; > > >> @@ -429,6 +426,8 @@ ngx_http_wait_request_handler(ngx_event_ > > >> b->end = b->last + size; > > >> } > > >> > > >> +size = b->end - b->last; > > >> + > > >> n = c->recv(c, b->last, size); > > >> > > >> if (n == NGX_AGAIN) { > > >> @@ -443,12 +442,16 @@ ngx_http_wait_request_handler(ngx_event_ > > >> return; > > >> } > > >> > > >> -/* > > >> - * We are trying to not hold c->buffer's memory for an idle > > >> connection. > > >> - */ > > >> - > > >> -if (ngx_pfree(c->pool, b->start) == NGX_OK) { > > >> -b->start = NULL; > > >> +if (b->pos == b->last) { > > >> + > > >> +/* > > >> + * We are trying to not hold c->buffer's memory for an > > >> + * idle connection. > > >> + */ > > >> + > > >> +if (ngx_pfree(c->pool, b->start) == NGX_OK) { > > >> +b->start = NULL; > > >> +} > > >> } > > >> > > >> return; > > >> @@ -489,10 +492,34 @@ ngx_http_wait_request_handler(ngx_event_ > > >> } > > >> } > > >> > > >> +ngx_reusable_connection(c, 0); > > >> + > > >> +#if (NGX_HTTP_V2) > > >> + > > >> +h2scf = ngx_http_get_module_srv_conf(hc->conf_ctx, > > >> ngx_http_v2_module); > > >> + > > >> +if (!c->ssl && (h2scf->enable || hc->addr_conf->http2)) { > > > > > > And one more fix for compilation with HTTP/2, but without SSL: > > > > > > diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c > > > --- a/src/http/ngx_http_request.c > > > +++ b/src/http/ngx_http_request.c > > > @@ -498,8 +498,12 @@ ngx_http_wait_request_handler(ngx_event_ > > > > > > h2scf = ngx_http_get_module_srv_conf(hc->conf_ctx, > > > ngx_http_v2_module); > > > > > > -if (!c->ssl && (h2scf->enable || hc->addr_conf->http2)) { > > > - > > > +if ((h2scf->enable || hc->addr_conf->http2) > > > +#if (NGX_HTTP_SSL) > > > +&& !c->ssl > > > +#endif > > > + ) > > > + { > > > size = ngx_min(sizeof(NGX_HTTP_V2_PREFACE) - 1, > > >(size_t) (b->last - b->pos)); > > > > > > > I think this test needs to be replaced with !hc->ssl. > > Otherwise, it would allow to establish (and keep) h2c on ssl-enabled > > sockets, which we likely do not want to allow. > > After a series of discussions we decided to
Re: [PATCH 2 of 3] HTTP/2: "http2" directive
On Tue, May 16, 2023 at 04:39:38PM +0400, Roman Arutyunyan wrote: > Hi, > > # HG changeset patch > # User Roman Arutyunyan > # Date 1684240208 -14400 > # Tue May 16 16:30:08 2023 +0400 > # Branch quic > # Node ID 4dcd2b42c23973815a6b8a7f54bbd1460c314c93 > # Parent d8272b84031bea1940ef8a5b8e2f79ec6a2dcfc1 > HTTP/2: "http2" directive. > > The directive enables HTTP/2 in the current server. The previous way to > enable HTTP/2 via "listen ... http2" is now deprecated. The new approach > allows to share HTTP/2 and HTTP/0.9-1.1 on the same port. > > For SSL connections, HTTP/2 is now selected by ALPN callback based on whether > the protocol is enabled in the virtual server chosen by SNI. This however > only > works since OpenSSL 1.0.2h, where ALPN callback is invoked after SNI callback. > For older versions of OpenSSL, HTTP/2 is enabled based on the default virtual > server configuration. > > For plain TCP connections, HTTP/2 is now auto-detected by HTTP/2 preface, if > HTTP/2 is enabled in the default virtual server. If preface is not matched, > HTTP/0.9-1.1 is assumed. You might also note that HTTP/2 detection by connection preface also affects the now deprecated "listen .. http2" configuration. Either way, looks good. ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: Q: http2 and http1 virtual hosts both works via HTTP/2 - bug of feature?
Hello! On Wed, May 24, 2023 at 01:07:00AM +0300, Andrey Kulikov wrote: > Observed nginx's version 1.22.1 questionable behaviour with two virtual > hosts, one with H2 - enabled, second without http2 support. > Both on the same IP and port, with different domain names/server names. > When browsers make requests to a second domain, h2 being ALPN-negotiated, > and data transferred via HTTP/2, in spite of http2 was not configured on > that virtual host. This is a mailing list about nginx development. For user-level questions, please use the nginx@ mailing list instead. Thank you. [...] -- Maxim Dounin http://mdounin.ru/ ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
Q: http2 and http1 virtual hosts both works via HTTP/2 - bug of feature?
Hello, Observed nginx's version 1.22.1 questionable behaviour with two virtual hosts, one with H2 - enabled, second without http2 support. Both on the same IP and port, with different domain names/server names. When browsers make requests to a second domain, h2 being ALPN-negotiated, and data transferred via HTTP/2, in spite of http2 was not configured on that virtual host. Sample config snippet: http { ... server { listen 1985 http2 ssl; server_name 'mavr.cp.eu'; ssl_certificate domain.cer; ssl_certificate_key domain.key; location / { return 302 https://zavr.cp.eu:1985$request_uri; } } server { listen 1985 ssl; # NO h2! server_name 'zavr.cp.eu'; ssl_certificate domain.cer; ssl_certificate_key domain.key; location / { # Doesn't really matter what's here, for simplicity I've used ngx_lua_module. echo "Server protocol: $server_protocol H2 connection: $http2 ."; } } ... } When I type https://mavr.cp.eu:1985 in browser I see: 1. Browser negotiates h2 ALPN with the server with SNI mavr.cp.eu ; 2. Server replied with 302 redirect to https://zavr.cp.eu:1985 (expected) 3. Browser makes NEW TCP-connection with the server with SNI zavr.cp.eu; (expected) 4. Server replied with h2 ALPN (unexpected) 5. Browser shows "Server protocol: HTTP/2.0 H2 connection: h2 ." When I browsed source code, I spotted following line: http://hg.nginx.org/nginx/file/tip/src/http/modules/ngx_http_ssl_module.c#l460 #if (NGX_HTTP_V2) <http://hg.nginx.org/nginx/file/tip/src/http/modules/ngx_http_ssl_module.c#l459> if (hc->addr_conf->http2) { <http://hg.nginx.org/nginx/file/tip/src/http/modules/ngx_http_ssl_module.c#l460> srv = (unsigned char *) NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS; <http://hg.nginx.org/nginx/file/tip/src/http/modules/ngx_http_ssl_module.c#l461> srvlen = sizeof(NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS) - 1; <http://hg.nginx.org/nginx/file/tip/src/http/modules/ngx_http_ssl_module.c#l462> } else <http://hg.nginx.org/nginx/file/tip/src/http/modules/ngx_http_ssl_module.c#l463>#endif My assumption (could be wrong) that it means, when http2 is enabled on the address, related to (possibly) many virtual hosts, we always add h2 ALPN. Regardless of negotiated SNI. At least I see that ngx_http_find_virtual_server() being called here: http://hg.nginx.org/nginx/file/tip/src/http/ngx_http_request.c#l2236 on already established http(s) connection, not during TLS handshake. This behaviour caused some pain in my neck today, so could please someone be so kind to enlighten me, is it like it supposed to be, or is it possible to change this in the way, so http1 and http2 virtual hosts could be served on the same IP:port? Is it a bug, feature, or just not needed by anyone? Quick Googling did not reveal that anyone had complained about it too much. If one will manage to implement a patch, correcting this behaviour, will it be even considered to review? -- Andrey P.S. Under "Browser" here I meant Chrome, Chromium, and Firefox on Windows. But, according to unconfirmed information, "some", yet unidentified, browsers do NOT make a second TCP-connection to another domain after redirection in the provided example. And put a second request to the same h2 connection, despite the fact that it was negotiated for different SNI. Shame on them. ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH 2 of 3] HTTP/2: "http2" directive
Hi, On Thu, Feb 09, 2023 at 07:56:55PM +0400, Sergey Kandaurov wrote: > > > On 9 Feb 2023, at 16:33, Roman Arutyunyan wrote: > > > > Hi, > > > > On Thu, Feb 09, 2023 at 04:02:34PM +0400, Roman Arutyunyan wrote: > > > > [..] > > > >> diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c > >> --- a/src/http/ngx_http_request.c > >> +++ b/src/http/ngx_http_request.c > >> @@ -318,12 +318,6 @@ ngx_http_init_connection(ngx_connection_ > >> rev->handler = ngx_http_wait_request_handler; > >> c->write->handler = ngx_http_empty_handler; > >> > >> -#if (NGX_HTTP_V2) > >> -if (hc->addr_conf->http2) { > >> -rev->handler = ngx_http_v2_init; > >> -} > >> -#endif > >> - > >> #if (NGX_HTTP_V3) > >> if (hc->addr_conf->quic) { > >> ngx_http_v3_init_stream(c); > >> @@ -383,6 +377,9 @@ ngx_http_wait_request_handler(ngx_event_ > >> ngx_buf_t *b; > >> ngx_connection_t *c; > >> ngx_http_connection_t *hc; > >> +#if (NGX_HTTP_V2) > >> +ngx_http_v2_srv_conf_t*h2scf; > >> +#endif > >> ngx_http_core_srv_conf_t *cscf; > >> > >> c = rev->data; > >> @@ -429,6 +426,8 @@ ngx_http_wait_request_handler(ngx_event_ > >> b->end = b->last + size; > >> } > >> > >> +size = b->end - b->last; > >> + > >> n = c->recv(c, b->last, size); > >> > >> if (n == NGX_AGAIN) { > >> @@ -443,12 +442,16 @@ ngx_http_wait_request_handler(ngx_event_ > >> return; > >> } > >> > >> -/* > >> - * We are trying to not hold c->buffer's memory for an idle > >> connection. > >> - */ > >> - > >> -if (ngx_pfree(c->pool, b->start) == NGX_OK) { > >> -b->start = NULL; > >> +if (b->pos == b->last) { > >> + > >> +/* > >> + * We are trying to not hold c->buffer's memory for an > >> + * idle connection. > >> + */ > >> + > >> +if (ngx_pfree(c->pool, b->start) == NGX_OK) { > >> +b->start = NULL; > >> +} > >> } > >> > >> return; > >> @@ -489,10 +492,34 @@ ngx_http_wait_request_handler(ngx_event_ > >> } > >> } > >> > >> +ngx_reusable_connection(c, 0); > >> + > >> +#if (NGX_HTTP_V2) > >> + > >> +h2scf = ngx_http_get_module_srv_conf(hc->conf_ctx, > >> ngx_http_v2_module); > >> + > >> +if (!c->ssl && (h2scf->enable || hc->addr_conf->http2)) { > > > > And one more fix for compilation with HTTP/2, but without SSL: > > > > diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c > > --- a/src/http/ngx_http_request.c > > +++ b/src/http/ngx_http_request.c > > @@ -498,8 +498,12 @@ ngx_http_wait_request_handler(ngx_event_ > > > > h2scf = ngx_http_get_module_srv_conf(hc->conf_ctx, ngx_http_v2_module); > > > > -if (!c->ssl && (h2scf->enable || hc->addr_conf->http2)) { > > - > > +if ((h2scf->enable || hc->addr_conf->http2) > > +#if (NGX_HTTP_SSL) > > + && !c->ssl > > +#endif > > + ) > > + { > > size = ngx_min(sizeof(NGX_HTTP_V2_PREFACE) - 1, > >(size_t) (b->last - b->pos)); > > > > I think this test needs to be replaced with !hc->ssl. > Otherwise, it would allow to establish (and keep) h2c on ssl-enabled > sockets, which we likely do not want to allow. After a series of discussions we decided to go with !hc->ssl. As a result, any non-SSL connection on an SSL port will trigger the HTTP/1 error page. Previous attempt to trigger the HTTP/2 error in case client request is recognized as HTTP/2, is discarded since the situation is unlikely. [..] -- Roman Arutyunyan # HG changeset patch # User Roman Arutyunyan # Date 1684240208 -14400 # Tue May 16 16:30:08 2023 +0400 # Branch quic # Node ID 4dcd2b42c23973815a6b8a7f54bbd1460c314c93 # Parent d8272b84031bea1940ef8a5b8e2f79ec6a2dcfc1 HTTP/2: "http2" directive. The directive enables HTTP/2 in the current server. The previous way t
Re: [PATCH 2 of 3] HTTP/2: "http2" directive
> On 9 Feb 2023, at 16:33, Roman Arutyunyan wrote: > > Hi, > > On Thu, Feb 09, 2023 at 04:02:34PM +0400, Roman Arutyunyan wrote: > > [..] > >> diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c >> --- a/src/http/ngx_http_request.c >> +++ b/src/http/ngx_http_request.c >> @@ -318,12 +318,6 @@ ngx_http_init_connection(ngx_connection_ >> rev->handler = ngx_http_wait_request_handler; >> c->write->handler = ngx_http_empty_handler; >> >> -#if (NGX_HTTP_V2) >> -if (hc->addr_conf->http2) { >> -rev->handler = ngx_http_v2_init; >> -} >> -#endif >> - >> #if (NGX_HTTP_V3) >> if (hc->addr_conf->quic) { >> ngx_http_v3_init_stream(c); >> @@ -383,6 +377,9 @@ ngx_http_wait_request_handler(ngx_event_ >> ngx_buf_t *b; >> ngx_connection_t *c; >> ngx_http_connection_t *hc; >> +#if (NGX_HTTP_V2) >> +ngx_http_v2_srv_conf_t*h2scf; >> +#endif >> ngx_http_core_srv_conf_t *cscf; >> >> c = rev->data; >> @@ -429,6 +426,8 @@ ngx_http_wait_request_handler(ngx_event_ >> b->end = b->last + size; >> } >> >> +size = b->end - b->last; >> + >> n = c->recv(c, b->last, size); >> >> if (n == NGX_AGAIN) { >> @@ -443,12 +442,16 @@ ngx_http_wait_request_handler(ngx_event_ >> return; >> } >> >> -/* >> - * We are trying to not hold c->buffer's memory for an idle >> connection. >> - */ >> - >> -if (ngx_pfree(c->pool, b->start) == NGX_OK) { >> -b->start = NULL; >> +if (b->pos == b->last) { >> + >> +/* >> + * We are trying to not hold c->buffer's memory for an >> + * idle connection. >> + */ >> + >> +if (ngx_pfree(c->pool, b->start) == NGX_OK) { >> +b->start = NULL; >> +} >> } >> >> return; >> @@ -489,10 +492,34 @@ ngx_http_wait_request_handler(ngx_event_ >> } >> } >> >> +ngx_reusable_connection(c, 0); >> + >> +#if (NGX_HTTP_V2) >> + >> +h2scf = ngx_http_get_module_srv_conf(hc->conf_ctx, ngx_http_v2_module); >> + >> +if (!c->ssl && (h2scf->enable || hc->addr_conf->http2)) { > > And one more fix for compilation with HTTP/2, but without SSL: > > diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c > --- a/src/http/ngx_http_request.c > +++ b/src/http/ngx_http_request.c > @@ -498,8 +498,12 @@ ngx_http_wait_request_handler(ngx_event_ > > h2scf = ngx_http_get_module_srv_conf(hc->conf_ctx, ngx_http_v2_module); > > -if (!c->ssl && (h2scf->enable || hc->addr_conf->http2)) { > - > +if ((h2scf->enable || hc->addr_conf->http2) > +#if (NGX_HTTP_SSL) > +&& !c->ssl > +#endif > + ) > + { > size = ngx_min(sizeof(NGX_HTTP_V2_PREFACE) - 1, >(size_t) (b->last - b->pos)); > I think this test needs to be replaced with !hc->ssl. Otherwise, it would allow to establish (and keep) h2c on ssl-enabled sockets, which we likely do not want to allow. > >> + >> +size = ngx_min(sizeof(NGX_HTTP_V2_PREFACE) - 1, >> + (size_t) (b->last - b->pos)); >> + >> +if (ngx_memcmp(b->pos, NGX_HTTP_V2_PREFACE, size) == 0) { >> + >> +if (size == sizeof(NGX_HTTP_V2_PREFACE) - 1) { >> +ngx_http_v2_init(rev); >> +return; >> +} >> + >> +c->log->action = "waiting for request"; >> +ngx_post_event(rev, _posted_events); >> +return; >> +} >> +} >> + >> +#endif >> + >> c->log->action = "reading client request line"; >> >> -ngx_reusable_connection(c, 0); >> - >> c->data = ngx_http_create_request(c); >> if (c->data == NULL) { >> ngx_http_close_connection(c); > > [..] > -- Sergey Kandaurov ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH 2 of 3] HTTP/2: "http2" directive
Hi, On Thu, Feb 09, 2023 at 04:02:34PM +0400, Roman Arutyunyan wrote: [..] > diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c > --- a/src/http/ngx_http_request.c > +++ b/src/http/ngx_http_request.c > @@ -318,12 +318,6 @@ ngx_http_init_connection(ngx_connection_ > rev->handler = ngx_http_wait_request_handler; > c->write->handler = ngx_http_empty_handler; > > -#if (NGX_HTTP_V2) > -if (hc->addr_conf->http2) { > -rev->handler = ngx_http_v2_init; > -} > -#endif > - > #if (NGX_HTTP_V3) > if (hc->addr_conf->quic) { > ngx_http_v3_init_stream(c); > @@ -383,6 +377,9 @@ ngx_http_wait_request_handler(ngx_event_ > ngx_buf_t *b; > ngx_connection_t *c; > ngx_http_connection_t *hc; > +#if (NGX_HTTP_V2) > +ngx_http_v2_srv_conf_t*h2scf; > +#endif > ngx_http_core_srv_conf_t *cscf; > > c = rev->data; > @@ -429,6 +426,8 @@ ngx_http_wait_request_handler(ngx_event_ > b->end = b->last + size; > } > > +size = b->end - b->last; > + > n = c->recv(c, b->last, size); > > if (n == NGX_AGAIN) { > @@ -443,12 +442,16 @@ ngx_http_wait_request_handler(ngx_event_ > return; > } > > -/* > - * We are trying to not hold c->buffer's memory for an idle > connection. > - */ > - > -if (ngx_pfree(c->pool, b->start) == NGX_OK) { > -b->start = NULL; > +if (b->pos == b->last) { > + > +/* > + * We are trying to not hold c->buffer's memory for an > + * idle connection. > + */ > + > +if (ngx_pfree(c->pool, b->start) == NGX_OK) { > +b->start = NULL; > +} > } > > return; > @@ -489,10 +492,34 @@ ngx_http_wait_request_handler(ngx_event_ > } > } > > +ngx_reusable_connection(c, 0); > + > +#if (NGX_HTTP_V2) > + > +h2scf = ngx_http_get_module_srv_conf(hc->conf_ctx, ngx_http_v2_module); > + > +if (!c->ssl && (h2scf->enable || hc->addr_conf->http2)) { And one more fix for compilation with HTTP/2, but without SSL: diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c --- a/src/http/ngx_http_request.c +++ b/src/http/ngx_http_request.c @@ -498,8 +498,12 @@ ngx_http_wait_request_handler(ngx_event_ h2scf = ngx_http_get_module_srv_conf(hc->conf_ctx, ngx_http_v2_module); -if (!c->ssl && (h2scf->enable || hc->addr_conf->http2)) { - +if ((h2scf->enable || hc->addr_conf->http2) +#if (NGX_HTTP_SSL) +&& !c->ssl +#endif + ) + { size = ngx_min(sizeof(NGX_HTTP_V2_PREFACE) - 1, (size_t) (b->last - b->pos)); > + > +size = ngx_min(sizeof(NGX_HTTP_V2_PREFACE) - 1, > + (size_t) (b->last - b->pos)); > + > +if (ngx_memcmp(b->pos, NGX_HTTP_V2_PREFACE, size) == 0) { > + > +if (size == sizeof(NGX_HTTP_V2_PREFACE) - 1) { > +ngx_http_v2_init(rev); > +return; > +} > + > +c->log->action = "waiting for request"; > +ngx_post_event(rev, _posted_events); > +return; > +} > +} > + > +#endif > + > c->log->action = "reading client request line"; > > -ngx_reusable_connection(c, 0); > - > c->data = ngx_http_create_request(c); > if (c->data == NULL) { > ngx_http_close_connection(c); [..] -- Roman Arutyunyan ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH 2 of 3] HTTP/2: "http2" directive
Hi, On Thu, Feb 09, 2023 at 03:28:02PM +0400, Sergey Kandaurov wrote: > > > On 7 Feb 2023, at 18:50, Roman Arutyunyan wrote: > > > > # HG changeset patch > > # User Roman Arutyunyan > > # Date 1675781276 -14400 > > # Tue Feb 07 18:47:56 2023 +0400 > > # Branch quic > > # Node ID 735f9e501922e4b0a1b20730d62bac35ea398336 > > # Parent 38eec3d9f2c0d2e6d041efe3ee6d9c1618d8f1e6 > > HTTP/2: "http2" directive. > > > > The directive enables HTTP/2 in the current server. The previous way to > > enable HTTP/2 via "listen ... http2" is now deprecated. The new approach > > allows to share HTTP/2 and HTTP/0.9-1.1 on the same port. > > > > For SSL connections, HTTP/2 is now selected by ALPN callback based on > > whether > > the protocol is enabled in the virtual server chosen by SNI. This however > > only > > works since OpenSSL 1.0.2h, where ALPN callback is invoked after SNI > > callback. > > For older versions of OpenSSL, HTTP/2 is enabled based on the default > > virtual > > server configuration. > > > > For plain TCP connections, HTTP/2 is now auto-detected by HTTP/2 preface, if > > HTTP/2 is enabled in the default virtual server. If preface is not matched, > > HTTP/0.9-1.1 is assumed. > > > > diff --git a/src/http/modules/ngx_http_ssl_module.c > > b/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 > > @@ -427,6 +427,9 @@ ngx_http_ssl_alpn_select(ngx_ssl_conn_t > > #if (NGX_HTTP_V2 || NGX_HTTP_V3) > > ngx_http_connection_t *hc; > > #endif > > +#if (NGX_HTTP_V2) > > +ngx_http_v2_srv_conf_t *h2scf; > > +#endif > > #if (NGX_HTTP_V3) > > ngx_http_v3_srv_conf_t *h3scf; > > #endif > > @@ -448,12 +451,9 @@ ngx_http_ssl_alpn_select(ngx_ssl_conn_t > > hc = c->data; > > #endif > > > > -#if (NGX_HTTP_V2) > > -if (hc->addr_conf->http2) { > > -srv = (unsigned char *) NGX_HTTP_V2_ALPN_PROTO > > NGX_HTTP_ALPN_PROTOS; > > -srvlen = sizeof(NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS) - 1; > > -} else > > -#endif > > +srv = (unsigned char *) NGX_HTTP_ALPN_PROTOS; > > +srvlen = sizeof(NGX_HTTP_ALPN_PROTOS) - 1; > > + > > #if (NGX_HTTP_V3) > > if (hc->addr_conf->quic) { > > > > @@ -479,10 +479,16 @@ ngx_http_ssl_alpn_select(ngx_ssl_conn_t > > > > } else > > #endif > > +#if (NGX_HTTP_V2) > > { > > -srv = (unsigned char *) NGX_HTTP_ALPN_PROTOS; > > -srvlen = sizeof(NGX_HTTP_ALPN_PROTOS) - 1; > > +h2scf = ngx_http_get_module_srv_conf(hc->conf_ctx, > > ngx_http_v2_module); > > + > > +if (h2scf->enable || hc->addr_conf->http2) { > > +srv = (unsigned char *) NGX_HTTP_V2_ALPN_PROTO > > NGX_HTTP_ALPN_PROTOS; > > +srvlen = sizeof(NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS) - > > 1; > > +} > > } > > +#endif > > With NGX_HTTP_V3 defined but NGX_HTTP_V2 not, > the else part will go to the SSL_select_next_proto() call. > So, to fix this, NGX_HTTP_ALPN_PROTOS still has to be the last resort, > for simplicity (and also reduces diff). > > My version: > > diff --git a/src/http/modules/ngx_http_ssl_module.c > b/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 > @@ -427,6 +427,9 @@ ngx_http_ssl_alpn_select(ngx_ssl_conn_t > #if (NGX_HTTP_V2 || NGX_HTTP_V3) > ngx_http_connection_t *hc; > #endif > +#if (NGX_HTTP_V2) > +ngx_http_v2_srv_conf_t *h2scf; > +#endif > #if (NGX_HTTP_V3) > ngx_http_v3_srv_conf_t *h3scf; > #endif > @@ -449,7 +452,9 @@ ngx_http_ssl_alpn_select(ngx_ssl_conn_t > #endif > > #if (NGX_HTTP_V2) > -if (hc->addr_conf->http2) { > +h2scf = ngx_http_get_module_srv_conf(hc->conf_ctx, ngx_http_v2_module); > + > +if (h2scf->enable || hc->addr_conf->http2) { > srv = (unsigned char *) NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS; > srvlen = sizeof(NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS) - 1; > } else > > > > > if (SSL_select_next_proto((unsigned char **) out, outlen, srv, srvlen, > > in, inlen) > Thanks for noticing this. If fixed this way, nginx will be able to negotiate HTTP/2 over QUIC, which is probably not what we want. Following a discussion with
Re: [PATCH 2 of 3] HTTP/2: "http2" directive
> On 7 Feb 2023, at 18:50, Roman Arutyunyan wrote: > > # HG changeset patch > # User Roman Arutyunyan > # Date 1675781276 -14400 > # Tue Feb 07 18:47:56 2023 +0400 > # Branch quic > # Node ID 735f9e501922e4b0a1b20730d62bac35ea398336 > # Parent 38eec3d9f2c0d2e6d041efe3ee6d9c1618d8f1e6 > HTTP/2: "http2" directive. > > The directive enables HTTP/2 in the current server. The previous way to > enable HTTP/2 via "listen ... http2" is now deprecated. The new approach > allows to share HTTP/2 and HTTP/0.9-1.1 on the same port. > > For SSL connections, HTTP/2 is now selected by ALPN callback based on whether > the protocol is enabled in the virtual server chosen by SNI. This however > only > works since OpenSSL 1.0.2h, where ALPN callback is invoked after SNI callback. > For older versions of OpenSSL, HTTP/2 is enabled based on the default virtual > server configuration. > > For plain TCP connections, HTTP/2 is now auto-detected by HTTP/2 preface, if > HTTP/2 is enabled in the default virtual server. If preface is not matched, > HTTP/0.9-1.1 is assumed. > > diff --git a/src/http/modules/ngx_http_ssl_module.c > b/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 > @@ -427,6 +427,9 @@ ngx_http_ssl_alpn_select(ngx_ssl_conn_t > #if (NGX_HTTP_V2 || NGX_HTTP_V3) > ngx_http_connection_t *hc; > #endif > +#if (NGX_HTTP_V2) > +ngx_http_v2_srv_conf_t *h2scf; > +#endif > #if (NGX_HTTP_V3) > ngx_http_v3_srv_conf_t *h3scf; > #endif > @@ -448,12 +451,9 @@ ngx_http_ssl_alpn_select(ngx_ssl_conn_t > hc = c->data; > #endif > > -#if (NGX_HTTP_V2) > -if (hc->addr_conf->http2) { > -srv = (unsigned char *) NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS; > -srvlen = sizeof(NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS) - 1; > -} else > -#endif > +srv = (unsigned char *) NGX_HTTP_ALPN_PROTOS; > +srvlen = sizeof(NGX_HTTP_ALPN_PROTOS) - 1; > + > #if (NGX_HTTP_V3) > if (hc->addr_conf->quic) { > > @@ -479,10 +479,16 @@ ngx_http_ssl_alpn_select(ngx_ssl_conn_t > > } else > #endif > +#if (NGX_HTTP_V2) > { > -srv = (unsigned char *) NGX_HTTP_ALPN_PROTOS; > -srvlen = sizeof(NGX_HTTP_ALPN_PROTOS) - 1; > +h2scf = ngx_http_get_module_srv_conf(hc->conf_ctx, > ngx_http_v2_module); > + > +if (h2scf->enable || hc->addr_conf->http2) { > +srv = (unsigned char *) NGX_HTTP_V2_ALPN_PROTO > NGX_HTTP_ALPN_PROTOS; > +srvlen = sizeof(NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS) - 1; > +} > } > +#endif With NGX_HTTP_V3 defined but NGX_HTTP_V2 not, the else part will go to the SSL_select_next_proto() call. So, to fix this, NGX_HTTP_ALPN_PROTOS still has to be the last resort, for simplicity (and also reduces diff). My version: diff --git a/src/http/modules/ngx_http_ssl_module.c b/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 @@ -427,6 +427,9 @@ ngx_http_ssl_alpn_select(ngx_ssl_conn_t #if (NGX_HTTP_V2 || NGX_HTTP_V3) ngx_http_connection_t *hc; #endif +#if (NGX_HTTP_V2) +ngx_http_v2_srv_conf_t *h2scf; +#endif #if (NGX_HTTP_V3) ngx_http_v3_srv_conf_t *h3scf; #endif @@ -449,7 +452,9 @@ ngx_http_ssl_alpn_select(ngx_ssl_conn_t #endif #if (NGX_HTTP_V2) -if (hc->addr_conf->http2) { +h2scf = ngx_http_get_module_srv_conf(hc->conf_ctx, ngx_http_v2_module); + +if (h2scf->enable || hc->addr_conf->http2) { srv = (unsigned char *) NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS; srvlen = sizeof(NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS) - 1; } else > > if (SSL_select_next_proto((unsigned char **) out, outlen, srv, srvlen, > in, inlen) [..] -- Sergey Kandaurov ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH 0 of 3] Directives for enabling http2 and http3
> On 7 Feb 2023, at 18:50, Roman Arutyunyan wrote: > > Hi, > > Updated the patches based on latest comments. Looks good to me. -- Sergey Kandaurov ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH 2 of 3] HTTP/2: "http2" directive
# HG changeset patch # User Roman Arutyunyan # Date 1675781276 -14400 # Tue Feb 07 18:47:56 2023 +0400 # Branch quic # Node ID 735f9e501922e4b0a1b20730d62bac35ea398336 # Parent 38eec3d9f2c0d2e6d041efe3ee6d9c1618d8f1e6 HTTP/2: "http2" directive. The directive enables HTTP/2 in the current server. The previous way to enable HTTP/2 via "listen ... http2" is now deprecated. The new approach allows to share HTTP/2 and HTTP/0.9-1.1 on the same port. For SSL connections, HTTP/2 is now selected by ALPN callback based on whether the protocol is enabled in the virtual server chosen by SNI. This however only works since OpenSSL 1.0.2h, where ALPN callback is invoked after SNI callback. For older versions of OpenSSL, HTTP/2 is enabled based on the default virtual server configuration. For plain TCP connections, HTTP/2 is now auto-detected by HTTP/2 preface, if HTTP/2 is enabled in the default virtual server. If preface is not matched, HTTP/0.9-1.1 is assumed. diff --git a/src/http/modules/ngx_http_ssl_module.c b/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 @@ -427,6 +427,9 @@ ngx_http_ssl_alpn_select(ngx_ssl_conn_t #if (NGX_HTTP_V2 || NGX_HTTP_V3) ngx_http_connection_t *hc; #endif +#if (NGX_HTTP_V2) +ngx_http_v2_srv_conf_t *h2scf; +#endif #if (NGX_HTTP_V3) ngx_http_v3_srv_conf_t *h3scf; #endif @@ -448,12 +451,9 @@ ngx_http_ssl_alpn_select(ngx_ssl_conn_t hc = c->data; #endif -#if (NGX_HTTP_V2) -if (hc->addr_conf->http2) { -srv = (unsigned char *) NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS; -srvlen = sizeof(NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS) - 1; -} else -#endif +srv = (unsigned char *) NGX_HTTP_ALPN_PROTOS; +srvlen = sizeof(NGX_HTTP_ALPN_PROTOS) - 1; + #if (NGX_HTTP_V3) if (hc->addr_conf->quic) { @@ -479,10 +479,16 @@ ngx_http_ssl_alpn_select(ngx_ssl_conn_t } else #endif +#if (NGX_HTTP_V2) { -srv = (unsigned char *) NGX_HTTP_ALPN_PROTOS; -srvlen = sizeof(NGX_HTTP_ALPN_PROTOS) - 1; +h2scf = ngx_http_get_module_srv_conf(hc->conf_ctx, ngx_http_v2_module); + + if (h2scf->enable || hc->addr_conf->http2) { +srv = (unsigned char *) NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS; +srvlen = sizeof(NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS) - 1; +} } +#endif if (SSL_select_next_proto((unsigned char **) out, outlen, srv, srvlen, in, inlen) diff --git a/src/http/ngx_http_core_module.c b/src/http/ngx_http_core_module.c --- a/src/http/ngx_http_core_module.c +++ b/src/http/ngx_http_core_module.c @@ -4179,6 +4179,11 @@ ngx_http_core_listen(ngx_conf_t *cf, ngx if (ngx_strcmp(value[n].data, "http2") == 0) { #if (NGX_HTTP_V2) +ngx_conf_log_error(NGX_LOG_WARN, cf, 0, + "the \"listen ... http2\" directive " + "is deprecated, use " + "the \"http2\" directive instead"); + lsopt.http2 = 1; continue; #else diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c --- a/src/http/ngx_http_request.c +++ b/src/http/ngx_http_request.c @@ -318,12 +318,6 @@ ngx_http_init_connection(ngx_connection_ rev->handler = ngx_http_wait_request_handler; c->write->handler = ngx_http_empty_handler; -#if (NGX_HTTP_V2) -if (hc->addr_conf->http2) { -rev->handler = ngx_http_v2_init; -} -#endif - #if (NGX_HTTP_V3) if (hc->addr_conf->quic) { ngx_http_v3_init_stream(c); @@ -383,6 +377,9 @@ ngx_http_wait_request_handler(ngx_event_ ngx_buf_t *b; ngx_connection_t *c; ngx_http_connection_t *hc; +#if (NGX_HTTP_V2) +ngx_http_v2_srv_conf_t*h2scf; +#endif ngx_http_core_srv_conf_t *cscf; c = rev->data; @@ -429,6 +426,8 @@ ngx_http_wait_request_handler(ngx_event_ b->end = b->last + size; } +size = b->end - b->last; + n = c->recv(c, b->last, size); if (n == NGX_AGAIN) { @@ -443,12 +442,16 @@ ngx_http_wait_request_handler(ngx_event_ return; } -/* - * We are trying to not hold c->buffer's memory for an idle connection. - */ - -if (ngx_pfree(c->pool, b->start) == NGX_OK) { -b->start = NULL; +if (b->pos == b->last) { + +/* + * We are trying to not hold c->buffer's memory for an + * idle connection. + */ + +if (ngx_pfree(c->pool, b->start) == NGX_OK) { +b->start = NULL; +} } return; @@ -489,10 +492,34 @@ ngx_http_wait_request_handler(ngx
[PATCH 0 of 3] Directives for enabling http2 and http3
Hi, Updated the patches based on latest comments. -- Roman Arutyunyan ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH 2 of 3] HTTP/2: "http2" directive
Hi, On Tue, Feb 07, 2023 at 06:12:50AM +0300, Maxim Dounin wrote: > Hello! > > On Wed, Feb 01, 2023 at 06:01:09PM +0400, Roman Arutyunyan wrote: > > > # HG changeset patch > > # User Roman Arutyunyan > > # Date 1675255790 -14400 > > # Wed Feb 01 16:49:50 2023 +0400 > > # Branch quic > > # Node ID 139b348815b3f19753176988f06b590a3e0af4c0 > > # Parent 2fcc1c60be1c89aad5464bcc06f1189d1adc885a > > HTTP/2: "http2" directive. > > > > The directive enables HTTP/2 in the current server. The previous way to > > enable HTTP/2 via "listen ... http2" is now deprecated. > > It might be a good idea to describe here how it works, notably > ALPN negotiation (and OpenSSL version where it works for > non-default virtual servers) and preface-based protocol detection > for non-SSL listening sockets. As well as benefits of this > approach. OK, added more details. > > > > diff --git a/src/http/modules/ngx_http_ssl_module.c > > b/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 > > @@ -427,6 +427,9 @@ ngx_http_ssl_alpn_select(ngx_ssl_conn_t > > #if (NGX_HTTP_V2 || NGX_HTTP_V3) > > ngx_http_connection_t *hc; > > #endif > > +#if (NGX_HTTP_V2) > > +ngx_http_v2_srv_conf_t *h2scf; > > +#endif > > #if (NGX_HTTP_V3) > > ngx_http_v3_srv_conf_t *h3scf; > > #endif > > @@ -448,12 +451,9 @@ ngx_http_ssl_alpn_select(ngx_ssl_conn_t > > hc = c->data; > > #endif > > > > -#if (NGX_HTTP_V2) > > -if (hc->addr_conf->http2) { > > -srv = (unsigned char *) NGX_HTTP_V2_ALPN_PROTO > > NGX_HTTP_ALPN_PROTOS; > > -srvlen = sizeof(NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS) - 1; > > -} else > > -#endif > > +srv = (unsigned char *) NGX_HTTP_ALPN_PROTOS; > > +srvlen = sizeof(NGX_HTTP_ALPN_PROTOS) - 1; > > + > > #if (NGX_HTTP_V3) > > if (hc->addr_conf->quic) { > > > > @@ -479,10 +479,16 @@ ngx_http_ssl_alpn_select(ngx_ssl_conn_t > > > > } else > > #endif > > +#if (NGX_HTTP_V2) > > { > > -srv = (unsigned char *) NGX_HTTP_ALPN_PROTOS; > > -srvlen = sizeof(NGX_HTTP_ALPN_PROTOS) - 1; > > +h2scf = ngx_http_get_module_srv_conf(hc->conf_ctx, > > ngx_http_v2_module); > > + > > +if (h2scf->enable || hc->addr_conf->http2) { > > +srv = (unsigned char *) NGX_HTTP_V2_ALPN_PROTO > > NGX_HTTP_ALPN_PROTOS; > > +srvlen = sizeof(NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS) - > > 1; > > +} > > } > > +#endif > > > > if (SSL_select_next_proto((unsigned char **) out, outlen, srv, srvlen, > > in, inlen) > > diff --git a/src/http/ngx_http_core_module.c > > b/src/http/ngx_http_core_module.c > > --- a/src/http/ngx_http_core_module.c > > +++ b/src/http/ngx_http_core_module.c > > @@ -4179,6 +4179,11 @@ ngx_http_core_listen(ngx_conf_t *cf, ngx > > > > if (ngx_strcmp(value[n].data, "http2") == 0) { > > #if (NGX_HTTP_V2) > > +ngx_conf_log_error(NGX_LOG_WARN, cf, 0, > > + "the \"listen ... http2\" directive " > > + "is deprecated, use " > > + "the \"http2\" directive instead"); > > + > > lsopt.http2 = 1; > > continue; > > #else > > diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c > > --- a/src/http/ngx_http_request.c > > +++ b/src/http/ngx_http_request.c > > @@ -318,12 +318,6 @@ ngx_http_init_connection(ngx_connection_ > > rev->handler = ngx_http_wait_request_handler; > > c->write->handler = ngx_http_empty_handler; > > > > -#if (NGX_HTTP_V2) > > -if (hc->addr_conf->http2) { > > -rev->handler = ngx_http_v2_init; > > -} > > -#endif > > - > > #if (NGX_HTTP_V3) > > if (hc->addr_conf->quic) { > > ngx_http_v3_init_stream(c); > > @@ -383,6 +377,9 @@ ngx_http_wait_request_handler(ngx_event_ > > ngx_buf_t *b; > > ngx_connection_t *c; > > ngx_http_connection_t *hc; > > +#if (NGX_HTTP_V2) > > +ngx_http_v2_srv_conf_t*h2scf; > > +#endif > > ngx_http_core_s
Re: [PATCH 2 of 3] HTTP/2: "http2" directive
Hi, On Tue, Feb 07, 2023 at 02:25:20PM +0400, Sergey Kandaurov wrote: > > > On 1 Feb 2023, at 18:01, Roman Arutyunyan wrote: > > > > # HG changeset patch > > # User Roman Arutyunyan > > # Date 1675255790 -14400 > > # Wed Feb 01 16:49:50 2023 +0400 > > # Branch quic > > # Node ID 139b348815b3f19753176988f06b590a3e0af4c0 > > # Parent 2fcc1c60be1c89aad5464bcc06f1189d1adc885a > > HTTP/2: "http2" directive. > > > > The directive enables HTTP/2 in the current server. The previous way to > > enable HTTP/2 via "listen ... http2" is now deprecated. > > > > diff --git a/src/http/modules/ngx_http_ssl_module.c > > b/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 > > @@ -427,6 +427,9 @@ ngx_http_ssl_alpn_select(ngx_ssl_conn_t > > #if (NGX_HTTP_V2 || NGX_HTTP_V3) > > ngx_http_connection_t *hc; > > #endif > > +#if (NGX_HTTP_V2) > > +ngx_http_v2_srv_conf_t *h2scf; > > +#endif > > #if (NGX_HTTP_V3) > > ngx_http_v3_srv_conf_t *h3scf; > > #endif > > @@ -448,12 +451,9 @@ ngx_http_ssl_alpn_select(ngx_ssl_conn_t > > hc = c->data; > > #endif > > > > -#if (NGX_HTTP_V2) > > -if (hc->addr_conf->http2) { > > -srv = (unsigned char *) NGX_HTTP_V2_ALPN_PROTO > > NGX_HTTP_ALPN_PROTOS; > > -srvlen = sizeof(NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS) - 1; > > -} else > > -#endif > > +srv = (unsigned char *) NGX_HTTP_ALPN_PROTOS; > > +srvlen = sizeof(NGX_HTTP_ALPN_PROTOS) - 1; > > + > > #if (NGX_HTTP_V3) > > if (hc->addr_conf->quic) { > > > > @@ -479,10 +479,16 @@ ngx_http_ssl_alpn_select(ngx_ssl_conn_t > > > > } else > > #endif > > +#if (NGX_HTTP_V2) > > { > > -srv = (unsigned char *) NGX_HTTP_ALPN_PROTOS; > > -srvlen = sizeof(NGX_HTTP_ALPN_PROTOS) - 1; > > +h2scf = ngx_http_get_module_srv_conf(hc->conf_ctx, > > ngx_http_v2_module); > > + > > +if (h2scf->enable || hc->addr_conf->http2) { > > +srv = (unsigned char *) NGX_HTTP_V2_ALPN_PROTO > > NGX_HTTP_ALPN_PROTOS; > > +srvlen = sizeof(NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS) - > > 1; > > +} > > } > > +#endif > > > > if (SSL_select_next_proto((unsigned char **) out, outlen, srv, srvlen, > > in, inlen) > > diff --git a/src/http/ngx_http_core_module.c > > b/src/http/ngx_http_core_module.c > > --- a/src/http/ngx_http_core_module.c > > +++ b/src/http/ngx_http_core_module.c > > @@ -4179,6 +4179,11 @@ ngx_http_core_listen(ngx_conf_t *cf, ngx > > > > if (ngx_strcmp(value[n].data, "http2") == 0) { > > #if (NGX_HTTP_V2) > > +ngx_conf_log_error(NGX_LOG_WARN, cf, 0, > > + "the \"listen ... http2\" directive " > > + "is deprecated, use " > > + "the \"http2\" directive instead"); > > + > > lsopt.http2 = 1; > > continue; > > #else > > diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c > > --- a/src/http/ngx_http_request.c > > +++ b/src/http/ngx_http_request.c > > @@ -318,12 +318,6 @@ ngx_http_init_connection(ngx_connection_ > > rev->handler = ngx_http_wait_request_handler; > > c->write->handler = ngx_http_empty_handler; > > > > -#if (NGX_HTTP_V2) > > -if (hc->addr_conf->http2) { > > -rev->handler = ngx_http_v2_init; > > -} > > -#endif > > - > > #if (NGX_HTTP_V3) > > if (hc->addr_conf->quic) { > > ngx_http_v3_init_stream(c); > > @@ -383,6 +377,9 @@ ngx_http_wait_request_handler(ngx_event_ > > ngx_buf_t *b; > > ngx_connection_t *c; > > ngx_http_connection_t *hc; > > +#if (NGX_HTTP_V2) > > +ngx_http_v2_srv_conf_t*h2scf; > > +#endif > > ngx_http_core_srv_conf_t *cscf; > > > > c = rev->data; > > @@ -429,6 +426,8 @@ ngx_http_wait_request_handler(ngx_event_ > > b->end = b->last + size; > > } > > > > +size = b->end - b->last; > > + > > n = c->recv(c, b->last, size); > > > > if (n == NGX_AGAIN) { > >
Re: [PATCH 2 of 3] HTTP/2: "http2" directive
> On 1 Feb 2023, at 18:01, Roman Arutyunyan wrote: > > # HG changeset patch > # User Roman Arutyunyan > # Date 1675255790 -14400 > # Wed Feb 01 16:49:50 2023 +0400 > # Branch quic > # Node ID 139b348815b3f19753176988f06b590a3e0af4c0 > # Parent 2fcc1c60be1c89aad5464bcc06f1189d1adc885a > HTTP/2: "http2" directive. > > The directive enables HTTP/2 in the current server. The previous way to > enable HTTP/2 via "listen ... http2" is now deprecated. > > diff --git a/src/http/modules/ngx_http_ssl_module.c > b/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 > @@ -427,6 +427,9 @@ ngx_http_ssl_alpn_select(ngx_ssl_conn_t > #if (NGX_HTTP_V2 || NGX_HTTP_V3) > ngx_http_connection_t *hc; > #endif > +#if (NGX_HTTP_V2) > +ngx_http_v2_srv_conf_t *h2scf; > +#endif > #if (NGX_HTTP_V3) > ngx_http_v3_srv_conf_t *h3scf; > #endif > @@ -448,12 +451,9 @@ ngx_http_ssl_alpn_select(ngx_ssl_conn_t > hc = c->data; > #endif > > -#if (NGX_HTTP_V2) > -if (hc->addr_conf->http2) { > -srv = (unsigned char *) NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS; > -srvlen = sizeof(NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS) - 1; > -} else > -#endif > +srv = (unsigned char *) NGX_HTTP_ALPN_PROTOS; > +srvlen = sizeof(NGX_HTTP_ALPN_PROTOS) - 1; > + > #if (NGX_HTTP_V3) > if (hc->addr_conf->quic) { > > @@ -479,10 +479,16 @@ ngx_http_ssl_alpn_select(ngx_ssl_conn_t > > } else > #endif > +#if (NGX_HTTP_V2) > { > -srv = (unsigned char *) NGX_HTTP_ALPN_PROTOS; > - srvlen = sizeof(NGX_HTTP_ALPN_PROTOS) - 1; > +h2scf = ngx_http_get_module_srv_conf(hc->conf_ctx, > ngx_http_v2_module); > + > +if (h2scf->enable || hc->addr_conf->http2) { > +srv = (unsigned char *) NGX_HTTP_V2_ALPN_PROTO > NGX_HTTP_ALPN_PROTOS; > +srvlen = sizeof(NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS) - 1; > +} > } > +#endif > > if (SSL_select_next_proto((unsigned char **) out, outlen, srv, srvlen, > in, inlen) > diff --git a/src/http/ngx_http_core_module.c b/src/http/ngx_http_core_module.c > --- a/src/http/ngx_http_core_module.c > +++ b/src/http/ngx_http_core_module.c > @@ -4179,6 +4179,11 @@ ngx_http_core_listen(ngx_conf_t *cf, ngx > > if (ngx_strcmp(value[n].data, "http2") == 0) { > #if (NGX_HTTP_V2) > +ngx_conf_log_error(NGX_LOG_WARN, cf, 0, > + "the \"listen ... http2\" directive " > + "is deprecated, use " > + "the \"http2\" directive instead"); > + > lsopt.http2 = 1; > continue; > #else > diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c > --- a/src/http/ngx_http_request.c > +++ b/src/http/ngx_http_request.c > @@ -318,12 +318,6 @@ ngx_http_init_connection(ngx_connection_ > rev->handler = ngx_http_wait_request_handler; > c->write->handler = ngx_http_empty_handler; > > -#if (NGX_HTTP_V2) > -if (hc->addr_conf->http2) { > -rev->handler = ngx_http_v2_init; > -} > -#endif > - > #if (NGX_HTTP_V3) > if (hc->addr_conf->quic) { > ngx_http_v3_init_stream(c); > @@ -383,6 +377,9 @@ ngx_http_wait_request_handler(ngx_event_ > ngx_buf_t *b; > ngx_connection_t *c; > ngx_http_connection_t *hc; > +#if (NGX_HTTP_V2) > +ngx_http_v2_srv_conf_t*h2scf; > +#endif > ngx_http_core_srv_conf_t *cscf; > > c = rev->data; > @@ -429,6 +426,8 @@ ngx_http_wait_request_handler(ngx_event_ > b->end = b->last + size; > } > > +size = b->end - b->last; > + > n = c->recv(c, b->last, size); > > if (n == NGX_AGAIN) { > @@ -443,12 +442,16 @@ ngx_http_wait_request_handler(ngx_event_ > return; > } > > -/* > - * We are trying to not hold c->buffer's memory for an idle > connection. > - */ > - > -if (ngx_pfree(c->pool, b->start) == NGX_OK) { > -b->start = NULL; > +if (b->pos == b->last) { > + > +/* > + * We are trying to not hold c->buffer's memory for an > + * idle connection. > + */ > + > +if (ngx_pfree(c->pool, b->start) == NGX_OK) { > +
Re: [PATCH 2 of 3] HTTP/2: "http2" directive
Hello! On Wed, Feb 01, 2023 at 06:01:09PM +0400, Roman Arutyunyan wrote: > # HG changeset patch > # User Roman Arutyunyan > # Date 1675255790 -14400 > # Wed Feb 01 16:49:50 2023 +0400 > # Branch quic > # Node ID 139b348815b3f19753176988f06b590a3e0af4c0 > # Parent 2fcc1c60be1c89aad5464bcc06f1189d1adc885a > HTTP/2: "http2" directive. > > The directive enables HTTP/2 in the current server. The previous way to > enable HTTP/2 via "listen ... http2" is now deprecated. It might be a good idea to describe here how it works, notably ALPN negotiation (and OpenSSL version where it works for non-default virtual servers) and preface-based protocol detection for non-SSL listening sockets. As well as benefits of this approach. > > diff --git a/src/http/modules/ngx_http_ssl_module.c > b/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 > @@ -427,6 +427,9 @@ ngx_http_ssl_alpn_select(ngx_ssl_conn_t > #if (NGX_HTTP_V2 || NGX_HTTP_V3) > ngx_http_connection_t *hc; > #endif > +#if (NGX_HTTP_V2) > +ngx_http_v2_srv_conf_t *h2scf; > +#endif > #if (NGX_HTTP_V3) > ngx_http_v3_srv_conf_t *h3scf; > #endif > @@ -448,12 +451,9 @@ ngx_http_ssl_alpn_select(ngx_ssl_conn_t > hc = c->data; > #endif > > -#if (NGX_HTTP_V2) > -if (hc->addr_conf->http2) { > -srv = (unsigned char *) NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS; > -srvlen = sizeof(NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS) - 1; > -} else > -#endif > +srv = (unsigned char *) NGX_HTTP_ALPN_PROTOS; > +srvlen = sizeof(NGX_HTTP_ALPN_PROTOS) - 1; > + > #if (NGX_HTTP_V3) > if (hc->addr_conf->quic) { > > @@ -479,10 +479,16 @@ ngx_http_ssl_alpn_select(ngx_ssl_conn_t > > } else > #endif > +#if (NGX_HTTP_V2) > { > -srv = (unsigned char *) NGX_HTTP_ALPN_PROTOS; > - srvlen = sizeof(NGX_HTTP_ALPN_PROTOS) - 1; > +h2scf = ngx_http_get_module_srv_conf(hc->conf_ctx, > ngx_http_v2_module); > + > +if (h2scf->enable || hc->addr_conf->http2) { > +srv = (unsigned char *) NGX_HTTP_V2_ALPN_PROTO > NGX_HTTP_ALPN_PROTOS; > +srvlen = sizeof(NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS) - 1; > +} > } > +#endif > > if (SSL_select_next_proto((unsigned char **) out, outlen, srv, srvlen, >in, inlen) > diff --git a/src/http/ngx_http_core_module.c b/src/http/ngx_http_core_module.c > --- a/src/http/ngx_http_core_module.c > +++ b/src/http/ngx_http_core_module.c > @@ -4179,6 +4179,11 @@ ngx_http_core_listen(ngx_conf_t *cf, ngx > > if (ngx_strcmp(value[n].data, "http2") == 0) { > #if (NGX_HTTP_V2) > +ngx_conf_log_error(NGX_LOG_WARN, cf, 0, > + "the \"listen ... http2\" directive " > + "is deprecated, use " > + "the \"http2\" directive instead"); > + > lsopt.http2 = 1; > continue; > #else > diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c > --- a/src/http/ngx_http_request.c > +++ b/src/http/ngx_http_request.c > @@ -318,12 +318,6 @@ ngx_http_init_connection(ngx_connection_ > rev->handler = ngx_http_wait_request_handler; > c->write->handler = ngx_http_empty_handler; > > -#if (NGX_HTTP_V2) > -if (hc->addr_conf->http2) { > -rev->handler = ngx_http_v2_init; > -} > -#endif > - > #if (NGX_HTTP_V3) > if (hc->addr_conf->quic) { > ngx_http_v3_init_stream(c); > @@ -383,6 +377,9 @@ ngx_http_wait_request_handler(ngx_event_ > ngx_buf_t *b; > ngx_connection_t *c; > ngx_http_connection_t *hc; > +#if (NGX_HTTP_V2) > +ngx_http_v2_srv_conf_t*h2scf; > +#endif > ngx_http_core_srv_conf_t *cscf; > > c = rev->data; > @@ -429,6 +426,8 @@ ngx_http_wait_request_handler(ngx_event_ > b->end = b->last + size; > } > > +size = b->end - b->last; > + > n = c->recv(c, b->last, size); > > if (n == NGX_AGAIN) { > @@ -443,12 +442,16 @@ ngx_http_wait_request_handler(ngx_event_ > return; > } > > -/* > - * We are trying to not hold c->buffer's memory for an idle > connection. > - */ > - > -if (ngx_pfree(c->pool, b->start) == NGX_OK) { > -b->start = NULL; > +if
[PATCH 2 of 3] HTTP/2: "http2" directive
# HG changeset patch # User Roman Arutyunyan # Date 1675255790 -14400 # Wed Feb 01 16:49:50 2023 +0400 # Branch quic # Node ID 139b348815b3f19753176988f06b590a3e0af4c0 # Parent 2fcc1c60be1c89aad5464bcc06f1189d1adc885a HTTP/2: "http2" directive. The directive enables HTTP/2 in the current server. The previous way to enable HTTP/2 via "listen ... http2" is now deprecated. diff --git a/src/http/modules/ngx_http_ssl_module.c b/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 @@ -427,6 +427,9 @@ ngx_http_ssl_alpn_select(ngx_ssl_conn_t #if (NGX_HTTP_V2 || NGX_HTTP_V3) ngx_http_connection_t *hc; #endif +#if (NGX_HTTP_V2) +ngx_http_v2_srv_conf_t *h2scf; +#endif #if (NGX_HTTP_V3) ngx_http_v3_srv_conf_t *h3scf; #endif @@ -448,12 +451,9 @@ ngx_http_ssl_alpn_select(ngx_ssl_conn_t hc = c->data; #endif -#if (NGX_HTTP_V2) -if (hc->addr_conf->http2) { -srv = (unsigned char *) NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS; -srvlen = sizeof(NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS) - 1; -} else -#endif +srv = (unsigned char *) NGX_HTTP_ALPN_PROTOS; +srvlen = sizeof(NGX_HTTP_ALPN_PROTOS) - 1; + #if (NGX_HTTP_V3) if (hc->addr_conf->quic) { @@ -479,10 +479,16 @@ ngx_http_ssl_alpn_select(ngx_ssl_conn_t } else #endif +#if (NGX_HTTP_V2) { -srv = (unsigned char *) NGX_HTTP_ALPN_PROTOS; -srvlen = sizeof(NGX_HTTP_ALPN_PROTOS) - 1; +h2scf = ngx_http_get_module_srv_conf(hc->conf_ctx, ngx_http_v2_module); + + if (h2scf->enable || hc->addr_conf->http2) { +srv = (unsigned char *) NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS; +srvlen = sizeof(NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS) - 1; +} } +#endif if (SSL_select_next_proto((unsigned char **) out, outlen, srv, srvlen, in, inlen) diff --git a/src/http/ngx_http_core_module.c b/src/http/ngx_http_core_module.c --- a/src/http/ngx_http_core_module.c +++ b/src/http/ngx_http_core_module.c @@ -4179,6 +4179,11 @@ ngx_http_core_listen(ngx_conf_t *cf, ngx if (ngx_strcmp(value[n].data, "http2") == 0) { #if (NGX_HTTP_V2) +ngx_conf_log_error(NGX_LOG_WARN, cf, 0, + "the \"listen ... http2\" directive " + "is deprecated, use " + "the \"http2\" directive instead"); + lsopt.http2 = 1; continue; #else diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c --- a/src/http/ngx_http_request.c +++ b/src/http/ngx_http_request.c @@ -318,12 +318,6 @@ ngx_http_init_connection(ngx_connection_ rev->handler = ngx_http_wait_request_handler; c->write->handler = ngx_http_empty_handler; -#if (NGX_HTTP_V2) -if (hc->addr_conf->http2) { -rev->handler = ngx_http_v2_init; -} -#endif - #if (NGX_HTTP_V3) if (hc->addr_conf->quic) { ngx_http_v3_init_stream(c); @@ -383,6 +377,9 @@ ngx_http_wait_request_handler(ngx_event_ ngx_buf_t *b; ngx_connection_t *c; ngx_http_connection_t *hc; +#if (NGX_HTTP_V2) +ngx_http_v2_srv_conf_t*h2scf; +#endif ngx_http_core_srv_conf_t *cscf; c = rev->data; @@ -429,6 +426,8 @@ ngx_http_wait_request_handler(ngx_event_ b->end = b->last + size; } +size = b->end - b->last; + n = c->recv(c, b->last, size); if (n == NGX_AGAIN) { @@ -443,12 +442,16 @@ ngx_http_wait_request_handler(ngx_event_ return; } -/* - * We are trying to not hold c->buffer's memory for an idle connection. - */ - -if (ngx_pfree(c->pool, b->start) == NGX_OK) { -b->start = NULL; +if (b->pos == b->last) { + +/* + * We are trying to not hold c->buffer's memory for an + * idle connection. + */ + +if (ngx_pfree(c->pool, b->start) == NGX_OK) { +b->start = NULL; +} } return; @@ -489,6 +492,30 @@ ngx_http_wait_request_handler(ngx_event_ } } +#if (NGX_HTTP_V2) + +h2scf = ngx_http_get_module_srv_conf(hc->conf_ctx, ngx_http_v2_module); + +if (!c->ssl && (h2scf->enable || hc->addr_conf->http2)) { + +size = ngx_min(sizeof(NGX_HTTP_V2_PREFACE) - 1, + (size_t) (b->last - b->pos)); + +if (ngx_memcmp(b->pos, NGX_HTTP_V2_PREFACE, size) == 0) { + +if (size == sizeof(NGX_HTTP_V2_PREFACE) - 1) { +ngx_http_v2_init(rev); +return; +} + +c->log->action = "waiting
[PATCH 0 of 3] Directives for enabling http2 and http3
Hi, Updates in this series: - Detect HTTP-level redirects to servers where the negotiated protocol is not available. Now 421 is returned. - Retained old http3 configuration for compatibility. - Reimplemented the http2 patch. Now http2 preface is read in ngx_http_wait_request_handler(). -- Roman Arutyunyan ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH 3 of 3] HTTP/2: "http2" directive
Hello! On Thu, Jan 26, 2023 at 03:50:47PM +0400, Roman Arutyunyan wrote: > # HG changeset patch > # User Roman Arutyunyan > # Date 1674649725 -14400 > # Wed Jan 25 16:28:45 2023 +0400 > # Branch quic > # Node ID 819737783463d7e38ea80109a976db1d3a9bb2db > # Parent 555913c358221f647bbace26165bef5eb614add4 > HTTP/2: "http2" directive. > > The directive enables HTTP/2 in the current server. The previous way to > enable HTTP/2 via "listen ... http2" is now deprecated. In no particular order: - The ngx_http_try_v2() should be a separate patch? - The "(buf[0] == 'P')" check is not going to work, since valid HTTP/1.x request can start with 'P' ('POST', 'PATCH', et cetera). This needs better checking. Further, it's not guaranteed to arrive in a single packet. - Virtual servers (and 421), see comments to the first patch. -- Maxim Dounin http://mdounin.ru/ ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH 3 of 3] HTTP/2: "http2" directive
# HG changeset patch # User Roman Arutyunyan # Date 1674649725 -14400 # Wed Jan 25 16:28:45 2023 +0400 # Branch quic # Node ID 819737783463d7e38ea80109a976db1d3a9bb2db # Parent 555913c358221f647bbace26165bef5eb614add4 HTTP/2: "http2" directive. The directive enables HTTP/2 in the current server. The previous way to enable HTTP/2 via "listen ... http2" is now deprecated. diff --git a/src/http/modules/ngx_http_ssl_module.c b/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 @@ -427,6 +427,9 @@ ngx_http_ssl_alpn_select(ngx_ssl_conn_t #if (NGX_HTTP_V2 || NGX_HTTP_V3) ngx_http_connection_t *hc; #endif +#if (NGX_HTTP_V2) +ngx_http_v2_srv_conf_t *h2scf; +#endif #if (NGX_HTTP_V3) ngx_http_v3_srv_conf_t *h3scf; #endif @@ -448,12 +451,9 @@ ngx_http_ssl_alpn_select(ngx_ssl_conn_t hc = c->data; #endif -#if (NGX_HTTP_V2) -if (hc->addr_conf->http2) { -srv = (unsigned char *) NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS; -srvlen = sizeof(NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS) - 1; -} else -#endif +srv = (unsigned char *) NGX_HTTP_ALPN_PROTOS; +srvlen = sizeof(NGX_HTTP_ALPN_PROTOS) - 1; + #if (NGX_HTTP_V3) if (hc->addr_conf->quic) { @@ -479,10 +479,16 @@ ngx_http_ssl_alpn_select(ngx_ssl_conn_t } else #endif +#if (NGX_HTTP_V2) { -srv = (unsigned char *) NGX_HTTP_ALPN_PROTOS; -srvlen = sizeof(NGX_HTTP_ALPN_PROTOS) - 1; +h2scf = ngx_http_get_module_srv_conf(hc->conf_ctx, ngx_http_v2_module); + +if (hc->addr_conf->http2 || h2scf->enable) { +srv = (unsigned char *) NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS; +srvlen = sizeof(NGX_HTTP_V2_ALPN_PROTO NGX_HTTP_ALPN_PROTOS) - 1; +} } +#endif if (SSL_select_next_proto((unsigned char **) out, outlen, srv, srvlen, in, inlen) diff --git a/src/http/ngx_http_core_module.c b/src/http/ngx_http_core_module.c --- a/src/http/ngx_http_core_module.c +++ b/src/http/ngx_http_core_module.c @@ -4179,6 +4179,11 @@ ngx_http_core_listen(ngx_conf_t *cf, ngx if (ngx_strcmp(value[n].data, "http2") == 0) { #if (NGX_HTTP_V2) +ngx_conf_log_error(NGX_LOG_WARN, cf, 0, + "the \"listen ... http2\" directive " + "is deprecated, use " + "the \"http2\" directive instead"); + lsopt.http2 = 1; continue; #else diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c --- a/src/http/ngx_http_request.c +++ b/src/http/ngx_http_request.c @@ -52,6 +52,10 @@ static u_char *ngx_http_log_error(ngx_lo static u_char *ngx_http_log_error_handler(ngx_http_request_t *r, ngx_http_request_t *sr, u_char *buf, size_t len); +#if (NGX_HTTP_V2) +static void ngx_http_try_v2(ngx_event_t *rev); +#endif + #if (NGX_HTTP_SSL) static void ngx_http_ssl_handshake(ngx_event_t *rev); static void ngx_http_ssl_handshake_handler(ngx_connection_t *c); @@ -319,8 +323,14 @@ ngx_http_init_connection(ngx_connection_ c->write->handler = ngx_http_empty_handler; #if (NGX_HTTP_V2) -if (hc->addr_conf->http2) { -rev->handler = ngx_http_v2_init; +{ +ngx_http_v2_srv_conf_t *h2scf; + + h2scf = ngx_http_get_module_srv_conf(hc->conf_ctx, ngx_http_v2_module); + +if (h2scf->enable || hc->addr_conf->http2) { +rev->handler = ngx_http_try_v2; +} } #endif @@ -638,6 +648,113 @@ ngx_http_alloc_request(ngx_connection_t } +#if (NGX_HTTP_V2) + +static void +ngx_http_try_v2(ngx_event_t *rev) +{ +u_char*p, buf[NGX_PROXY_PROTOCOL_MAX_HEADER + 1]; +size_t size; +ssize_tn; +ngx_err_t err; +ngx_connection_t *c; +ngx_http_connection_t *hc; +ngx_http_core_srv_conf_t *cscf; + +c = rev->data; +hc = c->data; + +ngx_log_debug0(NGX_LOG_DEBUG_HTTP, rev->log, 0, "http check v2"); + +if (rev->timedout) { +ngx_log_error(NGX_LOG_INFO, c->log, NGX_ETIMEDOUT, "client timed out"); +ngx_http_close_connection(c); +return; +} + +if (c->close) { +ngx_http_close_connection(c); +return; +} + +size = hc->proxy_protocol ? sizeof(buf) : 1; + +n = recv(c->fd, (char *) buf, size, MSG_PEEK); + +err = ngx_socket_errno; + +ngx_log_debug1(NGX_LOG_DEBUG_HTTP, rev->log, 0, "http recv(): %z", n); + +if (n == -1) { +if (err == NGX_EAGAIN) { +rev->ready = 0; + +if (!rev->timer_set) { +cscf = ngx_http_get_module_srv_conf(h
[PATCH 0 of 3] Directives for enabling http2 and http3
Hi, These patches change http2 and http3 configurations. Previously, both protocols were configured by "listen" parameters: listen 8800 ssl http2; listen 8443 http3; The patches introduce new directives for configuring http2 and http3 in server scope: server { listen 8800 ssl; listen 8443 quic; http2 on; http3 on; ... } Now protocol list is a property of server, not a listener. After choosing a server by SNI, nginx has a list of protocols supported in that particular server. The right protocol is then chosen in ALPN callback. For plain http2 connections, a simple preread is implemented which tells http/1 from http2 by the first symbol similar to TLS signature preread in http. -- Roman Arutyunyan ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: Http2 idle connection and worker shutdown
Thanks again Maxim. Response inlined. > On Oct 20, 2022, at 6:43 PM, Maxim Dounin wrote: > > Hello! > > On Thu, Oct 20, 2022 at 06:09:11PM -0700, Jojy Varghese via nginx-devel wrote: > > [...] > >>> If you think that nginx does something wrong, you may want to >>> elaborate on the details of the incorrect behaviour you observe >>> (as well as why you think it's incorrect, and how would you like >>> to improve it). >> >> We ran into a problem where the connection close (as described >> above) causes some clients (chrome browser) to fail long running >> (large) downloads using h2. This is I think because these >> clients send a WINDOW UPDATE towards the end and Nginx sends a >> TCP RST since the connection is closed. > > So it looks like you are running into the race condition the > lingering close is expected to solve. You may want to upgrade > nginx then. Indeed looks like that :). I think the lingering close should solve the problem/race condition we are seeing. > > Note well that lingering close support in HTTP/2 was added in > nginx 1.19.1, more than two years ago, and the last nginx version > without it is not supported for more than a year. > > -- > Maxim Dounin > http://mdounin.ru/ > ___ > nginx-devel mailing list -- nginx-devel@nginx.org > To unsubscribe send an email to nginx-devel-le...@nginx.org ___ nginx-devel mailing list -- nginx-devel@nginx.org To unsubscribe send an email to nginx-devel-le...@nginx.org
Re: Http2 idle connection and worker shutdown
Hello! On Thu, Oct 20, 2022 at 06:09:11PM -0700, Jojy Varghese via nginx-devel wrote: [...] > > If you think that nginx does something wrong, you may want to > > elaborate on the details of the incorrect behaviour you observe > > (as well as why you think it's incorrect, and how would you like > > to improve it). > > We ran into a problem where the connection close (as described > above) causes some clients (chrome browser) to fail long running > (large) downloads using h2. This is I think because these > clients send a WINDOW UPDATE towards the end and Nginx sends a > TCP RST since the connection is closed. So it looks like you are running into the race condition the lingering close is expected to solve. You may want to upgrade nginx then. Note well that lingering close support in HTTP/2 was added in nginx 1.19.1, more than two years ago, and the last nginx version without it is not supported for more than a year. -- Maxim Dounin http://mdounin.ru/ ___ nginx-devel mailing list -- nginx-devel@nginx.org To unsubscribe send an email to nginx-devel-le...@nginx.org
Re: Http2 idle connection and worker shutdown
Hi Maxim Thanks for the response. Answers inline. -jojy > On Oct 20, 2022, at 2:19 PM, Maxim Dounin wrote: > > Hello! > > On Thu, Oct 20, 2022 at 10:51:06AM -0700, Jojy Varghese via nginx-devel wrote: > >> Hi >> We noticed that the Http2 idle flag >> (https://github.com/nginx/nginx/blob/master/src/http/v2/ngx_http_v2.h#L126) >> is initialized to `1` >> (https://github.com/nginx/nginx/blob/master/src/http/v2/ngx_http_v2.c#L335) >> but not reset by the http2 state machine. It also looks like >> Nginx does not set keep alive timer for http2 as the keep >> alive setter code path is short circuited for http2 >> >> (https://github.com/nginx/nginx/blob/master/src/http/ngx_http_request.c#L2706). >> >> >> During worker shutdown when idle connections are reaped >> >> (https://github.com/nginx/nginx/blob/master/src/core/ngx_connection.c#L1363), >> >> due to the fact that http2 connections are always `idle`, >> ongoing streams end up prematurely getting closed on the read >> side(sends GOAWAY frame). > > No streams are closed by the http2 connection read handler when > it's called by ngx_close_idle_connections(). Consider re-checking > the ngx_http_v2_read_handler() code: > > http://hg.nginx.org/nginx/file/tip/src/http/v2/ngx_http_v2.c#l365 > > Instead, the handler closes the connection as long as there are no > active streams. If there are any active streams, it sends the > GOAWAY frame to the connection, so the client will know that no > additional streams will be accepted, and proceeds with normal > handling of the remaining streams. I should have been more clear. I meant that the connection sends a GOAWAY frame due to which during the finalizing of the request, nginx closes the server end of the connection (we have slightly older nginx version that closes the connection as opposed to lingering close). > >> It looks like the `idle` connection implementation as it is >> today only answers the question - “Can nginx prematurely close >> the streams when shutting down?” Is the above behavior >> intentional? >> >> It appears from the name of the flag (`idle`) that the intent is >> to reflect the dynamic state of the connection (are reads and >> writes happening for the connection for any of the streams) but >> the implementation does not reflect that. >> >> What is the intended behavior ? We would have liked a `idle` >> flag with its corresponding timeout configuration that lets us >> control the behavior during shutdown (and other flows). > > The "idle" flag used to be a way to distinguish HTTP connections > without active requests and therefore prepared to be closed during > shutdown by calling c->read->handler() with c->close set. With > HTTP/2, essentially all connections are prepared to be closed > during shutdown (in some cases this may take a while though), so > all HTTP/2 connections have c->idle flag set. While from > linguistic point of view this isn't exactly correct, it's > certainly correct from semantic point of view and believed to > result in correct behaviour. > > If you think that nginx does something wrong, you may want to > elaborate on the details of the incorrect behaviour you observe > (as well as why you think it's incorrect, and how would you like > to improve it). We ran into a problem where the connection close (as described above) causes some clients (chrome browser) to fail long running (large) downloads using h2. This is I think because these clients send a WINDOW UPDATE towards the end and Nginx sends a TCP RST since the connection is closed. We were wondering if there is any reason for `idle` flag to be always 1. Or we could introduce a “lingering” time concept so that we can control if we want to set the idle flag after the lingering time is over. > > Hope this helps. > > -- > Maxim Dounin > http://mdounin.ru/ > ___ > nginx-devel mailing list -- nginx-devel@nginx.org > <mailto:nginx-devel@nginx.org> > To unsubscribe send an email to nginx-devel-le...@nginx.org > <mailto:nginx-devel-le...@nginx.org> ___ nginx-devel mailing list -- nginx-devel@nginx.org To unsubscribe send an email to nginx-devel-le...@nginx.org
Re: Http2 idle connection and worker shutdown
Hello! On Thu, Oct 20, 2022 at 10:51:06AM -0700, Jojy Varghese via nginx-devel wrote: > Hi > We noticed that the Http2 idle flag > (https://github.com/nginx/nginx/blob/master/src/http/v2/ngx_http_v2.h#L126) > is initialized to `1` > (https://github.com/nginx/nginx/blob/master/src/http/v2/ngx_http_v2.c#L335) > but not reset by the http2 state machine. It also looks like > Nginx does not set keep alive timer for http2 as the keep > alive setter code path is short circuited for http2 > > (https://github.com/nginx/nginx/blob/master/src/http/ngx_http_request.c#L2706). > > > During worker shutdown when idle connections are reaped > > (https://github.com/nginx/nginx/blob/master/src/core/ngx_connection.c#L1363), > due to the fact that http2 connections are always `idle`, > ongoing streams end up prematurely getting closed on the read > side(sends GOAWAY frame). No streams are closed by the http2 connection read handler when it's called by ngx_close_idle_connections(). Consider re-checking the ngx_http_v2_read_handler() code: http://hg.nginx.org/nginx/file/tip/src/http/v2/ngx_http_v2.c#l365 Instead, the handler closes the connection as long as there are no active streams. If there are any active streams, it sends the GOAWAY frame to the connection, so the client will know that no additional streams will be accepted, and proceeds with normal handling of the remaining streams. > It looks like the `idle` connection implementation as it is > today only answers the question - “Can nginx prematurely close > the streams when shutting down?” Is the above behavior > intentional? > > It appears from the name of the flag (`idle`) that the intent is > to reflect the dynamic state of the connection (are reads and > writes happening for the connection for any of the streams) but > the implementation does not reflect that. > > What is the intended behavior ? We would have liked a `idle` > flag with its corresponding timeout configuration that lets us > control the behavior during shutdown (and other flows). The "idle" flag used to be a way to distinguish HTTP connections without active requests and therefore prepared to be closed during shutdown by calling c->read->handler() with c->close set. With HTTP/2, essentially all connections are prepared to be closed during shutdown (in some cases this may take a while though), so all HTTP/2 connections have c->idle flag set. While from linguistic point of view this isn't exactly correct, it's certainly correct from semantic point of view and believed to result in correct behaviour. If you think that nginx does something wrong, you may want to elaborate on the details of the incorrect behaviour you observe (as well as why you think it's incorrect, and how would you like to improve it). Hope this helps. -- Maxim Dounin http://mdounin.ru/ ___ nginx-devel mailing list -- nginx-devel@nginx.org To unsubscribe send an email to nginx-devel-le...@nginx.org
Http2 idle connection and worker shutdown
Hi We noticed that the Http2 idle flag (https://github.com/nginx/nginx/blob/master/src/http/v2/ngx_http_v2.h#L126) is initialized to `1` (https://github.com/nginx/nginx/blob/master/src/http/v2/ngx_http_v2.c#L335) but not reset by the http2 state machine. It also looks like Nginx does not set keep alive timer for http2 as the keep alive setter code path is short circuited for http2 (https://github.com/nginx/nginx/blob/master/src/http/ngx_http_request.c#L2706). During worker shutdown when idle connections are reaped (https://github.com/nginx/nginx/blob/master/src/core/ngx_connection.c#L1363), due to the fact that http2 connections are always `idle`, ongoing streams end up prematurely getting closed on the read side(sends GOAWAY frame). It looks like the `idle` connection implementation as it is today only answers the question - “Can nginx prematurely close the streams when shutting down?” Is the above behavior intentional? It appears from the name of the flag (`idle`) that the intent is to reflect the dynamic state of the connection (are reads and writes happening for the connection for any of the streams) but the implementation does not reflect that. What is the intended behavior ? We would have liked a `idle` flag with its corresponding timeout configuration that lets us control the behavior during shutdown (and other flows). Thanks in advance Jojy___ nginx-devel mailing list -- nginx-devel@nginx.org To unsubscribe send an email to nginx-devel-le...@nginx.org
Re: [PATCH] HTTP/2: premalloc http2 recv_buffer in init_conf
Hello! On Mon, Sep 07, 2020 at 08:06:06PM +0800, Kasei Wang wrote: > # HG changeset patch > # User Kasei Wang > # Date 1599480224 -28800 > # Mon Sep 07 20:03:44 2020 +0800 > # Node ID 1532a8fca7b24c3c3aa5cddd3362402d15f57a4f > # Parent 87d2ea860f380dc8418c97c0163412f53c2d008e > HTTP/2: premalloc http2 recv_buffer in init_conf > > There is no need to check if h2mcf->recv_buffer malloced every time. > > diff -r 87d2ea860f38 -r 1532a8fca7b2 src/http/v2/ngx_http_v2.c > --- a/src/http/v2/ngx_http_v2.cMon Sep 10 18:57:39 2018 +0300 > +++ b/src/http/v2/ngx_http_v2.cMon Sep 07 20:03:44 2020 +0800 > @@ -233,7 +233,6 @@ > ngx_pool_cleanup_t*cln; > ngx_http_connection_t *hc; > ngx_http_v2_srv_conf_t*h2scf; > -ngx_http_v2_main_conf_t *h2mcf; > ngx_http_v2_connection_t *h2c; > > c = rev->data; > @@ -243,17 +242,6 @@ > > c->log->action = "processing HTTP/2 connection"; > > -h2mcf = ngx_http_get_module_main_conf(hc->conf_ctx, ngx_http_v2_module); > - > -if (h2mcf->recv_buffer == NULL) { > -h2mcf->recv_buffer = ngx_palloc(ngx_cycle->pool, > -h2mcf->recv_buffer_size); > -if (h2mcf->recv_buffer == NULL) { > -ngx_http_close_connection(c); > -return; > -} > -} > - > h2c = ngx_pcalloc(c->pool, sizeof(ngx_http_v2_connection_t)); > if (h2c == NULL) { > ngx_http_close_connection(c); > diff -r 87d2ea860f38 -r 1532a8fca7b2 src/http/v2/ngx_http_v2_module.c > --- a/src/http/v2/ngx_http_v2_module.cMon Sep 10 18:57:39 2018 +0300 > +++ b/src/http/v2/ngx_http_v2_module.cMon Sep 07 20:03:44 2020 +0800 > @@ -334,6 +334,10 @@ > ngx_http_v2_main_conf_t *h2mcf = conf; > > ngx_conf_init_size_value(h2mcf->recv_buffer_size, 256 * 1024); > +h2mcf->recv_buffer = ngx_pcalloc(cf->pool, h2mcf->recv_buffer_size); > +if (h2mcf->recv_buffer == NULL) { > +return NGX_CONF_ERROR; > +} > > return NGX_CONF_OK; > } Thank you for the patch. While current approach implies some run-time costs, these costs are believed to be very small and not even measurable. On the other hand, it saves relatively large buffer allocation if HTTP/2 is compiled in but not used, and this might be important in some setups. -- Maxim Dounin http://mdounin.ru/ ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] HTTP/2: premalloc http2 recv_buffer in init_conf
# HG changeset patch # User Kasei Wang # Date 1599480224 -28800 # Mon Sep 07 20:03:44 2020 +0800 # Node ID 1532a8fca7b24c3c3aa5cddd3362402d15f57a4f # Parent 87d2ea860f380dc8418c97c0163412f53c2d008e HTTP/2: premalloc http2 recv_buffer in init_conf There is no need to check if h2mcf->recv_buffer malloced every time. diff -r 87d2ea860f38 -r 1532a8fca7b2 src/http/v2/ngx_http_v2.c --- a/src/http/v2/ngx_http_v2.cMon Sep 10 18:57:39 2018 +0300 +++ b/src/http/v2/ngx_http_v2.cMon Sep 07 20:03:44 2020 +0800 @@ -233,7 +233,6 @@ ngx_pool_cleanup_t*cln; ngx_http_connection_t *hc; ngx_http_v2_srv_conf_t*h2scf; -ngx_http_v2_main_conf_t *h2mcf; ngx_http_v2_connection_t *h2c; c = rev->data; @@ -243,17 +242,6 @@ c->log->action = "processing HTTP/2 connection"; -h2mcf = ngx_http_get_module_main_conf(hc->conf_ctx, ngx_http_v2_module); - -if (h2mcf->recv_buffer == NULL) { -h2mcf->recv_buffer = ngx_palloc(ngx_cycle->pool, -h2mcf->recv_buffer_size); -if (h2mcf->recv_buffer == NULL) { -ngx_http_close_connection(c); -return; -} -} - h2c = ngx_pcalloc(c->pool, sizeof(ngx_http_v2_connection_t)); if (h2c == NULL) { ngx_http_close_connection(c); diff -r 87d2ea860f38 -r 1532a8fca7b2 src/http/v2/ngx_http_v2_module.c --- a/src/http/v2/ngx_http_v2_module.cMon Sep 10 18:57:39 2018 +0300 +++ b/src/http/v2/ngx_http_v2_module.cMon Sep 07 20:03:44 2020 +0800 @@ -334,6 +334,10 @@ ngx_http_v2_main_conf_t *h2mcf = conf; ngx_conf_init_size_value(h2mcf->recv_buffer_size, 256 * 1024); +h2mcf->recv_buffer = ngx_pcalloc(cf->pool, h2mcf->recv_buffer_size); +if (h2mcf->recv_buffer == NULL) { +return NGX_CONF_ERROR; +} return NGX_CONF_OK; } ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
http2.
Hi, I am trying to understand how the modules work when dealing with http2 traffic? Trying to find any documentation or any other info that will tells me how a module should behave when handling http2 traffic. I am assuming the actual http2 protocol parsing is done by the http2 module. How can a module access the headers, params, body and other data in the request/response? Any info is greatly appreciated. Thanks. Dk. ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: http2 broken
Get it, I'll try it, thank you again. On Thu, Jul 12, 2018 at 1:02 AM Maxim Dounin wrote: > Hello! > > On Wed, Jul 11, 2018 at 08:56:56PM +0800, 洪志道 wrote: > > > But there are still some clients use the old version. > > Is it a way to be compatible with it? > > Or other suggestions? > > @Maxim Dounin > > As I already wrote in ticket #1397, we can try introducing a > workaround in nginx, but I'm not convinced it worth the effort and, > more importantly, associated downsides for non-broken clients. > > # HG changeset patch > # User Maxim Dounin > # Date 1531328359 -10800 > # Wed Jul 11 19:59:19 2018 +0300 > # Node ID f44e74d331885dce8682d2a9802eb707be7ae86f > # Parent 54683f650cbdcd73f7f8d845c843295978da5a85 > HTTP/2: workaround for clients which fail on table size updates. > > There are clients which cannot handle HPACK's dynamic table size updates > as added in 12cadc4669a7 (1.13.6). Notably, old versions of OkHttp library > are known to fail on it (ticket #1397). > > This change makes it possible to work with such clients by only sending > dynamic table size updates in response to SETTINGS_HEADER_TABLE_SIZE. As > a downside, clients which do not use SETTINGS_HEADER_TABLE_SIZE will > continue to maintain default 4k table. > > diff --git a/src/http/v2/ngx_http_v2.c b/src/http/v2/ngx_http_v2.c > --- a/src/http/v2/ngx_http_v2.c > +++ b/src/http/v2/ngx_http_v2.c > @@ -270,8 +270,6 @@ 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->concurrent_pushes = h2scf->concurrent_pushes; > @@ -2075,6 +2073,11 @@ ngx_http_v2_state_settings_params(ngx_ht > h2c->concurrent_pushes = ngx_min(value, > h2scf->concurrent_pushes); > break; > > +case NGX_HTTP_V2_HEADER_TABLE_SIZE_SETTING: > + > +h2c->table_update = 1; > +break; > + > default: > break; > } > > -- > Maxim Dounin > http://mdounin.ru/ > ___ > nginx-devel mailing list > nginx-devel@nginx.org > http://mailman.nginx.org/mailman/listinfo/nginx-devel ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: http2 broken
Hello! On Wed, Jul 11, 2018 at 08:56:56PM +0800, 洪志道 wrote: > But there are still some clients use the old version. > Is it a way to be compatible with it? > Or other suggestions? > @Maxim Dounin As I already wrote in ticket #1397, we can try introducing a workaround in nginx, but I'm not convinced it worth the effort and, more importantly, associated downsides for non-broken clients. # HG changeset patch # User Maxim Dounin # Date 1531328359 -10800 # Wed Jul 11 19:59:19 2018 +0300 # Node ID f44e74d331885dce8682d2a9802eb707be7ae86f # Parent 54683f650cbdcd73f7f8d845c843295978da5a85 HTTP/2: workaround for clients which fail on table size updates. There are clients which cannot handle HPACK's dynamic table size updates as added in 12cadc4669a7 (1.13.6). Notably, old versions of OkHttp library are known to fail on it (ticket #1397). This change makes it possible to work with such clients by only sending dynamic table size updates in response to SETTINGS_HEADER_TABLE_SIZE. As a downside, clients which do not use SETTINGS_HEADER_TABLE_SIZE will continue to maintain default 4k table. diff --git a/src/http/v2/ngx_http_v2.c b/src/http/v2/ngx_http_v2.c --- a/src/http/v2/ngx_http_v2.c +++ b/src/http/v2/ngx_http_v2.c @@ -270,8 +270,6 @@ 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->concurrent_pushes = h2scf->concurrent_pushes; @@ -2075,6 +2073,11 @@ ngx_http_v2_state_settings_params(ngx_ht h2c->concurrent_pushes = ngx_min(value, h2scf->concurrent_pushes); break; +case NGX_HTTP_V2_HEADER_TABLE_SIZE_SETTING: + +h2c->table_update = 1; +break; + default: break; } -- Maxim Dounin http://mdounin.ru/ ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: http2 broken
Thank you. But there are still some clients use the old version. Is it a way to be compatible with it? Or other suggestions? @Maxim Dounin On Wed, Jul 11, 2018 at 5:43 PM tokers wrote: > Hello! > > There is a known issue caused by some obsolete client like old version > okhttp (incomplete protocol implements). > Here is the ticket: https://trac.nginx.org/nginx/ticket/1397. > ___ > nginx-devel mailing list > nginx-devel@nginx.org > http://mailman.nginx.org/mailman/listinfo/nginx-devel ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: http2 broken
Hello! There is a known issue caused by some obsolete client like old version okhttp (incomplete protocol implements). Here is the ticket: https://trac.nginx.org/nginx/ticket/1397. ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: http2 broken
BTW. It works well by the temporary fix. --- a/src/http/v2/ngx_http_v2_filter_module.c +++ b/src/http/v2/ngx_http_v2_filter_module.c @@ -446,7 +446,6 @@ ngx_http_v2_header_filter(ngx_http_request_t *r) if (h2c->table_update) { ngx_log_debug0(NGX_LOG_DEBUG_HTTP, fc->log, 0, "http2 table size update: 0"); -*pos++ = (1 << 5) | 0; h2c->table_update = 0; } @@ -1010,7 +1009,6 @@ ngx_http_v2_push_resource(ngx_http_request_t *r, ngx_str_t *pat if (h2c->table_update) { ngx_log_debug0(NGX_LOG_DEBUG_HTTP, fc->log, 0, "http2 table size update: 0"); -*pos++ = (1 << 5) | 0; h2c->table_update = 0; } On Wed, Jul 11, 2018 at 5:28 PM 洪志道 wrote: > Hi. > > The client can't load the image served by nginx-1.13.6+. > [App with Android 7.1.2 4.4.4 4.12 series] > > It works well in nginx-1.13.5, but go wrong in ngnx-1.13.6+. > The change is http://hg.nginx.org/nginx/rev/12cadc4669a7. > > I found the similar problem. > https://github.com/cloudflare/sslconfig/issues/83 > > Now I don't know where the problem is. NGINX or Client? > > Does anyone know? > Thanks. > ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
http2 broken
Hi. The client can't load the image served by nginx-1.13.6+. [App with Android 7.1.2 4.4.4 4.12 series] It works well in nginx-1.13.5, but go wrong in ngnx-1.13.6+. The change is http://hg.nginx.org/nginx/rev/12cadc4669a7. I found the similar problem. https://github.com/cloudflare/sslconfig/issues/83 Now I don't know where the problem is. NGINX or Client? Does anyone know? Thanks. ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] HTTP/2: make http2 server support http1
hello? 发自我的 iPhone > 在 2018年4月2日,08:28,Haitao Lv 写道: > > Any body is here? > >> On Mar 21, 2018, at 11:36, Haitao Lv wrote: >> >> Thank you for reviewing. >> >> And here is the patch that fix the breaking PROXY protocol functionality. >> >> Sorry for disturbing. >> >> diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c >> index 2db7a627..9f1b8544 100644 >> --- a/src/http/ngx_http_request.c >> +++ b/src/http/ngx_http_request.c >> @@ -17,6 +17,10 @@ static ssize_t >> ngx_http_read_request_header(ngx_http_request_t *r); >> static ngx_int_t ngx_http_alloc_large_header_buffer(ngx_http_request_t *r, >>ngx_uint_t request_line); >> >> +#if (NGX_HTTP_V2) >> +static void ngx_http_wait_v2_preface_handler(ngx_event_t *rev); >> +#endif >> + >> 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_unique_header_line(ngx_http_request_t *r, >> @@ -325,7 +329,7 @@ ngx_http_init_connection(ngx_connection_t *c) >> >> #if (NGX_HTTP_V2) >>if (hc->addr_conf->http2) { >> -rev->handler = ngx_http_v2_init; >> +rev->handler = ngx_http_wait_v2_preface_handler; >>} >> #endif >> >> @@ -381,6 +385,131 @@ ngx_http_init_connection(ngx_connection_t *c) >> } >> >> >> +#if (NGX_HTTP_V2) >> +static void >> +ngx_http_wait_v2_preface_handler(ngx_event_t *rev) >> +{ >> +size_t size; >> +ssize_tn; >> +u_char*p; >> +ngx_buf_t *b; >> +ngx_connection_t *c; >> +ngx_http_connection_t *hc; >> +static const u_charpreface[] = "PRI"; >> + >> +c = rev->data; >> +hc = c->data; >> + >> +size = sizeof(preface) - 1; >> + >> +if (hc->proxy_protocol) { >> +size += NGX_PROXY_PROTOCOL_MAX_HEADER; >> +} >> + >> +ngx_log_debug0(NGX_LOG_DEBUG_HTTP, c->log, 0, >> +"http wait h2 preface handler"); >> + >> +if (rev->timedout) { >> +ngx_log_error(NGX_LOG_INFO, c->log, NGX_ETIMEDOUT, "client timed >> out"); >> +ngx_http_close_connection(c); >> +return; >> +} >> + >> +if (c->close) { >> +ngx_http_close_connection(c); >> +return; >> +} >> + >> +b = c->buffer; >> + >> +if (b == NULL) { >> +b = ngx_create_temp_buf(c->pool, size); >> +if (b == NULL) { >> +ngx_http_close_connection(c); >> +return; >> +} >> + >> +c->buffer = b; >> + >> +} else if (b->start == NULL) { >> + >> +b->start = ngx_palloc(c->pool, size); >> +if (b->start == NULL) { >> +ngx_http_close_connection(c); >> +return; >> +} >> + >> +b->pos = b->start; >> +b->last = b->start; >> +b->end = b->last + size; >> +} >> + >> +n = c->recv(c, b->last, b->end - b->last); >> + >> +if (n == NGX_AGAIN) { >> + >> +if (!rev->timer_set) { >> +ngx_add_timer(rev, c->listening->post_accept_timeout); >> +ngx_reusable_connection(c, 1); >> +} >> + >> +if (ngx_handle_read_event(rev, 0) != NGX_OK) { >> +ngx_http_close_connection(c); >> +return; >> +} >> + >> +/* >> + * We are trying to not hold c->buffer's memory for an idle >> connection. >> + */ >> + >> +if (ngx_pfree(c->pool, b->start) == NGX_OK) { >> +b->start = NULL; >> +} >> + >> +return; >> +} >> + >> +if (n == NGX_ERROR) { >> +ngx_http_close_connection(c); >> +return; >> +} >> + >> +if (n == 0) { >> +ngx_log_error(NGX_LOG_INFO, c->log, 0, >> + "client closed connection"); >> +ngx_http_close_connection(c); >> +return; >> +} >> + >> +b->last += n; >> + >> +if (hc->proxy_protocol) { >> +hc->proxy_protocol = 0; >> + >
Re: [PATCH] HTTP/2: make http2 server support http1
Any body is here? > On Mar 21, 2018, at 11:36, Haitao Lv <i...@lvht.net> wrote: > > Thank you for reviewing. > > And here is the patch that fix the breaking PROXY protocol functionality. > > Sorry for disturbing. > > diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c > index 2db7a627..9f1b8544 100644 > --- a/src/http/ngx_http_request.c > +++ b/src/http/ngx_http_request.c > @@ -17,6 +17,10 @@ static ssize_t > ngx_http_read_request_header(ngx_http_request_t *r); > static ngx_int_t ngx_http_alloc_large_header_buffer(ngx_http_request_t *r, > ngx_uint_t request_line); > > +#if (NGX_HTTP_V2) > +static void ngx_http_wait_v2_preface_handler(ngx_event_t *rev); > +#endif > + > 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_unique_header_line(ngx_http_request_t *r, > @@ -325,7 +329,7 @@ ngx_http_init_connection(ngx_connection_t *c) > > #if (NGX_HTTP_V2) > if (hc->addr_conf->http2) { > -rev->handler = ngx_http_v2_init; > +rev->handler = ngx_http_wait_v2_preface_handler; > } > #endif > > @@ -381,6 +385,131 @@ ngx_http_init_connection(ngx_connection_t *c) > } > > > +#if (NGX_HTTP_V2) > +static void > +ngx_http_wait_v2_preface_handler(ngx_event_t *rev) > +{ > +size_t size; > +ssize_tn; > +u_char*p; > +ngx_buf_t *b; > +ngx_connection_t *c; > +ngx_http_connection_t *hc; > +static const u_charpreface[] = "PRI"; > + > +c = rev->data; > +hc = c->data; > + > +size = sizeof(preface) - 1; > + > +if (hc->proxy_protocol) { > +size += NGX_PROXY_PROTOCOL_MAX_HEADER; > +} > + > +ngx_log_debug0(NGX_LOG_DEBUG_HTTP, c->log, 0, > +"http wait h2 preface handler"); > + > +if (rev->timedout) { > +ngx_log_error(NGX_LOG_INFO, c->log, NGX_ETIMEDOUT, "client timed > out"); > +ngx_http_close_connection(c); > +return; > +} > + > +if (c->close) { > +ngx_http_close_connection(c); > +return; > +} > + > +b = c->buffer; > + > +if (b == NULL) { > +b = ngx_create_temp_buf(c->pool, size); > +if (b == NULL) { > +ngx_http_close_connection(c); > +return; > +} > + > +c->buffer = b; > + > +} else if (b->start == NULL) { > + > +b->start = ngx_palloc(c->pool, size); > +if (b->start == NULL) { > +ngx_http_close_connection(c); > +return; > +} > + > +b->pos = b->start; > +b->last = b->start; > +b->end = b->last + size; > +} > + > +n = c->recv(c, b->last, b->end - b->last); > + > +if (n == NGX_AGAIN) { > + > +if (!rev->timer_set) { > +ngx_add_timer(rev, c->listening->post_accept_timeout); > +ngx_reusable_connection(c, 1); > +} > + > +if (ngx_handle_read_event(rev, 0) != NGX_OK) { > +ngx_http_close_connection(c); > +return; > +} > + > +/* > + * We are trying to not hold c->buffer's memory for an idle > connection. > + */ > + > +if (ngx_pfree(c->pool, b->start) == NGX_OK) { > +b->start = NULL; > +} > + > +return; > +} > + > +if (n == NGX_ERROR) { > +ngx_http_close_connection(c); > +return; > +} > + > +if (n == 0) { > +ngx_log_error(NGX_LOG_INFO, c->log, 0, > + "client closed connection"); > +ngx_http_close_connection(c); > +return; > +} > + > +b->last += n; > + > +if (hc->proxy_protocol) { > +hc->proxy_protocol = 0; > + > +p = ngx_proxy_protocol_read(c, b->pos, b->last); > + > +if (p == NULL) { > +ngx_http_close_connection(c); > +return; > +} > + > +b->pos = p; > +} > + > +if (b->last >= b->pos + sizeof(preface) - 1) { > +/* b will be freed in ngx_http_v2_init/ngx_http_wait_request_handler > */ > + > +if (ngx_strncmp(b->pos, preface, sizeof(preface) - 1) == 0) { > +ngx_http_v2_init(rev); > +} else { > +rev->handler = ngx_http_wait_request_h
Re: [PATCH] HTTP/2: make http2 server support http1
Thank you for reviewing. And here is the patch that fix the breaking PROXY protocol functionality. Sorry for disturbing. diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c index 2db7a627..9f1b8544 100644 --- a/src/http/ngx_http_request.c +++ b/src/http/ngx_http_request.c @@ -17,6 +17,10 @@ static ssize_t ngx_http_read_request_header(ngx_http_request_t *r); static ngx_int_t ngx_http_alloc_large_header_buffer(ngx_http_request_t *r, ngx_uint_t request_line); +#if (NGX_HTTP_V2) +static void ngx_http_wait_v2_preface_handler(ngx_event_t *rev); +#endif + 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_unique_header_line(ngx_http_request_t *r, @@ -325,7 +329,7 @@ ngx_http_init_connection(ngx_connection_t *c) #if (NGX_HTTP_V2) if (hc->addr_conf->http2) { -rev->handler = ngx_http_v2_init; +rev->handler = ngx_http_wait_v2_preface_handler; } #endif @@ -381,6 +385,131 @@ ngx_http_init_connection(ngx_connection_t *c) } +#if (NGX_HTTP_V2) +static void +ngx_http_wait_v2_preface_handler(ngx_event_t *rev) +{ +size_t size; +ssize_tn; +u_char*p; +ngx_buf_t *b; +ngx_connection_t *c; +ngx_http_connection_t *hc; +static const u_charpreface[] = "PRI"; + +c = rev->data; +hc = c->data; + +size = sizeof(preface) - 1; + +if (hc->proxy_protocol) { +size += NGX_PROXY_PROTOCOL_MAX_HEADER; +} + +ngx_log_debug0(NGX_LOG_DEBUG_HTTP, c->log, 0, +"http wait h2 preface handler"); + +if (rev->timedout) { +ngx_log_error(NGX_LOG_INFO, c->log, NGX_ETIMEDOUT, "client timed out"); +ngx_http_close_connection(c); +return; +} + +if (c->close) { +ngx_http_close_connection(c); +return; +} + +b = c->buffer; + +if (b == NULL) { +b = ngx_create_temp_buf(c->pool, size); +if (b == NULL) { +ngx_http_close_connection(c); +return; +} + +c->buffer = b; + +} else if (b->start == NULL) { + +b->start = ngx_palloc(c->pool, size); +if (b->start == NULL) { +ngx_http_close_connection(c); +return; +} + +b->pos = b->start; +b->last = b->start; +b->end = b->last + size; +} + +n = c->recv(c, b->last, b->end - b->last); + +if (n == NGX_AGAIN) { + +if (!rev->timer_set) { +ngx_add_timer(rev, c->listening->post_accept_timeout); +ngx_reusable_connection(c, 1); +} + +if (ngx_handle_read_event(rev, 0) != NGX_OK) { +ngx_http_close_connection(c); +return; +} + +/* + * We are trying to not hold c->buffer's memory for an idle connection. + */ + +if (ngx_pfree(c->pool, b->start) == NGX_OK) { +b->start = NULL; +} + +return; +} + +if (n == NGX_ERROR) { +ngx_http_close_connection(c); +return; +} + +if (n == 0) { +ngx_log_error(NGX_LOG_INFO, c->log, 0, + "client closed connection"); +ngx_http_close_connection(c); +return; +} + +b->last += n; + +if (hc->proxy_protocol) { +hc->proxy_protocol = 0; + +p = ngx_proxy_protocol_read(c, b->pos, b->last); + +if (p == NULL) { +ngx_http_close_connection(c); +return; +} + +b->pos = p; +} + +if (b->last >= b->pos + sizeof(preface) - 1) { +/* b will be freed in ngx_http_v2_init/ngx_http_wait_request_handler */ + +if (ngx_strncmp(b->pos, preface, sizeof(preface) - 1) == 0) { +ngx_http_v2_init(rev); +} else { +rev->handler = ngx_http_wait_request_handler; +ngx_http_wait_request_handler(rev); +} +} +} +#endif + + static void ngx_http_wait_request_handler(ngx_event_t *rev) { @@ -393,6 +522,7 @@ ngx_http_wait_request_handler(ngx_event_t *rev) ngx_http_core_srv_conf_t *cscf; c = rev->data; +n = NGX_AGAIN; ngx_log_debug0(NGX_LOG_DEBUG_HTTP, c->log, 0, "http wait request handler"); @@ -434,9 +564,27 @@ ngx_http_wait_request_handler(ngx_event_t *rev) b->pos = b->start; b->last = b->start; b->end = b->last + size; +} else { + +p = ngx_palloc(c->pool, size); +if (p == NULL) { +ngx_http_close_connection(c); +return; +} + +n = b->last - b->pos; +ngx_memcpy(p, b->pos, n); +ngx_pfree(c->pool, b->star
Re: [PATCH] HTTP/2: make http2 server support http1
On Thursday 08 March 2018 08:42:27 Haitao Lv wrote: > Sorry for disturbing. But I have to fix a buffer overflow bug. > Here is the latest patch. > > Sorry. But please make your comments. Thank you. [..] There's no way for this patch to be accepted as it breaks PROXY protocol functionality. wbr, Valentin V. Bartenev ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] HTTP/2: make http2 server support http1
ping @wbr > On Mar 13, 2018, at 01:11, Valentin V. Bartenev <vb...@nginx.com> wrote: > > I will look when time permits. > > But at the first glance the patch still look too complicated than it should > be. > > wbr, Valentin V. Bartenev > > > On Tuesday 13 March 2018 00:44:28 吕海涛 wrote: >> Is there any one who would like to review this patch? >> >> 发自我的 iPhone >> >>> 在 2018年3月8日,08:42,Haitao Lv <i...@lvht.net> 写道: >>> >>> Sorry for disturbing. But I have to fix a buffer overflow bug. >>> Here is the latest patch. >>> >>> Sorry. But please make your comments. Thank you. >>> >>> >>> diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c >>> index 89cfe77a..c51d8ace 100644 >>> --- a/src/http/ngx_http_request.c >>> +++ b/src/http/ngx_http_request.c >>> @@ -17,6 +17,10 @@ static ssize_t >>> ngx_http_read_request_header(ngx_http_request_t *r); >>> static ngx_int_t ngx_http_alloc_large_header_buffer(ngx_http_request_t *r, >>> ngx_uint_t request_line); >>> >>> +#if (NGX_HTTP_V2) >>> +static void ngx_http_wait_v2_preface_handler(ngx_event_t *rev); >>> +#endif >>> + >>> 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_unique_header_line(ngx_http_request_t *r, >>> @@ -321,7 +325,7 @@ ngx_http_init_connection(ngx_connection_t *c) >>> >>> #if (NGX_HTTP_V2) >>> if (hc->addr_conf->http2) { >>> -rev->handler = ngx_http_v2_init; >>> +rev->handler = ngx_http_wait_v2_preface_handler; >>> } >>> #endif >>> >>> @@ -377,6 +381,110 @@ ngx_http_init_connection(ngx_connection_t *c) >>> } >>> >>> >>> +#if (NGX_HTTP_V2) >>> +static void >>> +ngx_http_wait_v2_preface_handler(ngx_event_t *rev) >>> +{ >>> +size_t size; >>> +ssize_tn; >>> +ngx_buf_t *b; >>> +ngx_connection_t *c; >>> +static const u_charpreface[] = "PRI"; >>> + >>> +c = rev->data; >>> +size = sizeof(preface) - 1; >>> + >>> +ngx_log_debug0(NGX_LOG_DEBUG_HTTP, c->log, 0, >>> +"http wait h2 preface handler"); >>> + >>> +if (rev->timedout) { >>> +ngx_log_error(NGX_LOG_INFO, c->log, NGX_ETIMEDOUT, "client timed >>> out"); >>> +ngx_http_close_connection(c); >>> +return; >>> +} >>> + >>> +if (c->close) { >>> +ngx_http_close_connection(c); >>> +return; >>> +} >>> + >>> +b = c->buffer; >>> + >>> +if (b == NULL) { >>> +b = ngx_create_temp_buf(c->pool, size); >>> +if (b == NULL) { >>> +ngx_http_close_connection(c); >>> +return; >>> +} >>> + >>> +c->buffer = b; >>> + >>> +} else if (b->start == NULL) { >>> + >>> +b->start = ngx_palloc(c->pool, size); >>> +if (b->start == NULL) { >>> +ngx_http_close_connection(c); >>> +return; >>> +} >>> + >>> +b->pos = b->start; >>> +b->last = b->start; >>> +b->end = b->last + size; >>> +} >>> + >>> +n = c->recv(c, b->last, b->end - b->last); >>> + >>> +if (n == NGX_AGAIN) { >>> + >>> +if (!rev->timer_set) { >>> +ngx_add_timer(rev, c->listening->post_accept_timeout); >>> +ngx_reusable_connection(c, 1); >>> +} >>> + >>> +if (ngx_handle_read_event(rev, 0) != NGX_OK) { >>> +ngx_http_close_connection(c); >>> +return; >>> +} >>> + >>> +/* >>> + * We are trying to not hold c->buffer's memory for an idle >>> connection. >>> + */ >>> + >>> +if (ngx_pfree(c->pool, b->start) == NGX_OK) { >>> +b->start = NULL; >>> +} >>> + >>> +
Re: [PATCH] HTTP/2: make http2 server support http1
I will look when time permits. But at the first glance the patch still look too complicated than it should be. wbr, Valentin V. Bartenev On Tuesday 13 March 2018 00:44:28 吕海涛 wrote: > Is there any one who would like to review this patch? > > 发自我的 iPhone > > > 在 2018年3月8日,08:42,Haitao Lv <i...@lvht.net> 写道: > > > > Sorry for disturbing. But I have to fix a buffer overflow bug. > > Here is the latest patch. > > > > Sorry. But please make your comments. Thank you. > > > > > > diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c > > index 89cfe77a..c51d8ace 100644 > > --- a/src/http/ngx_http_request.c > > +++ b/src/http/ngx_http_request.c > > @@ -17,6 +17,10 @@ static ssize_t > > ngx_http_read_request_header(ngx_http_request_t *r); > > static ngx_int_t ngx_http_alloc_large_header_buffer(ngx_http_request_t *r, > >ngx_uint_t request_line); > > > > +#if (NGX_HTTP_V2) > > +static void ngx_http_wait_v2_preface_handler(ngx_event_t *rev); > > +#endif > > + > > 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_unique_header_line(ngx_http_request_t *r, > > @@ -321,7 +325,7 @@ ngx_http_init_connection(ngx_connection_t *c) > > > > #if (NGX_HTTP_V2) > >if (hc->addr_conf->http2) { > > -rev->handler = ngx_http_v2_init; > > +rev->handler = ngx_http_wait_v2_preface_handler; > >} > > #endif > > > > @@ -377,6 +381,110 @@ ngx_http_init_connection(ngx_connection_t *c) > > } > > > > > > +#if (NGX_HTTP_V2) > > +static void > > +ngx_http_wait_v2_preface_handler(ngx_event_t *rev) > > +{ > > +size_t size; > > +ssize_tn; > > +ngx_buf_t *b; > > +ngx_connection_t *c; > > +static const u_charpreface[] = "PRI"; > > + > > +c = rev->data; > > +size = sizeof(preface) - 1; > > + > > +ngx_log_debug0(NGX_LOG_DEBUG_HTTP, c->log, 0, > > +"http wait h2 preface handler"); > > + > > +if (rev->timedout) { > > +ngx_log_error(NGX_LOG_INFO, c->log, NGX_ETIMEDOUT, "client timed > > out"); > > +ngx_http_close_connection(c); > > +return; > > +} > > + > > +if (c->close) { > > +ngx_http_close_connection(c); > > +return; > > +} > > + > > +b = c->buffer; > > + > > +if (b == NULL) { > > +b = ngx_create_temp_buf(c->pool, size); > > +if (b == NULL) { > > +ngx_http_close_connection(c); > > +return; > > +} > > + > > +c->buffer = b; > > + > > +} else if (b->start == NULL) { > > + > > +b->start = ngx_palloc(c->pool, size); > > +if (b->start == NULL) { > > +ngx_http_close_connection(c); > > +return; > > +} > > + > > +b->pos = b->start; > > +b->last = b->start; > > +b->end = b->last + size; > > +} > > + > > +n = c->recv(c, b->last, b->end - b->last); > > + > > +if (n == NGX_AGAIN) { > > + > > +if (!rev->timer_set) { > > +ngx_add_timer(rev, c->listening->post_accept_timeout); > > +ngx_reusable_connection(c, 1); > > +} > > + > > +if (ngx_handle_read_event(rev, 0) != NGX_OK) { > > +ngx_http_close_connection(c); > > +return; > > +} > > + > > +/* > > + * We are trying to not hold c->buffer's memory for an idle > > connection. > > + */ > > + > > +if (ngx_pfree(c->pool, b->start) == NGX_OK) { > > +b->start = NULL; > > +} > > + > > +return; > > +} > > + > > +if (n == NGX_ERROR) { > > +ngx_http_close_connection(c); > > +return; > > +} > > + > > +if (n == 0) { > > +ngx_log_error(NGX_LOG_INFO, c->log, 0, > > + "client closed connection"); > > +ngx_http_close_connection(c); > > +return; > > +} > > + > > +b->last += n; > > + > > +if (b->la
Re: [PATCH] HTTP/2: make http2 server support http1
Is there any one who would like to review this patch? 发自我的 iPhone > 在 2018年3月8日,08:42,Haitao Lv <i...@lvht.net> 写道: > > Sorry for disturbing. But I have to fix a buffer overflow bug. > Here is the latest patch. > > Sorry. But please make your comments. Thank you. > > > diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c > index 89cfe77a..c51d8ace 100644 > --- a/src/http/ngx_http_request.c > +++ b/src/http/ngx_http_request.c > @@ -17,6 +17,10 @@ static ssize_t > ngx_http_read_request_header(ngx_http_request_t *r); > static ngx_int_t ngx_http_alloc_large_header_buffer(ngx_http_request_t *r, >ngx_uint_t request_line); > > +#if (NGX_HTTP_V2) > +static void ngx_http_wait_v2_preface_handler(ngx_event_t *rev); > +#endif > + > 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_unique_header_line(ngx_http_request_t *r, > @@ -321,7 +325,7 @@ ngx_http_init_connection(ngx_connection_t *c) > > #if (NGX_HTTP_V2) >if (hc->addr_conf->http2) { > -rev->handler = ngx_http_v2_init; > +rev->handler = ngx_http_wait_v2_preface_handler; >} > #endif > > @@ -377,6 +381,110 @@ ngx_http_init_connection(ngx_connection_t *c) > } > > > +#if (NGX_HTTP_V2) > +static void > +ngx_http_wait_v2_preface_handler(ngx_event_t *rev) > +{ > +size_t size; > +ssize_tn; > +ngx_buf_t *b; > +ngx_connection_t *c; > +static const u_charpreface[] = "PRI"; > + > +c = rev->data; > +size = sizeof(preface) - 1; > + > +ngx_log_debug0(NGX_LOG_DEBUG_HTTP, c->log, 0, > +"http wait h2 preface handler"); > + > +if (rev->timedout) { > +ngx_log_error(NGX_LOG_INFO, c->log, NGX_ETIMEDOUT, "client timed > out"); > +ngx_http_close_connection(c); > +return; > +} > + > +if (c->close) { > +ngx_http_close_connection(c); > +return; > +} > + > +b = c->buffer; > + > +if (b == NULL) { > +b = ngx_create_temp_buf(c->pool, size); > +if (b == NULL) { > +ngx_http_close_connection(c); > +return; > +} > + > +c->buffer = b; > + > +} else if (b->start == NULL) { > + > +b->start = ngx_palloc(c->pool, size); > +if (b->start == NULL) { > +ngx_http_close_connection(c); > +return; > +} > + > +b->pos = b->start; > +b->last = b->start; > +b->end = b->last + size; > +} > + > +n = c->recv(c, b->last, b->end - b->last); > + > +if (n == NGX_AGAIN) { > + > +if (!rev->timer_set) { > +ngx_add_timer(rev, c->listening->post_accept_timeout); > +ngx_reusable_connection(c, 1); > +} > + > +if (ngx_handle_read_event(rev, 0) != NGX_OK) { > +ngx_http_close_connection(c); > +return; > +} > + > +/* > + * We are trying to not hold c->buffer's memory for an idle > connection. > + */ > + > +if (ngx_pfree(c->pool, b->start) == NGX_OK) { > +b->start = NULL; > +} > + > +return; > +} > + > +if (n == NGX_ERROR) { > +ngx_http_close_connection(c); > +return; > +} > + > +if (n == 0) { > +ngx_log_error(NGX_LOG_INFO, c->log, 0, > + "client closed connection"); > +ngx_http_close_connection(c); > +return; > +} > + > +b->last += n; > + > +if (b->last == b->end) { > +/* b will be freed in ngx_http_v2_init/ngx_http_wait_request_handler > */ > + > +if (ngx_strncmp(b->start, preface, size) == 0) { > +ngx_http_v2_init(rev); > +} else { > +rev->handler = ngx_http_wait_request_handler; > +ngx_http_wait_request_handler(rev); > +} > +} > +} > +#endif > + > + > static void > ngx_http_wait_request_handler(ngx_event_t *rev) > { > @@ -430,6 +538,22 @@ ngx_http_wait_request_handler(ngx_event_t *rev) >b->pos = b->start; >b->last = b->start; >b->end = b->last + size; > +} else { > + > +p = ngx_palloc(c->pool, size); > +if (p == NULL) { > +ngx_http_close_connection(c); >
Re: [PATCH] HTTP/2: make http2 server support http1
Sorry for disturbing. But I have to fix a buffer overflow bug. Here is the latest patch. Sorry. But please make your comments. Thank you. diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c index 89cfe77a..c51d8ace 100644 --- a/src/http/ngx_http_request.c +++ b/src/http/ngx_http_request.c @@ -17,6 +17,10 @@ static ssize_t ngx_http_read_request_header(ngx_http_request_t *r); static ngx_int_t ngx_http_alloc_large_header_buffer(ngx_http_request_t *r, ngx_uint_t request_line); +#if (NGX_HTTP_V2) +static void ngx_http_wait_v2_preface_handler(ngx_event_t *rev); +#endif + 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_unique_header_line(ngx_http_request_t *r, @@ -321,7 +325,7 @@ ngx_http_init_connection(ngx_connection_t *c) #if (NGX_HTTP_V2) if (hc->addr_conf->http2) { -rev->handler = ngx_http_v2_init; +rev->handler = ngx_http_wait_v2_preface_handler; } #endif @@ -377,6 +381,110 @@ ngx_http_init_connection(ngx_connection_t *c) } +#if (NGX_HTTP_V2) +static void +ngx_http_wait_v2_preface_handler(ngx_event_t *rev) +{ +size_t size; +ssize_tn; +ngx_buf_t *b; +ngx_connection_t *c; +static const u_charpreface[] = "PRI"; + +c = rev->data; +size = sizeof(preface) - 1; + +ngx_log_debug0(NGX_LOG_DEBUG_HTTP, c->log, 0, +"http wait h2 preface handler"); + +if (rev->timedout) { +ngx_log_error(NGX_LOG_INFO, c->log, NGX_ETIMEDOUT, "client timed out"); +ngx_http_close_connection(c); +return; +} + +if (c->close) { +ngx_http_close_connection(c); +return; +} + +b = c->buffer; + +if (b == NULL) { +b = ngx_create_temp_buf(c->pool, size); +if (b == NULL) { +ngx_http_close_connection(c); +return; +} + +c->buffer = b; + +} else if (b->start == NULL) { + +b->start = ngx_palloc(c->pool, size); +if (b->start == NULL) { +ngx_http_close_connection(c); +return; +} + +b->pos = b->start; +b->last = b->start; +b->end = b->last + size; +} + +n = c->recv(c, b->last, b->end - b->last); + +if (n == NGX_AGAIN) { + +if (!rev->timer_set) { +ngx_add_timer(rev, c->listening->post_accept_timeout); +ngx_reusable_connection(c, 1); +} + +if (ngx_handle_read_event(rev, 0) != NGX_OK) { +ngx_http_close_connection(c); +return; +} + +/* + * We are trying to not hold c->buffer's memory for an idle connection. + */ + +if (ngx_pfree(c->pool, b->start) == NGX_OK) { +b->start = NULL; +} + +return; +} + +if (n == NGX_ERROR) { +ngx_http_close_connection(c); +return; +} + +if (n == 0) { +ngx_log_error(NGX_LOG_INFO, c->log, 0, + "client closed connection"); +ngx_http_close_connection(c); +return; +} + +b->last += n; + +if (b->last == b->end) { +/* b will be freed in ngx_http_v2_init/ngx_http_wait_request_handler */ + +if (ngx_strncmp(b->start, preface, size) == 0) { +ngx_http_v2_init(rev); +} else { +rev->handler = ngx_http_wait_request_handler; +ngx_http_wait_request_handler(rev); +} +} +} +#endif + + static void ngx_http_wait_request_handler(ngx_event_t *rev) { @@ -430,6 +538,22 @@ ngx_http_wait_request_handler(ngx_event_t *rev) b->pos = b->start; b->last = b->start; b->end = b->last + size; +} else { + +p = ngx_palloc(c->pool, size); +if (p == NULL) { +ngx_http_close_connection(c); +return; +} + +n = b->last - b->start; +ngx_memcpy(p, b->start, n); +ngx_pfree(c->pool, b->start); + +b->start = p; +b->pos = b->start; +b->last = b->start + n; +b->end = b->last + size; } n = c->recv(c, b->last, size); diff --git a/src/http/v2/ngx_http_v2.c b/src/http/v2/ngx_http_v2.c index d9df0f90..e36bf382 100644 --- a/src/http/v2/ngx_http_v2.c +++ b/src/http/v2/ngx_http_v2.c @@ -231,6 +231,8 @@ static ngx_http_v2_parse_header_t ngx_http_v2_parse_headers[] = { void ngx_http_v2_init(ngx_event_t *rev) { +size_t size; +ngx_buf_t *b; ngx_connection_t *c; ngx_pool_cleanup_t*cln; ngx_http_connection_t *hc; @@ -262,6 +264,23 @@ ngx_http_v2_i
Re: [PATCH] HTTP/2: make http2 server support http1
Here is a more simple patch. And the temp buffer has also been freed. Please make your comments. Thanks. diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c index 89cfe77a..d97952bc 100644 --- a/src/http/ngx_http_request.c +++ b/src/http/ngx_http_request.c @@ -17,6 +17,10 @@ static ssize_t ngx_http_read_request_header(ngx_http_request_t *r); static ngx_int_t ngx_http_alloc_large_header_buffer(ngx_http_request_t *r, ngx_uint_t request_line); +#if (NGX_HTTP_V2) +static void ngx_http_wait_v2_preface_handler(ngx_event_t *rev); +#endif + 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_unique_header_line(ngx_http_request_t *r, @@ -321,7 +325,7 @@ ngx_http_init_connection(ngx_connection_t *c) #if (NGX_HTTP_V2) if (hc->addr_conf->http2) { -rev->handler = ngx_http_v2_init; +rev->handler = ngx_http_wait_v2_preface_handler; } #endif @@ -377,6 +381,110 @@ ngx_http_init_connection(ngx_connection_t *c) } +#if (NGX_HTTP_V2) +static void +ngx_http_wait_v2_preface_handler(ngx_event_t *rev) +{ +size_t size; +ssize_tn; +ngx_buf_t *b; +ngx_connection_t *c; +static const u_charpreface[] = "PRI"; + +c = rev->data; +size = sizeof(preface) - 1; + +ngx_log_debug0(NGX_LOG_DEBUG_HTTP, c->log, 0, +"http wait h2 preface handler"); + +if (rev->timedout) { +ngx_log_error(NGX_LOG_INFO, c->log, NGX_ETIMEDOUT, "client timed out"); +ngx_http_close_connection(c); +return; +} + +if (c->close) { +ngx_http_close_connection(c); +return; +} + +b = c->buffer; + +if (b == NULL) { +b = ngx_create_temp_buf(c->pool, size); +if (b == NULL) { +ngx_http_close_connection(c); +return; +} + +c->buffer = b; + +} else if (b->start == NULL) { + +b->start = ngx_palloc(c->pool, size); +if (b->start == NULL) { +ngx_http_close_connection(c); +return; +} + +b->pos = b->start; +b->last = b->start; +b->end = b->last + size; +} + +n = c->recv(c, b->last, size); + +if (n == NGX_AGAIN) { + +if (!rev->timer_set) { +ngx_add_timer(rev, c->listening->post_accept_timeout); +ngx_reusable_connection(c, 1); +} + +if (ngx_handle_read_event(rev, 0) != NGX_OK) { +ngx_http_close_connection(c); +return; +} + +/* + * We are trying to not hold c->buffer's memory for an idle connection. + */ + +if (ngx_pfree(c->pool, b->start) == NGX_OK) { +b->start = NULL; +} + +return; +} + +if (n == NGX_ERROR) { +ngx_http_close_connection(c); +return; +} + +if (n == 0) { +ngx_log_error(NGX_LOG_INFO, c->log, 0, + "client closed connection"); +ngx_http_close_connection(c); +return; +} + +b->last += n; + +if (b->last == b->end) { +/* b will be freed in ngx_http_v2_init/ngx_http_wait_request_handler */ + +if (ngx_strncmp(b->start, preface, size) == 0) { +ngx_http_v2_init(rev); +} else { +rev->handler = ngx_http_wait_request_handler; +ngx_http_wait_request_handler(rev); +} +} +} +#endif + + static void ngx_http_wait_request_handler(ngx_event_t *rev) { @@ -430,6 +538,22 @@ ngx_http_wait_request_handler(ngx_event_t *rev) b->pos = b->start; b->last = b->start; b->end = b->last + size; +} else { + +p = ngx_palloc(c->pool, size); +if (p == NULL) { +ngx_http_close_connection(c); +return; +} + +n = b->last - b->start; +ngx_memcpy(p, b->start, n); +ngx_pfree(c->pool, b->start); + +b->start = p; +b->pos = b->start; +b->last = b->start + n; +b->end = b->last + size; } n = c->recv(c, b->last, size); diff --git a/src/http/v2/ngx_http_v2.c b/src/http/v2/ngx_http_v2.c index d9df0f90..e36bf382 100644 --- a/src/http/v2/ngx_http_v2.c +++ b/src/http/v2/ngx_http_v2.c @@ -231,6 +231,8 @@ static ngx_http_v2_parse_header_t ngx_http_v2_parse_headers[] = { void ngx_http_v2_init(ngx_event_t *rev) { +size_t size; +ngx_buf_t *b; ngx_connection_t *c; ngx_pool_cleanup_t*cln; ngx_http_connection_t *hc; @@ -262,6 +264,23 @@ ngx_http_v2_init(ngx_event_t *rev) return; } +b = c->buffer; + +
Re: [PATCH] HTTP/2: make http2 server support http1
Hello, here is another patch(more sample) according Maxim Dounin advice. Please offer your comment. Thanks. diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c index 89cfe77a..71bc7b59 100644 --- a/src/http/ngx_http_request.c +++ b/src/http/ngx_http_request.c @@ -17,6 +17,10 @@ static ssize_t ngx_http_read_request_header(ngx_http_request_t *r); static ngx_int_t ngx_http_alloc_large_header_buffer(ngx_http_request_t *r, ngx_uint_t request_line); +#if (NGX_HTTP_V2) +static void ngx_http_wait_v2_preface_handler(ngx_event_t *rev); +#endif + 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_unique_header_line(ngx_http_request_t *r, @@ -321,7 +325,7 @@ ngx_http_init_connection(ngx_connection_t *c) #if (NGX_HTTP_V2) if (hc->addr_conf->http2) { -rev->handler = ngx_http_v2_init; +rev->handler = ngx_http_wait_v2_preface_handler; } #endif @@ -377,6 +381,108 @@ ngx_http_init_connection(ngx_connection_t *c) } +#if (NGX_HTTP_V2) +static void +ngx_http_wait_v2_preface_handler(ngx_event_t *rev) +{ +size_t size; +ssize_tn; +ngx_buf_t *b; +ngx_connection_t *c; + +c = rev->data; + +ngx_log_debug0(NGX_LOG_DEBUG_HTTP, c->log, 0, +"http wait h2 preface handler"); + +if (rev->timedout) { +ngx_log_error(NGX_LOG_INFO, c->log, NGX_ETIMEDOUT, "client timed out"); +ngx_http_close_connection(c); +return; +} + +if (c->close) { +ngx_http_close_connection(c); +return; +} + +size = 5 /* strlen("PRI *") */; + +b = c->buffer; + +if (b == NULL) { +b = ngx_create_temp_buf(c->pool, size); +if (b == NULL) { +ngx_http_close_connection(c); +return; +} + +c->buffer = b; + +} else if (b->start == NULL) { + +b->start = ngx_palloc(c->pool, size); +if (b->start == NULL) { +ngx_http_close_connection(c); +return; +} + +b->pos = b->start; +b->last = b->start; +b->end = b->last + size; +} + +n = c->recv(c, b->last, size); + +if (n == NGX_AGAIN) { + +if (!rev->timer_set) { +ngx_add_timer(rev, c->listening->post_accept_timeout); +ngx_reusable_connection(c, 1); +} + +if (ngx_handle_read_event(rev, 0) != NGX_OK) { +ngx_http_close_connection(c); +return; +} + +/* + * We are trying to not hold c->buffer's memory for an idle connection. + */ + +if (ngx_pfree(c->pool, b->start) == NGX_OK) { +b->start = NULL; +} + +return; +} + +if (n == NGX_ERROR) { +ngx_http_close_connection(c); +return; +} + +if (n == 0) { +ngx_log_error(NGX_LOG_INFO, c->log, 0, + "client closed connection"); +ngx_http_close_connection(c); +return; +} + +b->last += n; + +if (b->last == b->end) { +if (ngx_strncmp(b->start, "PRI *", 5) == 0) { +ngx_http_v2_init_with_buf(rev, b); +} else { +rev->handler = ngx_http_wait_request_handler; +ngx_http_wait_request_handler(rev); +} +} +} +#endif + + static void ngx_http_wait_request_handler(ngx_event_t *rev) { @@ -430,6 +536,21 @@ ngx_http_wait_request_handler(ngx_event_t *rev) b->pos = b->start; b->last = b->start; b->end = b->last + size; +} else { + +p = ngx_palloc(c->pool, size); +if (p == NULL) { +ngx_http_close_connection(c); +return; +} + +n = b->last - b->start; +ngx_memcpy(p, b->start, n); + +b->start = p; +b->pos = b->start; +b->last = b->start + n; +b->end = b->last + size; } n = c->recv(c, b->last, size); diff --git a/src/http/v2/ngx_http_v2.c b/src/http/v2/ngx_http_v2.c index d9df0f90..a990c96f 100644 --- a/src/http/v2/ngx_http_v2.c +++ b/src/http/v2/ngx_http_v2.c @@ -231,6 +231,14 @@ static ngx_http_v2_parse_header_t ngx_http_v2_parse_headers[] = { void ngx_http_v2_init(ngx_event_t *rev) { +ngx_http_v2_init_with_buf(rev, NULL); +} + + +void +ngx_http_v2_init_with_buf(ngx_event_t *rev, ngx_buf_t *buf) +{ +size_t size; ngx_connection_t *c; ngx_pool_cleanup_t*cln; ngx_http_connection_t *hc; @@ -262,6 +270,17 @@ ngx_http_v2_init(ngx_event_t *rev) return; } +if (buf != NULL) { +size = buf->last - buf->start; + +if (size >
Re: [PATCH] HTTP/2: make http2 server support http1
Hello! On Mon, Mar 05, 2018 at 11:52:57PM +0800, Haitao Lv wrote: [...] > > Overall, the patch looks like a hack and introduces too much > > complexity for this feature. While I understand the reasoning, > > the proposed implementation cannot be accepted. > > Could you clarify that whether is this feature not accepted or this patch? > > If this feature is not needed, I will terminate this thread. > > If this patch only looks like a hack, would you like offer any advice to write > code with good smell? We've previously discussed this with Valentin, and our position is as follows: - The feature itself (autodetection between HTTP/2 and HTTP/1.x protocols) might be usable, and we can consider adding it if there will be a good and simple enough patch. (Moreover, we think that this probably should be the default if "listen ... http2" is configured - that is, no "http1" option.) - The patch suggested certainly doesn't meet the above criteria, and it does not look like it can be fixed. We don't know if a good and simple enough implementation is at all possible though. One of the possible approaches was already proposed by Valentin (detect HTTP/2 or HTTP/1.x before starting processing, may be similar to how we handle http-to-https requests), but it's now immediately clear if it will work or not. Sorry, but please don't expect any of us to provide further guidance. -- Maxim Dounin http://mdounin.ru/ ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] HTTP/2: make http2 server support http1
Hello wbr, Thank you for reviewing this patch. > On Mar 5, 2018, at 23:06, Valentin V. Bartenevwrote: > > On Monday 05 March 2018 11:11:08 Haitao Lv wrote: > [..] >>> @@ -145,10 +152,6 @@ ngx_http_parse_request_line(ngx_http_request_t *r, >>> ngx_buf_t *b) >>>case sw_start: >>>r->request_start = p; >>> >>> -if (ch == CR || ch == LF) { >>> -break; >>> -} >>> - >> >> I think Nginx should not allow any leading \r or \n. HTTP client should >> never send this chars >> before the request line. Support this feature makes the buffer management >> more harder. > > Support for this feature is required. See the notes from Ruslan. > Some clients send additional empty line between keepalive requests. Yes, this is a mistake. I did not read the RFC carefully. Sorry again. But this change isn't needed. > > [..] >>> +#if (NGX_HTTP_V2) >>> +case sw_h2_preface: >>> + >>> +if (ch == *n++) { >> >> I introduce a new sw_h2_preface state. In this state, the parser will check >> the h2 preface char by char. >> >> The parser only check at most 20 chars "* HTTP/2.0\r\n\r\nSM\r\n\r\n". So >> the buffer will not be fulfilled. >> > > The recv() operation gets buffer size as an argument. > > Thus, the buffer can be fully filled with all the data copied from > a socket buffer. HTTP/2 clients usually don't send preface alone. > They can send other data with the preface. Yes, the buffer could be full filled. HTTP/2 client will send more frames after the h2 preface, this is why I introduce the ngx_http_v2_init_after_preface(ngx_event_t *rev, ngx_buf_t *buf). All the remaining data in the buffer will be copied to the h2mcf->recv_buffer and be processed by the HTTP/2 ngx_http_v2_state_head handler. So this will not breaks the HTTP/2 handshake. However, if we received data is longer than the h2mcf->recv_buffer_size, the current patch will yield a bad request response. A bad case is the client_header_buffer_size(default 1k) set larger than the http2_recv_buffer_size(default 265k), we will may fully filled the header_in buffer but cannot be copied into the h2mcf->recv_buffer, which will cause the HTTP/2 handshake failed. But simple way to workaround this issue is to change the http2_recv_buffer_size to or larger than client_header_buffer_size. It is a hack. > > [..] >>> +#if (NGX_HTTP_V2) >>> +static void >>> +ngx_http_process_h2_preface(ngx_event_t *rev) >>> +{ >>> +size_t len; >>> +ngx_connection_t *c; >>> +ngx_http_request_t *r; >>> +ngx_http_connection_t *hc; >>> +ngx_http_v2_main_conf_t*h2mcf; >>> + >>> +c = rev->data; >>> +r = c->data; >>> +hc = r->http_connection; >>> + >>> +ngx_log_debug0(NGX_LOG_DEBUG_HTTP, c->log, 0, >>> +"http process h2 preface"); >>> + >>> +r->header_in->pos = r->header_in->start + 24; >>> + >>> +len = r->header_in->last - r->header_in->pos; >>> + >>> +h2mcf = ngx_http_get_module_main_conf(hc->conf_ctx, >>> ngx_http_v2_module); >>> + >>> +if (len <= h2mcf->recv_buffer_size) { >> >> We check the recv buffer size before do the ngx_http_v2_init_after_preface. >> > [..] > > See above. It's a bad idea to decline request with a "client sent > invalid h2 preface" message in error log just because client has > sent more HTTP/2 frames right after the preface. In this path, all the valid HTTP/2 handshake will never generate a error log. > > Overall, the patch looks like a hack and introduces too much > complexity for this feature. While I understand the reasoning, > the proposed implementation cannot be accepted. Could you clarify that whether is this feature not accepted or this patch? If this feature is not needed, I will terminate this thread. If this patch only looks like a hack, would you like offer any advice to write code with good smell? Thank you. > > Protocol detection before starting processing HTTP/1 or HTTP/2 > could be a better way to go. > > wbr, Valentin V. Bartenev > > ___ > nginx-devel mailing list > nginx-devel@nginx.org > http://mailman.nginx.org/mailman/listinfo/nginx-devel ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] HTTP/2: make http2 server support http1
On Monday 05 March 2018 11:11:08 Haitao Lv wrote: [..] > > @@ -145,10 +152,6 @@ ngx_http_parse_request_line(ngx_http_request_t *r, > > ngx_buf_t *b) > > case sw_start: > > r->request_start = p; > > > > -if (ch == CR || ch == LF) { > > -break; > > -} > > - > > I think Nginx should not allow any leading \r or \n. HTTP client should never > send this chars > before the request line. Support this feature makes the buffer management > more harder. Support for this feature is required. See the notes from Ruslan. Some clients send additional empty line between keepalive requests. [..] > > +#if (NGX_HTTP_V2) > > +case sw_h2_preface: > > + > > +if (ch == *n++) { > > I introduce a new sw_h2_preface state. In this state, the parser will check > the h2 preface char by char. > > The parser only check at most 20 chars "* HTTP/2.0\r\n\r\nSM\r\n\r\n". So the > buffer will not be fulfilled. > The recv() operation gets buffer size as an argument. Thus, the buffer can be fully filled with all the data copied from a socket buffer. HTTP/2 clients usually don't send preface alone. They can send other data with the preface. [..] > > +#if (NGX_HTTP_V2) > > +static void > > +ngx_http_process_h2_preface(ngx_event_t *rev) > > +{ > > +size_t len; > > +ngx_connection_t *c; > > +ngx_http_request_t *r; > > +ngx_http_connection_t *hc; > > +ngx_http_v2_main_conf_t*h2mcf; > > + > > +c = rev->data; > > +r = c->data; > > +hc = r->http_connection; > > + > > +ngx_log_debug0(NGX_LOG_DEBUG_HTTP, c->log, 0, > > +"http process h2 preface"); > > + > > +r->header_in->pos = r->header_in->start + 24; > > + > > +len = r->header_in->last - r->header_in->pos; > > + > > +h2mcf = ngx_http_get_module_main_conf(hc->conf_ctx, > > ngx_http_v2_module); > > + > > +if (len <= h2mcf->recv_buffer_size) { > > We check the recv buffer size before do the ngx_http_v2_init_after_preface. > [..] See above. It's a bad idea to decline request with a "client sent invalid h2 preface" message in error log just because client has sent more HTTP/2 frames right after the preface. Overall, the patch looks like a hack and introduces too much complexity for this feature. While I understand the reasoning, the proposed implementation cannot be accepted. Protocol detection before starting processing HTTP/1 or HTTP/2 could be a better way to go. wbr, Valentin V. Bartenev ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] HTTP/2: make http2 server support http1
> On Mar 5, 2018, at 17:02, Ruslan Ermilovwrote: > > On Mon, Mar 05, 2018 at 11:11:08AM +0800, Haitao Lv wrote: > [...] >>> @@ -145,10 +152,6 @@ ngx_http_parse_request_line(ngx_http_request_t *r, >>> ngx_buf_t *b) >>>case sw_start: >>>r->request_start = p; >>> >>> -if (ch == CR || ch == LF) { >>> -break; >>> -} >>> - >> >> I think Nginx should not allow any leading \r or \n. HTTP client should >> never send this chars >> before the request line. Support this feature makes the buffer management >> more harder. > > https://tools.ietf.org/html/rfc2616#section-4.1 > > In the interest of robustness, servers SHOULD ignore any empty > line(s) received where a Request-Line is expected. In other words, if > the server is reading the protocol stream at the beginning of a > message and receives a CRLF first, it should ignore the CRLF. > > > https://tools.ietf.org/html/rfc7230#section-3.5 > > In the interest of robustness, a server that is expecting to receive > and parse a request-line SHOULD ignore at least one empty line (CRLF) > received prior to the request-line. Thank you, it is a mistake. > ___ > nginx-devel mailing list > nginx-devel@nginx.org > http://mailman.nginx.org/mailman/listinfo/nginx-devel ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] HTTP/2: make http2 server support http1
On Mon, Mar 05, 2018 at 11:11:08AM +0800, Haitao Lv wrote: [...] > > @@ -145,10 +152,6 @@ ngx_http_parse_request_line(ngx_http_request_t *r, > > ngx_buf_t *b) > > case sw_start: > > r->request_start = p; > > > > -if (ch == CR || ch == LF) { > > -break; > > -} > > - > > I think Nginx should not allow any leading \r or \n. HTTP client should never > send this chars > before the request line. Support this feature makes the buffer management > more harder. https://tools.ietf.org/html/rfc2616#section-4.1 In the interest of robustness, servers SHOULD ignore any empty line(s) received where a Request-Line is expected. In other words, if the server is reading the protocol stream at the beginning of a message and receives a CRLF first, it should ignore the CRLF. https://tools.ietf.org/html/rfc7230#section-3.5 In the interest of robustness, a server that is expecting to receive and parse a request-line SHOULD ignore at least one empty line (CRLF) received prior to the request-line. ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] HTTP/2: make http2 server support http1
@wbr > On Mar 4, 2018, at 13:49, Haitao Lv <i...@lvht.net> wrote: > > Here is the new patch > > diff --git a/src/http/ngx_http.c b/src/http/ngx_http.c > index 9d8b6d79..c06ef7c7 100644 > --- a/src/http/ngx_http.c > +++ b/src/http/ngx_http.c > @@ -1197,6 +1197,7 @@ ngx_http_add_addresses(ngx_conf_t *cf, > ngx_http_core_srv_conf_t *cscf, > ngx_uint_t ssl; > #endif > #if (NGX_HTTP_V2) > +ngx_uint_t http1; > ngx_uint_t http2; > #endif > > @@ -1232,6 +1233,7 @@ ngx_http_add_addresses(ngx_conf_t *cf, > ngx_http_core_srv_conf_t *cscf, > ssl = lsopt->ssl || addr[i].opt.ssl; > #endif > #if (NGX_HTTP_V2) > + http1 = lsopt->http1 || addr[i].opt.http1; > http2 = lsopt->http2 || addr[i].opt.http2; > #endif > > @@ -1266,6 +1268,7 @@ ngx_http_add_addresses(ngx_conf_t *cf, > ngx_http_core_srv_conf_t *cscf, > addr[i].opt.ssl = ssl; > #endif > #if (NGX_HTTP_V2) > +addr[i].opt.http1 = http1; > addr[i].opt.http2 = http2; > #endif > > @@ -1802,6 +1805,7 @@ ngx_http_add_addrs(ngx_conf_t *cf, ngx_http_port_t > *hport, > addrs[i].conf.ssl = addr[i].opt.ssl; > #endif > #if (NGX_HTTP_V2) > +addrs[i].conf.http1 = addr[i].opt.http1; > addrs[i].conf.http2 = addr[i].opt.http2; > #endif > addrs[i].conf.proxy_protocol = addr[i].opt.proxy_protocol; > @@ -1867,6 +1871,7 @@ ngx_http_add_addrs6(ngx_conf_t *cf, ngx_http_port_t > *hport, > addrs6[i].conf.ssl = addr[i].opt.ssl; > #endif > #if (NGX_HTTP_V2) > +addrs6[i].conf.http1 = addr[i].opt.http1; > addrs6[i].conf.http2 = addr[i].opt.http2; > #endif > addrs6[i].conf.proxy_protocol = addr[i].opt.proxy_protocol; > diff --git a/src/http/ngx_http_core_module.c b/src/http/ngx_http_core_module.c > index 6b318dd0..741a4dd9 100644 > --- a/src/http/ngx_http_core_module.c > +++ b/src/http/ngx_http_core_module.c > @@ -3985,6 +3985,18 @@ ngx_http_core_listen(ngx_conf_t *cf, ngx_command_t > *cmd, void *conf) > #endif > } > > +if (ngx_strcmp(value[n].data, "http1") == 0) { > +#if (NGX_HTTP_V2) > +lsopt.http1 = 1; > +continue; > +#else > +ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, > + "the \"http1\" parameter requires " > + "ngx_http_v2_module"); > +return NGX_CONF_ERROR; > +#endif > +} > + > if (ngx_strcmp(value[n].data, "spdy") == 0) { > ngx_conf_log_error(NGX_LOG_WARN, cf, 0, >"invalid parameter \"spdy\": " > diff --git a/src/http/ngx_http_core_module.h b/src/http/ngx_http_core_module.h > index d7985049..68b3e7d9 100644 > --- a/src/http/ngx_http_core_module.h > +++ b/src/http/ngx_http_core_module.h > @@ -73,6 +73,7 @@ typedef struct { > unsigned bind:1; > unsigned wildcard:1; > unsigned ssl:1; > +unsigned http1:1; > unsigned http2:1; > #if (NGX_HAVE_INET6) > unsigned ipv6only:1; > @@ -234,6 +235,7 @@ struct ngx_http_addr_conf_s { > ngx_http_virtual_names_t *virtual_names; > > unsigned ssl:1; > +unsigned http1:1; > unsigned http2:1; > unsigned proxy_protocol:1; > }; > diff --git a/src/http/ngx_http_parse.c b/src/http/ngx_http_parse.c > index 844054c9..9eb713b7 100644 > --- a/src/http/ngx_http_parse.c > +++ b/src/http/ngx_http_parse.c > @@ -117,6 +117,7 @@ ngx_http_parse_request_line(ngx_http_request_t *r, > ngx_buf_t *b) > sw_host_ip_literal, > sw_port, > sw_host_http_09, > +sw_h2_preface, > sw_after_slash_in_uri, > sw_check_uri, > sw_check_uri_http_09, > @@ -134,6 +135,12 @@ ngx_http_parse_request_line(ngx_http_request_t *r, > ngx_buf_t *b) > sw_almost_done > } state; > > +#if (NGX_HTTP_V2) > +static u_char h2_preface[] = "PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n"; > +u_char *h2_preface_end = h2_preface + 24; > +u_char *n = h2_preface + 4; > +#endif > + > state = r->state; > > for (p = b->pos; p < b->last; p++) { > @@ -145,10 +152,6 @@ ngx_http_parse_request_line(ngx_http_request_t *r, > ngx_buf_t *b) > case sw_start: > r->request_start = p; > > -if (ch == CR || ch == LF) { > -break; > -
Re: [PATCH] HTTP/2: make http2 server support http1
> On Mar 4, 2018, at 21:00, Valentin V. Bartenevwrote: > > On Sunday, 4 March 2018 05:53:36 MSK Haitao Lv wrote: >> Hi, wbr, >> >> Thanks for your review. I don't know why I can't receive your email. >> Let me reply directly. >> >>> It doesn't look like a useful feature. >>> Could you please explain the use cases? >> >> The current implementation support both http/1 and http/2 over the >> same TLS listen port by the ALPN Extension. >> >> Nginx also support listening http/2 over plain tcp port, which is >> perfect for development and inner production environment. >> >> However, the current implementation cannot support http/1.1 and http/2 >> simultaneously. We have no choice but listen on two different ports. >> As a result, one same service has two ports and different clients >> should choose their suitable port. >> >> Besides, the http/2 is efficient, but http/1 is simple. So I see no >> chance that the http/2 will replace http/1 totally in the production >> inner environment. Will call the inner API by both http/1 and http/2. >> >> So I think support http/1 and http/2 on the plain tcp port will simplify >> both the production and development environment. >> > > HTTP/2 was designed to save connection time between a client and a web > server, which can be costly with significant RTT and full TLS handshake. > > Also it helps to overcome concurrency limitation in browsers, that usually > limited to 6 connection per host. > > All these problems usually don't exist in inner environments. > > HTTP/2 isn't efficient. In fact, it has more overhead and may require > more TCP packets to transfer the same amount of data. Moreover, more TCP > connections are usually faster than one, just because they have bigger start > window in total and less suffer from packet loss. > > Please, don't be fooled by aggressive marketing around HTTP/2. Most of > such articles are written by people who barely understand how network works. > HTTP/2 is neither a better version nor the "next" version of HTTP protocol > (the name is misleading). It's an another protocol designed to solve some > specific cases, while introducing many other problems. > > I don't recommend to use HTTP/2 in cases other than TLS connections between > browsers and web servers in public network. Also even for this purpose, > there are cases when HTTP/2 isn't recommended, e.g. unreliable networks > with packet loss (as mobile networks). > > You can easily find on YouTube a talk by Hooman Beheshti called > "HTTP/2: what no one is telling you" with some interesting research. > > nginx support HTTP/2 over plain TCP for a number of reasons: > > 1. for easy testing and debugging the protocol implementation in nginx; > > 2. that required almost no additional code to implement; > > 3. there's a use case when TLS termination is done by simple TCP proxy >and then the traffic routed to HTTP/1 or HTTP/2 nginx ports according >to ALPN. My actual use case is simple. My compony is going to introduce the gRPC. And the gRPC depends on the HTTP/2 and it's trailer headers. We have many php legacy code which expose API by HTTP/1. So we want to build gRPC server by PHP. A simple solution is to send gRPC payload by HTTP/1 and let nginx transfer it to HTTP/2. And the grpc-status, grpc-message header that should be transferred by the HTTP/2 trailer header could be transferred by the normal HTTP/1 header. By this way, we can support both HTTP/1 API and unary-call gRPC. > >> >>> What if the received data is bigger than h2mcf->recv_buffer? >> >> Yes, it is a problem. However, it can be fixed easily. >> >> As we know, the http/2 preface is PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n. >> We could modify the ngx_http_parse_request_line and when we got A PRI >> method, we need to get a single * uri. If we got things other than *, >> we just return a invalid request response. By this, we will never got >> a PRI to_much_long_uri HTTP/2.0 request line, and the buffer will not >> be exhausted. So the ngx_http_alloc_large_header_buffer will not be called >> during the handshake. After the ngx_http_parse_request_line, we will >> ensure we got a PRI request, and the buffer size is >> client_header_buffer_size. >> > > As far as I can see, your reasoning is based on assumption that > client_header_buffer_size is always smaller than http2_recv_buffer_size. > > That simply isn't true as both can be easily configured by users. Let me explain in the new patch. > > wbr, Valentin V. Bartenev > > ___ > nginx-devel mailing list > nginx-devel@nginx.org > http://mailman.nginx.org/mailman/listinfo/nginx-devel ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] HTTP/2: make http2 server support http1
Here is the new patch diff --git a/src/http/ngx_http.c b/src/http/ngx_http.c index 9d8b6d79..c06ef7c7 100644 --- a/src/http/ngx_http.c +++ b/src/http/ngx_http.c @@ -1197,6 +1197,7 @@ ngx_http_add_addresses(ngx_conf_t *cf, ngx_http_core_srv_conf_t *cscf, ngx_uint_t ssl; #endif #if (NGX_HTTP_V2) +ngx_uint_t http1; ngx_uint_t http2; #endif @@ -1232,6 +1233,7 @@ ngx_http_add_addresses(ngx_conf_t *cf, ngx_http_core_srv_conf_t *cscf, ssl = lsopt->ssl || addr[i].opt.ssl; #endif #if (NGX_HTTP_V2) +http1 = lsopt->http1 || addr[i].opt.http1; http2 = lsopt->http2 || addr[i].opt.http2; #endif @@ -1266,6 +1268,7 @@ ngx_http_add_addresses(ngx_conf_t *cf, ngx_http_core_srv_conf_t *cscf, addr[i].opt.ssl = ssl; #endif #if (NGX_HTTP_V2) +addr[i].opt.http1 = http1; addr[i].opt.http2 = http2; #endif @@ -1802,6 +1805,7 @@ ngx_http_add_addrs(ngx_conf_t *cf, ngx_http_port_t *hport, addrs[i].conf.ssl = addr[i].opt.ssl; #endif #if (NGX_HTTP_V2) +addrs[i].conf.http1 = addr[i].opt.http1; addrs[i].conf.http2 = addr[i].opt.http2; #endif addrs[i].conf.proxy_protocol = addr[i].opt.proxy_protocol; @@ -1867,6 +1871,7 @@ ngx_http_add_addrs6(ngx_conf_t *cf, ngx_http_port_t *hport, addrs6[i].conf.ssl = addr[i].opt.ssl; #endif #if (NGX_HTTP_V2) +addrs6[i].conf.http1 = addr[i].opt.http1; addrs6[i].conf.http2 = addr[i].opt.http2; #endif addrs6[i].conf.proxy_protocol = addr[i].opt.proxy_protocol; diff --git a/src/http/ngx_http_core_module.c b/src/http/ngx_http_core_module.c index 6b318dd0..741a4dd9 100644 --- a/src/http/ngx_http_core_module.c +++ b/src/http/ngx_http_core_module.c @@ -3985,6 +3985,18 @@ ngx_http_core_listen(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) #endif } +if (ngx_strcmp(value[n].data, "http1") == 0) { +#if (NGX_HTTP_V2) +lsopt.http1 = 1; +continue; +#else +ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, + "the \"http1\" parameter requires " + "ngx_http_v2_module"); +return NGX_CONF_ERROR; +#endif +} + if (ngx_strcmp(value[n].data, "spdy") == 0) { ngx_conf_log_error(NGX_LOG_WARN, cf, 0, "invalid parameter \"spdy\": " diff --git a/src/http/ngx_http_core_module.h b/src/http/ngx_http_core_module.h index d7985049..68b3e7d9 100644 --- a/src/http/ngx_http_core_module.h +++ b/src/http/ngx_http_core_module.h @@ -73,6 +73,7 @@ typedef struct { unsigned bind:1; unsigned wildcard:1; unsigned ssl:1; +unsigned http1:1; unsigned http2:1; #if (NGX_HAVE_INET6) unsigned ipv6only:1; @@ -234,6 +235,7 @@ struct ngx_http_addr_conf_s { ngx_http_virtual_names_t *virtual_names; unsigned ssl:1; +unsigned http1:1; unsigned http2:1; unsigned proxy_protocol:1; }; diff --git a/src/http/ngx_http_parse.c b/src/http/ngx_http_parse.c index 844054c9..9eb713b7 100644 --- a/src/http/ngx_http_parse.c +++ b/src/http/ngx_http_parse.c @@ -117,6 +117,7 @@ ngx_http_parse_request_line(ngx_http_request_t *r, ngx_buf_t *b) sw_host_ip_literal, sw_port, sw_host_http_09, +sw_h2_preface, sw_after_slash_in_uri, sw_check_uri, sw_check_uri_http_09, @@ -134,6 +135,12 @@ ngx_http_parse_request_line(ngx_http_request_t *r, ngx_buf_t *b) sw_almost_done } state; +#if (NGX_HTTP_V2) +static u_char h2_preface[] = "PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n"; +u_char *h2_preface_end = h2_preface + 24; +u_char *n = h2_preface + 4; +#endif + state = r->state; for (p = b->pos; p < b->last; p++) { @@ -145,10 +152,6 @@ ngx_http_parse_request_line(ngx_http_request_t *r, ngx_buf_t *b) case sw_start: r->request_start = p; -if (ch == CR || ch == LF) { -break; -} - if ((ch < 'A' || ch > 'Z') && ch != '_' && ch != '-') { return NGX_HTTP_PARSE_INVALID_METHOD; } @@ -174,6 +177,13 @@ ngx_http_parse_request_line(ngx_http_request_t *r, ngx_buf_t *b) break; } +#if (NGX_HTTP_V2) +if (ngx_str3_cmp(m, 'P', 'R', 'I', ' ')) { +r->method = NGX_HTTP_PRI; +break; +} +#endif + break; case 4: @@ -267,6 +277,14 @@ ngx_http_parse_request_line(ngx_http_request_t *r, ngx_buf_t *b)
Re: [PATCH] HTTP/2: make http2 server support http1
Hi, wbr, Thanks for your review. I don't know why I can't receive your email. Let me reply directly. > It doesn't look like a useful feature. > Could you please explain the use cases? The current implementation support both http/1 and http/2 over the same TLS listen port by the ALPN Extension. Nginx also support listening http/2 over plain tcp port, which is perfect for development and inner production environment. However, the current implementation cannot support http/1.1 and http/2 simultaneously. We have no choice but listen on two different ports. As a result, one same service has two ports and different clients should choose their suitable port. Besides, the http/2 is efficient, but http/1 is simple. So I see no chance that the http/2 will replace http/1 totally in the production inner environment. Will call the inner API by both http/1 and http/2. So I think support http/1 and http/2 on the plain tcp port will simplify both the production and development environment. > What if the received data is bigger than h2mcf->recv_buffer? Yes, it is a problem. However, it can be fixed easily. As we know, the http/2 preface is PRI * HTTP/2.0\r\n\r\nSM\r\n\r\n. We could modify the ngx_http_parse_request_line and when we got A PRI method, we need to get a single * uri. If we got things other than *, we just return a invalid request response. By this, we will never got a PRI to_much_long_uri HTTP/2.0 request line, and the buffer will not be exhausted. So the ngx_http_alloc_large_header_buffer will not be called during the handshake. After the ngx_http_parse_request_line, we will ensure we got a PRI request, and the buffer size is client_header_buffer_size. And then we should the compare the r->header_in buffer's length and the h2mcf->recv_buffer_size. If we got a buffered data larger than the h2mcf->recv_buffer_size, we should send a bad request response. If this feature can be accepted, I will send a new patch. Thanks. > On Mar 2, 2018, at 15:53, Haitao Lv <i...@lvht.net> wrote: > > # HG changeset patch > # User 吕海涛 <i...@lvht.net> > # Date 1519976498 -28800 > # Fri Mar 02 15:41:38 2018 +0800 > # Node ID 200955343460c4726015180f20c03e31c0b35ff6 > # Parent 81fae70d6cb81c67607931ec3ecc585a609c97e0 > make http2 server support http1 > > diff -r 81fae70d6cb8 -r 200955343460 src/http/ngx_http.c > --- a/src/http/ngx_http.c Thu Mar 01 20:25:50 2018 +0300 > +++ b/src/http/ngx_http.c Fri Mar 02 15:41:38 2018 +0800 > @@ -1197,6 +1197,7 @@ > ngx_uint_t ssl; > #endif > #if (NGX_HTTP_V2) > +ngx_uint_t http1; > ngx_uint_t http2; > #endif > > @@ -1232,6 +1233,7 @@ > ssl = lsopt->ssl || addr[i].opt.ssl; > #endif > #if (NGX_HTTP_V2) > +http1 = lsopt->http1 || addr[i].opt.http1; > http2 = lsopt->http2 || addr[i].opt.http2; > #endif > > @@ -1266,6 +1268,7 @@ > addr[i].opt.ssl = ssl; > #endif > #if (NGX_HTTP_V2) > +addr[i].opt.http1 = http1; > addr[i].opt.http2 = http2; > #endif > > @@ -1802,6 +1805,7 @@ > addrs[i].conf.ssl = addr[i].opt.ssl; > #endif > #if (NGX_HTTP_V2) > +addrs[i].conf.http1 = addr[i].opt.http1; > addrs[i].conf.http2 = addr[i].opt.http2; > #endif > addrs[i].conf.proxy_protocol = addr[i].opt.proxy_protocol; > @@ -1867,6 +1871,7 @@ > addrs6[i].conf.ssl = addr[i].opt.ssl; > #endif > #if (NGX_HTTP_V2) > +addrs6[i].conf.http1 = addr[i].opt.http1; > addrs6[i].conf.http2 = addr[i].opt.http2; > #endif > addrs6[i].conf.proxy_protocol = addr[i].opt.proxy_protocol; > diff -r 81fae70d6cb8 -r 200955343460 src/http/ngx_http_core_module.c > --- a/src/http/ngx_http_core_module.c Thu Mar 01 20:25:50 2018 +0300 > +++ b/src/http/ngx_http_core_module.c Fri Mar 02 15:41:38 2018 +0800 > @@ -3985,6 +3985,18 @@ > #endif > } > > +if (ngx_strcmp(value[n].data, "http1") == 0) { > +#if (NGX_HTTP_V2) > +lsopt.http1 = 1; > +continue; > +#else > +ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, > + "the \"http1\" parameter requires " > + "ngx_http_v2_module"); > +return NGX_CONF_ERROR; > +#endif > +} > + > if (ngx_strcmp(value[n].data, "spdy") == 0) { > ngx_conf_log_error(NGX_LOG_WARN, cf, 0, >"invalid parameter \"spdy\": " > diff -r 81fae70d6cb8 -r 200955343460 src/http/ngx_http_core_module.h > --- a/src/http/ngx_http_core_module.h Thu Mar 01 20:25:50 2018 +0300 > +++ b/src/http/ngx_http_core_module.h Fr
Re: [PATCH] HTTP/2: make http2 server support http1
On Friday 02 March 2018 15:53:07 Haitao Lv wrote: > # HG changeset patch > # User 吕海涛 <i...@lvht.net> > # Date 1519976498 -28800 > # Fri Mar 02 15:41:38 2018 +0800 > # Node ID 200955343460c4726015180f20c03e31c0b35ff6 > # Parent 81fae70d6cb81c67607931ec3ecc585a609c97e0 > make http2 server support http1 > [..] It doesn't look like a useful feature. Could you please explain the use cases? > diff -r 81fae70d6cb8 -r 200955343460 src/http/ngx_http.c > --- a/src/http/ngx_http.c Thu Mar 01 20:25:50 2018 +0300 > +++ b/src/http/ngx_http.c Fri Mar 02 15:41:38 2018 +0800 [..] > +void > +ngx_http_v2_init_after_preface(ngx_event_t *rev, ngx_buf_t *buf) > +{ > ngx_connection_t *c; > ngx_pool_cleanup_t*cln; > ngx_http_connection_t *hc; > @@ -316,6 +323,12 @@ > h2c->state.handler = hc->proxy_protocol ? > ngx_http_v2_state_proxy_protocol > : ngx_http_v2_state_preface; > > +if (buf != NULL) { > +ngx_memcpy(h2mcf->recv_buffer, buf->pos, buf->last - buf->pos); > +h2c->state.buffer_used = buf->last - buf->pos; > +h2c->state.handler = ngx_http_v2_state_head; > +} [..] What if the received data is bigger than h2mcf->recv_buffer? wbr, Valentin V. Bartenev ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] HTTP/2: make http2 server support http1
# HG changeset patch # User 吕海涛 <i...@lvht.net> # Date 1519976498 -28800 # Fri Mar 02 15:41:38 2018 +0800 # Node ID 200955343460c4726015180f20c03e31c0b35ff6 # Parent 81fae70d6cb81c67607931ec3ecc585a609c97e0 make http2 server support http1 diff -r 81fae70d6cb8 -r 200955343460 src/http/ngx_http.c --- a/src/http/ngx_http.c Thu Mar 01 20:25:50 2018 +0300 +++ b/src/http/ngx_http.c Fri Mar 02 15:41:38 2018 +0800 @@ -1197,6 +1197,7 @@ ngx_uint_t ssl; #endif #if (NGX_HTTP_V2) +ngx_uint_t http1; ngx_uint_t http2; #endif @@ -1232,6 +1233,7 @@ ssl = lsopt->ssl || addr[i].opt.ssl; #endif #if (NGX_HTTP_V2) +http1 = lsopt->http1 || addr[i].opt.http1; http2 = lsopt->http2 || addr[i].opt.http2; #endif @@ -1266,6 +1268,7 @@ addr[i].opt.ssl = ssl; #endif #if (NGX_HTTP_V2) +addr[i].opt.http1 = http1; addr[i].opt.http2 = http2; #endif @@ -1802,6 +1805,7 @@ addrs[i].conf.ssl = addr[i].opt.ssl; #endif #if (NGX_HTTP_V2) +addrs[i].conf.http1 = addr[i].opt.http1; addrs[i].conf.http2 = addr[i].opt.http2; #endif addrs[i].conf.proxy_protocol = addr[i].opt.proxy_protocol; @@ -1867,6 +1871,7 @@ addrs6[i].conf.ssl = addr[i].opt.ssl; #endif #if (NGX_HTTP_V2) +addrs6[i].conf.http1 = addr[i].opt.http1; addrs6[i].conf.http2 = addr[i].opt.http2; #endif addrs6[i].conf.proxy_protocol = addr[i].opt.proxy_protocol; diff -r 81fae70d6cb8 -r 200955343460 src/http/ngx_http_core_module.c --- a/src/http/ngx_http_core_module.c Thu Mar 01 20:25:50 2018 +0300 +++ b/src/http/ngx_http_core_module.c Fri Mar 02 15:41:38 2018 +0800 @@ -3985,6 +3985,18 @@ #endif } +if (ngx_strcmp(value[n].data, "http1") == 0) { +#if (NGX_HTTP_V2) +lsopt.http1 = 1; +continue; +#else +ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, + "the \"http1\" parameter requires " + "ngx_http_v2_module"); +return NGX_CONF_ERROR; +#endif +} + if (ngx_strcmp(value[n].data, "spdy") == 0) { ngx_conf_log_error(NGX_LOG_WARN, cf, 0, "invalid parameter \"spdy\": " diff -r 81fae70d6cb8 -r 200955343460 src/http/ngx_http_core_module.h --- a/src/http/ngx_http_core_module.h Thu Mar 01 20:25:50 2018 +0300 +++ b/src/http/ngx_http_core_module.h Fri Mar 02 15:41:38 2018 +0800 @@ -73,6 +73,7 @@ unsigned bind:1; unsigned wildcard:1; unsigned ssl:1; +unsigned http1:1; unsigned http2:1; #if (NGX_HAVE_INET6) unsigned ipv6only:1; @@ -234,6 +235,7 @@ ngx_http_virtual_names_t *virtual_names; unsigned ssl:1; +unsigned http1:1; unsigned http2:1; unsigned proxy_protocol:1; }; diff -r 81fae70d6cb8 -r 200955343460 src/http/ngx_http_parse.c --- a/src/http/ngx_http_parse.c Thu Mar 01 20:25:50 2018 +0300 +++ b/src/http/ngx_http_parse.c Fri Mar 02 15:41:38 2018 +0800 @@ -285,6 +285,14 @@ break; } +#if (NGX_HTTP_V2) +if (ch == '*') { +r->uri_start = p; +state = sw_uri; +break; +} +#endif + c = (u_char) (ch | 0x20); if (c >= 'a' && c <= 'z') { r->schema_start = p; @@ -724,9 +732,11 @@ r->http_major = ch - '0'; +#if !(NGX_HTTP_V2) if (r->http_major > 1) { return NGX_HTTP_PARSE_INVALID_VERSION; } +#endif state = sw_major_digit; break; @@ -744,9 +754,11 @@ r->http_major = r->http_major * 10 + (ch - '0'); +#if !(NGX_HTTP_V2) if (r->http_major > 1) { return NGX_HTTP_PARSE_INVALID_VERSION; } +#endif break; diff -r 81fae70d6cb8 -r 200955343460 src/http/ngx_http_request.c --- a/src/http/ngx_http_request.c Thu Mar 01 20:25:50 2018 +0300 +++ b/src/http/ngx_http_request.c Fri Mar 02 15:41:38 2018 +0800 @@ -17,6 +17,10 @@ static ngx_int_t ngx_http_alloc_large_header_buffer(ngx_http_request_t *r, ngx_uint_t request_line); +#if (NGX_HTTP_V2) +static void ngx_http_process_h2_preface(ngx_event_t *rev); +#endif + 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_unique_header_line(ngx_http_request_t *r, @@ -320,7 +324,7 @@ c->write->handler = ngx_http_empty_handler; #if (NGX_HTTP_V2) -if (hc->addr_conf->http2) { +if (hc->addr_co
Re: Contrib: http2 per server (was re: [nginx] support http2 per server)
Not that anybody has responded yet, but please find an important improvement over this patch: -if (hc->addr_conf->http2 && !sscf->h2) { +if (r->http_version == NGX_HTTP_VERSION_20 && !sscf->h2) { Full patch (with this improvement included) below: diff -r 6b6e15bbda92 -r 2806e0ba8e91 src/http/modules/ngx_http_ssl_module.c --- a/src/http/modules/ngx_http_ssl_module.cTue Sep 05 17:59:31 2017 +0300 +++ b/src/http/modules/ngx_http_ssl_module.cFri Sep 08 01:07:46 2017 + @@ -234,6 +234,13 @@ offsetof(ngx_http_ssl_srv_conf_t, stapling_verify), NULL }, +{ ngx_string("ssl_h2"), + NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_CONF_FLAG, + ngx_http_ssl_enable, + NGX_HTTP_SRV_CONF_OFFSET, + offsetof(ngx_http_ssl_srv_conf_t, h2), + NULL }, + ngx_null_command }; @@ -354,6 +361,7 @@ #endif #if (NGX_HTTP_V2) ngx_http_connection_t *hc; +ngx_http_ssl_srv_conf_t *sscf; #endif #if (NGX_HTTP_V2 || NGX_DEBUG) ngx_connection_t *c; @@ -372,7 +380,9 @@ #if (NGX_HTTP_V2) hc = c->data; -if (hc->addr_conf->http2) { +sscf = ngx_http_get_module_srv_conf(hc->conf_ctx, ngx_http_ssl_module); + +if (hc->addr_conf->http2 && sscf->h2) { srv = (unsigned char *) NGX_HTTP_V2_ALPN_ADVERTISE NGX_HTTP_NPN_ADVERTISE; srvlen = sizeof(NGX_HTTP_V2_ALPN_ADVERTISE NGX_HTTP_NPN_ADVERTISE) - 1; @@ -416,10 +426,13 @@ #if (NGX_HTTP_V2) { ngx_http_connection_t *hc; +ngx_http_ssl_srv_conf_t *sscf; hc = c->data; -if (hc->addr_conf->http2) { +sscf = ngx_http_get_module_srv_conf(hc->conf_ctx, ngx_http_ssl_module); + +if (hc->addr_conf->http2 && sscf->h2) { *out = (unsigned char *) NGX_HTTP_V2_NPN_ADVERTISE NGX_HTTP_NPN_ADVERTISE; *outlen = sizeof(NGX_HTTP_V2_NPN_ADVERTISE NGX_HTTP_NPN_ADVERTISE) - 1; @@ -559,6 +572,7 @@ sscf->session_ticket_keys = NGX_CONF_UNSET_PTR; sscf->stapling = NGX_CONF_UNSET; sscf->stapling_verify = NGX_CONF_UNSET; +sscf->h2 = NGX_CONF_UNSET; return sscf; } @@ -624,6 +638,8 @@ ngx_conf_merge_str_value(conf->stapling_responder, prev->stapling_responder, ""); +ngx_conf_merge_value(conf->h2, prev->h2, 1); + conf->ssl.log = cf->log; if (conf->enable) { diff -r 6b6e15bbda92 -r 2806e0ba8e91 src/http/modules/ngx_http_ssl_module.h --- a/src/http/modules/ngx_http_ssl_module.hTue Sep 05 17:59:31 2017 +0300 +++ b/src/http/modules/ngx_http_ssl_module.hFri Sep 08 01:07:46 2017 + @@ -57,6 +57,9 @@ u_char *file; ngx_uint_t line; + +ngx_flag_t h2; + } ngx_http_ssl_srv_conf_t; diff -r 6b6e15bbda92 -r 2806e0ba8e91 src/http/ngx_http_request.c --- a/src/http/ngx_http_request.c Tue Sep 05 17:59:31 2017 +0300 +++ b/src/http/ngx_http_request.c Fri Sep 08 01:07:46 2017 + @@ -795,6 +795,7 @@ unsigned intlen; const unsigned char*data; ngx_http_connection_t *hc; +ngx_http_ssl_srv_conf_t *sscf; hc = c->data; @@ -813,9 +814,15 @@ SSL_get0_next_proto_negotiated(c->ssl->connection, , ); #endif -if (len == 2 && data[0] == 'h' && data[1] == '2') { -ngx_http_v2_init(c->read); -return; +sscf = ngx_http_get_module_srv_conf(hc->conf_ctx, ngx_http_ssl_module); + +if (sscf->h2) { + +if (len == 2 && data[0] == 'h' && data[1] == '2') { +ngx_http_v2_init(c->read); +return; +} + } } } @@ -2106,6 +2113,15 @@ ngx_http_finalize_request(r, NGX_HTTP_MISDIRECTED_REQUEST); return NGX_ERROR; } +#if (NGX_HTTP_V2) +if (r->http_version == NGX_HTTP_VERSION_20 && !sscf->h2) { +ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, + "client attempted to request a server name " + "that does not have http2 enabled"); +ngx_http_finalize_request(r, NGX_HTTP_MISDIRECTED_REQUEST); +return NGX_ERROR; +} +#endif } #endif ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Contrib: http2 per server (was re: [nginx] support http2 per server)
Following on from the previous thread on this ( http://mailman.nginx.org/pipermail/nginx-devel/2017-June/010079.html ) , and after discussion with the original author < hongzhidao at gmail.com > , I'd like to present the patches we are using. Please note the following improvements / modifications: 1. Default state is set to off during conf merge, so that existing behaviour of enabling http2 directive on the listener and having it apply to all servers is preserved. 2. Attempts to re-use a connection via the listener to access a server without ssl_h2 enabled (i.e, explicitly set to off) are met with a 421 (Misdirected Request), enforcing the intended switch behavior on the server block, (in response to the comment made in http://mailman.nginx.org/pipermail/nginx-devel/2017-June/010091.html ) This works in much the same way as the ssl verify in that after SNI and in ngx_http_set_virtual_server(), we check for the switch state on the target server. 3. Tests added to the test suite. Patches follow below (decided not to patchbomb as this is across two repositories) , feedback welcome - we'd like to see this feature adopted as it is clearly needed. Patch against main project - # HG changeset patch # User David Freedman <david.freed...@uk.clara.net> # Date 1504832866 0 # Fri Sep 08 01:07:46 2017 + # Node ID 2806e0ba8e91978ad6ce18ff1605b89bdd7806f4 # Parent 6b6e15bbda9269d03d66130efd51921dfedd93cb Supports selective enabling of http2 for SSL/TLS sites using new ssl_h2 option. diff -r 6b6e15bbda92 -r 2806e0ba8e91 src/http/modules/ngx_http_ssl_module.c --- a/src/http/modules/ngx_http_ssl_module.cTue Sep 05 17:59:31 2017 +0300 +++ b/src/http/modules/ngx_http_ssl_module.cFri Sep 08 01:07:46 2017 + @@ -234,6 +234,13 @@ offsetof(ngx_http_ssl_srv_conf_t, stapling_verify), NULL }, +{ ngx_string("ssl_h2"), + NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_CONF_FLAG, + ngx_http_ssl_enable, + NGX_HTTP_SRV_CONF_OFFSET, + offsetof(ngx_http_ssl_srv_conf_t, h2), + NULL }, + ngx_null_command }; @@ -354,6 +361,7 @@ #endif #if (NGX_HTTP_V2) ngx_http_connection_t *hc; +ngx_http_ssl_srv_conf_t *sscf; #endif #if (NGX_HTTP_V2 || NGX_DEBUG) ngx_connection_t *c; @@ -372,7 +380,9 @@ #if (NGX_HTTP_V2) hc = c->data; - if (hc->addr_conf->http2) { +sscf = ngx_http_get_module_srv_conf(hc->conf_ctx, ngx_http_ssl_module); + +if (hc->addr_conf->http2 && sscf->h2) { srv = (unsigned char *) NGX_HTTP_V2_ALPN_ADVERTISE NGX_HTTP_NPN_ADVERTISE; srvlen = sizeof(NGX_HTTP_V2_ALPN_ADVERTISE NGX_HTTP_NPN_ADVERTISE) - 1; @@ -416,10 +426,13 @@ #if (NGX_HTTP_V2) { ngx_http_connection_t *hc; +ngx_http_ssl_srv_conf_t *sscf; hc = c->data; -if (hc->addr_conf->http2) { +sscf = ngx_http_get_module_srv_conf(hc->conf_ctx, ngx_http_ssl_module); + +if (hc->addr_conf->http2 && sscf->h2) { *out = (unsigned char *) NGX_HTTP_V2_NPN_ADVERTISE NGX_HTTP_NPN_ADVERTISE; *outlen = sizeof(NGX_HTTP_V2_NPN_ADVERTISE NGX_HTTP_NPN_ADVERTISE) - 1; @@ -559,6 +572,7 @@ sscf->session_ticket_keys = NGX_CONF_UNSET_PTR; sscf->stapling = NGX_CONF_UNSET; sscf->stapling_verify = NGX_CONF_UNSET; +sscf->h2 = NGX_CONF_UNSET; return sscf; } @@ -624,6 +638,8 @@ ngx_conf_merge_str_value(conf->stapling_responder, prev->stapling_responder, ""); +ngx_conf_merge_value(conf->h2, prev->h2, 1); + conf->ssl.log = cf->log; if (conf->enable) { diff -r 6b6e15bbda92 -r 2806e0ba8e91 src/http/modules/ngx_http_ssl_module.h --- a/src/http/modules/ngx_http_ssl_module.hTue Sep 05 17:59:31 2017 +0300 +++ b/src/http/modules/ngx_http_ssl_module.hFri Sep 08 01:07:46 2017 + @@ -57,6 +57,9 @@ u_char *file; ngx_uint_t line; + +ngx_flag_t h2; + } ngx_http_ssl_srv_conf_t; diff -r 6b6e15bbda92 -r 2806e0ba8e91 src/http/ngx_http_request.c --- a/src/http/ngx_http_request.c Tue Sep 05 17:59:31 2017 +0300 +++ b/src/http/ngx_http_request.c Fri Sep 08 01:07:46 2017 + @@ -795,6 +795,7 @@ unsigned intlen; const unsigned char*data; ngx_http_connection_t *hc; +ngx_http_ssl_srv_conf_t *sscf; hc = c->data; @@ -813,9 +814,15 @@ SSL_get0_next_proto_negotiated(c->ssl->connection, , ); #endif -if (len == 2 && data[0] == 'h' && data[1] == '2') { -ngx_http_v2_init(c->read); -return; +sscf = ngx_http_get_module_srv_conf(hc->conf_ctx, ngx_http_ssl_module); + +if (sscf
Re: [nginx] support http2 per server
Hi, Valentin. Please confirm again, thanks. diff -r 5e05118678af src/http/modules/ngx_http_ssl_module.c --- a/src/http/modules/ngx_http_ssl_module.c Mon May 29 23:33:38 2017 +0300 +++ b/src/http/modules/ngx_http_ssl_module.c Fri Jun 09 07:15:50 2017 -0400 @@ -234,6 +234,13 @@ offsetof(ngx_http_ssl_srv_conf_t, stapling_verify), NULL }, +{ ngx_string("ssl_h2"), + NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_CONF_FLAG, + ngx_http_ssl_enable, + NGX_HTTP_SRV_CONF_OFFSET, + offsetof(ngx_http_ssl_srv_conf_t, h2), + NULL }, + ngx_null_command }; @@ -343,16 +350,17 @@ unsigned char *outlen, const unsigned char *in, unsigned int inlen, void *arg) { -unsigned intsrvlen; -unsigned char *srv; +unsigned int srvlen; +unsigned char *srv; #if (NGX_DEBUG) -unsigned inti; +unsigned int i; #endif #if (NGX_HTTP_V2) -ngx_http_connection_t *hc; +ngx_http_connection_t *hc; +ngx_http_ssl_srv_conf_t *sscf; #endif #if (NGX_HTTP_V2 || NGX_DEBUG) -ngx_connection_t *c; +ngx_connection_t *c; c = ngx_ssl_get_connection(ssl_conn); #endif @@ -367,8 +375,9 @@ #if (NGX_HTTP_V2) hc = c->data; +sscf = ngx_http_get_module_srv_conf(hc->conf_ctx, ngx_http_ssl_module); -if (hc->addr_conf->http2) { + if (hc->addr_conf->http2 && sscf->h2) { srv = (unsigned char *) NGX_HTTP_V2_ALPN_ADVERTISE NGX_HTTP_NPN_ADVERTISE; srvlen = sizeof(NGX_HTTP_V2_ALPN_ADVERTISE NGX_HTTP_NPN_ADVERTISE) - 1; @@ -411,11 +420,13 @@ #if (NGX_HTTP_V2) { -ngx_http_connection_t *hc; +ngx_http_connection_t *hc; +ngx_http_ssl_srv_conf_t *sscf; hc = c->data; +sscf = ngx_http_get_module_srv_conf(hc->conf_ctx, ngx_http_ssl_module); -if (hc->addr_conf->http2) { +if (hc->addr_conf->http2 && sscf->h2) { *out = (unsigned char *) NGX_HTTP_V2_NPN_ADVERTISE NGX_HTTP_NPN_ADVERTISE; *outlen = sizeof(NGX_HTTP_V2_NPN_ADVERTISE NGX_HTTP_NPN_ADVERTISE) - 1; @@ -555,6 +566,7 @@ sscf->session_ticket_keys = NGX_CONF_UNSET_PTR; sscf->stapling = NGX_CONF_UNSET; sscf->stapling_verify = NGX_CONF_UNSET; +sscf->h2 = NGX_CONF_UNSET; return sscf; } @@ -620,6 +632,8 @@ ngx_conf_merge_str_value(conf->stapling_responder, prev->stapling_responder, ""); +ngx_conf_merge_value(conf->h2, prev->h2, 0); + conf->ssl.log = cf->log; if (conf->enable) { diff -r 5e05118678af src/http/modules/ngx_http_ssl_module.h --- a/src/http/modules/ngx_http_ssl_module.h Mon May 29 23:33:38 2017 +0300 +++ b/src/http/modules/ngx_http_ssl_module.h Fri Jun 09 07:15:50 2017 -0400 @@ -57,6 +57,8 @@ u_char *file; ngx_uint_t line; + +ngx_flag_t h2; } ngx_http_ssl_srv_conf_t; diff -r 5e05118678af src/http/ngx_http_request.c --- a/src/http/ngx_http_request.c Mon May 29 23:33:38 2017 +0300 +++ b/src/http/ngx_http_request.c Fri Jun 09 07:15:50 2017 -0400 @@ -784,9 +784,10 @@ && (defined TLSEXT_TYPE_application_layer_protocol_negotiation \ || defined TLSEXT_TYPE_next_proto_neg)) { -unsigned intlen; -const unsigned char*data; -ngx_http_connection_t *hc; +unsigned int len; +const unsigned char *data; +ngx_http_connection_t*hc; +ngx_http_ssl_srv_conf_t *sscf; hc = c->data; @@ -805,9 +806,14 @@ SSL_get0_next_proto_negotiated(c->ssl->connection, , ); #endif -if (len == 2 && data[0] == 'h' && data[1] == '2') { -ngx_http_v2_init(c->read); -return; +sscf = ngx_http_get_module_srv_conf(hc->conf_ctx, ngx_http_ssl_module); + +if (sscf->h2) { + +if (len == 2 && data[0] == 'h' && data[1] == '2') { +ngx_http_v2_init(c->read); +return; +} } } } ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [nginx] support http2 per server
Get it, thanks. On Fri, Jun 9, 2017 at 7:45 PM, Valentin V. Bartenevwrote: > On Friday 09 June 2017 13:40:23 洪志道 wrote: > > Hi, Valentin. > > > > " > > Also please note that with your patch clients are still able to > > negotiate HTTP/2 even if nginx doesn't announce it. > > " > > > > Two points: > > 1. The patch forbids the clients explicitly not support HTTP/2 doing v2 ( > > ngx_http_v2_init). > > How to follow you mean of "with the patch, clients are still able to > > negotiate HTTP/2" > > 2. "even if nginx doesn't announce it" > > Is it related to nginx? > > > [..] > > Your patch prevents advertising the protocol using ALPN and NPN, but > selecting protocol happens here (this part of code isn't touched): > http://hg.nginx.org/nginx/file/tip/src/http/ngx_http_request.c#l808 > > In case of NPN, client can select protocol even if it hasn't been > advertised by the server. > > wbr, Valentin V. Bartenev > > ___ > nginx-devel mailing list > nginx-devel@nginx.org > http://mailman.nginx.org/mailman/listinfo/nginx-devel > ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [nginx] support http2 per server
On Friday 09 June 2017 13:40:23 洪志道 wrote: > Hi, Valentin. > > " > Also please note that with your patch clients are still able to > negotiate HTTP/2 even if nginx doesn't announce it. > " > > Two points: > 1. The patch forbids the clients explicitly not support HTTP/2 doing v2 ( > ngx_http_v2_init). > How to follow you mean of "with the patch, clients are still able to > negotiate HTTP/2" > 2. "even if nginx doesn't announce it" > Is it related to nginx? > [..] Your patch prevents advertising the protocol using ALPN and NPN, but selecting protocol happens here (this part of code isn't touched): http://hg.nginx.org/nginx/file/tip/src/http/ngx_http_request.c#l808 In case of NPN, client can select protocol even if it hasn't been advertised by the server. wbr, Valentin V. Bartenev ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [nginx] support http2 per server
Hi, Valentin. " Also please note that with your patch clients are still able to negotiate HTTP/2 even if nginx doesn't announce it. " Two points: 1. The patch forbids the clients explicitly not support HTTP/2 doing v2 ( ngx_http_v2_init). How to follow you mean of "with the patch, clients are still able to negotiate HTTP/2" 2. "even if nginx doesn't announce it" Is it related to nginx? On Fri, Jun 9, 2017 at 1:09 AM, Valentin V. Bartenev <vb...@nginx.com> wrote: > On Friday 09 June 2017 00:08:06 洪志道 wrote: > > " > > > >For "https" resources, connection reuse additionally depends on > >having a certificate that is valid for the host in the URI. The > >certificate presented by the server MUST satisfy any checks that the > >client would perform when forming a new TLS connection for the host > >in the URI. > > > > " > > > > > > It seems the brower can prevent the unreasonable behavior. > > > > > > In reallity, It still exist some clients that dosen't perfom well in > http2. > > > > So it's kind of valuable to enable http2 by server. > > > > > > It's not a good idea the put the patch in nginx, > > > > Can you help to check the patch whether contains serious problem? > > > > > > Maybe it's helpful for other guys. > > > > > > Thanks again. > > > [..] > > The most serious problem with the patch, that it gives an illusion > that HTTP/2 can be enabled per virtual server basis, but in fact it > doesn't prevent requests to any server on particular listen socket > using already existing HTTP/2 connection. > > Also please note that with your patch clients are still able to > negotiate HTTP/2 even if nginx doesn't announce it. > > I don't see any other serious problems. > > wbr, Valentin V. Bartenev > > ___ > nginx-devel mailing list > nginx-devel@nginx.org > http://mailman.nginx.org/mailman/listinfo/nginx-devel > ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [nginx] support http2 per server
WRT the below, he H2 RFC includes a new status code to deal with thus, 421: https://tools.ietf.org/html/rfc7540#section-9.1.2 Client support is poor right no so it'd be good if sending 421 was optional perhaps. Cheers Sent from my iPhone On 8 Jun 2017, at 18:17, 洪志道 <hongzhi...@gmail.com<mailto:hongzhi...@gmail.com>> wrote: Thanks. On Fri, Jun 9, 2017 at 1:09 AM, Valentin V. Bartenev <vb...@nginx.com<mailto:vb...@nginx.com>> wrote: On Friday 09 June 2017 00:08:06 洪志道 wrote: > " > >For "https" resources, connection reuse additionally depends on >having a certificate that is valid for the host in the URI. The >certificate presented by the server MUST satisfy any checks that the >client would perform when forming a new TLS connection for the host >in the URI. > > " > > > It seems the brower can prevent the unreasonable behavior. > > > In reallity, It still exist some clients that dosen't perfom well in http2. > > So it's kind of valuable to enable http2 by server. > > > It's not a good idea the put the patch in nginx, > > Can you help to check the patch whether contains serious problem? > > > Maybe it's helpful for other guys. > > > Thanks again. > [..] The most serious problem with the patch, that it gives an illusion that HTTP/2 can be enabled per virtual server basis, but in fact it doesn't prevent requests to any server on particular listen socket using already existing HTTP/2 connection. Also please note that with your patch clients are still able to negotiate HTTP/2 even if nginx doesn't announce it. I don't see any other serious problems. wbr, Valentin V. Bartenev ___ nginx-devel mailing list nginx-devel@nginx.org<mailto:nginx-devel@nginx.org> http://mailman.nginx.org/mailman/listinfo/nginx-devel ___ nginx-devel mailing list nginx-devel@nginx.org<mailto:nginx-devel@nginx.org> http://mailman.nginx.org/mailman/listinfo/nginx-devel ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [nginx] support http2 per server
Thanks. On Fri, Jun 9, 2017 at 1:09 AM, Valentin V. Bartenev <vb...@nginx.com> wrote: > On Friday 09 June 2017 00:08:06 洪志道 wrote: > > " > > > >For "https" resources, connection reuse additionally depends on > >having a certificate that is valid for the host in the URI. The > >certificate presented by the server MUST satisfy any checks that the > >client would perform when forming a new TLS connection for the host > >in the URI. > > > > " > > > > > > It seems the brower can prevent the unreasonable behavior. > > > > > > In reallity, It still exist some clients that dosen't perfom well in > http2. > > > > So it's kind of valuable to enable http2 by server. > > > > > > It's not a good idea the put the patch in nginx, > > > > Can you help to check the patch whether contains serious problem? > > > > > > Maybe it's helpful for other guys. > > > > > > Thanks again. > > > [..] > > The most serious problem with the patch, that it gives an illusion > that HTTP/2 can be enabled per virtual server basis, but in fact it > doesn't prevent requests to any server on particular listen socket > using already existing HTTP/2 connection. > > Also please note that with your patch clients are still able to > negotiate HTTP/2 even if nginx doesn't announce it. > > I don't see any other serious problems. > > wbr, Valentin V. Bartenev > > ___ > nginx-devel mailing list > nginx-devel@nginx.org > http://mailman.nginx.org/mailman/listinfo/nginx-devel > ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [nginx] support http2 per server
On Friday 09 June 2017 00:08:06 洪志道 wrote: > " > >For "https" resources, connection reuse additionally depends on >having a certificate that is valid for the host in the URI. The >certificate presented by the server MUST satisfy any checks that the >client would perform when forming a new TLS connection for the host >in the URI. > > " > > > It seems the brower can prevent the unreasonable behavior. > > > In reallity, It still exist some clients that dosen't perfom well in http2. > > So it's kind of valuable to enable http2 by server. > > > It's not a good idea the put the patch in nginx, > > Can you help to check the patch whether contains serious problem? > > > Maybe it's helpful for other guys. > > > Thanks again. > [..] The most serious problem with the patch, that it gives an illusion that HTTP/2 can be enabled per virtual server basis, but in fact it doesn't prevent requests to any server on particular listen socket using already existing HTTP/2 connection. Also please note that with your patch clients are still able to negotiate HTTP/2 even if nginx doesn't announce it. I don't see any other serious problems. wbr, Valentin V. Bartenev ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [nginx] support http2 per server
" For "https" resources, connection reuse additionally depends on having a certificate that is valid for the host in the URI. The certificate presented by the server MUST satisfy any checks that the client would perform when forming a new TLS connection for the host in the URI. " It seems the brower can prevent the unreasonable behavior. In reallity, It still exist some clients that dosen't perfom well in http2. So it's kind of valuable to enable http2 by server. It's not a good idea the put the patch in nginx, Can you help to check the patch whether contains serious problem? Maybe it's helpful for other guys. Thanks again. On Thu, Jun 8, 2017 at 11:25 PM, Valentin V. Bartenev <vb...@nginx.com> wrote: > On Thursday 08 June 2017 23:19:23 洪志道 wrote: > > It sounds right. > > > > According to the same situation, how does http2 protocol force other > > virtual servers to process certificate (ssl handshake). > > > > Example: > > > > server { > > listen 443 http2; > > a.com; > > ssl_certi; > > } > > > > server { > > listen 443 http2; > > b.com; > > ssl_certi; > > } > > > > We assume sni is 'a.com', then the connection contains different > requests > > with different domains. > > That b.com will escape from ssl handshake. In fact, we need it to do the > > ssl handshake. > > > > Thanks. > > > > It is called "Connection Reuse" in HTTP/2: > https://tools.ietf.org/html/rfc7540#section-9.1.1 > > wbr, Valentin V. Bartenev > > ___ > nginx-devel mailing list > nginx-devel@nginx.org > http://mailman.nginx.org/mailman/listinfo/nginx-devel > ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [nginx] support http2 per server
On Thursday 08 June 2017 23:19:23 洪志道 wrote: > It sounds right. > > According to the same situation, how does http2 protocol force other > virtual servers to process certificate (ssl handshake). > > Example: > > server { > listen 443 http2; > a.com; > ssl_certi; > } > > server { > listen 443 http2; > b.com; > ssl_certi; > } > > We assume sni is 'a.com', then the connection contains different requests > with different domains. > That b.com will escape from ssl handshake. In fact, we need it to do the > ssl handshake. > > Thanks. > It is called "Connection Reuse" in HTTP/2: https://tools.ietf.org/html/rfc7540#section-9.1.1 wbr, Valentin V. Bartenev ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [nginx] support http2 per server
It sounds right. According to the same situation, how does http2 protocol force other virtual servers to process certificate (ssl handshake). Example: server { listen 443 http2; a.com; ssl_certi; } server { listen 443 http2; b.com; ssl_certi; } We assume sni is 'a.com', then the connection contains different requests with different domains. That b.com will escape from ssl handshake. In fact, we need it to do the ssl handshake. Thanks. On Thu, Jun 8, 2017 at 10:17 PM, Valentin V. Bartenev <vb...@nginx.com> wrote: > On Thursday 08 June 2017 12:07:29 洪志道 wrote: > > Hi! > > Now, http2 is enabled globally for 'listen' directive with ip:port. > > It seems it's possible to enable by server with sni, alpn, npn. > > Take a look, please. > > > [..] > > How will "sni, alpn, npn" prevent browser from asking other virtual > servers using already opened HTTP/2 connection? > >wbr, Valentin V. Bartenev > > ___ > nginx-devel mailing list > nginx-devel@nginx.org > http://mailman.nginx.org/mailman/listinfo/nginx-devel ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [nginx] support http2 per server
On Thursday 08 June 2017 12:07:29 洪志道 wrote: > Hi! > Now, http2 is enabled globally for 'listen' directive with ip:port. > It seems it's possible to enable by server with sni, alpn, npn. > Take a look, please. > [..] How will "sni, alpn, npn" prevent browser from asking other virtual servers using already opened HTTP/2 connection? wbr, Valentin V. Bartenev ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [nginx] support http2 per server
Sorry for the typo. diff -r 5e05118678af src/http/modules/ngx_http_ssl_module.c --- a/src/http/modules/ngx_http_ssl_module.c Mon May 29 23:33:38 2017 +0300 +++ b/src/http/modules/ngx_http_ssl_module.c Wed Jun 07 12:17:34 2017 -0400 @@ -234,6 +234,13 @@ offsetof(ngx_http_ssl_srv_conf_t, stapling_verify), NULL }, +{ ngx_string("ssl_h2"), + NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_CONF_FLAG, + ngx_conf_set_flag_slot, + NGX_HTTP_SRV_CONF_OFFSET, + offsetof(ngx_http_ssl_srv_conf_t, h2), + NULL }, + ngx_null_command }; @@ -343,16 +350,17 @@ unsigned char *outlen, const unsigned char *in, unsigned int inlen, void *arg) { -unsigned intsrvlen; -unsigned char *srv; +unsigned int srvlen; +unsigned char *srv; #if (NGX_DEBUG) -unsigned inti; +unsigned int i; #endif #if (NGX_HTTP_V2) -ngx_http_connection_t *hc; +ngx_http_connection_t *hc; +ngx_http_ssl_srv_conf_t *sscf; #endif #if (NGX_HTTP_V2 || NGX_DEBUG) -ngx_connection_t *c; +ngx_connection_t *c; c = ngx_ssl_get_connection(ssl_conn); #endif @@ -367,8 +375,9 @@ #if (NGX_HTTP_V2) hc = c->data; +sscf = ngx_http_get_module_srv_conf(hc->conf_ctx, ngx_http_ssl_module); -if (hc->addr_conf->http2) { + if (hc->addr_conf->http2 && sscf->h2) { srv = (unsigned char *) NGX_HTTP_V2_ALPN_ADVERTISE NGX_HTTP_NPN_ADVERTISE; srvlen = sizeof(NGX_HTTP_V2_ALPN_ADVERTISE NGX_HTTP_NPN_ADVERTISE) - 1; @@ -411,11 +420,13 @@ #if (NGX_HTTP_V2) { -ngx_http_connection_t *hc; +ngx_http_connection_t *hc; +ngx_http_ssl_srv_conf_t *sscf; hc = c->data; +sscf = ngx_http_get_module_srv_conf(hc->conf_ctx, ngx_http_ssl_module); -if (hc->addr_conf->http2) { +if (hc->addr_conf->http2 && sscf->h2) { *out = (unsigned char *) NGX_HTTP_V2_NPN_ADVERTISE NGX_HTTP_NPN_ADVERTISE; *outlen = sizeof(NGX_HTTP_V2_NPN_ADVERTISE NGX_HTTP_NPN_ADVERTISE) - 1; @@ -555,6 +566,7 @@ sscf->session_ticket_keys = NGX_CONF_UNSET_PTR; sscf->stapling = NGX_CONF_UNSET; sscf->stapling_verify = NGX_CONF_UNSET; +sscf->h2 = NGX_CONF_UNSET; return sscf; } @@ -620,6 +632,8 @@ ngx_conf_merge_str_value(conf->stapling_responder, prev->stapling_responder, ""); +ngx_conf_merge_value(conf->h2, prev->h2, 0); + conf->ssl.log = cf->log; if (conf->enable) { diff -r 5e05118678af src/http/modules/ngx_http_ssl_module.h --- a/src/http/modules/ngx_http_ssl_module.h Mon May 29 23:33:38 2017 +0300 +++ b/src/http/modules/ngx_http_ssl_module.h Wed Jun 07 12:17:34 2017 -0400 @@ -57,6 +57,8 @@ u_char *file; ngx_uint_t line; + +ngx_flag_t h2; } ngx_http_ssl_srv_conf_t; On Thu, Jun 8, 2017 at 12:07 PM, 洪志道 <hongzhi...@gmail.com> wrote: > Hi! > Now, http2 is enabled globally for 'listen' directive with ip:port. > It seems it's possible to enable by server with sni, alpn, npn. > Take a look, please. > > diff -r 5e05118678af src/http/modules/ngx_http_ssl_module.c > --- a/src/http/modules/ngx_http_ssl_module.c Mon May 29 23:33:38 2017 > +0300 > +++ b/src/http/modules/ngx_http_ssl_module.c Wed Jun 07 12:17:34 2017 > -0400 > @@ -234,6 +234,13 @@ >offsetof(ngx_http_ssl_srv_conf_t, stapling_verify), >NULL }, > > +{ ngx_string("ssl_h2"), > + NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_CONF_FLAG, > + ngx_http_ssl_enable, > + NGX_HTTP_SRV_CONF_OFFSET, > + offsetof(ngx_http_ssl_srv_conf_t, h2), > + NULL }, > + >ngx_null_command > }; > > @@ -343,16 +350,17 @@ > unsigned char *outlen, const unsigned char *in, unsigned int inlen, > void *arg) > { > -unsigned intsrvlen; > -unsigned char *srv; > +unsigned int srvlen; > +unsigned char *srv; > #if (NGX_DEBUG) > -unsigned inti; > +unsigned int i; > #endif > #if (NGX_HTTP_V2) > -ngx_http_connection_t *hc; > +ngx_http_connection_t *hc; > +ngx_http_ssl_srv_conf_t *sscf; > #endif > #if (NGX_HTTP_V2 || NGX_DEBUG) > -ngx_connection_t *c; > +ngx_connection_t *c; > > c = ngx_ssl_get_connection(ssl_conn); > #endif > @@ -367,8 +375,9 @@ > > #if (NGX_HTTP_V2) > hc = c->data; > +sscf = ngx_http_get_module_srv_conf(hc->conf_ctx, > ngx_http_ssl_module); > > -if (hc->addr_conf->http2) { > +if (hc->addr_conf->http2 &&am
[nginx] support http2 per server
Hi! Now, http2 is enabled globally for 'listen' directive with ip:port. It seems it's possible to enable by server with sni, alpn, npn. Take a look, please. diff -r 5e05118678af src/http/modules/ngx_http_ssl_module.c --- a/src/http/modules/ngx_http_ssl_module.c Mon May 29 23:33:38 2017 +0300 +++ b/src/http/modules/ngx_http_ssl_module.c Wed Jun 07 12:17:34 2017 -0400 @@ -234,6 +234,13 @@ offsetof(ngx_http_ssl_srv_conf_t, stapling_verify), NULL }, +{ ngx_string("ssl_h2"), + NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_CONF_FLAG, + ngx_http_ssl_enable, + NGX_HTTP_SRV_CONF_OFFSET, + offsetof(ngx_http_ssl_srv_conf_t, h2), + NULL }, + ngx_null_command }; @@ -343,16 +350,17 @@ unsigned char *outlen, const unsigned char *in, unsigned int inlen, void *arg) { -unsigned intsrvlen; -unsigned char *srv; +unsigned int srvlen; +unsigned char *srv; #if (NGX_DEBUG) -unsigned inti; +unsigned int i; #endif #if (NGX_HTTP_V2) -ngx_http_connection_t *hc; +ngx_http_connection_t *hc; +ngx_http_ssl_srv_conf_t *sscf; #endif #if (NGX_HTTP_V2 || NGX_DEBUG) -ngx_connection_t *c; +ngx_connection_t *c; c = ngx_ssl_get_connection(ssl_conn); #endif @@ -367,8 +375,9 @@ #if (NGX_HTTP_V2) hc = c->data; +sscf = ngx_http_get_module_srv_conf(hc->conf_ctx, ngx_http_ssl_module); -if (hc->addr_conf->http2) { + if (hc->addr_conf->http2 && sscf->h2) { srv = (unsigned char *) NGX_HTTP_V2_ALPN_ADVERTISE NGX_HTTP_NPN_ADVERTISE; srvlen = sizeof(NGX_HTTP_V2_ALPN_ADVERTISE NGX_HTTP_NPN_ADVERTISE) - 1; @@ -411,11 +420,13 @@ #if (NGX_HTTP_V2) { -ngx_http_connection_t *hc; +ngx_http_connection_t *hc; +ngx_http_ssl_srv_conf_t *sscf; hc = c->data; +sscf = ngx_http_get_module_srv_conf(hc->conf_ctx, ngx_http_ssl_module); -if (hc->addr_conf->http2) { +if (hc->addr_conf->http2 && sscf->h2) { *out = (unsigned char *) NGX_HTTP_V2_NPN_ADVERTISE NGX_HTTP_NPN_ADVERTISE; *outlen = sizeof(NGX_HTTP_V2_NPN_ADVERTISE NGX_HTTP_NPN_ADVERTISE) - 1; @@ -555,6 +566,7 @@ sscf->session_ticket_keys = NGX_CONF_UNSET_PTR; sscf->stapling = NGX_CONF_UNSET; sscf->stapling_verify = NGX_CONF_UNSET; +sscf->h2 = NGX_CONF_UNSET; return sscf; } @@ -620,6 +632,8 @@ ngx_conf_merge_str_value(conf->stapling_responder, prev->stapling_responder, ""); +ngx_conf_merge_value(conf->h2, prev->h2, 0); + conf->ssl.log = cf->log; if (conf->enable) { diff -r 5e05118678af src/http/modules/ngx_http_ssl_module.h --- a/src/http/modules/ngx_http_ssl_module.h Mon May 29 23:33:38 2017 +0300 +++ b/src/http/modules/ngx_http_ssl_module.h Wed Jun 07 12:17:34 2017 -0400 @@ -57,6 +57,8 @@ u_char *file; ngx_uint_t line; + +ngx_flag_t h2; } ngx_http_ssl_srv_conf_t; Thanks. B.R. ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[nginx] Modules compatibility: http2.
details: http://hg.nginx.org/nginx/rev/bdf64ae3376b branches: changeset: 6718:bdf64ae3376b user: Maxim Dounin <mdou...@mdounin.ru> date: Mon Oct 03 15:58:22 2016 +0300 description: Modules compatibility: http2. HTTP/2-specific fields in structures are now available unconditionally. Removed NGX_HTTP_V2 from the signature accordingly. diffstat: src/core/ngx_module.h | 4 src/http/ngx_http.h | 3 --- src/http/ngx_http_core_module.h | 4 src/http/ngx_http_request.h | 4 4 files changed, 0 insertions(+), 15 deletions(-) diffs (75 lines): diff -r 3b522d7a5b34 -r bdf64ae3376b src/core/ngx_module.h --- a/src/core/ngx_module.h Mon Oct 03 15:58:19 2016 +0300 +++ b/src/core/ngx_module.h Mon Oct 03 15:58:22 2016 +0300 @@ -158,11 +158,7 @@ #define NGX_MODULE_SIGNATURE_24 "0" #endif -#if (NGX_HTTP_V2) #define NGX_MODULE_SIGNATURE_25 "1" -#else -#define NGX_MODULE_SIGNATURE_25 "0" -#endif #if (NGX_HTTP_GZIP) #define NGX_MODULE_SIGNATURE_26 "1" diff -r 3b522d7a5b34 -r bdf64ae3376b src/http/ngx_http.h --- a/src/http/ngx_http.h Mon Oct 03 15:58:19 2016 +0300 +++ b/src/http/ngx_http.h Mon Oct 03 15:58:22 2016 +0300 @@ -19,10 +19,7 @@ typedef struct ngx_http_cache_s ng typedef struct ngx_http_file_cache_s ngx_http_file_cache_t; typedef struct ngx_http_log_ctx_s ngx_http_log_ctx_t; typedef struct ngx_http_chunked_s ngx_http_chunked_t; - -#if (NGX_HTTP_V2) typedef struct ngx_http_v2_stream_s ngx_http_v2_stream_t; -#endif typedef ngx_int_t (*ngx_http_header_handler_pt)(ngx_http_request_t *r, ngx_table_elt_t *h, ngx_uint_t offset); diff -r 3b522d7a5b34 -r bdf64ae3376b src/http/ngx_http_core_module.h --- a/src/http/ngx_http_core_module.h Mon Oct 03 15:58:19 2016 +0300 +++ b/src/http/ngx_http_core_module.h Mon Oct 03 15:58:22 2016 +0300 @@ -68,9 +68,7 @@ typedef struct { #if (NGX_HTTP_SSL) unsigned ssl:1; #endif -#if (NGX_HTTP_V2) unsigned http2:1; -#endif #if (NGX_HAVE_INET6 && defined IPV6_V6ONLY) unsigned ipv6only:1; #endif @@ -237,9 +235,7 @@ struct ngx_http_addr_conf_s { #if (NGX_HTTP_SSL) unsigned ssl:1; #endif -#if (NGX_HTTP_V2) unsigned http2:1; -#endif unsigned proxy_protocol:1; }; diff -r 3b522d7a5b34 -r bdf64ae3376b src/http/ngx_http_request.h --- a/src/http/ngx_http_request.h Mon Oct 03 15:58:19 2016 +0300 +++ b/src/http/ngx_http_request.h Mon Oct 03 15:58:22 2016 +0300 @@ -286,9 +286,7 @@ typedef struct { ngx_chain_t *bufs; ngx_buf_t*buf; off_t rest; -#if (NGX_HTTP_V2) off_t received; -#endif ngx_chain_t *free; ngx_chain_t *busy; ngx_http_chunked_t *chunked; @@ -438,9 +436,7 @@ struct ngx_http_request_s { ngx_uint_terr_status; ngx_http_connection_t*http_connection; -#if (NGX_HTTP_V2) ngx_http_v2_stream_t *stream; -#endif ngx_http_log_handler_pt log_handler; ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: 1.9.14 - http2 protocol violations
On Wednesday 13 April 2016 19:35:25 Валентин Бартенев wrote: > On Wednesday 13 April 2016 18:00:26 Валентин Бартенев wrote: > > On Wednesday 13 April 2016 15:08:29 Otto van der Schaaf wrote: > > > I'm sorry to say that the patch does not make a difference. > > > > > > I collected Chrome's http/2 trace for a post to a non-existant url: > > > https://gist.github.com/oschaaf/da273f96fad5e22890981fcd4a1a4376 > > > > > > I'm wondering about that last HTTP2_SESSION_RST_STREAM entry.. it looks > > > like > > > the httpv2 mod wants to cancel the incoming post data stream at an odd > > > point in time? > > > But I'm no expert, so I'm not sure at all. > > > > > [..] > > > > I was able to capture quite the same Chrome trace with the actual bytes > > sent/received and this last RST_STREAM entry with status 6 (which is > > FRAME_SIZE_ERROR) looks really strange. > > > > > > It isn't presented in nginx debug log (nginx hasn't sent, nor has received > > it). Moreover, the actual bytes sent/received trace, that you can get from > > Chrome if you set the "Include the actual bytes sent/received." flag in > > chrome://net-internals/#capture), doesn't contain it either. > > > > The trace (and nginx log) contains RST_STREAM sent by nginx with status 0 > > (NO_ERROR), which is correct: > > > > t= 94141 [st= 6088]SSL_SOCKET_BYTES_RECEIVED > >--> byte_count = 13 > >--> hex_encoded_bytes = > > 00 00 04 03 00 00 00 00 03 00 00 00 00 > > > > As you can see it has 4 bytes length, type 3, flags 0, stream id 3 > > and error code 0. > > > > +---+ > > | Length (24) | > > +---+---+---+ > > | Type (8)| Flags (8) | > > +-+-+---+---+ > > |R| Stream Identifier (31) | > > +=+=+ > > | Frame Payload (0...) ... > > +---+ > > > > Figure 1: Frame Layout > > > > > > https://tools.ietf.org/html/rfc7540#section-4.1 > > > > > > +---+ > > |Error Code (32)| > > +---+ > > > > Figure 9: RST_STREAM Frame Payload > > > > https://tools.ietf.org/html/rfc7540#section-6.4 > > > > I've attached the full trace. > > > > Maybe I missed something, but after manual decoding bytes I don't see any > > problems with the frame sizes received from nginx. > > > > However, that can be a result of this change: > > http://hg.nginx.org/nginx/rev/92464ebace8e > > > > Firefox perfectly fine with it. It looks like the problem in Chrome. > > > > Well, I've found the key how to interpret these statuses in Chromium sources: > > // Status codes for RST_STREAM frames. > enum SpdyRstStreamStatus { > RST_STREAM_INVALID = 0, > RST_STREAM_PROTOCOL_ERROR = 1, > RST_STREAM_INVALID_STREAM = 2, > RST_STREAM_STREAM_CLOSED = 2, // Equivalent to INVALID_STREAM > RST_STREAM_REFUSED_STREAM = 3, > RST_STREAM_UNSUPPORTED_VERSION = 4, > RST_STREAM_CANCEL = 5, > RST_STREAM_INTERNAL_ERROR = 6, > RST_STREAM_FLOW_CONTROL_ERROR = 7, > RST_STREAM_STREAM_IN_USE = 8, > RST_STREAM_STREAM_ALREADY_CLOSED = 9, > RST_STREAM_INVALID_CREDENTIALS = 10, > // FRAME_TOO_LARGE (defined by SPDY versions 3.1 and below), and > // FRAME_SIZE_ERROR (defined by HTTP/2) are mapped to the same internal > // reset status. > RST_STREAM_FRAME_TOO_LARGE = 11, > RST_STREAM_FRAME_SIZE_ERROR = 11, > RST_STREAM_SETTINGS_TIMEOUT = 12, > RST_STREAM_CONNECT_ERROR = 13, > RST_STREAM_ENHANCE_YOUR_CALM = 14, > RST_STREAM_NUM_STATUS_CODES = 15 > }; > > looks like NO_ERROR is handled as internal error, which is obviously wrong. > I've opened a ticket for this: https://bugs.chromium.org/p/chromium/issues/detail?id=603182 wbr, Valentin V. Bartenev ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: 1.9.14 - http2 protocol violations
On Wednesday 13 April 2016 18:00:26 Валентин Бартенев wrote: > On Wednesday 13 April 2016 15:08:29 Otto van der Schaaf wrote: > > I'm sorry to say that the patch does not make a difference. > > > > I collected Chrome's http/2 trace for a post to a non-existant url: > > https://gist.github.com/oschaaf/da273f96fad5e22890981fcd4a1a4376 > > > > I'm wondering about that last HTTP2_SESSION_RST_STREAM entry.. it looks > > like > > the httpv2 mod wants to cancel the incoming post data stream at an odd > > point in time? > > But I'm no expert, so I'm not sure at all. > > > [..] > > I was able to capture quite the same Chrome trace with the actual bytes > sent/received and this last RST_STREAM entry with status 6 (which is > FRAME_SIZE_ERROR) looks really strange. > > > It isn't presented in nginx debug log (nginx hasn't sent, nor has received > it). Moreover, the actual bytes sent/received trace, that you can get from > Chrome if you set the "Include the actual bytes sent/received." flag in > chrome://net-internals/#capture), doesn't contain it either. > > The trace (and nginx log) contains RST_STREAM sent by nginx with status 0 > (NO_ERROR), which is correct: > > t= 94141 [st= 6088]SSL_SOCKET_BYTES_RECEIVED >--> byte_count = 13 >--> hex_encoded_bytes = > 00 00 04 03 00 00 00 00 03 00 00 00 00 > > As you can see it has 4 bytes length, type 3, flags 0, stream id 3 > and error code 0. > > +---+ > | Length (24) | > +---+---+---+ > | Type (8)| Flags (8) | > +-+-+---+---+ > |R| Stream Identifier (31) | > +=+=+ > | Frame Payload (0...) ... > +---+ > > Figure 1: Frame Layout > > > https://tools.ietf.org/html/rfc7540#section-4.1 > > > +---+ > |Error Code (32)| > +---+ > > Figure 9: RST_STREAM Frame Payload > > https://tools.ietf.org/html/rfc7540#section-6.4 > > I've attached the full trace. > > Maybe I missed something, but after manual decoding bytes I don't see any > problems with the frame sizes received from nginx. > > However, that can be a result of this change: > http://hg.nginx.org/nginx/rev/92464ebace8e > > Firefox perfectly fine with it. It looks like the problem in Chrome. > Well, I've found the key how to interpret these statuses in Chromium sources: // Status codes for RST_STREAM frames. enum SpdyRstStreamStatus { RST_STREAM_INVALID = 0, RST_STREAM_PROTOCOL_ERROR = 1, RST_STREAM_INVALID_STREAM = 2, RST_STREAM_STREAM_CLOSED = 2, // Equivalent to INVALID_STREAM RST_STREAM_REFUSED_STREAM = 3, RST_STREAM_UNSUPPORTED_VERSION = 4, RST_STREAM_CANCEL = 5, RST_STREAM_INTERNAL_ERROR = 6, RST_STREAM_FLOW_CONTROL_ERROR = 7, RST_STREAM_STREAM_IN_USE = 8, RST_STREAM_STREAM_ALREADY_CLOSED = 9, RST_STREAM_INVALID_CREDENTIALS = 10, // FRAME_TOO_LARGE (defined by SPDY versions 3.1 and below), and // FRAME_SIZE_ERROR (defined by HTTP/2) are mapped to the same internal // reset status. RST_STREAM_FRAME_TOO_LARGE = 11, RST_STREAM_FRAME_SIZE_ERROR = 11, RST_STREAM_SETTINGS_TIMEOUT = 12, RST_STREAM_CONNECT_ERROR = 13, RST_STREAM_ENHANCE_YOUR_CALM = 14, RST_STREAM_NUM_STATUS_CODES = 15 }; looks like NO_ERROR is handled as internal error, which is obviously wrong. wbr, Valentin V. Bartenev ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: 1.9.14 - http2 protocol violations
On Wednesday 13 April 2016 15:08:29 Otto van der Schaaf wrote: > I'm sorry to say that the patch does not make a difference. > > I collected Chrome's http/2 trace for a post to a non-existant url: > https://gist.github.com/oschaaf/da273f96fad5e22890981fcd4a1a4376 > > I'm wondering about that last HTTP2_SESSION_RST_STREAM entry.. it looks > like > the httpv2 mod wants to cancel the incoming post data stream at an odd > point in time? > But I'm no expert, so I'm not sure at all. > [..] I was able to capture quite the same Chrome trace with the actual bytes sent/received and this last RST_STREAM entry with status 6 (which is FRAME_SIZE_ERROR) looks really strange. It isn't presented in nginx debug log (nginx hasn't sent, nor has received it). Moreover, the actual bytes sent/received trace, that you can get from Chrome if you set the "Include the actual bytes sent/received." flag in chrome://net-internals/#capture), doesn't contain it either. The trace (and nginx log) contains RST_STREAM sent by nginx with status 0 (NO_ERROR), which is correct: t= 94141 [st= 6088]SSL_SOCKET_BYTES_RECEIVED --> byte_count = 13 --> hex_encoded_bytes = 00 00 04 03 00 00 00 00 03 00 00 00 00 As you can see it has 4 bytes length, type 3, flags 0, stream id 3 and error code 0. +---+ | Length (24) | +---+---+---+ | Type (8)| Flags (8) | +-+-+---+---+ |R| Stream Identifier (31) | +=+=+ | Frame Payload (0...) ... +---+ Figure 1: Frame Layout https://tools.ietf.org/html/rfc7540#section-4.1 +---+ |Error Code (32)| +---+ Figure 9: RST_STREAM Frame Payload https://tools.ietf.org/html/rfc7540#section-6.4 I've attached the full trace. Maybe I missed something, but after manual decoding bytes I don't see any problems with the frame sizes received from nginx. However, that can be a result of this change: http://hg.nginx.org/nginx/rev/92464ebace8e Firefox perfectly fine with it. It looks like the problem in Chrome. wbr, Valentin V. Bartenev 12281: SOCKET ssl/localhost:4433 Start Time: 2016-04-13 16:57:18.765 t= 88053 [st=0] +SOCKET_ALIVE [dt=16927] --> source_dependency = 12280 (CONNECT_JOB) t= 88053 [st=0] +TCP_CONNECT [dt=0] --> address_list = ["[::1]:4433","127.0.0.1:4433"] t= 88053 [st=0] +TCP_CONNECT_ATTEMPT [dt=0] --> address = "[::1]:4433" t= 88053 [st=0] -TCP_CONNECT_ATTEMPT --> os_error = 111 t= 88053 [st=0] TCP_CONNECT_ATTEMPT [dt=0] --> address = "127.0.0.1:4433" t= 88053 [st=0] -TCP_CONNECT --> source_address = "127.0.0.1:35904" t= 88053 [st=0] +SOCKET_IN_USE [dt=16927] --> source_dependency = 12279 (CONNECT_JOB) t= 88053 [st=0] +SSL_CONNECT [dt=3] t= 88053 [st=0]SOCKET_BYTES_SENT --> byte_count = 517 --> hex_encoded_bytes = 16 03 01 02 00 01 00 01 FC 03 03 DC 52 24 92 CC . .R$.. F0 24 33 1A 45 CA DA 1A A7 80 90 92 DB 9F 31 15 .$3.E.1. F8 BC B4 F9 43 6D 3F 77 79 74 AC 20 B4 EE 6A D6 Cm?wyt. ..j. D1 DB 99 40 9B A0 A8 D4 F3 13 46 D3 05 69 0E AB ...@..F..i.. BF 04 D3 A2 48 88 0A B5 46 3D 42 2B 00 1C C0 2B H...F=B+ ..+ C0 2F CC A9 CC A8 CC 14 CC 13 C0 0A C0 14 C0 09 ./.. C0 13 00 9C 00 35 00 2F 00 0A 01 00 01 97 FF 01 .. . 5 / .. 00 01 00 00 00 00 0E 00 0C 00 00 09 6C 6F 63 61 .. . .loca 6C 68 6F 73 74 00 17 00 00 00 23 00 C0 FE CA E1 lhost . # 59 34 4A 05 54 33 8D 42 56 A8 48 6F DA BD 35 CE Y4J.T3.BV.Ho..5. 95 31 B2 48 70 74 CC 54 04 E8 B4 56 AF 91 29 12 .1.Hpt.T...V..). 84 26 4A BF 80 A5 80 B0 E7 B3 53 7D F1 68 4B CC .}.hK. BD AF 0D 18 B8 5F A4 0F AB 5C 10 E6 71 2F A5 84 ._...\..q/..
Re: 1.9.14 - http2 protocol violations
I'm sorry to say that the patch does not make a difference. I collected Chrome's http/2 trace for a post to a non-existant url: https://gist.github.com/oschaaf/da273f96fad5e22890981fcd4a1a4376 I'm wondering about that last HTTP2_SESSION_RST_STREAM entry.. it looks like the httpv2 mod wants to cancel the incoming post data stream at an odd point in time? But I'm no expert, so I'm not sure at all. Otto On Wed, Apr 13, 2016 at 2:44 PM, Otto van der Schaafwrote: > I'm pretty sure these logs correlate to the problems I am seeing, yes. > Indeed the error.log samples are free from warnings and errors, but it > seems the protocol is violated nevertheless. At least Chrome says so. So > the client does report an error afaict. > I'll patch in the change you posted and let you know how that goes, thanks. > > Otto > > On Wed, Apr 13, 2016 at 2:34 PM, Валентин Бартенев > wrote: > >> On Wednesday 13 April 2016 15:30:39 Валентин Бартенев wrote: >> > On Wednesday 13 April 2016 13:54:24 Otto van der Schaaf wrote: >> > > Sure. >> > > >> > > - This sample contains to or three requests, the last request is >> where rate >> > > limiting kicks in and the protocol error happens (the earlier >> requests are >> > > problem-free): >> > > https://gist.github.com/oschaaf/281b7a0fed9954dd960adac55e96f2cd >> > > >> > > - This is from a post to a non-existant url: >> > > https://gist.github.com/oschaaf/6396a614ce599d5003e50bb8e7106bed >> > > >> > [..] >> > >> > Are you sure that you saw this problem while were collecting the logs? >> > There's nothing suspicious in them, and no errors are reported by nginx >> > and by the client. >> > >> > The only problem that I could see should be fixed by the patch below. >> > Please try it and let me know if it will help. >> > >> > wbr, Valentin V. Bartenev >> > >> > >> > # HG changeset patch >> > # User Valentin Bartenev >> > # Date 1460529455 -10800 >> > # Wed Apr 13 09:37:35 2016 +0300 >> > # Node ID 80d0c9e314f62b594e634cd9318fe84afc50bb2c >> > # Parent 640288d0e1bc449a54ac1c85905e6fce780fac7d >> > HTTP/2: refuse streams with data until SETTINGS is acknowledged. >> > >> > A client can start sending data before receiving and acknowledging >> > the SETTINGS frame, thus having a wrong idea about the stream's >> > initial window. This can violate flow control. >> > >> [..] >> >> Sorry, automatic word wrapping broke the patch. >> >> Here is another attempt: >> >> # HG changeset patch >> # User Valentin Bartenev >> # Date 1460529455 -10800 >> # Wed Apr 13 09:37:35 2016 +0300 >> # Node ID 80d0c9e314f62b594e634cd9318fe84afc50bb2c >> # Parent 640288d0e1bc449a54ac1c85905e6fce780fac7d >> HTTP/2: refuse streams with data until SETTINGS is acknowledged. >> >> A client can start sending data before receiving and acknowledging >> the SETTINGS frame, thus having a wrong idea about the stream's >> initial window. This can violate flow control. >> >> diff -r 640288d0e1bc -r 80d0c9e314f6 src/http/v2/ngx_http_v2.c >> --- a/src/http/v2/ngx_http_v2.c Tue Apr 12 19:01:56 2016 +0300 >> +++ b/src/http/v2/ngx_http_v2.c Wed Apr 13 09:37:35 2016 +0300 >> @@ -1071,6 +1071,19 @@ ngx_http_v2_state_headers(ngx_http_v2_co >> return ngx_http_v2_state_header_block(h2c, pos, end); >> } >> >> +if (!h2c->settings_ack && !(h2c->state.flags & >> NGX_HTTP_V2_END_STREAM_FLAG)) >> +{ >> +if (ngx_http_v2_send_rst_stream(h2c, h2c->state.sid, >> +NGX_HTTP_V2_REFUSED_STREAM) >> +!= NGX_OK) >> +{ >> +return ngx_http_v2_connection_error(h2c, >> + >> NGX_HTTP_V2_INTERNAL_ERROR); >> +} >> + >> +return ngx_http_v2_state_header_block(h2c, pos, end); >> +} >> + >> node = ngx_http_v2_get_node_by_id(h2c, h2c->state.sid, 1); >> >> if (node == NULL) { >> @@ -1883,7 +1896,7 @@ ngx_http_v2_state_settings(ngx_http_v2_c >> return ngx_http_v2_connection_error(h2c, >> NGX_HTTP_V2_SIZE_ERROR); >> } >> >> -/* TODO settings acknowledged */ >> +h2c->settings_ack = 1; >> >> return ngx_http_v2_state_complete(h2c, pos, end); >> } >> diff -r 640288d0e1bc -r 80d0c9e314f6 src/http/v2/ngx_http_v2.h >> --- a/src/http/v2/ngx_http_v2.h Tue Apr 12 19:01:56 2016 +0300 >> +++ b/src/http/v2/ngx_http_v2.h Wed Apr 13 09:37:35 2016 +0300 >> @@ -141,6 +141,7 @@ struct ngx_http_v2_connection_s { >> ngx_uint_t last_sid; >> >> unsigned closed_nodes:8; >> +unsigned settings_ack:1; >> unsigned blocked:1; >> }; >> >> >> ___ >> nginx-devel mailing list >> nginx-devel@nginx.org >> http://mailman.nginx.org/mailman/listinfo/nginx-devel >> > > ___ nginx-devel mailing list nginx-devel@nginx.org
Re: 1.9.14 - http2 protocol violations
On Wednesday 13 April 2016 15:30:39 Валентин Бартенев wrote: > On Wednesday 13 April 2016 13:54:24 Otto van der Schaaf wrote: > > Sure. > > > > - This sample contains to or three requests, the last request is where rate > > limiting kicks in and the protocol error happens (the earlier requests are > > problem-free): > > https://gist.github.com/oschaaf/281b7a0fed9954dd960adac55e96f2cd > > > > - This is from a post to a non-existant url: > > https://gist.github.com/oschaaf/6396a614ce599d5003e50bb8e7106bed > > > [..] > > Are you sure that you saw this problem while were collecting the logs? > There's nothing suspicious in them, and no errors are reported by nginx > and by the client. > > The only problem that I could see should be fixed by the patch below. > Please try it and let me know if it will help. > > wbr, Valentin V. Bartenev > > > # HG changeset patch > # User Valentin Bartenev> # Date 1460529455 -10800 > # Wed Apr 13 09:37:35 2016 +0300 > # Node ID 80d0c9e314f62b594e634cd9318fe84afc50bb2c > # Parent 640288d0e1bc449a54ac1c85905e6fce780fac7d > HTTP/2: refuse streams with data until SETTINGS is acknowledged. > > A client can start sending data before receiving and acknowledging > the SETTINGS frame, thus having a wrong idea about the stream's > initial window. This can violate flow control. > [..] Sorry, automatic word wrapping broke the patch. Here is another attempt: # HG changeset patch # User Valentin Bartenev # Date 1460529455 -10800 # Wed Apr 13 09:37:35 2016 +0300 # Node ID 80d0c9e314f62b594e634cd9318fe84afc50bb2c # Parent 640288d0e1bc449a54ac1c85905e6fce780fac7d HTTP/2: refuse streams with data until SETTINGS is acknowledged. A client can start sending data before receiving and acknowledging the SETTINGS frame, thus having a wrong idea about the stream's initial window. This can violate flow control. diff -r 640288d0e1bc -r 80d0c9e314f6 src/http/v2/ngx_http_v2.c --- a/src/http/v2/ngx_http_v2.c Tue Apr 12 19:01:56 2016 +0300 +++ b/src/http/v2/ngx_http_v2.c Wed Apr 13 09:37:35 2016 +0300 @@ -1071,6 +1071,19 @@ ngx_http_v2_state_headers(ngx_http_v2_co return ngx_http_v2_state_header_block(h2c, pos, end); } +if (!h2c->settings_ack && !(h2c->state.flags & NGX_HTTP_V2_END_STREAM_FLAG)) +{ +if (ngx_http_v2_send_rst_stream(h2c, h2c->state.sid, +NGX_HTTP_V2_REFUSED_STREAM) +!= NGX_OK) +{ +return ngx_http_v2_connection_error(h2c, +NGX_HTTP_V2_INTERNAL_ERROR); +} + +return ngx_http_v2_state_header_block(h2c, pos, end); +} + node = ngx_http_v2_get_node_by_id(h2c, h2c->state.sid, 1); if (node == NULL) { @@ -1883,7 +1896,7 @@ ngx_http_v2_state_settings(ngx_http_v2_c return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_SIZE_ERROR); } -/* TODO settings acknowledged */ +h2c->settings_ack = 1; return ngx_http_v2_state_complete(h2c, pos, end); } diff -r 640288d0e1bc -r 80d0c9e314f6 src/http/v2/ngx_http_v2.h --- a/src/http/v2/ngx_http_v2.h Tue Apr 12 19:01:56 2016 +0300 +++ b/src/http/v2/ngx_http_v2.h Wed Apr 13 09:37:35 2016 +0300 @@ -141,6 +141,7 @@ struct ngx_http_v2_connection_s { ngx_uint_t last_sid; unsigned closed_nodes:8; +unsigned settings_ack:1; unsigned blocked:1; }; ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: 1.9.14 - http2 protocol violations
Sure. - This sample contains to or three requests, the last request is where rate limiting kicks in and the protocol error happens (the earlier requests are problem-free): https://gist.github.com/oschaaf/281b7a0fed9954dd960adac55e96f2cd - This is from a post to a non-existant url: https://gist.github.com/oschaaf/6396a614ce599d5003e50bb8e7106bed Otto On Wed, Apr 13, 2016 at 12:52 AM, Валентин Бартенев <vb...@nginx.com> wrote: > On Wednesday 13 April 2016 00:08:43 Otto van der Schaaf wrote: > > Hello, > > > > While looking into > https://github.com/pagespeed/ngx_pagespeed/issues/1175 I > > noticed that when performing a POST to a non-existing page, Chrome will > > complain about the response (net::ERR_SPDY_PROTOCOL_ERROR). > > > > This happens with a plain nginx build, configure arguments: > > --prefix=/home/oschaaf/nginx-tmpbuild --with-http_v2_module > > --with-http_ssl_module > > > > 1.9.14 seems to have introduced some behavioral changes only noticable on > > http2, there seem to be similar breakages in some of ngx_pagespeed's > flows. > > > > One thing we do is perform an async lookup in the preaccess phase - doing > > so when processing a POST request also results in a protocol violation. > > As this path looks very similar to the limit request module, I tested > that > > as well. And indeed this also seems to give the same error on http/2 > > > [..] > > Could you provide the debug log? > > wbr, Valentin V. Bartenev > > ___ > nginx-devel mailing list > nginx-devel@nginx.org > http://mailman.nginx.org/mailman/listinfo/nginx-devel > ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: 1.9.14 - http2 protocol violations
On Wednesday 13 April 2016 00:08:43 Otto van der Schaaf wrote: > Hello, > > While looking into https://github.com/pagespeed/ngx_pagespeed/issues/1175 I > noticed that when performing a POST to a non-existing page, Chrome will > complain about the response (net::ERR_SPDY_PROTOCOL_ERROR). > > This happens with a plain nginx build, configure arguments: > --prefix=/home/oschaaf/nginx-tmpbuild --with-http_v2_module > --with-http_ssl_module > > 1.9.14 seems to have introduced some behavioral changes only noticable on > http2, there seem to be similar breakages in some of ngx_pagespeed's flows. > > One thing we do is perform an async lookup in the preaccess phase - doing > so when processing a POST request also results in a protocol violation. > As this path looks very similar to the limit request module, I tested that > as well. And indeed this also seems to give the same error on http/2 > [..] Could you provide the debug log? wbr, Valentin V. Bartenev ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
1.9.14 - http2 protocol violations
Hello, While looking into https://github.com/pagespeed/ngx_pagespeed/issues/1175 I noticed that when performing a POST to a non-existing page, Chrome will complain about the response (net::ERR_SPDY_PROTOCOL_ERROR). This happens with a plain nginx build, configure arguments: --prefix=/home/oschaaf/nginx-tmpbuild --with-http_v2_module --with-http_ssl_module 1.9.14 seems to have introduced some behavioral changes only noticable on http2, there seem to be similar breakages in some of ngx_pagespeed's flows. One thing we do is perform an async lookup in the preaccess phase - doing so when processing a POST request also results in a protocol violation. As this path looks very similar to the limit request module, I tested that as well. And indeed this also seems to give the same error on http/2 Regards, Otto ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[nginx] SSL: only select HTTP/2 using NPN if "http2" is enabled.
details: http://hg.nginx.org/nginx/rev/909b5b191f25 branches: changeset: 6289:909b5b191f25 user: Valentin Bartenev <vb...@nginx.com> date: Thu Nov 05 15:01:09 2015 +0300 description: SSL: only select HTTP/2 using NPN if "http2" is enabled. OpenSSL doesn't check if the negotiated protocol has been announced. As a result, the client might force using HTTP/2 even if it wasn't enabled in configuration. diffstat: src/http/ngx_http_request.c | 30 ++ 1 files changed, 18 insertions(+), 12 deletions(-) diffs (47 lines): diff -r 0f4b7800e681 -r 909b5b191f25 src/http/ngx_http_request.c --- a/src/http/ngx_http_request.c Thu Nov 05 15:01:01 2015 +0300 +++ b/src/http/ngx_http_request.c Thu Nov 05 15:01:09 2015 +0300 @@ -768,25 +768,31 @@ ngx_http_ssl_handshake_handler(ngx_conne && (defined TLSEXT_TYPE_application_layer_protocol_negotiation \ || defined TLSEXT_TYPE_next_proto_neg)) { -unsigned int len; -const unsigned char *data; +unsigned intlen; +const unsigned char*data; +ngx_http_connection_t *hc; + +hc = c->data; + + if (hc->addr_conf->http2) { #ifdef TLSEXT_TYPE_application_layer_protocol_negotiation -SSL_get0_alpn_selected(c->ssl->connection, , ); +SSL_get0_alpn_selected(c->ssl->connection, , ); #ifdef TLSEXT_TYPE_next_proto_neg -if (len == 0) { +if (len == 0) { +SSL_get0_next_proto_negotiated(c->ssl->connection, , ); +} +#endif + +#else /* TLSEXT_TYPE_next_proto_neg */ SSL_get0_next_proto_negotiated(c->ssl->connection, , ); -} #endif -#else /* TLSEXT_TYPE_next_proto_neg */ -SSL_get0_next_proto_negotiated(c->ssl->connection, , ); -#endif - -if (len == 2 && data[0] == 'h' && data[1] == '2') { -ngx_http_v2_init(c->read); -return; +if (len == 2 && data[0] == 'h' && data[1] == '2') { +ngx_http_v2_init(c->read); +return; +} } } #endif ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: subrequest error with http2
Hello! On Tue, Oct 6, 2015 at 5:33 PM, Carlos Eduardo Ferreira Rodrigues wrote: > I'm aware 1.9.5 isn't supported yet and that SPDY is mentioned in the docs as > non-working for certain API calls > as well. However, we are have been using location.capture with SPDY for a > while now and haven't seen any issues. > Information on why and what issues are present is a little fuzzy (and > searching only returns old stuff that some > sources claim not to apply anymore), can you provide some details? > Well, I just haven't had the time to look close enough to SPDY or HTTP/2. I've been mainly relying on the user feedback on such (unsupported) usage. In theory, they *should* work with ngx.location.capture*. But there might be some knobs to resolve. I'm not sure and I cannot give you any details at this point. Sorry. Regards, -agentzh ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
RE: subrequest error with http2
Hi, I'm aware 1.9.5 isn't supported yet and that SPDY is mentioned in the docs as non-working for certain API calls as well. However, we are have been using location.capture with SPDY for a while now and haven't seen any issues. Information on why and what issues are present is a little fuzzy (and searching only returns old stuff that some sources claim not to apply anymore), can you provide some details? Best regards, -- Carlos Rodrigues From: nginx-devel [nginx-devel-boun...@nginx.org] On Behalf Of Yichun Zhang (agentzh) [agen...@gmail.com] Sent: Tuesday, October 06, 2015 4:01 To: nginx-devel@nginx.org Subject: Re: subrequest error with http2 Hello! On Wed, Sep 23, 2015 at 1:14 AM, Carlos Eduardo Ferreira Rodrigues wrote: > I just upgraded nginx to 1.9.5 on our testing enviroment, and immediately > started seeing this error on http2 requests: > > 2015/09/22 18:04:06 [alert] 27305#27305: *1 epoll_ctl(1, 17) failed (17: File > exists), client: x.x.x.x, server: _, request: "GET ... HTTP/2.0", subrequest: > "...", host: "..." > > The subrequest is being made from Lua code using "ngx.location.capture()", so > I understand this may be an issue with ngx_lua and not nginx itself. However, > this worked fine with nginx 1.8.0/1.9.4 and spdy and nothing else has changed. > The ngx.location.capture* API does not support SPDY nor HTTP 2.0 (yet). We may add such support in the future. Oh, BTW, nginx 1.9.5 is not listed in the compatible nginx core list yet: https://github.com/openresty/lua-nginx-module#nginx-compatibility Regards, -agentzh ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: subrequest error with http2
Hello! On Wed, Sep 23, 2015 at 1:14 AM, Carlos Eduardo Ferreira Rodrigues wrote: > I just upgraded nginx to 1.9.5 on our testing enviroment, and immediately > started seeing this error on http2 requests: > > 2015/09/22 18:04:06 [alert] 27305#27305: *1 epoll_ctl(1, 17) failed (17: File > exists), client: x.x.x.x, server: _, request: "GET ... HTTP/2.0", subrequest: > "...", host: "..." > > The subrequest is being made from Lua code using "ngx.location.capture()", so > I understand this may be an issue with ngx_lua and not nginx itself. However, > this worked fine with nginx 1.8.0/1.9.4 and spdy and nothing else has changed. > The ngx.location.capture* API does not support SPDY nor HTTP 2.0 (yet). We may add such support in the future. Oh, BTW, nginx 1.9.5 is not listed in the compatible nginx core list yet: https://github.com/openresty/lua-nginx-module#nginx-compatibility Regards, -agentzh ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: subrequest error with http2
Hello! On Wed, Sep 23, 2015 at 1:34 AM, Maxim Dounin wrote: > > The lua module deeply integrates into nginx internals (far beyond > what we consider to be nginx modules API), and there is no surprise > it's broken by the changes in nginx 1.9.5. > True. This is also why ngx_lua has so many advanced features :) Actually I quite like the fact that NGINX does not enforce a limited set of API for 3rd-party modules to use and 3rd-party module authors can almost always find their way to implement bold imaginations *grin* For the same reason, I'll hate to see that NGINX enforces a well defined ABI, BTW :) Regards, -agentzh ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: subrequest error with http2
Hello! On Tue, Sep 22, 2015 at 06:14:56PM +0100, Carlos Eduardo Ferreira Rodrigues wrote: > Hi, > > I just upgraded nginx to 1.9.5 on our testing enviroment, and immediately > started seeing this error on http2 requests: > > 2015/09/22 18:04:06 [alert] 27305#27305: *1 epoll_ctl(1, 17) failed (17: File > exists), client: x.x.x.x, server: _, request: "GET ... HTTP/2.0", subrequest: > "...", host: "..." > > The subrequest is being made from Lua code using "ngx.location.capture()", so > I understand this may be an issue with ngx_lua and not nginx itself. However, > this worked fine with nginx 1.8.0/1.9.4 and spdy and nothing else has changed. The lua module deeply integrates into nginx internals (far beyond what we consider to be nginx modules API), and there is no surprise it's broken by the changes in nginx 1.9.5. -- Maxim Dounin http://nginx.org/ ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: http2 slow loading
On Monday 17 August 2015 22:50:29 Chris Vancoillie wrote: Hi Herewith attached debug log for slow loading issue testing with patch v2. The log looks incomplete, it seems you put the error_log directive with the debug option not in the main level of configuration, but in some level below. As a result it didn't catch all the events. Please also provide your configuration. At the first glance it more looks like some client bandwidth limitation. Could you describe your environment and how do you test it? wbr, Valentin V. Bartenev ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: http2 broke links in code
Thanks - work fine now! Valentin V. Bartenev mailto:vb...@nginx.com August 14, 2015 at 7:11 PM [...] Thanks for the report. Please, try the new patch: http://nginx.org/patches/http2/ wbr, Valentin V. Bartenev ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel dims mailto:dims.m...@gmail.com August 14, 2015 at 3:02 PM Yes, you right, my mistake But found problem why dont work our code. We use nginx + php-fpm and variable HTTP_HOST in code Config: location ~ \.(html|php)$ { rewrite ^(.+)\.html$ $1.php break; include /etc/nginx/fastcgi_params; fastcgi_pass unix:/var/lib/nginx/php-fpm.sock; fastcgi_index index.php; fastcgi_param SCRIPT_FILENAME $document_root$fastcgi_script_name; fastcgi_split_path_info ^(.+\.php)(/.+)$; try_files $uri =404; } When enable http2 variable HTTP_HOST absent in phpinfo If set fastcgi_param HTTP_HOST $http_host; in phpinfo(): _SERVER[HTTP_HOST]no value Valentin V. Bartenev mailto:vb...@nginx.com August 14, 2015 at 2:29 PM HTTP/2 does nothing with page content. You're probably using 3-rd party modules like mod_pagespeed or something else. wbr, Valentin V. Bartenev ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
http2 broke links in code
Default ssl configuration: link rel=stylesheet media=all href=//www.domain.ru/css2/reset-min.css When enable http2 link brokes link rel=stylesheet media=all href=///css2/reset-min.css ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: http2 broke links in code
Yes, you right, my mistake But found problem why dont work our code. We use nginx + php-fpm and variable HTTP_HOST in code Config: location ~ \.(html|php)$ { rewrite ^(.+)\.html$ $1.php break; include /etc/nginx/fastcgi_params; fastcgi_pass unix:/var/lib/nginx/php-fpm.sock; fastcgi_index index.php; fastcgi_param SCRIPT_FILENAME $document_root$fastcgi_script_name; fastcgi_split_path_info ^(.+\.php)(/.+)$; try_files $uri =404; } When enable http2 variable HTTP_HOST absent in phpinfo If set fastcgi_param HTTP_HOST $http_host; in phpinfo(): _SERVER[HTTP_HOST]no value Valentin V. Bartenev mailto:vb...@nginx.com August 14, 2015 at 2:29 PM HTTP/2 does nothing with page content. You're probably using 3-rd party modules like mod_pagespeed or something else. wbr, Valentin V. Bartenev ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel