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

[Sergey Kandaurov]

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

> 
>> +
>> +        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, &ngx_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