Opened 10 months ago

Closed 9 months ago

#2142 closed defect (fixed)

keepalive_timeout 0 will break http2

Reported by: Jeroen Owned by:
Priority: major Milestone: nginx-1.19
Component: nginx-core Version: 1.19.x
Keywords: keepalive_timeout Cc:
uname -a:
nginx -V: nginx version: nginx/1.19.7
built by gcc 10.2.1 20201203 (Alpine 10.2.1_pre1)
built with OpenSSL 1.1.1i 8 Dec 2020 (running with OpenSSL 1.1.1j 16 Feb 2021)
TLS SNI support enabled
configure arguments: --prefix=/etc/nginx --sbin-path=/usr/sbin/nginx --modules-path=/usr/lib/nginx/modules --conf-path=/etc/nginx/nginx.conf --error-log-path=/var/log/nginx/error.log --http-log-path=/var/log/nginx/access.log --pid-path=/var/run/nginx.pid --lock-path=/var/run/nginx.lock --http-client-body-temp-path=/var/cache/nginx/client_temp --http-proxy-temp-path=/var/cache/nginx/proxy_temp --http-fastcgi-temp-path=/var/cache/nginx/fastcgi_temp --http-uwsgi-temp-path=/var/cache/nginx/uwsgi_temp --http-scgi-temp-path=/var/cache/nginx/scgi_temp --with-perl_modules_path=/usr/lib/perl5/vendor_perl --user=nginx --group=nginx --with-compat --with-file-aio --with-threads --with-http_addition_module --with-http_auth_request_module --with-http_dav_module --with-http_flv_module --with-http_gunzip_module --with-http_gzip_static_module --with-http_mp4_module --with-http_random_index_module --with-http_realip_module --with-http_secure_link_module --with-http_slice_module --with-http_ssl_module --with-http_stub_status_module --with-http_sub_module --with-http_v2_module --with-mail --with-mail_ssl_module --with-stream --with-stream_realip_module --with-stream_ssl_module --with-stream_ssl_preread_module --with-cc-opt='-Os -fomit-frame-pointer -g' --with-ld-opt=-Wl,--as-needed

Description (last modified by Jeroen)

From the https://nginx.org/en/CHANGES and the latest change 1.19.7

*) Change: connections handling in HTTP/2 has been changed to better

match HTTP/1.x; the "http2_recv_timeout", "http2_idle_timeout", and
"http2_max_requests" directives have been removed, the
"keepalive_timeout" and "keepalive_requests" directives should be
used instead.

If someone is upgrading from earlier versions in this patch release, http2 will break if there is a 0 for keepalive_timeout.
Because we had different parameters in the past, this will silently break your system for http2 clients.

I think what happens is that the requesting end will keeps it's stream open, but nginx will terminate it because 0 has passed.

We use nginx:alpine docker image, which was automatically updated.

To reproduce using docker, but will work on any environment:

Start nginx with self-signed certificate and http2 enabled

docker run --rm -it --name=host --entrypoint=/bin/sh nginx:1.19.7-alpine

apk add --no-cache openssl

cd /etc/nginx
openssl req -keyout cert.key -nodes -out cert.crt -newkey rsa:4096 -days 365 -x509
# ENTER A FEW TIMES

# update keepalive and logging
sed -e 's/keepalive_timeout\s*65;/keepalive_timeout 0;/' -e 's#error_log\s*/var/log/nginx/error.log#error_log stdout#' -e 's#access_log */var/log/nginx/access.log#access_log /dev/stdout#' -i nginx.conf

conf.d/default.conf

server {
    listen       80;
    listen      443 ssl http2;
    server_name  localhost;

ssl_certificate cert.crt;
ssl_certificate_key cert.key;

... snap


start nginx without a daemon: nginx -g'daemon off;'

In a new console:

docker run --rm -it --link host alpine

apk add --no-cache curl

curl -vk 'https://host/' -http2 # http2 is the default in latest curl's
# repeat several times, sometimes a succesfull response can be generated

change the value to something else than 0 and start again.

sed -e 's/keepalive_timeout\s*0;/keepalive_timeout 65;/' -i nginx.conf

nginx -g'daemon off;'

If you curl again, everything will work just fine.

My suggestion would be to raise an error/warning on start when a zero value is set and http2 is enabled, http2 should never accept a zero value for keepalive_timeout

Change History (5)

comment:1 by Jeroen, 10 months ago

Description: modified (diff)

comment:2 by Jeroen, 10 months ago

A reason for it to have a 0 comes from using Nginx as a proxy.

comment:3 by Maxim Dounin, 10 months ago

Status: newaccepted

Thanks for the report, this certainly needs to be addressed.

An obvious workaround would be to use keepalive_requests 1; instead of keepalive_timeout 0; to disable keepalive connections, this will properly allow just one request with both HTTP/1.x and HTTP/2. But keepalive_timeout 0; is a well-known way to do the same, and should work as well.

The following patch should fix this, please test if it works for you.

# HG changeset patch
# User Maxim Dounin <mdounin@mdounin.ru>
# Date 1614362311 -10800
#      Fri Feb 26 20:58:31 2021 +0300
# Node ID 4fa6e0c129d738dc434b7c94da4f6093d972c685
# Parent  dea93b6dce9458d20cef3478a6f3f7b24b87de4a
HTTP/2: client_header_timeout before first request (ticket #2142).

With this change, behaviour of HTTP/2 becomes even closer to HTTP/1.x,
and client_header_timeout instead of keepalive_timeout is used before
the first request is received.

This fixes HTTP/2 connections being closed even before the first request,
if "keepalive_timeout 0;" was used in the configuration; the problem appeared
in f790816a0e87 (1.19.7).

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
@@ -238,6 +238,7 @@ ngx_http_v2_init(ngx_event_t *rev)
     ngx_http_v2_srv_conf_t    *h2scf;
     ngx_http_v2_main_conf_t   *h2mcf;
     ngx_http_v2_connection_t  *h2c;
+    ngx_http_core_srv_conf_t  *cscf;
 
     c = rev->data;
     hc = c->data;
@@ -325,8 +326,10 @@ ngx_http_v2_init(ngx_event_t *rev)
     rev->handler = ngx_http_v2_read_handler;
     c->write->handler = ngx_http_v2_write_handler;
 
-    if (c->read->timer_set) {
-        ngx_del_timer(c->read);
+    if (!rev->timer_set) {
+        cscf = ngx_http_get_module_srv_conf(hc->conf_ctx,
+                                            ngx_http_core_module);
+        ngx_add_timer(rev, cscf->client_header_timeout);
     }
 
     c->idle = 1;

comment:4 by Maxim Dounin <mdounin@…>, 9 months ago

In 7783:171682010da4/nginx:

HTTP/2: client_header_timeout before first request (ticket #2142).

With this change, behaviour of HTTP/2 becomes even closer to HTTP/1.x,
and client_header_timeout instead of keepalive_timeout is used before
the first request is received.

This fixes HTTP/2 connections being closed even before the first request
if "keepalive_timeout 0;" was used in the configuration; the problem
appeared in f790816a0e87 (1.19.7).

comment:5 by Maxim Dounin, 9 months ago

Resolution: fixed
Status: acceptedclosed

Fix committed, thanks for reporting this.

Note: See TracTickets for help on using tickets.