[PATCH 2 of 3] HTTP/2: "http2" directive
Maxim Dounin
mdounin at mdounin.ru
Wed Jun 7 15:50:57 UTC 2023
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 <arut at nginx.com> 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 <arut at nginx.com>
> > > # 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.
> > >
> > > 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
> > > @@ -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 --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
> > > @@ -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 --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);
> >
> > I don't think it should be moved.
> >
> > Non-reusable connections are basically ones with a request
> > created, so nginx won't try to terminate running requests. This
> > does not apply to a connection waiting for an HTTP/2 preface.
> >
> > It might need to be called in (or just before) ngx_http_v2_init()
> > though, since HTTP/2 code assumes that the connection is not
> > reusable by default.
>
> OK
>
> > > +
> > > +#if (NGX_HTTP_V2)
> > > +
> > > + h2scf = ngx_http_get_module_srv_conf(hc->conf_ctx, ngx_http_v2_module);
> > > +
> > > + if (!hc->ssl && (h2scf->enable || hc->addr_conf->http2)) {
> >
> > For the record: we might consider introducing a way to explicitly
> > control "establishment of HTTP/2 connections over cleartext TCP"
> > (quote from RFC 9113, section 3.3. "Starting HTTP/2 with Prior
> > Knowledge"), something like "http2_cleartext on|off;". Might not
> > worth the effort though.
>
> Why do you think we need this directive?
HTTP/2 over cleartext TCP implies different code paths, which are
better avoided unless needed. And it is only needed in very
specific cases, since browsers do not support HTTP/2 over
cleartext TCP. Further, it implies different resource usage ratio
between client and server, and enabling it might make some attacks
more feasible.
That is, there are no specific reasons to say “we certainly need
it”, but there are generic concerns that it would be safer to not
enable HTTP/2 over cleartext TCP in most setups using HTTP/2.
While it would be possible to only enable HTTP/2 in specific
server blocks with only SSL listening sockets, having a distinct
directive would be easier to configure.
> > > +
> > > + 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";
> >
> > Any reasons to change the log action here?
> >
> > Note that for proxy_protocol handling above, c->log->action is set
> > to "reading PROXY protocol", and therefore needs to be reset to
> > "waiting for request" when PROXY protocol header is fully
> > processed. This is not the case for HTTP/2 preface though, as it
> > does not use a special log action.
> >
> > Further, in this particular point HTTP/2 preface reading is not
> > complete, so changing the log action would be wrong if there was
> > a special log action set.
>
> You're right, removed this one.
>
> > > + ngx_post_event(rev, &ngx_posted_events);
> > > + return;
> >
> > For the record: as far as I understand, there is no real need to
> > post an event here, and just adding a timer and calling
> > ngx_handle_read_event() (much like in case of EAGAIN after recv())
> > would be slightly more optimal. I don't object keeping it this
> > way though, since it's not a hot path, have some chance to
> > actually read the rest of the preface in the posted event, and
> > proxy_protocol handling already uses the same approach.
> >
> > > + }
> > > + }
> > > +
> > > +#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);
> > > @@ -808,13 +835,16 @@ ngx_http_ssl_handshake_handler(ngx_conne
> > > #if (NGX_HTTP_V2 \
> > > && defined TLSEXT_TYPE_application_layer_protocol_negotiation)
> > > {
> > > - unsigned int len;
> > > - const unsigned char *data;
> > > - ngx_http_connection_t *hc;
> > > + unsigned int len;
> > > + const unsigned char *data;
> > > + ngx_http_connection_t *hc;
> > > + ngx_http_v2_srv_conf_t *h2scf;
> > >
> > > hc = c->data;
> > >
> > > - 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) {
> > >
> > > SSL_get0_alpn_selected(c->ssl->connection, &data, &len);
> > >
> > > 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
> > > @@ -63,8 +63,6 @@ static void ngx_http_v2_handle_connectio
> > > static void ngx_http_v2_lingering_close(ngx_connection_t *c);
> > > static void ngx_http_v2_lingering_close_handler(ngx_event_t *rev);
> > >
> > > -static u_char *ngx_http_v2_state_proxy_protocol(ngx_http_v2_connection_t *h2c,
> > > - u_char *pos, u_char *end);
> > > static u_char *ngx_http_v2_state_preface(ngx_http_v2_connection_t *h2c,
> > > u_char *pos, u_char *end);
> > > static u_char *ngx_http_v2_state_preface_end(ngx_http_v2_connection_t *h2c,
> > > @@ -232,6 +230,7 @@ static ngx_http_v2_parse_header_t ngx_h
> > > void
> > > ngx_http_v2_init(ngx_event_t *rev)
> > > {
> > > + u_char *p, *end;
> > > ngx_connection_t *c;
> > > ngx_pool_cleanup_t *cln;
> > > ngx_http_connection_t *hc;
> > > @@ -314,8 +313,7 @@ ngx_http_v2_init(ngx_event_t *rev)
> > > return;
> > > }
> > >
> > > - h2c->state.handler = hc->proxy_protocol ? ngx_http_v2_state_proxy_protocol
> > > - : ngx_http_v2_state_preface;
> > > + h2c->state.handler = ngx_http_v2_state_preface;
> > >
> > > ngx_queue_init(&h2c->waiting);
> > > ngx_queue_init(&h2c->dependencies);
> > > @@ -333,7 +331,23 @@ ngx_http_v2_init(ngx_event_t *rev)
> > > }
> > >
> > > c->idle = 1;
> > > - ngx_reusable_connection(c, 0);
> >
> > See above.
>
> Brought this back along with the above mentioned calls.
>
> > > +
> > > + if (c->buffer) {
> > > + p = c->buffer->pos;
> > > + end = c->buffer->last;
> > > +
> > > + do {
> > > + p = h2c->state.handler(h2c, p, end);
> > > +
> > > + if (p == NULL) {
> > > + return;
> > > + }
> > > +
> > > + } while (p != end);
> > > +
> > > + h2c->total_bytes += p - c->buffer->pos;
> > > + c->buffer->pos = p;
> > > + }
> > >
> > > ngx_http_v2_read_handler(rev);
> > > }
> >
> > For the record: it might be better to integrate this into
> > ngx_http_v2_read_handler(), similarly to how it is done in
> > ngx_event_pipe_read_upstream(). On the other hand, suggested
> > approach looks simple enough and might be better.
>
> Yes, this would make ngx_http_v2_read_handler() too complex.
>
> > Note well: this implies an extra recv() call on each cleartext
> > HTTP/2 connection establishment. Probably not very important
> > though. On the other hand, we can consider optimizing out an
> > extra call if there are space in c->buffer.
>
> Why not check rev->ready at the beginning of the loop in
> ngx_http_v2_read_handler() ?
>
> while (rev->ready) { ... }
>
> Currently it's checked at the end.
Usually rev->ready is checked at the end, since read handlers are
not expected to be called with rev->ready set to 0, so checking it
on the first iteration would be a waste of resources. This
approach also guarantees some safety against event handling bugs
(that is, if rev->ready wouldn't be set for some reason).
A better approach might be to check rev->ready before calling the
ngx_http_v2_read_handler() handler, much like in
ngx_http_init_connection().
> > > @@ -847,31 +861,10 @@ ngx_http_v2_lingering_close_handler(ngx_
> > >
> > >
> > > static u_char *
> > > -ngx_http_v2_state_proxy_protocol(ngx_http_v2_connection_t *h2c, u_char *pos,
> > > - u_char *end)
> > > -{
> > > - ngx_log_t *log;
> > > -
> > > - log = h2c->connection->log;
> > > - log->action = "reading PROXY protocol";
> > > -
> > > - pos = ngx_proxy_protocol_read(h2c->connection, pos, end);
> > > -
> > > - log->action = "processing HTTP/2 connection";
> > > -
> > > - if (pos == NULL) {
> > > - return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_PROTOCOL_ERROR);
> > > - }
> > > -
> > > - return ngx_http_v2_state_preface(h2c, pos, end);
> > > -}
> > > -
> > > -
> > > -static u_char *
> > > ngx_http_v2_state_preface(ngx_http_v2_connection_t *h2c, u_char *pos,
> > > u_char *end)
> > > {
> > > - static const u_char preface[] = "PRI * HTTP/2.0\r\n";
> > > + static const u_char preface[] = NGX_HTTP_V2_PREFACE_START;
> > >
> > > if ((size_t) (end - pos) < sizeof(preface) - 1) {
> > > return ngx_http_v2_state_save(h2c, pos, end, ngx_http_v2_state_preface);
> > > @@ -892,7 +885,7 @@ static u_char *
> > > ngx_http_v2_state_preface_end(ngx_http_v2_connection_t *h2c, u_char *pos,
> > > u_char *end)
> > > {
> > > - static const u_char preface[] = "\r\nSM\r\n\r\n";
> > > + static const u_char preface[] = NGX_HTTP_V2_PREFACE_END;
> > >
> > > if ((size_t) (end - pos) < sizeof(preface) - 1) {
> > > return ngx_http_v2_state_save(h2c, pos, end,
> > > @@ -3943,10 +3936,22 @@ static void
> > > ngx_http_v2_run_request(ngx_http_request_t *r)
> > > {
> > > ngx_connection_t *fc;
> > > + ngx_http_v2_srv_conf_t *h2scf;
> > > ngx_http_v2_connection_t *h2c;
> > >
> > > fc = r->connection;
> > >
> > > + h2scf = ngx_http_get_module_srv_conf(r, ngx_http_v2_module);
> > > +
> > > + if (!h2scf->enable && !r->http_connection->addr_conf->http2) {
> > > + ngx_log_error(NGX_LOG_INFO, fc->log, 0,
> > > + "client attempted to request the server name "
> > > + "for which the negotiated protocol is disabled");
> > > +
> > > + ngx_http_finalize_request(r, NGX_HTTP_MISDIRECTED_REQUEST);
> > > + goto failed;
> > > + }
> > > +
> > > if (ngx_http_v2_construct_request_line(r) != NGX_OK) {
> > > goto failed;
> > > }
> > > diff --git a/src/http/v2/ngx_http_v2.h b/src/http/v2/ngx_http_v2.h
> > > --- a/src/http/v2/ngx_http_v2.h
> > > +++ b/src/http/v2/ngx_http_v2.h
> > > @@ -64,6 +64,16 @@ typedef u_char *(*ngx_http_v2_handler_pt
> > >
> > >
> > > typedef struct {
> > > + ngx_flag_t enable;
> > > + size_t pool_size;
> > > + ngx_uint_t concurrent_streams;
> > > + ngx_uint_t concurrent_pushes;
> > > + size_t preread_size;
> > > + ngx_uint_t streams_index_mask;
> > > +} ngx_http_v2_srv_conf_t;
> > > +
> > > +
> > > +typedef struct {
> > > ngx_str_t name;
> > > ngx_str_t value;
> > > } ngx_http_v2_header_t;
> > > @@ -408,9 +418,17 @@ ngx_int_t ngx_http_v2_table_size(ngx_htt
> > > #define NGX_HTTP_V2_USER_AGENT_INDEX 58
> > > #define NGX_HTTP_V2_VARY_INDEX 59
> > >
> > > +#define NGX_HTTP_V2_PREFACE_START "PRI * HTTP/2.0\r\n"
> > > +#define NGX_HTTP_V2_PREFACE_END "\r\nSM\r\n\r\n"
> > > +#define NGX_HTTP_V2_PREFACE NGX_HTTP_V2_PREFACE_START \
> > > + NGX_HTTP_V2_PREFACE_END
> > > +
> > >
> > > u_char *ngx_http_v2_string_encode(u_char *dst, u_char *src, size_t len,
> > > u_char *tmp, ngx_uint_t lower);
> > >
> > >
> > > +extern ngx_module_t ngx_http_v2_module;
> > > +
> > > +
> > > #endif /* _NGX_HTTP_V2_H_INCLUDED_ */
> > > diff --git a/src/http/v2/ngx_http_v2_module.c b/src/http/v2/ngx_http_v2_module.c
> > > --- a/src/http/v2/ngx_http_v2_module.c
> > > +++ b/src/http/v2/ngx_http_v2_module.c
> > > @@ -75,6 +75,13 @@ static ngx_conf_post_t ngx_http_v2_chun
> > >
> > > static ngx_command_t ngx_http_v2_commands[] = {
> > >
> > > + { ngx_string("http2"),
> > > + NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_CONF_FLAG,
> > > + ngx_conf_set_flag_slot,
> > > + NGX_HTTP_SRV_CONF_OFFSET,
> > > + offsetof(ngx_http_v2_srv_conf_t, enable),
> > > + NULL },
> > > +
> > > { ngx_string("http2_recv_buffer_size"),
> > > NGX_HTTP_MAIN_CONF|NGX_CONF_TAKE1,
> > > ngx_conf_set_size_slot,
> > > @@ -314,6 +321,8 @@ ngx_http_v2_create_srv_conf(ngx_conf_t *
> > > return NULL;
> > > }
> > >
> > > + h2scf->enable = NGX_CONF_UNSET;
> > > +
> > > h2scf->pool_size = NGX_CONF_UNSET_SIZE;
> > >
> > > h2scf->concurrent_streams = NGX_CONF_UNSET_UINT;
> > > @@ -333,6 +342,8 @@ ngx_http_v2_merge_srv_conf(ngx_conf_t *c
> > > ngx_http_v2_srv_conf_t *prev = parent;
> > > ngx_http_v2_srv_conf_t *conf = child;
> > >
> > > + ngx_conf_merge_value(conf->enable, prev->enable, 0);
> > > +
> > > ngx_conf_merge_size_value(conf->pool_size, prev->pool_size, 4096);
> > >
> > > ngx_conf_merge_uint_value(conf->concurrent_streams,
> > > diff --git a/src/http/v2/ngx_http_v2_module.h b/src/http/v2/ngx_http_v2_module.h
> > > --- a/src/http/v2/ngx_http_v2_module.h
> > > +++ b/src/http/v2/ngx_http_v2_module.h
> > > @@ -21,15 +21,6 @@ typedef struct {
> > >
> > >
> > > typedef struct {
> > > - size_t pool_size;
> > > - ngx_uint_t concurrent_streams;
> > > - ngx_uint_t concurrent_pushes;
> > > - size_t preread_size;
> > > - ngx_uint_t streams_index_mask;
> > > -} ngx_http_v2_srv_conf_t;
> > > -
> > > -
> > > -typedef struct {
> > > size_t chunk_size;
> > >
> > > ngx_flag_t push_preload;
> > > @@ -39,7 +30,4 @@ typedef struct {
> > > } ngx_http_v2_loc_conf_t;
> > >
> > >
> > > -extern ngx_module_t ngx_http_v2_module;
> > > -
> > > -
> > > #endif /* _NGX_HTTP_V2_MODULE_H_INCLUDED_ */
> >
> > Overall looks good.
> >
> > --
> > Maxim Dounin
> > http://mdounin.ru/
> > _______________________________________________
> > nginx-devel mailing list
> > nginx-devel at nginx.org
> > https://mailman.nginx.org/mailman/listinfo/nginx-devel
>
> --
> Roman Arutyunyan
> # HG changeset patch
> # User Roman Arutyunyan <arut at nginx.com>
> # Date 1684240208 -14400
> # Tue May 16 16:30:08 2023 +0400
> # Node ID 6d3648910d8ea0bf8891670c7b9ea5539a3d39f9
> # Parent 564c8f447ea4d76d80251bbf07df412942a2261f
> 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
> @@ -421,6 +421,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
> @@ -442,12 +445,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) {
>
> @@ -474,8 +471,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 --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
> @@ -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 --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);
> @@ -377,6 +371,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;
> @@ -423,6 +420,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) {
> @@ -437,12 +436,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;
> @@ -483,6 +486,29 @@ 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 (!hc->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;
> + }
> +
> + ngx_post_event(rev, &ngx_posted_events);
> + return;
> + }
> + }
> +
> +#endif
> +
> c->log->action = "reading client request line";
>
> ngx_reusable_connection(c, 0);
> @@ -802,13 +828,16 @@ ngx_http_ssl_handshake_handler(ngx_conne
> #if (NGX_HTTP_V2 \
> && defined TLSEXT_TYPE_application_layer_protocol_negotiation)
> {
> - unsigned int len;
> - const unsigned char *data;
> - ngx_http_connection_t *hc;
> + unsigned int len;
> + const unsigned char *data;
> + ngx_http_connection_t *hc;
> + ngx_http_v2_srv_conf_t *h2scf;
>
> hc = c->data;
>
> - 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) {
>
> SSL_get0_alpn_selected(c->ssl->connection, &data, &len);
>
> 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
> @@ -63,8 +63,6 @@ static void ngx_http_v2_handle_connectio
> static void ngx_http_v2_lingering_close(ngx_connection_t *c);
> static void ngx_http_v2_lingering_close_handler(ngx_event_t *rev);
>
> -static u_char *ngx_http_v2_state_proxy_protocol(ngx_http_v2_connection_t *h2c,
> - u_char *pos, u_char *end);
> static u_char *ngx_http_v2_state_preface(ngx_http_v2_connection_t *h2c,
> u_char *pos, u_char *end);
> static u_char *ngx_http_v2_state_preface_end(ngx_http_v2_connection_t *h2c,
> @@ -232,6 +230,7 @@ static ngx_http_v2_parse_header_t ngx_h
> void
> ngx_http_v2_init(ngx_event_t *rev)
> {
> + u_char *p, *end;
> ngx_connection_t *c;
> ngx_pool_cleanup_t *cln;
> ngx_http_connection_t *hc;
> @@ -314,8 +313,7 @@ ngx_http_v2_init(ngx_event_t *rev)
> return;
> }
>
> - h2c->state.handler = hc->proxy_protocol ? ngx_http_v2_state_proxy_protocol
> - : ngx_http_v2_state_preface;
> + h2c->state.handler = ngx_http_v2_state_preface;
>
> ngx_queue_init(&h2c->waiting);
> ngx_queue_init(&h2c->dependencies);
> @@ -335,6 +333,23 @@ ngx_http_v2_init(ngx_event_t *rev)
> c->idle = 1;
> ngx_reusable_connection(c, 0);
>
> + if (c->buffer) {
> + p = c->buffer->pos;
> + end = c->buffer->last;
> +
> + do {
> + p = h2c->state.handler(h2c, p, end);
> +
> + if (p == NULL) {
> + return;
> + }
> +
> + } while (p != end);
> +
> + h2c->total_bytes += p - c->buffer->pos;
> + c->buffer->pos = p;
> + }
> +
> ngx_http_v2_read_handler(rev);
> }
>
> @@ -847,31 +862,10 @@ ngx_http_v2_lingering_close_handler(ngx_
>
>
> static u_char *
> -ngx_http_v2_state_proxy_protocol(ngx_http_v2_connection_t *h2c, u_char *pos,
> - u_char *end)
> -{
> - ngx_log_t *log;
> -
> - log = h2c->connection->log;
> - log->action = "reading PROXY protocol";
> -
> - pos = ngx_proxy_protocol_read(h2c->connection, pos, end);
> -
> - log->action = "processing HTTP/2 connection";
> -
> - if (pos == NULL) {
> - return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_PROTOCOL_ERROR);
> - }
> -
> - return ngx_http_v2_state_preface(h2c, pos, end);
> -}
> -
> -
> -static u_char *
> ngx_http_v2_state_preface(ngx_http_v2_connection_t *h2c, u_char *pos,
> u_char *end)
> {
> - static const u_char preface[] = "PRI * HTTP/2.0\r\n";
> + static const u_char preface[] = NGX_HTTP_V2_PREFACE_START;
>
> if ((size_t) (end - pos) < sizeof(preface) - 1) {
> return ngx_http_v2_state_save(h2c, pos, end, ngx_http_v2_state_preface);
> @@ -892,7 +886,7 @@ static u_char *
> ngx_http_v2_state_preface_end(ngx_http_v2_connection_t *h2c, u_char *pos,
> u_char *end)
> {
> - static const u_char preface[] = "\r\nSM\r\n\r\n";
> + static const u_char preface[] = NGX_HTTP_V2_PREFACE_END;
>
> if ((size_t) (end - pos) < sizeof(preface) - 1) {
> return ngx_http_v2_state_save(h2c, pos, end,
> @@ -3943,10 +3937,22 @@ static void
> ngx_http_v2_run_request(ngx_http_request_t *r)
> {
> ngx_connection_t *fc;
> + ngx_http_v2_srv_conf_t *h2scf;
> ngx_http_v2_connection_t *h2c;
>
> fc = r->connection;
>
> + h2scf = ngx_http_get_module_srv_conf(r, ngx_http_v2_module);
> +
> + if (!h2scf->enable && !r->http_connection->addr_conf->http2) {
> + ngx_log_error(NGX_LOG_INFO, fc->log, 0,
> + "client attempted to request the server name "
> + "for which the negotiated protocol is disabled");
> +
> + ngx_http_finalize_request(r, NGX_HTTP_MISDIRECTED_REQUEST);
> + goto failed;
> + }
> +
> if (ngx_http_v2_construct_request_line(r) != NGX_OK) {
> goto failed;
> }
> diff --git a/src/http/v2/ngx_http_v2.h b/src/http/v2/ngx_http_v2.h
> --- a/src/http/v2/ngx_http_v2.h
> +++ b/src/http/v2/ngx_http_v2.h
> @@ -64,6 +64,16 @@ typedef u_char *(*ngx_http_v2_handler_pt
>
>
> typedef struct {
> + ngx_flag_t enable;
> + size_t pool_size;
> + ngx_uint_t concurrent_streams;
> + ngx_uint_t concurrent_pushes;
> + size_t preread_size;
> + ngx_uint_t streams_index_mask;
> +} ngx_http_v2_srv_conf_t;
> +
> +
> +typedef struct {
> ngx_str_t name;
> ngx_str_t value;
> } ngx_http_v2_header_t;
> @@ -408,9 +418,17 @@ ngx_int_t ngx_http_v2_table_size(ngx_htt
> #define NGX_HTTP_V2_USER_AGENT_INDEX 58
> #define NGX_HTTP_V2_VARY_INDEX 59
>
> +#define NGX_HTTP_V2_PREFACE_START "PRI * HTTP/2.0\r\n"
> +#define NGX_HTTP_V2_PREFACE_END "\r\nSM\r\n\r\n"
> +#define NGX_HTTP_V2_PREFACE NGX_HTTP_V2_PREFACE_START \
> + NGX_HTTP_V2_PREFACE_END
> +
>
> u_char *ngx_http_v2_string_encode(u_char *dst, u_char *src, size_t len,
> u_char *tmp, ngx_uint_t lower);
>
>
> +extern ngx_module_t ngx_http_v2_module;
> +
> +
> #endif /* _NGX_HTTP_V2_H_INCLUDED_ */
> diff --git a/src/http/v2/ngx_http_v2_module.c b/src/http/v2/ngx_http_v2_module.c
> --- a/src/http/v2/ngx_http_v2_module.c
> +++ b/src/http/v2/ngx_http_v2_module.c
> @@ -75,6 +75,13 @@ static ngx_conf_post_t ngx_http_v2_chun
>
> static ngx_command_t ngx_http_v2_commands[] = {
>
> + { ngx_string("http2"),
> + NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_CONF_FLAG,
> + ngx_conf_set_flag_slot,
> + NGX_HTTP_SRV_CONF_OFFSET,
> + offsetof(ngx_http_v2_srv_conf_t, enable),
> + NULL },
> +
> { ngx_string("http2_recv_buffer_size"),
> NGX_HTTP_MAIN_CONF|NGX_CONF_TAKE1,
> ngx_conf_set_size_slot,
> @@ -314,6 +321,8 @@ ngx_http_v2_create_srv_conf(ngx_conf_t *
> return NULL;
> }
>
> + h2scf->enable = NGX_CONF_UNSET;
> +
> h2scf->pool_size = NGX_CONF_UNSET_SIZE;
>
> h2scf->concurrent_streams = NGX_CONF_UNSET_UINT;
> @@ -333,6 +342,8 @@ ngx_http_v2_merge_srv_conf(ngx_conf_t *c
> ngx_http_v2_srv_conf_t *prev = parent;
> ngx_http_v2_srv_conf_t *conf = child;
>
> + ngx_conf_merge_value(conf->enable, prev->enable, 0);
> +
> ngx_conf_merge_size_value(conf->pool_size, prev->pool_size, 4096);
>
> ngx_conf_merge_uint_value(conf->concurrent_streams,
> diff --git a/src/http/v2/ngx_http_v2_module.h b/src/http/v2/ngx_http_v2_module.h
> --- a/src/http/v2/ngx_http_v2_module.h
> +++ b/src/http/v2/ngx_http_v2_module.h
> @@ -21,15 +21,6 @@ typedef struct {
>
>
> typedef struct {
> - size_t pool_size;
> - ngx_uint_t concurrent_streams;
> - ngx_uint_t concurrent_pushes;
> - size_t preread_size;
> - ngx_uint_t streams_index_mask;
> -} ngx_http_v2_srv_conf_t;
> -
> -
> -typedef struct {
> size_t chunk_size;
>
> ngx_flag_t push_preload;
> @@ -39,7 +30,4 @@ typedef struct {
> } ngx_http_v2_loc_conf_t;
>
>
> -extern ngx_module_t ngx_http_v2_module;
> -
> -
> #endif /* _NGX_HTTP_V2_MODULE_H_INCLUDED_ */
Looks good.
--
Maxim Dounin
http://mdounin.ru/
More information about the nginx-devel
mailing list