From 2485681308bd8d3108da31546cb91bb97813a3fb Mon Sep 17 00:00:00 2001 From: Maxim Dounin Date: Thu, 2 Feb 2023 23:38:48 +0300 Subject: Lingering close for connections with pipelined requests. This is expected to help with clients using pipelining with some constant depth, such as apt[1][2]. When downloading many resources, apt uses pipelining with some constant depth, a number of requests in flight. This essentially means that after receiving a response it sends an additional request to the server, and this can result in requests arriving to the server at any time. Further, additional requests are sent one-by-one, and can be easily seen as such (neither as pipelined, nor followed by pipelined requests). The only safe approach to close such connections (for example, when keepalive_requests is reached) is with lingering. To do so, now nginx monitors if pipelining was used on the connection, and if it was, closes the connection with lingering. [1] https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=973861#10 [2] https://mailman.nginx.org/pipermail/nginx-devel/2023-January/ZA2SP5SJU55LHEBCJMFDB2AZVELRLTHI.html --- src/core/ngx_connection.h | 1 + 1 file changed, 1 insertion(+) (limited to 'src/core') diff --git a/src/core/ngx_connection.h b/src/core/ngx_connection.h index 6d3348d9c..36e1be27c 100644 --- a/src/core/ngx_connection.h +++ b/src/core/ngx_connection.h @@ -172,6 +172,7 @@ struct ngx_connection_s { unsigned timedout:1; unsigned error:1; unsigned destroyed:1; + unsigned pipeline:1; unsigned idle:1; unsigned reusable:1; -- cgit From 2c5fccd4693c0a68e1c72d65e016ba83e861120e Mon Sep 17 00:00:00 2001 From: Yugo Horie Date: Thu, 23 Feb 2023 08:09:50 +0900 Subject: Core: stricter UTF-8 handling in ngx_utf8_decode(). An UTF-8 octet sequence cannot start with a 11111xxx byte (above 0xf8), see https://datatracker.ietf.org/doc/html/rfc3629#section-3. Previously, such bytes were accepted by ngx_utf8_decode() and misinterpreted as 11110xxx bytes (as in a 4-byte sequence). While unlikely, this can potentially cause issues. Fix is to explicitly reject such bytes in ngx_utf8_decode(). --- src/core/ngx_string.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) (limited to 'src/core') diff --git a/src/core/ngx_string.c b/src/core/ngx_string.c index 98f270aca..f8f738472 100644 --- a/src/core/ngx_string.c +++ b/src/core/ngx_string.c @@ -1364,7 +1364,12 @@ ngx_utf8_decode(u_char **p, size_t n) u = **p; - if (u >= 0xf0) { + if (u >= 0xf8) { + + (*p)++; + return 0xffffffff; + + } else if (u >= 0xf0) { u &= 0x07; valid = 0xffff; -- cgit From 853912986d9568b049ecb5499b6af987cb13cb14 Mon Sep 17 00:00:00 2001 From: Maxim Dounin Date: Fri, 10 Mar 2023 07:43:40 +0300 Subject: Syslog: removed usage of ngx_cycle->log and ngx_cycle->hostname. During initial startup the ngx_cycle->hostname is not available, and previously this resulted in incorrect logging. Instead, hostname from the configuration being parsed is now preserved in the syslog peer structure and then used during logging. Similarly, ngx_cycle->log might not match the configuration where the syslog peer is defined if the configuration is not yet fully applied, and previously this resulted in unexpected logging of syslog errors and debug information. Instead, cf->cycle->new_log is now referenced in the syslog peer structure and used for logging, similarly to how it is done in other modules. --- src/core/ngx_syslog.c | 21 +++++++++++---------- src/core/ngx_syslog.h | 21 +++++++++++++-------- 2 files changed, 24 insertions(+), 18 deletions(-) (limited to 'src/core') diff --git a/src/core/ngx_syslog.c b/src/core/ngx_syslog.c index 3c7b63a62..0a64e10b8 100644 --- a/src/core/ngx_syslog.c +++ b/src/core/ngx_syslog.c @@ -66,6 +66,9 @@ ngx_syslog_process_conf(ngx_conf_t *cf, ngx_syslog_peer_t *peer) ngx_str_set(&peer->tag, "nginx"); } + peer->hostname = &cf->cycle->hostname; + peer->log = &cf->cycle->new_log; + peer->conn.fd = (ngx_socket_t) -1; peer->conn.read = &ngx_syslog_dummy_event; @@ -243,7 +246,7 @@ ngx_syslog_add_header(ngx_syslog_peer_t *peer, u_char *buf) } return ngx_sprintf(buf, "<%ui>%V %V %V: ", pri, &ngx_cached_syslog_time, - &ngx_cycle->hostname, &peer->tag); + peer->hostname, &peer->tag); } @@ -292,9 +295,6 @@ ngx_syslog_send(ngx_syslog_peer_t *peer, u_char *buf, size_t len) } } - /* log syslog socket events with valid log */ - peer->conn.log = ngx_cycle->log; - if (ngx_send) { n = ngx_send(&peer->conn, buf, len); @@ -306,7 +306,7 @@ ngx_syslog_send(ngx_syslog_peer_t *peer, u_char *buf, size_t len) if (n == NGX_ERROR) { if (ngx_close_socket(peer->conn.fd) == -1) { - ngx_log_error(NGX_LOG_ALERT, ngx_cycle->log, ngx_socket_errno, + ngx_log_error(NGX_LOG_ALERT, peer->log, ngx_socket_errno, ngx_close_socket_n " failed"); } @@ -324,24 +324,25 @@ ngx_syslog_init_peer(ngx_syslog_peer_t *peer) fd = ngx_socket(peer->server.sockaddr->sa_family, SOCK_DGRAM, 0); if (fd == (ngx_socket_t) -1) { - ngx_log_error(NGX_LOG_ALERT, ngx_cycle->log, ngx_socket_errno, + ngx_log_error(NGX_LOG_ALERT, peer->log, ngx_socket_errno, ngx_socket_n " failed"); return NGX_ERROR; } if (ngx_nonblocking(fd) == -1) { - ngx_log_error(NGX_LOG_ALERT, ngx_cycle->log, ngx_socket_errno, + ngx_log_error(NGX_LOG_ALERT, peer->log, ngx_socket_errno, ngx_nonblocking_n " failed"); goto failed; } if (connect(fd, peer->server.sockaddr, peer->server.socklen) == -1) { - ngx_log_error(NGX_LOG_ALERT, ngx_cycle->log, ngx_socket_errno, + ngx_log_error(NGX_LOG_ALERT, peer->log, ngx_socket_errno, "connect() failed"); goto failed; } peer->conn.fd = fd; + peer->conn.log = peer->log; /* UDP sockets are always ready to write */ peer->conn.write->ready = 1; @@ -351,7 +352,7 @@ ngx_syslog_init_peer(ngx_syslog_peer_t *peer) failed: if (ngx_close_socket(fd) == -1) { - ngx_log_error(NGX_LOG_ALERT, ngx_cycle->log, ngx_socket_errno, + ngx_log_error(NGX_LOG_ALERT, peer->log, ngx_socket_errno, ngx_close_socket_n " failed"); } @@ -372,7 +373,7 @@ ngx_syslog_cleanup(void *data) } if (ngx_close_socket(peer->conn.fd) == -1) { - ngx_log_error(NGX_LOG_ALERT, ngx_cycle->log, ngx_socket_errno, + ngx_log_error(NGX_LOG_ALERT, peer->log, ngx_socket_errno, ngx_close_socket_n " failed"); } } diff --git a/src/core/ngx_syslog.h b/src/core/ngx_syslog.h index 50dcd3511..181ebe7b3 100644 --- a/src/core/ngx_syslog.h +++ b/src/core/ngx_syslog.h @@ -9,14 +9,19 @@ typedef struct { - ngx_uint_t facility; - ngx_uint_t severity; - ngx_str_t tag; - - ngx_addr_t server; - ngx_connection_t conn; - unsigned busy:1; - unsigned nohostname:1; + ngx_uint_t facility; + ngx_uint_t severity; + ngx_str_t tag; + + ngx_str_t *hostname; + + ngx_addr_t server; + ngx_connection_t conn; + + ngx_log_t *log; + + unsigned busy:1; + unsigned nohostname:1; } ngx_syslog_peer_t; -- cgit From 11ed95bb53210c322c16bb0897f0cb3b5726ed57 Mon Sep 17 00:00:00 2001 From: Maxim Dounin Date: Fri, 10 Mar 2023 07:43:50 +0300 Subject: Syslog: introduced error log handler. This ensures that errors which happen during logging to syslog are logged with proper context, such as "while logging to syslog" and the server name. Prodded by Safar Safarly. --- src/core/ngx_syslog.c | 47 +++++++++++++++++++++++++++++++++++++++-------- src/core/ngx_syslog.h | 3 ++- 2 files changed, 41 insertions(+), 9 deletions(-) (limited to 'src/core') diff --git a/src/core/ngx_syslog.c b/src/core/ngx_syslog.c index 0a64e10b8..bad45bd16 100644 --- a/src/core/ngx_syslog.c +++ b/src/core/ngx_syslog.c @@ -18,6 +18,7 @@ static char *ngx_syslog_parse_args(ngx_conf_t *cf, ngx_syslog_peer_t *peer); static ngx_int_t ngx_syslog_init_peer(ngx_syslog_peer_t *peer); static void ngx_syslog_cleanup(void *data); +static u_char *ngx_syslog_log_error(ngx_log_t *log, u_char *buf, size_t len); static char *facilities[] = { @@ -67,7 +68,7 @@ ngx_syslog_process_conf(ngx_conf_t *cf, ngx_syslog_peer_t *peer) } peer->hostname = &cf->cycle->hostname; - peer->log = &cf->cycle->new_log; + peer->logp = &cf->cycle->new_log; peer->conn.fd = (ngx_socket_t) -1; @@ -289,6 +290,13 @@ ngx_syslog_send(ngx_syslog_peer_t *peer, u_char *buf, size_t len) { ssize_t n; + if (peer->log.handler == NULL) { + peer->log = *peer->logp; + peer->log.handler = ngx_syslog_log_error; + peer->log.data = peer; + peer->log.action = "logging to syslog"; + } + if (peer->conn.fd == (ngx_socket_t) -1) { if (ngx_syslog_init_peer(peer) != NGX_OK) { return NGX_ERROR; @@ -306,7 +314,7 @@ ngx_syslog_send(ngx_syslog_peer_t *peer, u_char *buf, size_t len) if (n == NGX_ERROR) { if (ngx_close_socket(peer->conn.fd) == -1) { - ngx_log_error(NGX_LOG_ALERT, peer->log, ngx_socket_errno, + ngx_log_error(NGX_LOG_ALERT, &peer->log, ngx_socket_errno, ngx_close_socket_n " failed"); } @@ -324,25 +332,25 @@ ngx_syslog_init_peer(ngx_syslog_peer_t *peer) fd = ngx_socket(peer->server.sockaddr->sa_family, SOCK_DGRAM, 0); if (fd == (ngx_socket_t) -1) { - ngx_log_error(NGX_LOG_ALERT, peer->log, ngx_socket_errno, + ngx_log_error(NGX_LOG_ALERT, &peer->log, ngx_socket_errno, ngx_socket_n " failed"); return NGX_ERROR; } if (ngx_nonblocking(fd) == -1) { - ngx_log_error(NGX_LOG_ALERT, peer->log, ngx_socket_errno, + ngx_log_error(NGX_LOG_ALERT, &peer->log, ngx_socket_errno, ngx_nonblocking_n " failed"); goto failed; } if (connect(fd, peer->server.sockaddr, peer->server.socklen) == -1) { - ngx_log_error(NGX_LOG_ALERT, peer->log, ngx_socket_errno, + ngx_log_error(NGX_LOG_ALERT, &peer->log, ngx_socket_errno, "connect() failed"); goto failed; } peer->conn.fd = fd; - peer->conn.log = peer->log; + peer->conn.log = &peer->log; /* UDP sockets are always ready to write */ peer->conn.write->ready = 1; @@ -352,7 +360,7 @@ ngx_syslog_init_peer(ngx_syslog_peer_t *peer) failed: if (ngx_close_socket(fd) == -1) { - ngx_log_error(NGX_LOG_ALERT, peer->log, ngx_socket_errno, + ngx_log_error(NGX_LOG_ALERT, &peer->log, ngx_socket_errno, ngx_close_socket_n " failed"); } @@ -373,7 +381,30 @@ ngx_syslog_cleanup(void *data) } if (ngx_close_socket(peer->conn.fd) == -1) { - ngx_log_error(NGX_LOG_ALERT, peer->log, ngx_socket_errno, + ngx_log_error(NGX_LOG_ALERT, &peer->log, ngx_socket_errno, ngx_close_socket_n " failed"); } } + + +static u_char * +ngx_syslog_log_error(ngx_log_t *log, u_char *buf, size_t len) +{ + u_char *p; + ngx_syslog_peer_t *peer; + + p = buf; + + if (log->action) { + p = ngx_snprintf(buf, len, " while %s", log->action); + len -= p - buf; + } + + peer = log->data; + + if (peer) { + p = ngx_snprintf(p, len, ", server: %V", &peer->server.name); + } + + return p; +} diff --git a/src/core/ngx_syslog.h b/src/core/ngx_syslog.h index 181ebe7b3..e2d54acdb 100644 --- a/src/core/ngx_syslog.h +++ b/src/core/ngx_syslog.h @@ -18,7 +18,8 @@ typedef struct { ngx_addr_t server; ngx_connection_t conn; - ngx_log_t *log; + ngx_log_t log; + ngx_log_t *logp; unsigned busy:1; unsigned nohostname:1; -- cgit