[PATCH 2 of 3] HTTP/2: "http2" directive

Maxim Dounin mdounin at mdounin.ru
Tue Feb 7 03:12:50 UTC 2023


Hello!

On Wed, Feb 01, 2023 at 06:01:09PM +0400, Roman Arutyunyan wrote:

> # HG changeset patch
> # User Roman Arutyunyan <arut at nginx.com>
> # 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 (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;
> +            }

Note that ngx_http_wait_request_handler() is used for both 
new and keepalive connections.  It does not look like a good idea 
to start an HTTP/2 connection after processing some HTTP/1.x 
requests.

> +
> +            c->log->action = "waiting for request";
> +            ngx_post_event(rev, &ngx_posted_events);
> +            return;
> +        }
> +    }
> +
> +#endif
> +
>      c->log->action = "reading client request line";
>  
>      ngx_reusable_connection(c, 0);
> @@ -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
> @@ -232,6 +232,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;
> @@ -335,6 +336,29 @@ 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;
> +
> +        if (h2c->total_bytes / 8 > h2c->payload_bytes + 1048576) {
> +            ngx_log_error(NGX_LOG_INFO, c->log, 0, "http2 flood detected");
> +            ngx_http_v2_finalize_connection(h2c, NGX_HTTP_V2_NO_ERROR);
> +            return;
> +        }

I think flood protection can be safely omitted here, given that 
client_header_buffer_size is expected to be small (1k by default), 
and flood protection is not going to kick in till at least 8M of 
total bytes from the client.

> +    }
> +
>      ngx_http_v2_read_handler(rev);
>  }
>  
> @@ -871,7 +895,7 @@ 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 +916,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,
> @@ -3946,10 +3970,22 @@ static void
>  ngx_http_v2_run_request(ngx_http_request_t *r)
>  {
>      ngx_connection_t          *fc;
> +    ngx_http_v3_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 negotitated protocol is unavailable");

Typo, should be "negotiated".

> +
> +        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_ */

Otherwise looks good.

-- 
Maxim Dounin
http://mdounin.ru/


More information about the nginx-devel mailing list