[PATCH] HTTP: stop emitting server version by default
# HG changeset patch # User Piotr Sikora # Date 1708977611 0 # Mon Feb 26 20:00:11 2024 + # Branch patch001 # Node ID a8a592b9b62eff7bca03e8b46669f59d2da689ed # Parent 89bff782528a91ad123b63b624f798e6fd9c8e68 HTTP: stop emitting server version by default. This information is only useful to attackers. The previous behavior can be restored using "server_tokens on". Signed-off-by: Piotr Sikora diff -r 89bff782528a -r a8a592b9b62e src/http/ngx_http_core_module.c --- a/src/http/ngx_http_core_module.c Wed Feb 14 20:03:00 2024 +0400 +++ b/src/http/ngx_http_core_module.c Mon Feb 26 20:00:11 2024 + @@ -3899,7 +3899,7 @@ ngx_conf_merge_value(conf->etag, prev->etag, 1); ngx_conf_merge_uint_value(conf->server_tokens, prev->server_tokens, - NGX_HTTP_SERVER_TOKENS_ON); + NGX_HTTP_SERVER_TOKENS_OFF); ngx_conf_merge_ptr_value(conf->open_file_cache, prev->open_file_cache, NULL); ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] Core: free connections and read/write events at shutdown
# HG changeset patch # User Piotr Sikora # Date 1708977616 0 # Mon Feb 26 20:00:16 2024 + # Branch patch002 # Node ID f8d9fb94eab212f6e640b7a68ed111562e3157d5 # Parent a8a592b9b62eff7bca03e8b46669f59d2da689ed Core: free connections and read/write events at shutdown. Found with LeakSanitizer. Signed-off-by: Piotr Sikora diff -r a8a592b9b62e -r f8d9fb94eab2 src/os/unix/ngx_process_cycle.c --- a/src/os/unix/ngx_process_cycle.c Mon Feb 26 20:00:11 2024 + +++ b/src/os/unix/ngx_process_cycle.c Mon Feb 26 20:00:16 2024 + @@ -940,6 +940,7 @@ ngx_worker_process_exit(ngx_cycle_t *cycle) { ngx_uint_t i; +ngx_event_t *rev, *wev; ngx_connection_t *c; for (i = 0; cycle->modules[i]; i++) { @@ -989,8 +990,16 @@ ngx_exit_cycle.files_n = ngx_cycle->files_n; ngx_cycle = &ngx_exit_cycle; +c = cycle->connections; +rev = cycle->read_events; +wev = cycle->write_events; + ngx_destroy_pool(cycle->pool); +ngx_free(c); +ngx_free(rev); +ngx_free(wev); + ngx_log_error(NGX_LOG_NOTICE, ngx_cycle->log, 0, "exit"); exit(0); ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] Upstream: cleanup at shutdown
# HG changeset patch # User Piotr Sikora # Date 1708977618 0 # Mon Feb 26 20:00:18 2024 + # Branch patch003 # Node ID 8edb4003177dac56301aed7f86f8d2a564b47552 # Parent f8d9fb94eab212f6e640b7a68ed111562e3157d5 Upstream: cleanup at shutdown. Add "free_upstream" callback called on worker exit to free any per-upstream objects allocated from the heap. Found with LeakSanitizer. Signed-off-by: Piotr Sikora diff -r f8d9fb94eab2 -r 8edb4003177d src/http/modules/ngx_http_upstream_random_module.c --- a/src/http/modules/ngx_http_upstream_random_module.cMon Feb 26 20:00:16 2024 + +++ b/src/http/modules/ngx_http_upstream_random_module.cMon Feb 26 20:00:18 2024 + @@ -114,6 +114,35 @@ } +static void +ngx_http_upstream_free_random(ngx_http_upstream_srv_conf_t *us) +{ +#if (NGX_HTTP_UPSTREAM_ZONE) + +ngx_http_upstream_rr_peers_t *peers; +ngx_http_upstream_random_srv_conf_t *rcf; + +peers = us->peer.data; + +if (peers->shpool) { + +rcf = ngx_http_conf_upstream_srv_conf(us, + ngx_http_upstream_random_module); + +if (rcf->ranges) { +ngx_log_debug1(NGX_LOG_DEBUG_HTTP, ngx_cycle->log, 0, + "free ranges: %p", rcf->ranges); +ngx_free(rcf->ranges); +rcf->ranges = NULL; +} +} + +#endif + +ngx_http_upstream_free_round_robin(us); +} + + static ngx_int_t ngx_http_upstream_update_random(ngx_pool_t *pool, ngx_http_upstream_srv_conf_t *us) @@ -465,6 +494,7 @@ } uscf->peer.init_upstream = ngx_http_upstream_init_random; +uscf->peer.free_upstream = ngx_http_upstream_free_random; uscf->flags = NGX_HTTP_UPSTREAM_CREATE |NGX_HTTP_UPSTREAM_WEIGHT diff -r f8d9fb94eab2 -r 8edb4003177d src/http/ngx_http_upstream.c --- a/src/http/ngx_http_upstream.c Mon Feb 26 20:00:16 2024 + +++ b/src/http/ngx_http_upstream.c Mon Feb 26 20:00:18 2024 + @@ -189,6 +189,8 @@ ngx_http_upstream_t *u, ngx_connection_t *c); #endif +static void ngx_http_upstream_worker_cleanup(ngx_cycle_t *cycle); + static ngx_http_upstream_header_t ngx_http_upstream_headers_in[] = { @@ -368,7 +370,7 @@ NULL, /* init process */ NULL, /* init thread */ NULL, /* exit thread */ -NULL, /* exit process */ +ngx_http_upstream_worker_cleanup, /* exit process */ NULL, /* exit master */ NGX_MODULE_V1_PADDING }; @@ -6829,3 +6831,29 @@ return NGX_CONF_OK; } + + +static void +ngx_http_upstream_worker_cleanup(ngx_cycle_t *cycle) +{ +ngx_uint_t i; +ngx_http_upstream_free_ptfree; +ngx_http_upstream_srv_conf_t **uscfp; +ngx_http_upstream_main_conf_t *umcf; + +umcf = ngx_http_cycle_get_module_main_conf(cycle, ngx_http_upstream_module); + +if (umcf) { + +uscfp = umcf->upstreams.elts; + +for (i = 0; i < umcf->upstreams.nelts; i++) { + +free = uscfp[i]->peer.free_upstream + ? uscfp[i]->peer.free_upstream + : ngx_http_upstream_free_round_robin; + +free(uscfp[i]); +} +} +} diff -r f8d9fb94eab2 -r 8edb4003177d src/http/ngx_http_upstream.h --- a/src/http/ngx_http_upstream.h Mon Feb 26 20:00:16 2024 + +++ b/src/http/ngx_http_upstream.h Mon Feb 26 20:00:18 2024 + @@ -82,11 +82,13 @@ ngx_http_upstream_srv_conf_t *us); typedef ngx_int_t (*ngx_http_upstream_init_peer_pt)(ngx_http_request_t *r, ngx_http_upstream_srv_conf_t *us); +typedef void (*ngx_http_upstream_free_pt)(ngx_http_upstream_srv_conf_t *us); typedef struct { ngx_http_upstream_init_ptinit_upstream; ngx_http_upstream_init_peer_pt init; +ngx_http_upstream_free_ptfree_upstream; void*data; } ngx_http_upstream_peer_t; diff -r f8d9fb94eab2 -r 8edb4003177d src/http/ngx_http_upstream_round_robin.c --- a/src/http/ngx_http_upstream_round_robin.c Mon Feb 26 20:00:16 2024 + +++ b/src/http/ngx_http_upstream_round_robin.c Mon Feb 26 20:00:18 2024 + @@ -851,3 +851,34 @@ } #endif + + +void +ngx_http_upstream_free_round_robin(ngx_http_upstream_srv_conf_t *us) +{ +#if (NGX_HTTP_SSL) + +ngx_uint_t i; +ngx_http_upstream_rr_peer_t *peer; +ngx_http_upstream_rr_peers_t *peers; + +peers = us->peer.data; + +#if (NGX_HTTP_UPSTREAM_ZONE) +if (peers->shpool) { +return; +} +#endif + +for (peer = peers->peer, i = 0; peer; peer = peer->next, i++) { + +if (peer->ssl_session) { +ngx_log_debug1(NGX_LOG_DEBUG_HTTP, ngx_cycle->log, 0, + "free session: %p", peer->ssl_session); +ngx_ssl_free_session(peer->ssl_session);
[PATCH] Correctly initialize ngx_str_t
# HG changeset patch # User Piotr Sikora # Date 1708977619 0 # Mon Feb 26 20:00:19 2024 + # Branch patch004 # Node ID 52936793ac076072c3544aa4e27f973d2f8fecda # Parent 8edb4003177dac56301aed7f86f8d2a564b47552 Correctly initialize ngx_str_t. Previously, only the "len" field was set, which resulted in an uninitialized "data" field accessed elsewhere in the code. Note that "r->uri" is initialized to an empty string to avoid changing the existing value for "$uri" in case of invalid URI. Found with MemorySanitizer. Signed-off-by: Piotr Sikora diff -r 8edb4003177d -r 52936793ac07 src/event/ngx_event_openssl.c --- a/src/event/ngx_event_openssl.c Mon Feb 26 20:00:18 2024 + +++ b/src/event/ngx_event_openssl.c Mon Feb 26 20:00:19 2024 + @@ -5064,7 +5064,7 @@ n = SSL_get0_raw_cipherlist(c->ssl->connection, &ciphers); if (n <= 0) { -s->len = 0; +ngx_str_null(s); return NGX_OK; } @@ -5116,7 +5116,7 @@ if (SSL_get_shared_ciphers(c->ssl->connection, (char *) buf, 4096) == NULL) { -s->len = 0; +ngx_str_null(s); return NGX_OK; } @@ -5165,7 +5165,7 @@ #endif -s->len = 0; +ngx_str_null(s); return NGX_OK; } @@ -5182,7 +5182,7 @@ n = SSL_get1_curves(c->ssl->connection, NULL); if (n <= 0) { -s->len = 0; +ngx_str_null(s); return NGX_OK; } @@ -5233,7 +5233,7 @@ #else -s->len = 0; +ngx_str_null(s); #endif @@ -5250,7 +5250,7 @@ sess = SSL_get0_session(c->ssl->connection); if (sess == NULL) { -s->len = 0; +ngx_str_null(s); return NGX_OK; } @@ -5285,7 +5285,7 @@ ngx_int_t ngx_ssl_get_early_data(ngx_connection_t *c, ngx_pool_t *pool, ngx_str_t *s) { -s->len = 0; +ngx_str_null(s); #ifdef SSL_ERROR_EARLY_DATA_REJECTED @@ -5335,7 +5335,7 @@ #endif -s->len = 0; +ngx_str_null(s); return NGX_OK; } @@ -5365,7 +5365,7 @@ #endif -s->len = 0; +ngx_str_null(s); return NGX_OK; } @@ -5377,10 +5377,9 @@ BIO *bio; X509*cert; -s->len = 0; - cert = SSL_get_peer_certificate(c->ssl->connection); if (cert == NULL) { +ngx_str_null(s); return NGX_OK; } @@ -5433,7 +5432,7 @@ } if (cert.len == 0) { -s->len = 0; +ngx_str_null(s); return NGX_OK; } @@ -5476,7 +5475,7 @@ } if (cert.len == 0) { -s->len = 0; +ngx_str_null(s); return NGX_OK; } @@ -5501,10 +5500,9 @@ X509 *cert; X509_NAME *name; -s->len = 0; - cert = SSL_get_peer_certificate(c->ssl->connection); if (cert == NULL) { +ngx_str_null(s); return NGX_OK; } @@ -,10 +5553,9 @@ X509 *cert; X509_NAME *name; -s->len = 0; - cert = SSL_get_peer_certificate(c->ssl->connection); if (cert == NULL) { +ngx_str_null(s); return NGX_OK; } @@ -5611,10 +5608,9 @@ X509 *cert; X509_NAME *name; -s->len = 0; - cert = SSL_get_peer_certificate(c->ssl->connection); if (cert == NULL) { +ngx_str_null(s); return NGX_OK; } @@ -5659,10 +5655,9 @@ X509 *cert; X509_NAME *name; -s->len = 0; - cert = SSL_get_peer_certificate(c->ssl->connection); if (cert == NULL) { +ngx_str_null(s); return NGX_OK; } @@ -5705,10 +5700,9 @@ X509*cert; BIO *bio; -s->len = 0; - cert = SSL_get_peer_certificate(c->ssl->connection); if (cert == NULL) { +ngx_str_null(s); return NGX_OK; } @@ -5745,10 +5739,9 @@ unsigned int len; u_char buf[EVP_MAX_MD_SIZE]; -s->len = 0; - cert = SSL_get_peer_certificate(c->ssl->connection); if (cert == NULL) { +ngx_str_null(s); return NGX_OK; } @@ -5818,10 +5811,9 @@ X509*cert; size_t len; -s->len = 0; - cert = SSL_get_peer_certificate(c->ssl->connection); if (cert == NULL) { +ngx_str_null(s); return NGX_OK; } @@ -5863,10 +5855,9 @@ X509*cert; size_t len; -s->len = 0; - cert = SSL_get_peer_certificate(c->ssl->connection); if (cert == NULL) { +ngx_str_null(s); return NGX_OK; } @@ -5907,10 +5898,9 @@ X509*cert; time_t now, end; -s->len = 0; - cert = SSL_get_peer_certificate(c->ssl->connection); if (cert == NULL) { +ngx_str_null(s); return NGX_OK; } diff -r 8edb4003177d -r 52936793ac07 src/event/quic/ngx_event_quic_streams.c --- a/src/event/quic/ngx_event_quic_streams.c Mon Feb 26 20:00:18 2024 + +++ b/src/event/quic/ngx_event_quic_streams.c Mon Feb 26 20:00:19 2024 + @@ -719,8 +719,7 @@ addr_text.len = c->addr_text.len; } else { -addr_text
[PATCH] Geo: fix uninitialized memory access
# HG changeset patch # User Piotr Sikora # Date 1708977621 0 # Mon Feb 26 20:00:21 2024 + # Branch patch005 # Node ID fe6f8a72d42970df176ea53f4f0aea16947ba5b8 # Parent 52936793ac076072c3544aa4e27f973d2f8fecda Geo: fix uninitialized memory access. Found with MemorySanitizer. Signed-off-by: Piotr Sikora diff -r 52936793ac07 -r fe6f8a72d429 src/http/modules/ngx_http_geo_module.c --- a/src/http/modules/ngx_http_geo_module.cMon Feb 26 20:00:19 2024 + +++ b/src/http/modules/ngx_http_geo_module.cMon Feb 26 20:00:21 2024 + @@ -1259,7 +1259,7 @@ return gvvn->value; } -val = ngx_palloc(ctx->pool, sizeof(ngx_http_variable_value_t)); +val = ngx_pcalloc(ctx->pool, sizeof(ngx_http_variable_value_t)); if (val == NULL) { return NULL; } diff -r 52936793ac07 -r fe6f8a72d429 src/stream/ngx_stream_geo_module.c --- a/src/stream/ngx_stream_geo_module.cMon Feb 26 20:00:19 2024 + +++ b/src/stream/ngx_stream_geo_module.cMon Feb 26 20:00:21 2024 + @@ -1209,7 +1209,7 @@ return gvvn->value; } -val = ngx_palloc(ctx->pool, sizeof(ngx_stream_variable_value_t)); +val = ngx_pcalloc(ctx->pool, sizeof(ngx_stream_variable_value_t)); if (val == NULL) { return NULL; } ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] Core: fix conversion of IPv4-mapped IPv6 addresses
# HG changeset patch # User Piotr Sikora # Date 1708977626 0 # Mon Feb 26 20:00:26 2024 + # Branch patch007 # Node ID 5584232259d28489efba149f2f5ae730691ff0d4 # Parent 03e5549976765912818120e11f6b08410a2af6a9 Core: fix conversion of IPv4-mapped IPv6 addresses. Found with UndefinedBehaviorSanitizer (shift). Signed-off-by: Piotr Sikora diff -r 03e554997676 -r 5584232259d2 src/core/ngx_inet.c --- a/src/core/ngx_inet.c Mon Feb 26 20:00:23 2024 + +++ b/src/core/ngx_inet.c Mon Feb 26 20:00:26 2024 + @@ -507,10 +507,10 @@ p = inaddr6->s6_addr; -inaddr = p[12] << 24; -inaddr += p[13] << 16; -inaddr += p[14] << 8; -inaddr += p[15]; +inaddr = (in_addr_t) p[12] << 24; +inaddr += (in_addr_t) p[13] << 16; +inaddr += (in_addr_t) p[14] << 8; +inaddr += (in_addr_t) p[15]; inaddr = htonl(inaddr); } diff -r 03e554997676 -r 5584232259d2 src/http/modules/ngx_http_access_module.c --- a/src/http/modules/ngx_http_access_module.c Mon Feb 26 20:00:23 2024 + +++ b/src/http/modules/ngx_http_access_module.c Mon Feb 26 20:00:26 2024 + @@ -148,10 +148,10 @@ p = sin6->sin6_addr.s6_addr; if (alcf->rules && IN6_IS_ADDR_V4MAPPED(&sin6->sin6_addr)) { -addr = p[12] << 24; -addr += p[13] << 16; -addr += p[14] << 8; -addr += p[15]; +addr = (in_addr_t) p[12] << 24; +addr += (in_addr_t) p[13] << 16; +addr += (in_addr_t) p[14] << 8; +addr += (in_addr_t) p[15]; return ngx_http_access_inet(r, alcf, htonl(addr)); } diff -r 03e554997676 -r 5584232259d2 src/http/modules/ngx_http_geo_module.c --- a/src/http/modules/ngx_http_geo_module.cMon Feb 26 20:00:23 2024 + +++ b/src/http/modules/ngx_http_geo_module.cMon Feb 26 20:00:26 2024 + @@ -199,10 +199,10 @@ p = inaddr6->s6_addr; if (IN6_IS_ADDR_V4MAPPED(inaddr6)) { -inaddr = p[12] << 24; -inaddr += p[13] << 16; -inaddr += p[14] << 8; -inaddr += p[15]; +inaddr = (in_addr_t) p[12] << 24; +inaddr += (in_addr_t) p[13] << 16; +inaddr += (in_addr_t) p[14] << 8; +inaddr += (in_addr_t) p[15]; vv = (ngx_http_variable_value_t *) ngx_radix32tree_find(ctx->u.trees.tree, inaddr); @@ -272,10 +272,10 @@ if (IN6_IS_ADDR_V4MAPPED(inaddr6)) { p = inaddr6->s6_addr; -inaddr = p[12] << 24; -inaddr += p[13] << 16; -inaddr += p[14] << 8; -inaddr += p[15]; +inaddr = (in_addr_t) p[12] << 24; +inaddr += (in_addr_t) p[13] << 16; +inaddr += (in_addr_t) p[14] << 8; +inaddr += (in_addr_t) p[15]; } else { inaddr = INADDR_NONE; diff -r 03e554997676 -r 5584232259d2 src/http/modules/ngx_http_geoip_module.c --- a/src/http/modules/ngx_http_geoip_module.c Mon Feb 26 20:00:23 2024 + +++ b/src/http/modules/ngx_http_geoip_module.c Mon Feb 26 20:00:26 2024 + @@ -266,10 +266,10 @@ if (IN6_IS_ADDR_V4MAPPED(inaddr6)) { p = inaddr6->s6_addr; -inaddr = p[12] << 24; -inaddr += p[13] << 16; -inaddr += p[14] << 8; -inaddr += p[15]; +inaddr = (in_addr_t) p[12] << 24; +inaddr += (in_addr_t) p[13] << 16; +inaddr += (in_addr_t) p[14] << 8; +inaddr += (in_addr_t) p[15]; return inaddr; } diff -r 03e554997676 -r 5584232259d2 src/stream/ngx_stream_access_module.c --- a/src/stream/ngx_stream_access_module.c Mon Feb 26 20:00:23 2024 + +++ b/src/stream/ngx_stream_access_module.c Mon Feb 26 20:00:26 2024 + @@ -144,10 +144,10 @@ p = sin6->sin6_addr.s6_addr; if (ascf->rules && IN6_IS_ADDR_V4MAPPED(&sin6->sin6_addr)) { -addr = p[12] << 24; -addr += p[13] << 16; -addr += p[14] << 8; -addr += p[15]; +addr = (in_addr_t) p[12] << 24; +addr += (in_addr_t) p[13] << 16; +addr += (in_addr_t) p[14] << 8; +addr += (in_addr_t) p[15]; return ngx_stream_access_inet(s, ascf, htonl(addr)); } diff -r 03e554997676 -r 5584232259d2 src/stream/ngx_stream_geo_module.c --- a/src/stream/ngx_stream_geo_module.cMon Feb 26 20:00:23 2024 + +++ b/src/stream/ngx_stream_geo_module.cMon Feb 26 20:00:26 2024 + @@ -190,10 +190,10 @@ p = inaddr6->s6_addr; if (IN6_IS_ADDR_V4MAPPED(inaddr6)) { -inaddr = p[12] << 24; -inaddr += p[13] << 16; -inaddr += p[14] << 8; -inaddr += p[15]; +inaddr = (in_addr_t) p[12] << 24; +in
[PATCH] Rewrite: fix "return" directive without response text
# HG changeset patch # User Piotr Sikora # Date 1708977628 0 # Mon Feb 26 20:00:28 2024 + # Branch patch008 # Node ID 3cde11b747c08c69889edc014a700317fe4d1d88 # Parent 5584232259d28489efba149f2f5ae730691ff0d4 Rewrite: fix "return" directive without response text. Previously, the response text wasn't initialized and the rewrite module was sending response body set to NULL. Found with UndefinedBehaviorSanitizer (pointer-overflow). Signed-off-by: Piotr Sikora diff -r 5584232259d2 -r 3cde11b747c0 src/http/modules/ngx_http_rewrite_module.c --- a/src/http/modules/ngx_http_rewrite_module.cMon Feb 26 20:00:26 2024 + +++ b/src/http/modules/ngx_http_rewrite_module.cMon Feb 26 20:00:28 2024 + @@ -489,6 +489,7 @@ } if (cf->args->nelts == 2) { +ngx_str_set(&ret->text.value, ""); return NGX_CONF_OK; } ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH 2 of 2] SSL: add $ssl_curve when using AWS-LC
# HG changeset patch # User Piotr Sikora # Date 1708977632 0 # Mon Feb 26 20:00:32 2024 + # Branch patch009 # Node ID dfffc67d286b788204f60701ef4179566d933a1b # Parent 5e923992006199748e79b08b1e65c4ef41f07495 SSL: add $ssl_curve when using AWS-LC. Signed-off-by: Piotr Sikora diff -r 5e9239920061 -r dfffc67d286b src/event/ngx_event_openssl.c --- a/src/event/ngx_event_openssl.c Mon Feb 26 20:00:30 2024 + +++ b/src/event/ngx_event_openssl.c Mon Feb 26 20:00:32 2024 + @@ -5163,6 +5163,72 @@ return NGX_OK; } +#elif defined(OPENSSL_IS_AWSLC) + +uint16_t curve_id; + +curve_id = SSL_get_curve_id(c->ssl->connection); + +/* + * Hardcoded table with ANSI / SECG curve names (e.g. "prime256v1"), + * which is the same format that OpenSSL returns for $ssl_curve. + * + * Without this table, we'd need to make 3 additional library calls + * to convert from curve_id to ANSI / SECG curve name: + * + * nist_name = SSL_get_curve_name(curve_id); + * nid = EC_curve_nist2nid(nist_name); + * ansi_name = OBJ_nid2sn(nid); + */ + +switch (curve_id) { + +#ifdef SSL_CURVE_SECP224R1 +case SSL_CURVE_SECP224R1: +ngx_str_set(s, "secp224r1"); +return NGX_OK; +#endif + +#ifdef SSL_CURVE_SECP256R1 +case SSL_CURVE_SECP256R1: +ngx_str_set(s, "prime256v1"); +return NGX_OK; +#endif + +#ifdef SSL_CURVE_SECP384R1 +case SSL_CURVE_SECP384R1: +ngx_str_set(s, "secp384r1"); +return NGX_OK; +#endif + +#ifdef SSL_CURVE_SECP521R1 +case SSL_CURVE_SECP521R1: +ngx_str_set(s, "secp521r1"); +return NGX_OK; +#endif + +#ifdef SSL_CURVE_X25519 +case SSL_CURVE_X25519: +ngx_str_set(s, "x25519"); +return NGX_OK; +#endif + +case 0: +break; + +default: +s->len = sizeof("0x") - 1; + +s->data = ngx_pnalloc(pool, s->len); +if (s->data == NULL) { +return NGX_ERROR; +} + +ngx_sprintf(s->data, "0x%04xd", curve_id); + +return NGX_OK; +} + #endif ngx_str_null(s); ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH 1 of 2] SSL: add support for AWS-LC
# HG changeset patch # User Piotr Sikora # Date 1708977630 0 # Mon Feb 26 20:00:30 2024 + # Branch patch009 # Node ID 5e923992006199748e79b08b1e65c4ef41f07495 # Parent 3cde11b747c08c69889edc014a700317fe4d1d88 SSL: add support for AWS-LC. AWS-LC is a fork of BoringSSL with some performance improvements, useful features (OCSP and multiple certificates), and support for more platforms. Signed-off-by: Piotr Sikora diff -r 3cde11b747c0 -r 5e9239920061 src/event/ngx_event_openssl.h --- a/src/event/ngx_event_openssl.h Mon Feb 26 20:00:28 2024 + +++ b/src/event/ngx_event_openssl.h Mon Feb 26 20:00:30 2024 + @@ -25,7 +25,7 @@ #endif #include #if (NGX_QUIC) -#ifdef OPENSSL_IS_BORINGSSL +#if defined(OPENSSL_IS_BORINGSSL) || defined(OPENSSL_IS_AWSLC) #include #include #else diff -r 3cde11b747c0 -r 5e9239920061 src/event/quic/ngx_event_quic.c --- a/src/event/quic/ngx_event_quic.c Mon Feb 26 20:00:28 2024 + +++ b/src/event/quic/ngx_event_quic.c Mon Feb 26 20:00:30 2024 + @@ -962,7 +962,7 @@ return NGX_DECLINED; } -#if !defined (OPENSSL_IS_BORINGSSL) +#if !defined(OPENSSL_IS_BORINGSSL) && !defined(OPENSSL_IS_AWSLC) /* OpenSSL provides read keys for an application level before it's ready */ if (pkt->level == ssl_encryption_application && !c->ssl->handshaked) { diff -r 3cde11b747c0 -r 5e9239920061 src/event/quic/ngx_event_quic_protection.c --- a/src/event/quic/ngx_event_quic_protection.cMon Feb 26 20:00:28 2024 + +++ b/src/event/quic/ngx_event_quic_protection.cMon Feb 26 20:00:30 2024 + @@ -30,7 +30,7 @@ static ngx_int_t ngx_quic_crypto_open(ngx_quic_secret_t *s, ngx_str_t *out, u_char *nonce, ngx_str_t *in, ngx_str_t *ad, ngx_log_t *log); -#ifndef OPENSSL_IS_BORINGSSL +#if !defined(OPENSSL_IS_BORINGSSL) && !defined(OPENSSL_IS_AWSLC) static ngx_int_t ngx_quic_crypto_common(ngx_quic_secret_t *s, ngx_str_t *out, u_char *nonce, ngx_str_t *in, ngx_str_t *ad, ngx_log_t *log); #endif @@ -55,7 +55,7 @@ switch (id) { case TLS1_3_CK_AES_128_GCM_SHA256: -#ifdef OPENSSL_IS_BORINGSSL +#if defined(OPENSSL_IS_BORINGSSL) || defined(OPENSSL_IS_AWSLC) ciphers->c = EVP_aead_aes_128_gcm(); #else ciphers->c = EVP_aes_128_gcm(); @@ -66,7 +66,7 @@ break; case TLS1_3_CK_AES_256_GCM_SHA384: -#ifdef OPENSSL_IS_BORINGSSL +#if defined(OPENSSL_IS_BORINGSSL) || defined(OPENSSL_IS_AWSLC) ciphers->c = EVP_aead_aes_256_gcm(); #else ciphers->c = EVP_aes_256_gcm(); @@ -77,12 +77,12 @@ break; case TLS1_3_CK_CHACHA20_POLY1305_SHA256: -#ifdef OPENSSL_IS_BORINGSSL +#if defined(OPENSSL_IS_BORINGSSL) || defined(OPENSSL_IS_AWSLC) ciphers->c = EVP_aead_chacha20_poly1305(); #else ciphers->c = EVP_chacha20_poly1305(); #endif -#ifdef OPENSSL_IS_BORINGSSL +#if defined(OPENSSL_IS_BORINGSSL) || defined(OPENSSL_IS_AWSLC) ciphers->hp = (const EVP_CIPHER *) EVP_aead_chacha20_poly1305(); #else ciphers->hp = EVP_chacha20(); @@ -91,7 +91,7 @@ len = 32; break; -#ifndef OPENSSL_IS_BORINGSSL +#if !defined(OPENSSL_IS_BORINGSSL) && !defined(OPENSSL_IS_AWSLC) case TLS1_3_CK_AES_128_CCM_SHA256: ciphers->c = EVP_aes_128_ccm(); ciphers->hp = EVP_aes_128_ctr(); @@ -259,7 +259,7 @@ ngx_hkdf_expand(u_char *out_key, size_t out_len, const EVP_MD *digest, const uint8_t *prk, size_t prk_len, const u_char *info, size_t info_len) { -#ifdef OPENSSL_IS_BORINGSSL +#if defined(OPENSSL_IS_BORINGSSL) || defined(OPENSSL_IS_AWSLC) if (HKDF_expand(out_key, out_len, digest, prk, prk_len, info, info_len) == 0) @@ -321,7 +321,7 @@ const u_char *secret, size_t secret_len, const u_char *salt, size_t salt_len) { -#ifdef OPENSSL_IS_BORINGSSL +#if defined(OPENSSL_IS_BORINGSSL) || defined(OPENSSL_IS_AWSLC) if (HKDF_extract(out_key, out_len, digest, secret, secret_len, salt, salt_len) @@ -384,7 +384,7 @@ ngx_quic_md_t *key, ngx_int_t enc, ngx_log_t *log) { -#ifdef OPENSSL_IS_BORINGSSL +#if defined(OPENSSL_IS_BORINGSSL) || defined(OPENSSL_IS_AWSLC) EVP_AEAD_CTX *ctx; ctx = EVP_AEAD_CTX_new(cipher, key->data, key->len, @@ -444,7 +444,7 @@ ngx_quic_crypto_open(ngx_quic_secret_t *s, ngx_str_t *out, u_char *nonce, ngx_str_t *in, ngx_str_t *ad, ngx_log_t *log) { -#ifdef OPENSSL_IS_BORINGSSL +#if defined(OPENSSL_IS_BORINGSSL) || defined(OPENSSL_IS_AWSLC) if (EVP_AEAD_CTX_open(s->ctx, out->data, &out->len, out->len, nonce, s->iv.len, in->data, in->len, ad->data, ad->len) != 1) @@ -464,7 +464,7 @@ ngx_quic_crypto_seal(ngx_quic_secret_t *s, ngx_str_t *out, u_char *nonce, ngx_str_t *in, ngx_str_t *ad, ngx_log_t *log) { -#ifdef OPENSSL_IS_BORINGSSL +#if defined(OPENSSL_IS_BORINGSSL) || defined(OPENSSL_IS_AWSLC) if (EVP_AEAD_CTX_seal(s->ctx, out->data, &out->len, out->len, nonce,
[PATCH] Core: fix build without libcrypt
# HG changeset patch # User Piotr Sikora # Date 1708977637 0 # Mon Feb 26 20:00:37 2024 + # Branch patch013 # Node ID cdc173477ea99fd6c952a85e5cd11db66452076a # Parent 04e3155b3b9651fee708898aaf82ac35532806ee Core: fix build without libcrypt. libcrypt is no longer part of glibc, so it might not be available. Signed-off-by: Piotr Sikora diff -r 04e3155b3b96 -r cdc173477ea9 auto/unix --- a/auto/unix Mon Feb 26 20:00:35 2024 + +++ b/auto/unix Mon Feb 26 20:00:37 2024 + @@ -150,7 +150,7 @@ ngx_feature="crypt()" -ngx_feature_name= +ngx_feature_name="NGX_HAVE_CRYPT" ngx_feature_run=no ngx_feature_incs= ngx_feature_path= @@ -162,7 +162,7 @@ if [ $ngx_found = no ]; then ngx_feature="crypt() in libcrypt" -ngx_feature_name= +ngx_feature_name="NGX_HAVE_CRYPT" ngx_feature_run=no ngx_feature_incs= ngx_feature_path= diff -r 04e3155b3b96 -r cdc173477ea9 src/os/unix/ngx_linux_config.h --- a/src/os/unix/ngx_linux_config.hMon Feb 26 20:00:35 2024 + +++ b/src/os/unix/ngx_linux_config.hMon Feb 26 20:00:37 2024 + @@ -52,7 +52,6 @@ #include /* memalign() */ #include /* IOV_MAX */ #include -#include #include /* uname() */ #include @@ -61,6 +60,11 @@ #include +#if (NGX_HAVE_CRYPT_H) +#include +#endif + + #if (NGX_HAVE_POSIX_SEM) #include #endif diff -r 04e3155b3b96 -r cdc173477ea9 src/os/unix/ngx_user.c --- a/src/os/unix/ngx_user.cMon Feb 26 20:00:35 2024 + +++ b/src/os/unix/ngx_user.cMon Feb 26 20:00:37 2024 + @@ -44,7 +44,7 @@ return NGX_ERROR; } -#else +#elif (NGX_HAVE_CRYPT) ngx_int_t ngx_libc_crypt(ngx_pool_t *pool, u_char *key, u_char *salt, u_char **encrypted) @@ -76,6 +76,14 @@ return NGX_ERROR; } +#else + +ngx_int_t +ngx_libc_crypt(ngx_pool_t *pool, u_char *key, u_char *salt, u_char **encrypted) +{ +return NGX_ERROR; +} + #endif #endif /* NGX_CRYPT */ ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] Configure: link libcrypt when a feature using it is detected
# HG changeset patch # User Piotr Sikora # Date 1708977638 0 # Mon Feb 26 20:00:38 2024 + # Branch patch014 # Node ID 570e97dddeeddb79c71587aa8a10150b64404beb # Parent cdc173477ea99fd6c952a85e5cd11db66452076a Configure: link libcrypt when a feature using it is detected. Previously, this worked only because libcrypt was added in a separate test for crypt() in auto/unix. Signed-off-by: Piotr Sikora diff -r cdc173477ea9 -r 570e97dddeed auto/os/linux --- a/auto/os/linux Mon Feb 26 20:00:37 2024 + +++ b/auto/os/linux Mon Feb 26 20:00:38 2024 + @@ -228,6 +228,10 @@ crypt_r(\"key\", \"salt\", &cd);" . auto/feature +if [ $ngx_found = yes ]; then +CRYPT_LIB="-lcrypt" +fi + ngx_include="sys/vfs.h"; . auto/include ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] Configure: set cache line sizes for more architectures
# HG changeset patch # User Piotr Sikora # Date 1708977642 0 # Mon Feb 26 20:00:42 2024 + # Branch patch016 # Node ID bb99cbe3a343ae581d2369b990aee66e69679ca2 # Parent f58bc1041ebca635517b919d58b49923bf24f76d Configure: set cache line sizes for more architectures. Signed-off-by: Piotr Sikora diff -r f58bc1041ebc -r bb99cbe3a343 auto/os/conf --- a/auto/os/conf Mon Feb 26 20:00:40 2024 + +++ b/auto/os/conf Mon Feb 26 20:00:42 2024 + @@ -115,6 +115,21 @@ NGX_MACH_CACHE_LINE=64 ;; +ppc64 | ppc64le) +have=NGX_ALIGNMENT value=16 . auto/define +NGX_MACH_CACHE_LINE=128 +;; + +riscv64) +have=NGX_ALIGNMENT value=16 . auto/define +NGX_MACH_CACHE_LINE=64 +;; + +s390x) +have=NGX_ALIGNMENT value=16 . auto/define +NGX_MACH_CACHE_LINE=256 +;; + *) have=NGX_ALIGNMENT value=16 . auto/define NGX_MACH_CACHE_LINE=32 ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] macOS: detect cache line size at runtime
# HG changeset patch # User Piotr Sikora # Date 1708977640 0 # Mon Feb 26 20:00:40 2024 + # Branch patch015 # Node ID f58bc1041ebca635517b919d58b49923bf24f76d # Parent 570e97dddeeddb79c71587aa8a10150b64404beb macOS: detect cache line size at runtime. Notably, Apple Silicon CPUs have 128 byte cache line size, which is twice the default configured for generic aarch64. Signed-off-by: Piotr Sikora diff -r 570e97dddeed -r f58bc1041ebc src/os/unix/ngx_darwin_init.c --- a/src/os/unix/ngx_darwin_init.c Mon Feb 26 20:00:38 2024 + +++ b/src/os/unix/ngx_darwin_init.c Mon Feb 26 20:00:40 2024 + @@ -11,6 +11,7 @@ charngx_darwin_kern_ostype[16]; charngx_darwin_kern_osrelease[128]; +int64_t ngx_darwin_hw_cachelinesize; int ngx_darwin_hw_ncpu; int ngx_darwin_kern_ipc_somaxconn; u_long ngx_darwin_net_inet_tcp_sendspace; @@ -44,6 +45,10 @@ sysctl_t sysctls[] = { +{ "hw.cachelinesize", + &ngx_darwin_hw_cachelinesize, + sizeof(ngx_darwin_hw_cachelinesize), 0 }, + { "hw.ncpu", &ngx_darwin_hw_ncpu, sizeof(ngx_darwin_hw_ncpu), 0 }, @@ -155,6 +160,7 @@ return NGX_ERROR; } +ngx_cacheline_size = ngx_darwin_hw_cachelinesize; ngx_ncpu = ngx_darwin_hw_ncpu; if (ngx_darwin_kern_ipc_somaxconn > 32767) { diff -r 570e97dddeed -r f58bc1041ebc src/os/unix/ngx_posix_init.c --- a/src/os/unix/ngx_posix_init.c Mon Feb 26 20:00:38 2024 + +++ b/src/os/unix/ngx_posix_init.c Mon Feb 26 20:00:40 2024 + @@ -51,7 +51,10 @@ } ngx_pagesize = getpagesize(); -ngx_cacheline_size = NGX_CPU_CACHE_LINE; + +if (ngx_cacheline_size == 0) { +ngx_cacheline_size = NGX_CPU_CACHE_LINE; +} for (n = ngx_pagesize; n >>= 1; ngx_pagesize_shift++) { /* void */ } ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] Configure: add support for Homebrew on Apple Silicon
# HG changeset patch # User Piotr Sikora # Date 1708977643 0 # Mon Feb 26 20:00:43 2024 + # Branch patch017 # Node ID dd95daa55cf6131a7e845edd6ad3b429bcef6f98 # Parent bb99cbe3a343ae581d2369b990aee66e69679ca2 Configure: add support for Homebrew on Apple Silicon. Signed-off-by: Piotr Sikora diff -r bb99cbe3a343 -r dd95daa55cf6 auto/lib/geoip/conf --- a/auto/lib/geoip/conf Mon Feb 26 20:00:42 2024 + +++ b/auto/lib/geoip/conf Mon Feb 26 20:00:43 2024 + @@ -64,6 +64,23 @@ fi +if [ $ngx_found = no ]; then + +# Homebrew on Apple Silicon + +ngx_feature="GeoIP library in /opt/homebrew/" +ngx_feature_path="/opt/homebrew/include" + +if [ $NGX_RPATH = YES ]; then +ngx_feature_libs="-R/opt/homebrew/lib -L/opt/homebrew/lib -lGeoIP" +else +ngx_feature_libs="-L/opt/homebrew/lib -lGeoIP" +fi + +. auto/feature +fi + + if [ $ngx_found = yes ]; then CORE_INCS="$CORE_INCS $ngx_feature_path" diff -r bb99cbe3a343 -r dd95daa55cf6 auto/lib/google-perftools/conf --- a/auto/lib/google-perftools/confMon Feb 26 20:00:42 2024 + +++ b/auto/lib/google-perftools/confMon Feb 26 20:00:43 2024 + @@ -46,6 +46,22 @@ fi +if [ $ngx_found = no ]; then + +# Homebrew on Apple Silicon + +ngx_feature="Google perftools in /opt/homebrew/" + +if [ $NGX_RPATH = YES ]; then +ngx_feature_libs="-R/opt/homebrew/lib -L/opt/homebrew/lib -lprofiler" +else +ngx_feature_libs="-L/opt/homebrew/lib -lprofiler" +fi + +. auto/feature +fi + + if [ $ngx_found = yes ]; then CORE_LIBS="$CORE_LIBS $ngx_feature_libs" diff -r bb99cbe3a343 -r dd95daa55cf6 auto/lib/libgd/conf --- a/auto/lib/libgd/conf Mon Feb 26 20:00:42 2024 + +++ b/auto/lib/libgd/conf Mon Feb 26 20:00:43 2024 + @@ -65,6 +65,23 @@ fi +if [ $ngx_found = no ]; then + +# Homebrew on Apple Silicon + +ngx_feature="GD library in /opt/homebrew/" +ngx_feature_path="/opt/homebrew/include" + +if [ $NGX_RPATH = YES ]; then +ngx_feature_libs="-R/opt/homebrew/lib -L/opt/homebrew/lib -lgd" +else +ngx_feature_libs="-L/opt/homebrew/lib -lgd" +fi + +. auto/feature +fi + + if [ $ngx_found = yes ]; then CORE_INCS="$CORE_INCS $ngx_feature_path" diff -r bb99cbe3a343 -r dd95daa55cf6 auto/lib/openssl/conf --- a/auto/lib/openssl/conf Mon Feb 26 20:00:42 2024 + +++ b/auto/lib/openssl/conf Mon Feb 26 20:00:43 2024 + @@ -122,6 +122,24 @@ . auto/feature fi +if [ $ngx_found = no ]; then + +# Homebrew on Apple Silicon + +ngx_feature="OpenSSL library in /opt/homebrew/" +ngx_feature_path="/opt/homebrew/include" + +if [ $NGX_RPATH = YES ]; then +ngx_feature_libs="-R/opt/homebrew/lib -L/opt/homebrew/lib -lssl -lcrypto" +else +ngx_feature_libs="-L/opt/homebrew/lib -lssl -lcrypto" +fi + +ngx_feature_libs="$ngx_feature_libs $NGX_LIBDL $NGX_LIBPTHREAD" + +. auto/feature +fi + if [ $ngx_found = yes ]; then have=NGX_SSL . auto/have CORE_INCS="$CORE_INCS $ngx_feature_path" diff -r bb99cbe3a343 -r dd95daa55cf6 auto/lib/pcre/conf --- a/auto/lib/pcre/confMon Feb 26 20:00:42 2024 + +++ b/auto/lib/pcre/confMon Feb 26 20:00:43 2024 + @@ -182,6 +182,22 @@ . auto/feature fi +if [ $ngx_found = no ]; then + +# Homebrew on Apple Silicon + +ngx_feature="PCRE library in /opt/homebrew/" +ngx_feature_path="/opt/homebrew/include" + +if [ $NGX_RPATH = YES ]; then +ngx_feature_libs="-R/opt/homebrew/lib -L/opt/homebrew/lib -lpcre" +else +ngx_feature_libs="-L/opt/homebrew/lib -lpcre" +fi + +. auto/feature +fi + if [ $ngx_found = yes ]; then CORE_INCS="$CORE_INCS $ngx_feature_path" CORE_LIBS="$CORE_LIBS $ngx_feature_libs" ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] Win32: include missing
# HG changeset patch # User Piotr Sikora # Date 1708977633 0 # Mon Feb 26 20:00:33 2024 + # Branch patch011 # Node ID 9b57470dc49f8d8d10abe30a5df628732d7618dc # Parent 480071fe7251829912a4f42301e8fc85da2d1905 Win32: include missing . Signed-off-by: Piotr Sikora diff -r 480071fe7251 -r 9b57470dc49f src/os/win32/ngx_win32_config.h --- a/src/os/win32/ngx_win32_config.h Mon Feb 26 20:00:32 2024 + +++ b/src/os/win32/ngx_win32_config.h Mon Feb 26 20:00:33 2024 + @@ -62,6 +62,7 @@ #include #endif #include +#include #include #ifdef __WATCOMC__ ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] Configure: allow cross-compiling to Windows using Clang
# HG changeset patch # User Piotr Sikora # Date 1708977648 0 # Mon Feb 26 20:00:48 2024 + # Branch patch019 # Node ID 77eab4d83413b053d9681611d243335a95ee5567 # Parent ea1ab31c166c52372b40429a1cccece9ec9e003b Configure: allow cross-compiling to Windows using Clang. Signed-off-by: Piotr Sikora diff -r ea1ab31c166c -r 77eab4d83413 auto/os/win32 --- a/auto/os/win32 Mon Feb 26 20:00:46 2024 + +++ b/auto/os/win32 Mon Feb 26 20:00:48 2024 + @@ -18,7 +18,7 @@ case "$NGX_CC_NAME" in -gcc) +clang | gcc) CORE_LIBS="$CORE_LIBS -ladvapi32 -lws2_32" MAIN_LINK="$MAIN_LINK -Wl,--export-all-symbols" MAIN_LINK="$MAIN_LINK -Wl,--out-implib=$NGX_OBJS/libnginx.a" ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] Win32: fix unique file index calculations
# HG changeset patch # User Piotr Sikora # Date 1708977635 0 # Mon Feb 26 20:00:35 2024 + # Branch patch012 # Node ID 04e3155b3b9651fee708898aaf82ac35532806ee # Parent 9b57470dc49f8d8d10abe30a5df628732d7618dc Win32: fix unique file index calculations. The old code was breaking strict aliasing rules. Signed-off-by: Piotr Sikora diff -r 9b57470dc49f -r 04e3155b3b96 src/os/win32/ngx_files.h --- a/src/os/win32/ngx_files.h Mon Feb 26 20:00:33 2024 + +++ b/src/os/win32/ngx_files.h Mon Feb 26 20:00:35 2024 + @@ -154,7 +154,8 @@ (((off_t) (fi)->nFileSizeHigh << 32) | (fi)->nFileSizeLow) #define ngx_file_fs_size(fi)ngx_file_size(fi) -#define ngx_file_uniq(fi) (*(ngx_file_uniq_t *) &(fi)->nFileIndexHigh) +#define ngx_file_uniq(fi)\ +(((ngx_file_uniq_t) (fi)->nFileIndexHigh << 32) | (fi)->nFileIndexLow) /* 1164447360 is commented in src/os/win32/ngx_time.c */ ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] Configure: fix "make install" when cross-compiling to Windows
# HG changeset patch # User Piotr Sikora # Date 1708977646 0 # Mon Feb 26 20:00:46 2024 + # Branch patch018 # Node ID ea1ab31c166c52372b40429a1cccece9ec9e003b # Parent dd95daa55cf6131a7e845edd6ad3b429bcef6f98 Configure: fix "make install" when cross-compiling to Windows. Signed-off-by: Piotr Sikora diff -r dd95daa55cf6 -r ea1ab31c166c auto/install --- a/auto/install Mon Feb 26 20:00:43 2024 + +++ b/auto/install Mon Feb 26 20:00:46 2024 + @@ -112,7 +112,7 @@ test ! -f '\$(DESTDIR)$NGX_SBIN_PATH' \\ || mv '\$(DESTDIR)$NGX_SBIN_PATH' \\ '\$(DESTDIR)$NGX_SBIN_PATH.old' - cp $NGX_OBJS/nginx '\$(DESTDIR)$NGX_SBIN_PATH' + cp $NGX_OBJS/nginx$ngx_binext '\$(DESTDIR)$NGX_SBIN_PATH' test -d '\$(DESTDIR)$NGX_CONF_PREFIX' \\ || mkdir -p '\$(DESTDIR)$NGX_CONF_PREFIX' ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] HTTP: stop emitting server version by default
Hi Sergey, > I don't think this is a good idea to change the default behaviour > for the directive we have for a long-long time. But it's arguably a wrong behavior, and keeping it forever wrong because of a decision made ~20 years ago, doesn't seem like a particularly great idea. Also, while I'm usually all for retaining backward-compatibility, I cannot imagine this breaking anything. > It's always possible > to set `server_tokens off;' in the configuration file. Right, but if you require majority of users to change the defaults, then those defaults are not very good. > Also, this change is required a corresponding change in the > documentation on the nginx.org website. I'm happy to submit the corresponding change if the patch is accepted. Best regards, Piotr Sikora ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] Configure: add support for Homebrew on Apple Silicon
Hi Sergey, > An obvious question is why do you need this change. Homebrew seems > to be quite niche to pay attention. Homebrew [1] is orders of magnitude more popular than MacPorts [2], which is already supported by the configure script. > Using appropriate paths in > --with-cc-opt / --with-ld-opt should work (not tested). Everything under auto/lib can be replaced with --with-{cc,ld}-opt, so I don't really understand this reasoning. > A quick grep for MacPorts search paths suggests that some libraries > are missing in the change. If this is on purpose, please reflect > this in the description. libxml2, libxslt, and libexslt are all installed as part of Xcode, which is required to use Homebrew and compile anything on macOS. I'll ship update patch in a moment. > Apple Silicon is something from the marketing language, > using Apple ARM instead should be fine. > > Notably, Homebrew uses Hardware::CPU.arm Ruby language boolean > to make the distinction. There is no such thing as "Apple ARM". The official documentation uses the term "Apple silicon" [3], Homebrew refers to the supported platform as "Apple Silicon" [4], and Wikipedia has an article about "Apple silicon" [5]. > Further, given the smooth decay on Intel-based hardware, > I'd reduce this just to "Homebrew". But that would be misleading, seeing that the new code path doesn't do anything for Homebrew on Intel. And then, there is Homebrew on Linux [6]. [1] https://formulae.brew.sh/analytics/install-on-request/30d/ [2] https://ports.macports.org/statistics/ports/ [3] https://support.apple.com/en-us/116943 [4] https://formulae.brew.sh/formula/nginx#default [5] https://en.wikipedia.org/wiki/Apple_silicon [6] https://docs.brew.sh/Homebrew-on-Linux Best regards, Piotr Sikora ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] Core: free connections and read/write events at shutdown
Hi Sergey, > Since this is not a real memory leak (the allocations made in the > init_process method of the ngx_event_core_module module are used up > to this function and already freed on exit automatically, as this > function never returns), I don't think there is something to fix. Agreed, this is not a "real" leak. But fixing those "false" leaks allows you to run test with sanitizers on the CI, which can protect you and/or external contributors from introducing new bugs. > (Further, the patch misses cycle->files for NGX_USE_FD_EVENT event > methods. Good catch, thanks! > Also, it's probably better to free the memory in the > exit_process method to obey the ownership (this would also fix > missing ngx_free() calls on win32), but this would require moving > code that uses these connections afterwards to catch socket leaks.) Freeing memory in exit_process could result in use-after-free, since cleanups for the cycle->pool might still access those connections. Best regards, Piotr Sikora ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] Core: free connections and read/write events at shutdown
Hi Sergey, > While I agree that false positives do not allow to run LeakSanitizer > in a clean fashion, I don't think it is nginx which should be fixed. > Rather, sanitizer analysis could be improved instead to prevent FPs. Patches welcome? > Meanwhile, leak sanitizer can be used with suppressions as appropriate > to run tests cleanly. For example, this allows to suppress memory leak > reports for allocations made during worker process init, such as cycle > connections and read/write events: > > $ cat suppr.txt > leak:ngx_worker_process_init > $ LSAN_OPTIONS=suppressions=suppr.txt prove -r Right, but that's an unnecessary step that prevents potential contributors from using LeakSanitizer with NGINX. Also, I don't think that you're using those tools either, likely because of those few false positives. Best regards, Piotr Sikora ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] Geo: fix uninitialized memory access
Hi Roman, > Thanks for the patch, looks valid, except we no longer need to explicitly > initialize fields to zero. Right, I was going back-and-forth between which version I should send. > Also, I think we need more details about the > uninitialized memory access. See updated patch. LGTM, thanks! Best regards, Piotr Sikora ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] Core: fix conversion of IPv4-mapped IPv6 addresses
Hi Sergey, > The "shift" remark doesn't describe a problem in details. It's not a remark, it's the name of the UndefinedBehaviorSanitizer check that caught the issue [1]. > @@ -507,7 +507,7 @@ ngx_cidr_match(struct sockaddr *sa, ngx_ > > p = inaddr6->s6_addr; > > -inaddr = p[12] << 24; > +inaddr = (in_addr_t) p[12] << 24; > inaddr += p[13] << 16; > inaddr += p[14] << 8; > inaddr += p[15]; While this minimizes the diff and silences the error at hand, I find my version more readable. But you're obviously welcome to commit either version. [1] https://clang.llvm.org/docs/UndefinedBehaviorSanitizer.html Best regards, Piotr Sikora ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH 1 of 2] SSL: add support for AWS-LC
Hi Roman, > It looks like this library is not super popular, but the patch is relatively > large. Perhaps it's not as widely used as the forks that started ~10 years ago, but it's basically a version of BoringSSL that's more suitable to use with NGINX than BoringSSL itself: - it ships releases and it's versioned, - it supports OCSP stapling, - it supports multiple TLS certificates, - it supports big endian platforms supported by NGINX. Also, the patch is pretty small. > Also, compiling nginx with -DOPENSSL_IS_BORINGSSL should probably solve > the issue. For the time being, probably, but AWS folks are actively developing it, so I'd expect it to led to issues sooner rather than later. Best regards, Piotr Sikora ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] Core: fix build without libcrypt
Hi Roman, > Can you provide an example of a system where this fix is needed? 1. When linking against musl-libc on a system where libcrypt is linked against glibc and thus not linkable. 2. When cross-compiling, for similar reasons. Best regards, Piotr Sikora ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] macOS: detect cache line size at runtime
Hi Sergey, > I prefer not to introduce more ad-hoc prefixes in the log summary. > Something like moving the "macOS" part to the end should be fine. That's fine with me. > style: this breaks a perfect indentation of two spaces after type; > further, it appears to be unsorted by type; I'd put it after u_long Good catch, thanks! > This makes the following slight update to the patch. > If you're okey with it, I will commit it then. LGTM. Best regards, Piotr Sikora ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] Configure: link libcrypt when a feature using it is detected
Hi Sergey, > I'd rewrote the description to be more specific: > > : Configure: fixed Linux crypt_r() test to add libcrypt. > : > : Previously, the resulting binary was successfully linked > : because libcrypt was added in a separate test for crypt(). That's fine with me. Best regards, Piotr Sikora ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH 2 of 2] SSL: add $ssl_curve when using AWS-LC
Hi Sergey, > Not sure nginx needs such complexity. > Instead, I'd expect the missing API to be added to the library, > especially that both parent and grandparent implement it. > > BoringSSL didn't have such API for quite some time until recently. Right, this patch predates addition of this API to BoringSSL, and I only removed BoringSSL's #ifdef when submitting it last month. Feel free to skip it. > Since AWS-LC is a fork of BoringSSL, it is welcome to sync up. > As far as I can see, they used to sync quite often, so it is ought > to be resolved automagically after the next sync. To ease to task, > here I provide certain git hashes: > > 6cf98208371e5c2c8b9d34ce3b8c452ea90e2963 - SSL_get_negotiated_group > 28c24092e39bfd70852afa2923a3d12d2e9be2f5 - TLSEXT_nid_unknown You'd assume that's the case, but they've merged a lot of commits more recent than those two, so they've skipped them (intentionally or not). Best regards, Piotr Sikora ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] QUIC: fix build against musl-libc when using Clang
# HG changeset patch # User Piotr Sikora # Date 1714589495 0 # Wed May 01 18:51:35 2024 + # Node ID 962cddbaecf02b9c213dca492a74b23924b8f24c # Parent 49dce50fad40bf09db81ca2a35983ecd7b740e43 QUIC: fix build against musl-libc when using Clang. Signed-off-by: Piotr Sikora diff -r 49dce50fad40 -r 962cddbaecf0 src/event/ngx_event_udp.c --- a/src/event/ngx_event_udp.c Tue Apr 16 18:29:59 2024 +0400 +++ b/src/event/ngx_event_udp.c Wed May 01 18:51:35 2024 + @@ -138,6 +138,14 @@ ngx_memcpy(&lsa, local_sockaddr, local_socklen); local_sockaddr = &lsa.sockaddr; +#ifndef __GLIBC__ +/* Silence warnings from expanded CMSG_NXTHDR macro when using musl-libc. */ +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wunknown-pragmas" +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wsign-compare" +#endif + for (cmsg = CMSG_FIRSTHDR(&msg); cmsg != NULL; cmsg = CMSG_NXTHDR(&msg, cmsg)) @@ -148,6 +156,11 @@ } } +#ifndef __GLIBC__ +#pragma clang diagnostic pop +#pragma GCC diagnostic pop +#endif + #endif c = ngx_lookup_udp_connection(ls, sockaddr, socklen, local_sockaddr, diff -r 49dce50fad40 -r 962cddbaecf0 src/event/quic/ngx_event_quic_output.c --- a/src/event/quic/ngx_event_quic_output.cTue Apr 16 18:29:59 2024 +0400 +++ b/src/event/quic/ngx_event_quic_output.cWed May 01 18:51:35 2024 + @@ -434,10 +434,25 @@ *valp = segment; #if (NGX_HAVE_ADDRINFO_CMSG) + +#ifndef __GLIBC__ +/* Silence warnings from expanded CMSG_NXTHDR macro when using musl-libc. */ +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wunknown-pragmas" +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wsign-compare" +#endif + if (c->listening && c->listening->wildcard && c->local_sockaddr) { cmsg = CMSG_NXTHDR(&msg, cmsg); clen += ngx_set_srcaddr_cmsg(cmsg, c->local_sockaddr); } + +#ifndef __GLIBC__ +#pragma clang diagnostic pop +#pragma GCC diagnostic pop +#endif + #endif msg.msg_controllen = clen; diff -r 49dce50fad40 -r 962cddbaecf0 src/event/quic/ngx_event_quic_udp.c --- a/src/event/quic/ngx_event_quic_udp.c Tue Apr 16 18:29:59 2024 +0400 +++ b/src/event/quic/ngx_event_quic_udp.c Wed May 01 18:51:35 2024 + @@ -140,6 +140,14 @@ ngx_memcpy(&lsa, local_sockaddr, local_socklen); local_sockaddr = &lsa.sockaddr; +#ifndef __GLIBC__ +/* Silence warnings from expanded CMSG_NXTHDR macro when using musl-libc. */ +#pragma GCC diagnostic push +#pragma GCC diagnostic ignored "-Wunknown-pragmas" +#pragma clang diagnostic push +#pragma clang diagnostic ignored "-Wsign-compare" +#endif + for (cmsg = CMSG_FIRSTHDR(&msg); cmsg != NULL; cmsg = CMSG_NXTHDR(&msg, cmsg)) @@ -150,6 +158,11 @@ } } +#ifndef __GLIBC__ +#pragma clang diagnostic pop +#pragma GCC diagnostic pop +#endif + #endif if (ngx_quic_get_packet_dcid(ev->log, buffer, n, &key) != NGX_OK) { ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] Configure: test --with-cc-opt options
# HG changeset patch # User Piotr Sikora # Date 1714589524 0 # Wed May 01 18:52:04 2024 + # Node ID 0d5498e86bf8a7f119ed83dbc0789be37d728334 # Parent 49dce50fad40bf09db81ca2a35983ecd7b740e43 Configure: test --with-cc-opt options. Previously, invalid C compiler options would fail with an unrelated error much later in the ./configure tests. This matches the existing test for --with-ld-opt options. Signed-off-by: Piotr Sikora diff -r 49dce50fad40 -r 0d5498e86bf8 auto/cc/conf --- a/auto/cc/conf Tue Apr 16 18:29:59 2024 +0400 +++ b/auto/cc/conf Wed May 01 18:52:04 2024 + @@ -142,6 +142,29 @@ fi CFLAGS="$CFLAGS $NGX_CC_OPT" + +if [ "$NGX_PLATFORM" != win32 ]; then + +if test -n "$NGX_CC_OPT"; then +ngx_feature=--with-cc-opt=\"$NGX_CC_OPT\" +ngx_feature_name= +ngx_feature_run=no +ngx_feature_incs= +ngx_feature_path= +ngx_feature_libs= +ngx_feature_test= +. auto/feature + +if [ $ngx_found = no ]; then +echo +echo $0: error: the invalid value in --with-cc-opt=\"$NGX_CC_OPT\" +echo +exit 1 +fi +fi + +fi + NGX_TEST_LD_OPT="$NGX_LD_OPT" if [ "$NGX_PLATFORM" != win32 ]; then ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] Configure: fix warnings in a few feature tests
# HG changeset patch # User Piotr Sikora # Date 1714589586 0 # Wed May 01 18:53:06 2024 + # Node ID c083cd8ead811426c6f7bd7c4ab58a413d80be52 # Parent 49dce50fad40bf09db81ca2a35983ecd7b740e43 Configure: fix warnings in a few feature tests. Signed-off-by: Piotr Sikora diff -r 49dce50fad40 -r c083cd8ead81 auto/unix --- a/auto/unix Tue Apr 16 18:29:59 2024 +0400 +++ b/auto/unix Wed May 01 18:53:06 2024 + @@ -731,7 +731,7 @@ ngx_feature_incs= ngx_feature_path= ngx_feature_libs= -ngx_feature_test="char buf[1]; ssize_t n; n = pwrite(1, buf, 1, 0); +ngx_feature_test="const char buf[1] = \"\"; ssize_t n; n = pwrite(1, buf, 1, 0); if (n == -1) return 1" . auto/feature @@ -760,7 +760,7 @@ ngx_feature_incs='#include ' ngx_feature_path= ngx_feature_libs= -ngx_feature_test="char *p; p = strerrordesc_np(0); +ngx_feature_test="const char *p; p = strerrordesc_np(0); if (p == NULL) return 1" . auto/feature ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] Configure: always test with provided C compiler options
# HG changeset patch # User Piotr Sikora # Date 1714589692 0 # Wed May 01 18:54:52 2024 + # Node ID df39b5d3c3a0c670f3d94e623351b6c659f5be84 # Parent 49dce50fad40bf09db81ca2a35983ecd7b740e43 Configure: always test with provided C compiler options. Previously, build in auto/include didn't use C compiler options, which didn't allow using a custom sysroot. While there, update auto/types/uintptr_t to match others. Signed-off-by: Piotr Sikora diff -r 49dce50fad40 -r df39b5d3c3a0 auto/include --- a/auto/include Tue Apr 16 18:29:59 2024 +0400 +++ b/auto/include Wed May 01 18:54:52 2024 + @@ -27,7 +27,8 @@ END -ngx_test="$CC -o $NGX_AUTOTEST $NGX_AUTOTEST.c" +ngx_test="$CC $CC_TEST_FLAGS $CC_AUX_FLAGS \ + -o $NGX_AUTOTEST $NGX_AUTOTEST.c $NGX_LD_OPT $ngx_feature_libs" eval "$ngx_test >> $NGX_AUTOCONF_ERR 2>&1" diff -r 49dce50fad40 -r df39b5d3c3a0 auto/types/uintptr_t --- a/auto/types/uintptr_t Tue Apr 16 18:29:59 2024 +0400 +++ b/auto/types/uintptr_t Wed May 01 18:54:52 2024 + @@ -27,7 +27,7 @@ END ngx_test="$CC $CC_TEST_FLAGS $CC_AUX_FLAGS \ - -o $NGX_AUTOTEST $NGX_AUTOTEST.c $NGX_LD_OPT" + -o $NGX_AUTOTEST $NGX_AUTOTEST.c $NGX_LD_OPT $ngx_feature_libs" eval "$ngx_test >> $NGX_AUTOCONF_ERR 2>&1" ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] Configure: fix build on Windows using non-MSVC compilers
# HG changeset patch # User Piotr Sikora # Date 1714589717 0 # Wed May 01 18:55:17 2024 + # Node ID 43b04ece77b7132db868122a20c99c8ba89adfb5 # Parent 49dce50fad40bf09db81ca2a35983ecd7b740e43 Configure: fix build on Windows using non-MSVC compilers. Previously, kernel32 and user32 libraries were linked only when using MSVC, but they are needed regardless of the compiler. Signed-off-by: Piotr Sikora diff -r 49dce50fad40 -r 43b04ece77b7 auto/cc/msvc --- a/auto/cc/msvc Tue Apr 16 18:29:59 2024 +0400 +++ b/auto/cc/msvc Wed May 01 18:55:17 2024 + @@ -114,8 +114,6 @@ CFLAGS="$CFLAGS $LIBC" -CORE_LIBS="$CORE_LIBS kernel32.lib user32.lib" - # Win32 GUI mode application #CORE_LINK="$CORE_LINK -subsystem:windows -entry:mainCRTStartup" diff -r 49dce50fad40 -r 43b04ece77b7 auto/os/win32 --- a/auto/os/win32 Tue Apr 16 18:29:59 2024 +0400 +++ b/auto/os/win32 Wed May 01 18:55:17 2024 + @@ -19,14 +19,14 @@ case "$NGX_CC_NAME" in clang | gcc) -CORE_LIBS="$CORE_LIBS -ladvapi32 -lws2_32" +CORE_LIBS="$CORE_LIBS -lkernel32 -luser32 -ladvapi32 -lws2_32" MAIN_LINK="$MAIN_LINK -Wl,--export-all-symbols" MAIN_LINK="$MAIN_LINK -Wl,--out-implib=$NGX_OBJS/libnginx.a" MODULE_LINK="-shared -L $NGX_OBJS -lnginx" ;; *) -CORE_LIBS="$CORE_LIBS advapi32.lib ws2_32.lib" +CORE_LIBS="$CORE_LIBS kernel32.lib user32.lib advapi32.lib ws2_32.lib" ;; esac ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] Configure: build C++ test module using C++ compiler
# HG changeset patch # User Piotr Sikora # Date 1714589815 0 # Wed May 01 18:56:55 2024 + # Node ID 787e1adea9aa7f681884657f119e864af0be3e0d # Parent 49dce50fad40bf09db81ca2a35983ecd7b740e43 Configure: build C++ test module using C++ compiler. This fixes build when using C compiler that doesn't support C++. Signed-off-by: Piotr Sikora diff -r 49dce50fad40 -r 787e1adea9aa auto/cc/conf --- a/auto/cc/conf Tue Apr 16 18:29:59 2024 +0400 +++ b/auto/cc/conf Wed May 01 18:56:55 2024 + @@ -142,6 +142,7 @@ fi CFLAGS="$CFLAGS $NGX_CC_OPT" +CXXFLAGS="$CXXFLAGS $NGX_CXX_OPT" NGX_TEST_LD_OPT="$NGX_LD_OPT" if [ "$NGX_PLATFORM" != win32 ]; then diff -r 49dce50fad40 -r 787e1adea9aa auto/make --- a/auto/make Tue Apr 16 18:29:59 2024 +0400 +++ b/auto/make Wed May 01 18:56:55 2024 + @@ -23,6 +23,8 @@ CC = $CC CFLAGS = $CFLAGS +CXX = $CXX +CXXFLAGS = $CXXFLAGS CPP = $CPP LINK = $LINK @@ -384,22 +386,36 @@ if test -n "$MISC_SRCS"; then ngx_cc="\$(CC) $ngx_compile_opt \$(CFLAGS) $ngx_use_pch \$(ALL_INCS)" +ngx_cxx="\$(CXX) $ngx_compile_opt \$(CXXFLAGS) $ngx_use_pch \$(ALL_INCS)" -for ngx_src in $MISC_SRCS +for ngx_source in $MISC_SRCS do -ngx_src=`echo $ngx_src | sed -e "s/\//$ngx_regex_dirsep/g"` +ngx_src=`echo $ngx_source | sed -e "s/\//$ngx_regex_dirsep/g"` ngx_obj=`echo $ngx_src \ | sed -e "s#^\(.*\.\)cpp\\$#$ngx_objs_dir\1$ngx_objext#g" \ -e "s#^\(.*\.\)cc\\$#$ngx_objs_dir\1$ngx_objext#g" \ -e "s#^\(.*\.\)c\\$#$ngx_objs_dir\1$ngx_objext#g" \ -e "s#^\(.*\.\)S\\$#$ngx_objs_dir\1$ngx_objext#g"` -cat << END>> $NGX_MAKEFILE +if [ $ngx_source = src/misc/ngx_cpp_test_module.cpp ]; then + +cat << END>> $NGX_MAKEFILE + +$ngx_obj: \$(CORE_DEPS) $ngx_cont$ngx_src + $ngx_cxx$ngx_tab$ngx_objout$ngx_obj$ngx_tab$ngx_src$NGX_AUX + +END + +else + +cat << END>> $NGX_MAKEFILE $ngx_obj: \$(CORE_DEPS) $ngx_cont$ngx_src $ngx_cc$ngx_tab$ngx_objout$ngx_obj$ngx_tab$ngx_src$NGX_AUX END + +fi done fi diff -r 49dce50fad40 -r 787e1adea9aa auto/options --- a/auto/options Tue Apr 16 18:29:59 2024 +0400 +++ b/auto/options Wed May 01 18:56:55 2024 + @@ -18,11 +18,13 @@ NGX_BUILD= CC=${CC:-cc} +CXX=${CXX:-c++} CPP= NGX_OBJS=objs NGX_DEBUG=NO NGX_CC_OPT= +NGX_CXX_OPT= NGX_LD_OPT= CPU=NO @@ -358,8 +360,10 @@ --with-compat) NGX_COMPAT=YES ;; --with-cc=*) CC="$value";; +--with-cxx=*)CXX="$value" ;; --with-cpp=*)CPP="$value" ;; --with-cc-opt=*) NGX_CC_OPT="$value";; +--with-cxx-opt=*)NGX_CXX_OPT="$value" ;; --with-ld-opt=*) NGX_LD_OPT="$value";; --with-cpu-opt=*)CPU="$value" ;; --with-debug)NGX_DEBUG=YES ;; @@ -578,8 +582,10 @@ --with-compat dynamic modules compatibility --with-cc=PATH set C compiler pathname + --with-cxx=PATHset C++ compiler pathname --with-cpp=PATHset C preprocessor pathname --with-cc-opt=OPTIONS set additional C compiler options + --with-cxx-opt=OPTIONS set additional C++ compiler options --with-ld-opt=OPTIONS set additional linker options --with-cpu-opt=CPU build for the specified CPU, valid values: pentium, pentiumpro, pentium3, pentium4, ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] Configure: postpone running the test binary
# HG changeset patch # User Piotr Sikora # Date 1714589827 0 # Wed May 01 18:57:07 2024 + # Node ID 2d5e754e3a4e7a59eaf7f653ad4fd5346f53eab4 # Parent 49dce50fad40bf09db81ca2a35983ecd7b740e43 Configure: postpone running the test binary. Previously, the ./configure script would attempt to execute the test binary during the C compiler check, and it would abort with an error saying that the compiler doesn't work if there was any issue running the test binary (e.g. due to missing compiler and/or linker flags). This change delays that check and runs the test binary compiled with all provided compiler and linker flags. Signed-off-by: Piotr Sikora diff -r 49dce50fad40 -r 2d5e754e3a4e auto/cc/conf --- a/auto/cc/conf Tue Apr 16 18:29:59 2024 +0400 +++ b/auto/cc/conf Wed May 01 18:57:07 2024 + @@ -164,6 +164,23 @@ fi +ngx_feature="working test binary" +ngx_feature_name= +ngx_feature_run=yes +ngx_feature_incs= +ngx_feature_path= +ngx_feature_libs= +ngx_feature_test= +. auto/feature + +if [ $ngx_found = no ]; then +echo +echo $0: error: cannot execute test binary +echo +exit 1 +fi + + ngx_feature="-Wl,-E switch" ngx_feature_name= ngx_feature_run=no diff -r 49dce50fad40 -r 2d5e754e3a4e auto/cc/name --- a/auto/cc/name Tue Apr 16 18:29:59 2024 +0400 +++ b/auto/cc/name Wed May 01 18:57:07 2024 + @@ -7,7 +7,7 @@ ngx_feature="C compiler" ngx_feature_name= -ngx_feature_run=yes +ngx_feature_run=no ngx_feature_incs= ngx_feature_path= ngx_feature_libs= ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] Configure: increase the default optimization level to -O2
# HG changeset patch # User Piotr Sikora # Date 1714589900 0 # Wed May 01 18:58:20 2024 + # Node ID 429191aad8d3ab8f70d40dfdd7684a256357bc48 # Parent 49dce50fad40bf09db81ca2a35983ecd7b740e43 Configure: increase the default optimization level to -O2. Signed-off-by: Piotr Sikora diff -r 49dce50fad40 -r 429191aad8d3 auto/cc/clang --- a/auto/cc/clang Tue Apr 16 18:29:59 2024 +0400 +++ b/auto/cc/clang Wed May 01 18:58:20 2024 + @@ -19,9 +19,9 @@ # optimizations -#NGX_CLANG_OPT="-O2" +NGX_CLANG_OPT="-O2" #NGX_CLANG_OPT="-Oz" -NGX_CLANG_OPT="-O" +#NGX_CLANG_OPT="-O" case $CPU in pentium) diff -r 49dce50fad40 -r 429191aad8d3 auto/cc/gcc --- a/auto/cc/gcc Tue Apr 16 18:29:59 2024 +0400 +++ b/auto/cc/gcc Wed May 01 18:58:20 2024 + @@ -48,9 +48,9 @@ # optimizations -#NGX_GCC_OPT="-O2" +NGX_GCC_OPT="-O2" #NGX_GCC_OPT="-Os" -NGX_GCC_OPT="-O" +#NGX_GCC_OPT="-O" #CFLAGS="$CFLAGS -fomit-frame-pointer" ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] Follow OpenSSL's switch from AES128 to AES256 for session tickets
Hey Christian, > # HG changeset patch > # User Christian Klinger > # Date 1478383992 -3600 > # Node ID 5719a734584d23a6bcd22a3e59dd36138d06b803 > # Parent 92ad1c92bcf93310bf59447dd581cac37af87adb > Follow OpenSSL's switch from AES128 to AES256 for session tickets > > OpenSSL switched from AES128 to AES256 for de-/encryption of session > tickets in May 2016 (commit 05df5c2036f1244fe3df70de7d8079a5d86b999d), > while we still used AES128, if `ssl_session_ticket_key` was set. > > Using AES128 for session tickets means that even when using an AES256 > cipher suite, the session's master key (and hence the whole > connection) is effectively only protected by AES128, which silently > weakens encryption. > > Hence, we follow OpenSSL's lead and switch from AES128 to AES256 for > session tickets, if `ssl_session_ticket_key` is set. > > While the switch from AES128 to AES256 warrants reading additional > 16 bytes from the key file, we nonetheless increase by 32 bytes, > reading a total of 80 bytes instead of previously 48 bytes. > The additional 16 bytes flow into the key for the SHA256 HMAC. > Previously, the SHA256 HMAC only used a 16 byte key, and thereby used > only half of SHA256 HMAC's 32 byte key space. We now provide a 32 byte > key to the SHA256 HMAC and finally fully use the available key space. While I agree that we should bump this to AES256 (or at least, make it work with both), your change to use AES256 with keys that are half-filled with zeros doesn't seem very appealing... I suggest that "ssl_session_ticket_key" should either accept only 80 byte files (for use with AES256) or both: 48 byte files (for use with AES128) & 80 byte files (for use with AES256). Whether or not we want to support both is up to Maxim, but considering that this is "advanced" feature, which users are expected to know how to generate new keys, I'd recommend that NGINX should migrate to 80 byte files for use with AES256, without backward-compatibility for 48 byte keys. Also, considering that recent versions of OpenSSL use AES256 by default (i.e. when keys are not provided using "ssl_session_ticket_key" directive), we shouldn't provide a way lower the security of Session Tickets. Best regards, Piotr Sikora ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] Follow OpenSSL's switch from AES128 to AES256 for session tickets
Hey, > While I agree that we should bump this to AES256 (or at least, make it > work with both), your change to use AES256 with keys that are > half-filled with zeros doesn't seem very appealing... > > I suggest that "ssl_session_ticket_key" should either accept only 80 > byte files (for use with AES256) or both: 48 byte files (for use with > AES128) & 80 byte files (for use with AES256). > > Whether or not we want to support both is up to Maxim, but considering > that this is "advanced" feature, which users are expected to know how > to generate new keys, I'd recommend that NGINX should migrate to 80 > byte files for use with AES256, without backward-compatibility for 48 > byte keys. > > Also, considering that recent versions of OpenSSL use AES256 by > default (i.e. when keys are not provided using > "ssl_session_ticket_key" directive), we shouldn't provide a way lower > the security of Session Tickets. ...and while we're breaking backward-compatibility, we should also change the order in which values are read from files to match what OpenSSL, BoringSSL & Apache are doing, i.e. key name, HMAC key, AES key: ngx_memcpy(key->name, buf, 16); ngx_memcpy(key->hmac_key, buf + 16, 32); ngx_memcpy(key->aes_key, buf + 48, 32); instead of the current: key name, AES key, HMAC key, which I introduced by mistake and which has been annoying me ever since. Best regards, Piotr Sikora ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] Follow OpenSSL's switch from AES128 to AES256 for session tickets
Hey Christian, > # HG changeset patch > # User Christian Klinger > # Date 1478468739 -3600 > # Node ID 9cfbbce1ec24a31c29ea2f20cb21e32e5173bc60 > # Parent 92ad1c92bcf93310bf59447dd581cac37af87adb > Follow OpenSSL's switch from AES128 to AES256 for session tickets This should be: SSL: switch from AES128 to AES256 for TLS Session Tickets. > > OpenSSL switched from AES128 to AES256 for de-/encryption of session > tickets in May 2016 (commit 05df5c2036f1244fe3df70de7d8079a5d86b999d), > while we still used AES128, if `ssl_session_ticket_key` was set. > > Using AES128 for session tickets means that even when using an AES256 > cipher suite, the session's master key (and hence the whole > connection) is effectively only protected by AES128, which silently > weakens encryption. > > Hence, we follow OpenSSL's lead and switch from AES128 to AES256 for > session tickets, if `ssl_session_ticket_key` is set. > > While the switch from AES128 to AES256 warrants reading additional > 16 bytes from the key file, we nonetheless increase by 32 bytes, > reading a total of 80 bytes instead of previously 48 bytes. > The additional 16 bytes flow into the key for the SHA256 HMAC. > Previously, the SHA256 HMAC only used a 16 byte key, and thereby used > only half of SHA256 HMAC's 32 byte key space. We now provide a 32 byte > key to the SHA256 HMAC and finally fully use the available key space. > > diff -r 92ad1c92bcf9 -r 9cfbbce1ec24 src/event/ngx_event_openssl.c > --- a/src/event/ngx_event_openssl.c Fri Nov 04 19:12:19 2016 +0300 > +++ b/src/event/ngx_event_openssl.c Sun Nov 06 22:45:39 2016 +0100 > @@ -2832,7 +2832,7 @@ > ngx_int_t > ngx_ssl_session_ticket_keys(ngx_conf_t *cf, ngx_ssl_t *ssl, ngx_array_t > *paths) > { > -u_char buf[48]; > +u_char buf[80]; > ssize_tn; > ngx_str_t *path; > ngx_file_t file; > @@ -2875,13 +2875,13 @@ > goto failed; > } > > -if (ngx_file_size(&fi) != 48) { > +if (ngx_file_size(&fi) != 80) { > ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, > - "\"%V\" must be 48 bytes", &file.name); > + "\"%V\" must be 80 bytes", &file.name); > goto failed; > } > > -n = ngx_read_file(&file, buf, 48, 0); > +n = ngx_read_file(&file, buf, 80, 0); > > if (n == NGX_ERROR) { > ngx_conf_log_error(NGX_LOG_CRIT, cf, ngx_errno, > @@ -2889,10 +2889,10 @@ > goto failed; > } > > -if (n != 48) { > +if (n != 80) { > ngx_conf_log_error(NGX_LOG_CRIT, cf, 0, > ngx_read_file_n " \"%V\" returned only " > - "%z bytes instead of 48", &file.name, n); > + "%z bytes instead of 80", &file.name, n); > goto failed; > } > > @@ -2900,10 +2900,9 @@ > if (key == NULL) { > goto failed; > } > - Style: leave empty line. > ngx_memcpy(key->name, buf, 16); > -ngx_memcpy(key->aes_key, buf + 16, 16); > -ngx_memcpy(key->hmac_key, buf + 32, 16); > +ngx_memcpy(key->hmac_key, buf + 16, 32); > +ngx_memcpy(key->aes_key, buf + 48, 32); > > if (ngx_close_file(file.fd) == NGX_FILE_ERROR) { > ngx_log_error(NGX_LOG_ALERT, cf->log, ngx_errno, > @@ -2962,7 +2961,7 @@ > c = ngx_ssl_get_connection(ssl_conn); > ssl_ctx = c->ssl->session_ctx; > > -cipher = EVP_aes_128_cbc(); > +cipher = EVP_aes_256_cbc(); > #ifdef OPENSSL_NO_SHA256 > digest = EVP_sha1(); > #else > @@ -2996,12 +2995,14 @@ > } > > #if OPENSSL_VERSION_NUMBER >= 0x1000L > -if (HMAC_Init_ex(hctx, key[0].hmac_key, 16, digest, NULL) != 1) { > +if (HMAC_Init_ex(hctx, key[0].hmac_key, sizeof(key[0].hmac_key), > + digest, NULL) != 1) { Style, this should be: if (HMAC_Init_ex(hctx, key[0].hmac_key, sizeof(key[0].hmac_key), digest, NULL) != 1) { which is why I'm not sure if the use of sizeof() instead of hardcoded "32" is a good choice, since it changes 1 line to 4 lines (here and in other places). > ngx_ssl_error(NGX_LOG_ALERT, c->log, 0, "HMAC_Init_ex() failed"); > return -1; > } > #else > -HMAC_Init_ex(hctx, key[0].hmac_key, 16, digest, NULL); > +HMAC_Init_ex(hctx, key[0].hmac_key, sizeof(key[0].hmac_key), digest, > + NULL); > #endif > > ngx_memcpy(name, key[0].name, 16); > @@ -3031,12 +3032,14 @@ > (i == 0) ? " (default)" : ""); > > #if OPENSSL_VERSION_NUMBER >= 0x1000L > -if (HMAC_Init_ex(hctx, key[i].hmac_key, 16, digest, NULL) != 1) { > +if (HMAC_Init_ex(hctx, key[i].hmac_key, sizeof(key[i].hmac_key), > +
Re: [PATCH 1 of 6] SSL: define NGX_SSL_VERIFY constants
Hey Maxim, > How is that related to the commit in question? > > Please note that I pinged you on 3 out of 6 commits, which I'm > interested in getting in, regardless of ngx_ssl_verify_client() & > friends. Ping. Best regards, Piotr Sikora ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH 1 of 2] HTTP: add support for "429 Too Many Requests" response (RFC6585)
Hey, > # HG changeset patch > # User Piotr Sikora > # Date 1476859139 25200 > # Tue Oct 18 23:38:59 2016 -0700 > # Node ID 1eec5355ef1e4a8b0aecebdec84c744734c0d36e > # Parent 8081e1f3ab8b9ccb4e2d7f9240cbfb8e404a3c95 > HTTP: add support for "429 Too Many Requests" response (RFC6585). > > This change adds reason phrase in status line and pretty response body > when "429" status code is used in "return", "limit_conn_status" and/or > "limit_req_status" directives. > > It also adds "http_429" parameter to "proxy_next_upstream" for retrying > rate-limited requests, and to "proxy_cache_use_stale" for serving stale > cached responses after being rate-limited. Ping. Best regards, Piotr Sikora ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] SSL: fix call to BIO_get_mem_data()
# HG changeset patch # User Piotr Sikora # Date 1481667570 28800 # Tue Dec 13 14:19:30 2016 -0800 # Node ID c0b6eef901895a4790db1e62d2822651977399a4 # Parent 25a64c864f4d31761eb42d39cda8b0e80277816d SSL: fix call to BIO_get_mem_data(). Fixes build with BoringSSL. Signed-off-by: Piotr Sikora diff -r 25a64c864f4d -r c0b6eef90189 src/event/ngx_event_openssl.c --- a/src/event/ngx_event_openssl.c +++ b/src/event/ngx_event_openssl.c @@ -4069,7 +4069,7 @@ ngx_ssl_parse_time( BIO_write(bio, "Tue ", sizeof("Tue ") - 1); ASN1_TIME_print(bio, asn1time); -len = BIO_get_mem_data(bio, &value); +len = BIO_get_mem_data(bio, (char **) &value); time = ngx_parse_http_time(value, len); ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [nginx] SSL: $ssl_curves (ticket #1088).
Hey Maxim, > details: http://hg.nginx.org/nginx/rev/e75e854657ba > branches: > changeset: 6817:e75e854657ba > user: Maxim Dounin > date: Mon Dec 05 22:23:23 2016 +0300 > description: > SSL: $ssl_curves (ticket #1088). > > The variable contains a list of curves as supported by the client. Why this isn't called $ssl_client_curves, then? Same question applies to $ssl_ciphers. Best regards, Piotr Sikora ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] SSL: fix call to BIO_get_mem_data()
# HG changeset patch # User Piotr Sikora # Date 1481667570 28800 # Tue Dec 13 14:19:30 2016 -0800 # Node ID 4848b2eea6ba373d29c036b3b5acbeaf0f038587 # Parent 25a64c864f4d31761eb42d39cda8b0e80277816d SSL: fix call to BIO_get_mem_data(). Fixes build with BoringSSL. Signed-off-by: Piotr Sikora diff -r 25a64c864f4d -r 4848b2eea6ba src/event/ngx_event_openssl.c --- a/src/event/ngx_event_openssl.c +++ b/src/event/ngx_event_openssl.c @@ -4069,7 +4069,7 @@ ngx_ssl_parse_time( BIO_write(bio, "Tue ", sizeof("Tue ") - 1); ASN1_TIME_print(bio, asn1time); -len = BIO_get_mem_data(bio, &value); +len = BIO_get_mem_data(bio, (char **) &value); time = ngx_parse_http_time(value, len); diff -r 25a64c864f4d -r 4848b2eea6ba src/event/ngx_event_openssl_stapling.c --- a/src/event/ngx_event_openssl_stapling.c +++ b/src/event/ngx_event_openssl_stapling.c @@ -793,7 +793,7 @@ ngx_ssl_stapling_time(ASN1_GENERALIZEDTI BIO_write(bio, "Tue ", sizeof("Tue ") - 1); ASN1_GENERALIZEDTIME_print(bio, asn1time); -len = BIO_get_mem_data(bio, &value); +len = BIO_get_mem_data(bio, (char **) &value); time = ngx_parse_http_time(value, len); ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] SSL: fix call to BIO_get_mem_data()
Hey Maxim, > If the goal is to fix the call, shouldn't identical one in > src/event/ngx_event_openssl_stapling.c be fixed as well? Good catch, thanks! Best regards, Piotr Sikora ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH 1 of 2] HTTP: add support for "429 Too Many Requests" response (RFC6585)
Hey, > # HG changeset patch > # User Piotr Sikora > # Date 1476859139 25200 > # Tue Oct 18 23:38:59 2016 -0700 > # Node ID 1eec5355ef1e4a8b0aecebdec84c744734c0d36e > # Parent 8081e1f3ab8b9ccb4e2d7f9240cbfb8e404a3c95 > HTTP: add support for "429 Too Many Requests" response (RFC6585). > > This change adds reason phrase in status line and pretty response body > when "429" status code is used in "return", "limit_conn_status" and/or > "limit_req_status" directives. > > It also adds "http_429" parameter to "proxy_next_upstream" for retrying > rate-limited requests, and to "proxy_cache_use_stale" for serving stale > cached responses after being rate-limited. Ping. Best regards, Piotr Sikora ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH 1 of 2] HTTP: add support for "429 Too Many Requests" response (RFC6585)
Hey, > # HG changeset patch > # User Piotr Sikora > # Date 1476859139 25200 > # Tue Oct 18 23:38:59 2016 -0700 > # Node ID 1eec5355ef1e4a8b0aecebdec84c744734c0d36e > # Parent 8081e1f3ab8b9ccb4e2d7f9240cbfb8e404a3c95 > HTTP: add support for "429 Too Many Requests" response (RFC6585). > > This change adds reason phrase in status line and pretty response body > when "429" status code is used in "return", "limit_conn_status" and/or > "limit_req_status" directives. > > It also adds "http_429" parameter to "proxy_next_upstream" for retrying > rate-limited requests, and to "proxy_cache_use_stale" for serving stale > cached responses after being rate-limited. Ping... Best regards, Piotr Sikora ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH 1 of 3] HTTP: add support for "429 Too Many Requests" response (RFC6585)
# HG changeset patch # User Piotr Sikora # Date 1488324535 28800 # Tue Feb 28 15:28:55 2017 -0800 # Node ID 9a63d6e990d230db0ec6b03250265447f648526e # Parent 8b7fd958c59f8280d167fe7dd93f1942bfed5876 HTTP: add support for "429 Too Many Requests" response (RFC6585). This change adds reason phrase in status line and pretty response body when "429" status code is used in "return", "limit_conn_status" and/or "limit_req_status" directives. Signed-off-by: Piotr Sikora diff -r 8b7fd958c59f -r 9a63d6e990d2 src/http/ngx_http_header_filter_module.c --- a/src/http/ngx_http_header_filter_module.c +++ b/src/http/ngx_http_header_filter_module.c @@ -101,12 +101,16 @@ static ngx_str_t ngx_http_status_lines[] ngx_null_string, /* "419 unused" */ ngx_null_string, /* "420 unused" */ ngx_string("421 Misdirected Request"), +ngx_null_string, /* "422 Unprocessable Entity" */ +ngx_null_string, /* "423 Locked" */ +ngx_null_string, /* "424 Failed Dependency" */ +ngx_null_string, /* "425 unused" */ +ngx_null_string, /* "426 Upgrade Required" */ +ngx_null_string, /* "427 unused" */ +ngx_null_string, /* "428 Precondition Required" */ +ngx_string("429 Too Many Requests"), -/* ngx_null_string, */ /* "422 Unprocessable Entity" */ -/* ngx_null_string, */ /* "423 Locked" */ -/* ngx_null_string, */ /* "424 Failed Dependency" */ - -#define NGX_HTTP_LAST_4XX 422 +#define NGX_HTTP_LAST_4XX 430 #define NGX_HTTP_OFF_5XX (NGX_HTTP_LAST_4XX - 400 + NGX_HTTP_OFF_4XX) ngx_string("500 Internal Server Error"), diff -r 8b7fd958c59f -r 9a63d6e990d2 src/http/ngx_http_request.h --- a/src/http/ngx_http_request.h +++ b/src/http/ngx_http_request.h @@ -98,6 +98,7 @@ #define NGX_HTTP_UNSUPPORTED_MEDIA_TYPE415 #define NGX_HTTP_RANGE_NOT_SATISFIABLE 416 #define NGX_HTTP_MISDIRECTED_REQUEST 421 +#define NGX_HTTP_TOO_MANY_REQUESTS 429 /* Our own HTTP codes */ diff -r 8b7fd958c59f -r 9a63d6e990d2 src/http/ngx_http_special_response.c --- a/src/http/ngx_http_special_response.c +++ b/src/http/ngx_http_special_response.c @@ -225,6 +225,14 @@ static char ngx_http_error_421_page[] = ; +static char ngx_http_error_429_page[] = +"" CRLF +"429 Too Many Requests" CRLF +"" CRLF +"429 Too Many Requests" CRLF +; + + static char ngx_http_error_494_page[] = "" CRLF "400 Request Header Or Cookie Too Large" @@ -354,8 +362,16 @@ static ngx_str_t ngx_http_error_pages[] ngx_null_string, /* 419 */ ngx_null_string, /* 420 */ ngx_string(ngx_http_error_421_page), +ngx_null_string, /* 422 */ +ngx_null_string, /* 423 */ +ngx_null_string, /* 424 */ +ngx_null_string, /* 425 */ +ngx_null_string, /* 426 */ +ngx_null_string, /* 427 */ +ngx_null_string, /* 428 */ +ngx_string(ngx_http_error_429_page), -#define NGX_HTTP_LAST_4XX 422 +#define NGX_HTTP_LAST_4XX 430 #define NGX_HTTP_OFF_5XX (NGX_HTTP_LAST_4XX - 400 + NGX_HTTP_OFF_4XX) ngx_string(ngx_http_error_494_page), /* 494, request header too large */ ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH 3 of 3] Limit req: change default response code when rate-limiting
# HG changeset patch # User Piotr Sikora # Date 1488324535 28800 # Tue Feb 28 15:28:55 2017 -0800 # Node ID c9d43c652ac776068e78f695dde00606eed184f8 # Parent e21f12a958010e1f3e5cdc1640859e335e032ca5 Limit req: change default response code when rate-limiting. Previously, "503 Service Unavailable" response code was used, but the new "429 Too Many Requests" response code is more appropriate. Signed-off-by: Piotr Sikora diff -r e21f12a95801 -r c9d43c652ac7 src/http/modules/ngx_http_limit_req_module.c --- a/src/http/modules/ngx_http_limit_req_module.c +++ b/src/http/modules/ngx_http_limit_req_module.c @@ -712,7 +712,7 @@ ngx_http_limit_req_merge_conf(ngx_conf_t NGX_LOG_INFO : conf->limit_log_level + 1; ngx_conf_merge_uint_value(conf->status_code, prev->status_code, - NGX_HTTP_SERVICE_UNAVAILABLE); + NGX_HTTP_TOO_MANY_REQUESTS); return NGX_CONF_OK; } ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH 2 of 3] Upstream: allow recovery from "429 Too Many Requests" response
# HG changeset patch # User Piotr Sikora # Date 1488324535 28800 # Tue Feb 28 15:28:55 2017 -0800 # Node ID e21f12a958010e1f3e5cdc1640859e335e032ca5 # Parent 9a63d6e990d230db0ec6b03250265447f648526e Upstream: allow recovery from "429 Too Many Requests" response. This change adds "http_429" parameter to "proxy_next_upstream" for retrying rate-limited requests, and to "proxy_cache_use_stale" for serving stale cached responses after being rate-limited. Signed-off-by: Piotr Sikora diff -r 9a63d6e990d2 -r e21f12a95801 src/http/modules/ngx_http_fastcgi_module.c --- a/src/http/modules/ngx_http_fastcgi_module.c +++ b/src/http/modules/ngx_http_fastcgi_module.c @@ -211,6 +211,7 @@ static ngx_conf_bitmask_t ngx_http_fast { ngx_string("http_503"), NGX_HTTP_UPSTREAM_FT_HTTP_503 }, { ngx_string("http_403"), NGX_HTTP_UPSTREAM_FT_HTTP_403 }, { ngx_string("http_404"), NGX_HTTP_UPSTREAM_FT_HTTP_404 }, +{ ngx_string("http_429"), NGX_HTTP_UPSTREAM_FT_HTTP_429 }, { ngx_string("updating"), NGX_HTTP_UPSTREAM_FT_UPDATING }, { ngx_string("off"), NGX_HTTP_UPSTREAM_FT_OFF }, { ngx_null_string, 0 } diff -r 9a63d6e990d2 -r e21f12a95801 src/http/modules/ngx_http_proxy_module.c --- a/src/http/modules/ngx_http_proxy_module.c +++ b/src/http/modules/ngx_http_proxy_module.c @@ -220,6 +220,7 @@ static ngx_conf_bitmask_t ngx_http_prox { ngx_string("http_504"), NGX_HTTP_UPSTREAM_FT_HTTP_504 }, { ngx_string("http_403"), NGX_HTTP_UPSTREAM_FT_HTTP_403 }, { ngx_string("http_404"), NGX_HTTP_UPSTREAM_FT_HTTP_404 }, +{ ngx_string("http_429"), NGX_HTTP_UPSTREAM_FT_HTTP_429 }, { ngx_string("updating"), NGX_HTTP_UPSTREAM_FT_UPDATING }, { ngx_string("off"), NGX_HTTP_UPSTREAM_FT_OFF }, { ngx_null_string, 0 } diff -r 9a63d6e990d2 -r e21f12a95801 src/http/modules/ngx_http_scgi_module.c --- a/src/http/modules/ngx_http_scgi_module.c +++ b/src/http/modules/ngx_http_scgi_module.c @@ -82,6 +82,7 @@ static ngx_conf_bitmask_t ngx_http_scgi_ { ngx_string("http_503"), NGX_HTTP_UPSTREAM_FT_HTTP_503 }, { ngx_string("http_403"), NGX_HTTP_UPSTREAM_FT_HTTP_403 }, { ngx_string("http_404"), NGX_HTTP_UPSTREAM_FT_HTTP_404 }, +{ ngx_string("http_429"), NGX_HTTP_UPSTREAM_FT_HTTP_429 }, { ngx_string("updating"), NGX_HTTP_UPSTREAM_FT_UPDATING }, { ngx_string("off"), NGX_HTTP_UPSTREAM_FT_OFF }, { ngx_null_string, 0 } diff -r 9a63d6e990d2 -r e21f12a95801 src/http/modules/ngx_http_uwsgi_module.c --- a/src/http/modules/ngx_http_uwsgi_module.c +++ b/src/http/modules/ngx_http_uwsgi_module.c @@ -114,6 +114,7 @@ static ngx_conf_bitmask_t ngx_http_uwsgi { ngx_string("http_503"), NGX_HTTP_UPSTREAM_FT_HTTP_503 }, { ngx_string("http_403"), NGX_HTTP_UPSTREAM_FT_HTTP_403 }, { ngx_string("http_404"), NGX_HTTP_UPSTREAM_FT_HTTP_404 }, +{ ngx_string("http_429"), NGX_HTTP_UPSTREAM_FT_HTTP_429 }, { ngx_string("updating"), NGX_HTTP_UPSTREAM_FT_UPDATING }, { ngx_string("off"), NGX_HTTP_UPSTREAM_FT_OFF }, { ngx_null_string, 0 } diff -r 9a63d6e990d2 -r e21f12a95801 src/http/ngx_http_upstream.c --- a/src/http/ngx_http_upstream.c +++ b/src/http/ngx_http_upstream.c @@ -436,6 +436,7 @@ static ngx_http_upstream_next_t ngx_htt { 504, NGX_HTTP_UPSTREAM_FT_HTTP_504 }, { 403, NGX_HTTP_UPSTREAM_FT_HTTP_403 }, { 404, NGX_HTTP_UPSTREAM_FT_HTTP_404 }, +{ 429, NGX_HTTP_UPSTREAM_FT_HTTP_429 }, { 0, 0 } }; @@ -4115,7 +4116,8 @@ ngx_http_upstream_next(ngx_http_request_ if (u->peer.sockaddr) { if (ft_type == NGX_HTTP_UPSTREAM_FT_HTTP_403 -|| ft_type == NGX_HTTP_UPSTREAM_FT_HTTP_404) +|| ft_type == NGX_HTTP_UPSTREAM_FT_HTTP_404 +|| ft_type == NGX_HTTP_UPSTREAM_FT_HTTP_429) { state = NGX_PEER_NEXT; @@ -4155,6 +4157,10 @@ ngx_http_upstream_next(ngx_http_request_ status = NGX_HTTP_NOT_FOUND; break; +case NGX_HTTP_UPSTREAM_FT_HTTP_429: +status = NGX_HTTP_TOO_MANY_REQUESTS; +break; + /* * NGX_HTTP_UPSTREAM_FT_BUSY_LOCK and NGX_HTTP_UPSTREAM_FT_MAX_WAITING * never reach here diff -r 9a63d6e990d2 -r e21f12a95801 src/http/ngx_http_upstream.h --- a/src/http/ngx_http_upstream.h +++ b/src/http/ngx_http_upstream.h @@ -26,10 +26,11 @@ #define NGX_HTTP_UPSTREAM_FT_HTTP_5040x0080 #define NGX_HTTP_UPSTREAM_FT_HTTP_4030x0100 #define NGX_HTTP_UPSTREAM_FT_HTTP_4040x0200 -#define NGX_HTTP_UPSTREAM_FT_UPDATING0x0400 -#define NGX_HTTP_UPSTREAM_FT_BUSY_LOCK 0x0800 -#define NGX_HTTP_UPSTREAM_FT_MAX_WAITING 0x1000 -#define NGX_HTTP_UPSTREAM_FT_NON_IDEMPOTENT 0x2000 +#define NGX_HTTP_UPSTREAM_FT_HTTP_4290x0400 +#define NGX_HTTP_UPSTREAM_FT_UPDATING0x0800 +#define NGX_HTTP_UPSTREAM_FT_BUSY_LOCK 0x1000 +#define NGX_HTTP_UPSTREAM_FT_MAX_WAITING 0x2000 +#define NGX_HTTP_UPSTREAM_FT_NON_IDEMPOTENT 0x4000 #define NGX_HTTP_UPSTRE
Re: [PATCH 2 of 3] Upstream: allow recovery from "429 Too Many Requests" response
Hi Maxim, > Not sure it's good idea to don't count a 429 response as a peer > failure. Contrary, counting it as a failure will naturally reduce > load on the particular server, resulting in less rejects. But 429 can be returned on a per request basis (think client IP exceeding limit_req limits, or a logged in user that's rate-limited based on API usage), so I'd question marking backends as failed because of that. Best regards, Piotr Sikora ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH 3 of 3] Limit req: change default response code when rate-limiting
Hi Maxim, > As I already wrote, I certainly disagree with this change. Yes, that was expected (that's why it was originally split into separate change). Is there a particular reason why do you disagree? It's hard to have a constructive discussion if you don't provide any reasoning for your (possibly correct) opinion. Best regards, Piotr Sikora ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH 1 of 2] HTTP: add support for "429 Too Many Requests" response (RFC6585)
# HG changeset patch # User Piotr Sikora # Date 1490348883 25200 # Fri Mar 24 02:48:03 2017 -0700 # Node ID 799ef976b58cadbc212bd790a666033d3777c10d # Parent 39ff6939266e913e8bfd400e60f9520e70725a4d HTTP: add support for "429 Too Many Requests" response (RFC6585). This change adds reason phrase in status line and pretty response body when "429" status code is used in "return", "limit_conn_status" and/or "limit_req_status" directives. Signed-off-by: Piotr Sikora diff -r 39ff6939266e -r 799ef976b58c src/http/ngx_http_header_filter_module.c --- a/src/http/ngx_http_header_filter_module.c +++ b/src/http/ngx_http_header_filter_module.c @@ -101,12 +101,16 @@ static ngx_str_t ngx_http_status_lines[] ngx_null_string, /* "419 unused" */ ngx_null_string, /* "420 unused" */ ngx_string("421 Misdirected Request"), +ngx_null_string, /* "422 Unprocessable Entity" */ +ngx_null_string, /* "423 Locked" */ +ngx_null_string, /* "424 Failed Dependency" */ +ngx_null_string, /* "425 unused" */ +ngx_null_string, /* "426 Upgrade Required" */ +ngx_null_string, /* "427 unused" */ +ngx_null_string, /* "428 Precondition Required" */ +ngx_string("429 Too Many Requests"), -/* ngx_null_string, */ /* "422 Unprocessable Entity" */ -/* ngx_null_string, */ /* "423 Locked" */ -/* ngx_null_string, */ /* "424 Failed Dependency" */ - -#define NGX_HTTP_LAST_4XX 422 +#define NGX_HTTP_LAST_4XX 430 #define NGX_HTTP_OFF_5XX (NGX_HTTP_LAST_4XX - 400 + NGX_HTTP_OFF_4XX) ngx_string("500 Internal Server Error"), diff -r 39ff6939266e -r 799ef976b58c src/http/ngx_http_request.h --- a/src/http/ngx_http_request.h +++ b/src/http/ngx_http_request.h @@ -98,6 +98,7 @@ #define NGX_HTTP_UNSUPPORTED_MEDIA_TYPE415 #define NGX_HTTP_RANGE_NOT_SATISFIABLE 416 #define NGX_HTTP_MISDIRECTED_REQUEST 421 +#define NGX_HTTP_TOO_MANY_REQUESTS 429 /* Our own HTTP codes */ diff -r 39ff6939266e -r 799ef976b58c src/http/ngx_http_special_response.c --- a/src/http/ngx_http_special_response.c +++ b/src/http/ngx_http_special_response.c @@ -225,6 +225,14 @@ static char ngx_http_error_421_page[] = ; +static char ngx_http_error_429_page[] = +"" CRLF +"429 Too Many Requests" CRLF +"" CRLF +"429 Too Many Requests" CRLF +; + + static char ngx_http_error_494_page[] = "" CRLF "400 Request Header Or Cookie Too Large" @@ -354,8 +362,16 @@ static ngx_str_t ngx_http_error_pages[] ngx_null_string, /* 419 */ ngx_null_string, /* 420 */ ngx_string(ngx_http_error_421_page), +ngx_null_string, /* 422 */ +ngx_null_string, /* 423 */ +ngx_null_string, /* 424 */ +ngx_null_string, /* 425 */ +ngx_null_string, /* 426 */ +ngx_null_string, /* 427 */ +ngx_null_string, /* 428 */ +ngx_string(ngx_http_error_429_page), -#define NGX_HTTP_LAST_4XX 422 +#define NGX_HTTP_LAST_4XX 430 #define NGX_HTTP_OFF_5XX (NGX_HTTP_LAST_4XX - 400 + NGX_HTTP_OFF_4XX) ngx_string(ngx_http_error_494_page), /* 494, request header too large */ ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH 2 of 2] Upstream: allow recovery from "429 Too Many Requests" response
# HG changeset patch # User Piotr Sikora # Date 1490348883 25200 # Fri Mar 24 02:48:03 2017 -0700 # Node ID b377cfedf632b14a3d459e12342a0557a25a790c # Parent 799ef976b58cadbc212bd790a666033d3777c10d Upstream: allow recovery from "429 Too Many Requests" response. This change adds "http_429" parameter to "proxy_next_upstream" for retrying rate-limited requests, and to "proxy_cache_use_stale" for serving stale cached responses after being rate-limited. Signed-off-by: Piotr Sikora diff -r 799ef976b58c -r b377cfedf632 src/http/modules/ngx_http_fastcgi_module.c --- a/src/http/modules/ngx_http_fastcgi_module.c +++ b/src/http/modules/ngx_http_fastcgi_module.c @@ -211,6 +211,7 @@ static ngx_conf_bitmask_t ngx_http_fast { ngx_string("http_503"), NGX_HTTP_UPSTREAM_FT_HTTP_503 }, { ngx_string("http_403"), NGX_HTTP_UPSTREAM_FT_HTTP_403 }, { ngx_string("http_404"), NGX_HTTP_UPSTREAM_FT_HTTP_404 }, +{ ngx_string("http_429"), NGX_HTTP_UPSTREAM_FT_HTTP_429 }, { ngx_string("updating"), NGX_HTTP_UPSTREAM_FT_UPDATING }, { ngx_string("off"), NGX_HTTP_UPSTREAM_FT_OFF }, { ngx_null_string, 0 } diff -r 799ef976b58c -r b377cfedf632 src/http/modules/ngx_http_proxy_module.c --- a/src/http/modules/ngx_http_proxy_module.c +++ b/src/http/modules/ngx_http_proxy_module.c @@ -220,6 +220,7 @@ static ngx_conf_bitmask_t ngx_http_prox { ngx_string("http_504"), NGX_HTTP_UPSTREAM_FT_HTTP_504 }, { ngx_string("http_403"), NGX_HTTP_UPSTREAM_FT_HTTP_403 }, { ngx_string("http_404"), NGX_HTTP_UPSTREAM_FT_HTTP_404 }, +{ ngx_string("http_429"), NGX_HTTP_UPSTREAM_FT_HTTP_429 }, { ngx_string("updating"), NGX_HTTP_UPSTREAM_FT_UPDATING }, { ngx_string("off"), NGX_HTTP_UPSTREAM_FT_OFF }, { ngx_null_string, 0 } diff -r 799ef976b58c -r b377cfedf632 src/http/modules/ngx_http_scgi_module.c --- a/src/http/modules/ngx_http_scgi_module.c +++ b/src/http/modules/ngx_http_scgi_module.c @@ -82,6 +82,7 @@ static ngx_conf_bitmask_t ngx_http_scgi_ { ngx_string("http_503"), NGX_HTTP_UPSTREAM_FT_HTTP_503 }, { ngx_string("http_403"), NGX_HTTP_UPSTREAM_FT_HTTP_403 }, { ngx_string("http_404"), NGX_HTTP_UPSTREAM_FT_HTTP_404 }, +{ ngx_string("http_429"), NGX_HTTP_UPSTREAM_FT_HTTP_429 }, { ngx_string("updating"), NGX_HTTP_UPSTREAM_FT_UPDATING }, { ngx_string("off"), NGX_HTTP_UPSTREAM_FT_OFF }, { ngx_null_string, 0 } diff -r 799ef976b58c -r b377cfedf632 src/http/modules/ngx_http_uwsgi_module.c --- a/src/http/modules/ngx_http_uwsgi_module.c +++ b/src/http/modules/ngx_http_uwsgi_module.c @@ -114,6 +114,7 @@ static ngx_conf_bitmask_t ngx_http_uwsgi { ngx_string("http_503"), NGX_HTTP_UPSTREAM_FT_HTTP_503 }, { ngx_string("http_403"), NGX_HTTP_UPSTREAM_FT_HTTP_403 }, { ngx_string("http_404"), NGX_HTTP_UPSTREAM_FT_HTTP_404 }, +{ ngx_string("http_429"), NGX_HTTP_UPSTREAM_FT_HTTP_429 }, { ngx_string("updating"), NGX_HTTP_UPSTREAM_FT_UPDATING }, { ngx_string("off"), NGX_HTTP_UPSTREAM_FT_OFF }, { ngx_null_string, 0 } diff -r 799ef976b58c -r b377cfedf632 src/http/ngx_http_upstream.c --- a/src/http/ngx_http_upstream.c +++ b/src/http/ngx_http_upstream.c @@ -436,6 +436,7 @@ static ngx_http_upstream_next_t ngx_htt { 504, NGX_HTTP_UPSTREAM_FT_HTTP_504 }, { 403, NGX_HTTP_UPSTREAM_FT_HTTP_403 }, { 404, NGX_HTTP_UPSTREAM_FT_HTTP_404 }, +{ 429, NGX_HTTP_UPSTREAM_FT_HTTP_429 }, { 0, 0 } }; @@ -4155,6 +4156,10 @@ ngx_http_upstream_next(ngx_http_request_ status = NGX_HTTP_NOT_FOUND; break; +case NGX_HTTP_UPSTREAM_FT_HTTP_429: +status = NGX_HTTP_TOO_MANY_REQUESTS; +break; + /* * NGX_HTTP_UPSTREAM_FT_BUSY_LOCK and NGX_HTTP_UPSTREAM_FT_MAX_WAITING * never reach here diff -r 799ef976b58c -r b377cfedf632 src/http/ngx_http_upstream.h --- a/src/http/ngx_http_upstream.h +++ b/src/http/ngx_http_upstream.h @@ -26,10 +26,11 @@ #define NGX_HTTP_UPSTREAM_FT_HTTP_5040x0080 #define NGX_HTTP_UPSTREAM_FT_HTTP_4030x0100 #define NGX_HTTP_UPSTREAM_FT_HTTP_4040x0200 -#define NGX_HTTP_UPSTREAM_FT_UPDATING0x0400 -#define NGX_HTTP_UPSTREAM_FT_BUSY_LOCK 0x0800 -#define NGX_HTTP_UPSTREAM_FT_MAX_WAITING 0x1000 -#define NGX_HTTP_UPSTREAM_FT_NON_IDEMPOTENT 0x2000 +#define NGX_HTTP_UPSTREAM_FT_HTTP_4290x0400 +#define NGX_HTTP_UPSTREAM_FT_UPDATING0x0800 +#define NGX_HTTP_UPSTREAM_FT_BUSY_LOCK 0x1000 +#define NGX_HTTP_UPSTREAM_FT_MAX_WAITING 0x2000 +#define NGX_HTTP_UPSTREAM_FT_NON_IDEMPOTENT 0x4000 #define NGX_HTTP_UPSTREAM_FT_NOLIVE 0x4000 #define NGX_HTTP_UPSTREAM_FT_OFF 0x8000 @@ -38,7 +39,8 @@ |NGX_HTTP_UPSTREAM_FT_HTTP_503 \ |NGX_HTTP_UPSTREAM_FT_HTTP_504 \ |NGX_HTTP_UPSTREAM_FT_HTTP_403 \ -
Re: [PATCH 2 of 3] Upstream: allow recovery from "429 Too Many Requests" response
Hey Maxim, > Sure, but why one would use "proxy_next_upstream http_429" then? > > If one of your backends reject a requests based on client's IP / > login, then you probably don't want nginx to retry such a request > on other servers, as this will just allow the user to do more > requests when you already know the limit was reached. And it > doesn't look like an effective way to build a system with > distributed limits. > > In contrast, if a limit affects nginx's IP and/or group of > services on a backend, retrying on a different backend may make > sense. But use case suggests that 429 should be counted as > failure. That's a good point. Fixed, thanks! Best regards, Piotr Sikora ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH 2 of 3] Headers filter: add "add_trailer" directive
# HG changeset patch # User Piotr Sikora # Date 1490351854 25200 # Fri Mar 24 03:37:34 2017 -0700 # Node ID 6d878f4585b977053a9d00053bbb7aff263eb96c # Parent a0f291f0a903b863161900f4d0cbb405f5d7a735 Headers filter: add "add_trailer" directive. Trailers added using this directive are evaluated after response body is processed by output filters (but before it's written to the wire), so it's possible to use variables calculated from the response body as the trailer value. Signed-off-by: Piotr Sikora diff -r a0f291f0a903 -r 6d878f4585b9 src/http/modules/ngx_http_chunked_filter_module.c --- a/src/http/modules/ngx_http_chunked_filter_module.c +++ b/src/http/modules/ngx_http_chunked_filter_module.c @@ -256,6 +256,10 @@ ngx_http_chunked_create_trailers(ngx_htt ngx_table_elt_t*header; ngx_http_chunked_filter_ctx_t *ctx; +if (ngx_http_eval_trailers(r) != NGX_OK) { +return NULL; +} + len = 0; part = &r->headers_out.trailers.part; diff -r a0f291f0a903 -r 6d878f4585b9 src/http/modules/ngx_http_headers_filter_module.c --- a/src/http/modules/ngx_http_headers_filter_module.c +++ b/src/http/modules/ngx_http_headers_filter_module.c @@ -48,6 +48,7 @@ typedef struct { time_t expires_time; ngx_http_complex_value_t *expires_value; ngx_array_t *headers; +ngx_array_t *trailers; } ngx_http_headers_conf_t; @@ -72,6 +73,8 @@ static char *ngx_http_headers_expires(ng void *conf); static char *ngx_http_headers_add(ngx_conf_t *cf, ngx_command_t *cmd, void *conf); +static char *ngx_http_headers_add_trailer(ngx_conf_t *cf, ngx_command_t *cmd, +void *conf); static ngx_http_set_header_t ngx_http_set_headers[] = { @@ -108,6 +111,14 @@ static ngx_command_t ngx_http_headers_f 0, NULL}, +{ ngx_string("add_trailer"), + NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_HTTP_LIF_CONF +|NGX_CONF_TAKE23, + ngx_http_headers_add_trailer, + NGX_HTTP_LOC_CONF_OFFSET, + 0, + NULL}, + ngx_null_command }; @@ -149,15 +160,24 @@ static ngx_http_output_header_filter_pt static ngx_int_t ngx_http_headers_filter(ngx_http_request_t *r) { -ngx_str_t value; -ngx_uint_ti, safe_status; -ngx_http_header_val_t*h; -ngx_http_headers_conf_t *conf; +u_char*p, *data; +size_t len; +ngx_str_t value; +ngx_uint_t i, safe_status; +ngx_table_elt_t *t; +ngx_http_header_val_t *h; +ngx_http_headers_conf_t *conf; +ngx_http_core_loc_conf_t *clcf; + +if (r != r->main) { +return ngx_http_next_header_filter(r); +} conf = ngx_http_get_module_loc_conf(r, ngx_http_headers_filter_module); -if ((conf->expires == NGX_HTTP_EXPIRES_OFF && conf->headers == NULL) -|| r != r->main) +if (conf->expires == NGX_HTTP_EXPIRES_OFF +&& conf->headers == NULL +&& conf->trailers == NULL) { return ngx_http_next_header_filter(r); } @@ -205,6 +225,84 @@ ngx_http_headers_filter(ngx_http_request } } +if (conf->trailers && r->allow_trailers) { + +if (r->http_version < NGX_HTTP_VERSION_20) { +if (r->header_only +|| r->headers_out.status == NGX_HTTP_NOT_MODIFIED +|| r->headers_out.status == NGX_HTTP_NO_CONTENT +|| r->headers_out.status < NGX_HTTP_OK +|| r->method == NGX_HTTP_HEAD) +{ + return ngx_http_next_header_filter(r); +} + +clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module); + +if (!clcf->chunked_transfer_encoding) { +return ngx_http_next_header_filter(r); +} +} + +len = 0; + +h = conf->trailers->elts; +for (i = 0; i < conf->trailers->nelts; i++) { + +if (!safe_status && !h[i].always) { +continue; +} + +if (h[i].value.value.len) { +len += h[i].key.len + sizeof(", ") - 1; +} +} + +if (len == 0) { +return ngx_http_next_header_filter(r); +} + +len -= sizeof(", ") - 1; + +t = ngx_list_push(&r->headers_out.headers); +if (t == NULL) { +return NGX_ERROR; +} + +data = ngx_pnalloc(r->pool, len); +if (data == NULL) { +return NGX_ERROR; +} + +p = data; + +h = conf->trailers->elts; +for (i = 0; i < conf->trailers->nelts; i++) { + +if (!safe_status && !h[i].always) { +continue; +} + +if (h[i].value.value.len) { +p = ngx_copy(p, h[i].key.data, h[i].key.len); + +if (p == data + len) { +
[PATCH 3 of 3] Upstream: add support for trailers in HTTP responses
# HG changeset patch # User Piotr Sikora # Date 1490351854 25200 # Fri Mar 24 03:37:34 2017 -0700 # Node ID 94e49d08e3b4de32416f5841e38a2388e092191d # Parent 6d878f4585b977053a9d00053bbb7aff263eb96c Upstream: add support for trailers in HTTP responses. Please note that due to how upstream module terminates processing of responses that cannot have message body (responses to HEAD requests, and responses with 1xx, 204 and 304 status codes), trailers of those responses won't be passed to the downstream. This change also adds $upstream_trailer_* variables. Signed-off-by: Piotr Sikora diff -r 6d878f4585b9 -r 94e49d08e3b4 src/http/modules/ngx_http_fastcgi_module.c --- a/src/http/modules/ngx_http_fastcgi_module.c +++ b/src/http/modules/ngx_http_fastcgi_module.c @@ -2784,10 +2784,10 @@ ngx_http_fastcgi_create_loc_conf(ngx_con conf->upstream.intercept_errors = NGX_CONF_UNSET; -/* "fastcgi_cyclic_temp_file" is disabled */ +/* the hardcoded values */ conf->upstream.cyclic_temp_file = 0; - conf->upstream.change_buffering = 1; +conf->upstream.pass_trailers = 0; conf->catch_stderr = NGX_CONF_UNSET_PTR; diff -r 6d878f4585b9 -r 94e49d08e3b4 src/http/modules/ngx_http_memcached_module.c --- a/src/http/modules/ngx_http_memcached_module.c +++ b/src/http/modules/ngx_http_memcached_module.c @@ -619,6 +619,7 @@ ngx_http_memcached_create_loc_conf(ngx_c conf->upstream.pass_request_headers = 0; conf->upstream.pass_request_body = 0; conf->upstream.force_ranges = 1; +conf->upstream.pass_trailers = 0; conf->index = NGX_CONF_UNSET; conf->gzip_flag = NGX_CONF_UNSET_UINT; diff -r 6d878f4585b9 -r 94e49d08e3b4 src/http/modules/ngx_http_proxy_module.c --- a/src/http/modules/ngx_http_proxy_module.c +++ b/src/http/modules/ngx_http_proxy_module.c @@ -2886,11 +2886,12 @@ ngx_http_proxy_create_loc_conf(ngx_conf_ conf->ssl_passwords = NGX_CONF_UNSET_PTR; #endif -/* "proxy_cyclic_temp_file" is disabled */ +/* the hardcoded values */ conf->upstream.cyclic_temp_file = 0; +conf->upstream.change_buffering = 1; +conf->upstream.pass_trailers = 0; conf->redirect = NGX_CONF_UNSET; -conf->upstream.change_buffering = 1; conf->cookie_domains = NGX_CONF_UNSET_PTR; conf->cookie_paths = NGX_CONF_UNSET_PTR; diff -r 6d878f4585b9 -r 94e49d08e3b4 src/http/modules/ngx_http_scgi_module.c --- a/src/http/modules/ngx_http_scgi_module.c +++ b/src/http/modules/ngx_http_scgi_module.c @@ -1234,10 +1234,10 @@ ngx_http_scgi_create_loc_conf(ngx_conf_t conf->upstream.intercept_errors = NGX_CONF_UNSET; -/* "scgi_cyclic_temp_file" is disabled */ +/* the hardcoded values */ conf->upstream.cyclic_temp_file = 0; - conf->upstream.change_buffering = 1; +conf->upstream.pass_trailers = 0; ngx_str_set(&conf->upstream.module, "scgi"); diff -r 6d878f4585b9 -r 94e49d08e3b4 src/http/modules/ngx_http_uwsgi_module.c --- a/src/http/modules/ngx_http_uwsgi_module.c +++ b/src/http/modules/ngx_http_uwsgi_module.c @@ -1448,10 +1448,10 @@ ngx_http_uwsgi_create_loc_conf(ngx_conf_ conf->ssl_passwords = NGX_CONF_UNSET_PTR; #endif -/* "uwsgi_cyclic_temp_file" is disabled */ +/* the hardcoded values */ conf->upstream.cyclic_temp_file = 0; - conf->upstream.change_buffering = 1; +conf->upstream.pass_trailers = 0; ngx_str_set(&conf->upstream.module, "uwsgi"); diff -r 6d878f4585b9 -r 94e49d08e3b4 src/http/ngx_http_upstream.c --- a/src/http/ngx_http_upstream.c +++ b/src/http/ngx_http_upstream.c @@ -55,6 +55,8 @@ static ngx_int_t ngx_http_upstream_inter static ngx_int_t ngx_http_upstream_test_connect(ngx_connection_t *c); static ngx_int_t ngx_http_upstream_process_headers(ngx_http_request_t *r, ngx_http_upstream_t *u); +static ngx_int_t ngx_http_upstream_process_trailers(ngx_http_request_t *r, +ngx_http_upstream_t *u); static void ngx_http_upstream_process_body_in_memory(ngx_http_request_t *r, ngx_http_upstream_t *u); static void ngx_http_upstream_send_response(ngx_http_request_t *r, @@ -166,6 +168,8 @@ static ngx_int_t ngx_http_upstream_respo ngx_http_request_t *r, ngx_http_variable_value_t *v, uintptr_t data); static ngx_int_t ngx_http_upstream_header_variable(ngx_http_request_t *r, ngx_http_variable_value_t *v, uintptr_t data); +static ngx_int_t ngx_http_upstream_trailer_variable(ngx_http_request_t *r, +ngx_http_variable_value_t *v, uintptr_t data); static ngx_int_t ngx_http_upstream_cookie_variable(ngx_http_request_t *r, ngx_http_variable_value_t *v, uintptr_t data); @@ -422,6 +426,9 @@ static ngx_http_variable_t ngx_http_ups { ngx_string("upstream_http_"), NULL, ngx_http_upstream_header_variable, 0, NGX_HTTP_VAR_NOCACHEABLE|NGX_HTTP_VAR_PREFIX, 0 }, +{ ngx_string("upstream_trailer_"), NULL, ngx_http_upstream_trailer_variable, + 0, NGX_HTTP_VAR_NOCACHEABLE|NGX_HTTP_VAR_PREFIX, 0 }, + { ngx_string("upstream_cookie_"), NULL, ngx_
[PATCH 1 of 3] HTTP: add support for trailers in HTTP responses
# HG changeset patch # User Piotr Sikora # Date 1490351854 25200 # Fri Mar 24 03:37:34 2017 -0700 # Node ID a0f291f0a903b863161900f4d0cbb405f5d7a735 # Parent 39ff6939266e913e8bfd400e60f9520e70725a4d HTTP: add support for trailers in HTTP responses. Example: ngx_table_elt_t *h; h = ngx_list_push(&r->headers_out.trailers); if (h == NULL) { return NGX_ERROR; } ngx_str_set(&h->key, "Fun"); ngx_str_set(&h->value, "with trailers"); h->hash = ngx_hash_key_lc(h->key.data, h->key.len); The code above adds "Fun: with trailers" trailer to the response to the request with "TE: trailers" header (which indicates support for trailers). Modules that want to emit trailers must set r->expect_trailers = 1, otherwise they are going to be ignored. This change also adds $sent_trailer_* variables. Signed-off-by: Piotr Sikora diff -r 39ff6939266e -r a0f291f0a903 src/http/modules/ngx_http_chunked_filter_module.c --- a/src/http/modules/ngx_http_chunked_filter_module.c +++ b/src/http/modules/ngx_http_chunked_filter_module.c @@ -17,6 +17,7 @@ typedef struct { static ngx_int_t ngx_http_chunked_filter_init(ngx_conf_t *cf); +static ngx_chain_t *ngx_http_chunked_create_trailers(ngx_http_request_t *r); static ngx_http_module_t ngx_http_chunked_filter_module_ctx = { @@ -69,28 +70,33 @@ ngx_http_chunked_header_filter(ngx_http_ return ngx_http_next_header_filter(r); } -if (r->headers_out.content_length_n == -1) { +clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module); + +if (clcf->chunked_transfer_encoding +&& r->allow_trailers && r->expect_trailers) +{ +ngx_http_clear_content_length(r); +r->chunked = 1; + +} else if (r->headers_out.content_length_n == -1) { if (r->http_version < NGX_HTTP_VERSION_11) { r->keepalive = 0; +} else if (clcf->chunked_transfer_encoding) { +r->chunked = 1; + } else { -clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module); +r->keepalive = 0; +} +} -if (clcf->chunked_transfer_encoding) { -r->chunked = 1; +if (r->chunked) { +ctx = ngx_pcalloc(r->pool, sizeof(ngx_http_chunked_filter_ctx_t)); +if (ctx == NULL) { +return NGX_ERROR; +} -ctx = ngx_pcalloc(r->pool, - sizeof(ngx_http_chunked_filter_ctx_t)); -if (ctx == NULL) { -return NGX_ERROR; -} - -ngx_http_set_ctx(r, ctx, ngx_http_chunked_filter_module); - -} else { -r->keepalive = 0; -} -} +ngx_http_set_ctx(r, ctx, ngx_http_chunked_filter_module); } return ngx_http_next_header_filter(r); @@ -201,6 +207,15 @@ ngx_http_chunked_body_filter(ngx_http_re b->pos += 2; } +if (r->allow_trailers && r->expect_trailers) { +tl->next = ngx_http_chunked_create_trailers(r); + +if (tl->next != NULL) { +b->last -= 2; +b->last_buf = 0; +} +} + } else if (size > 0) { tl = ngx_chain_get_free_buf(r->pool, &ctx->free); if (tl == NULL) { @@ -230,6 +245,108 @@ ngx_http_chunked_body_filter(ngx_http_re } +static ngx_chain_t * +ngx_http_chunked_create_trailers(ngx_http_request_t *r) +{ +size_t len; +ngx_buf_t *b; +ngx_uint_t i; +ngx_chain_t*cl; +ngx_list_part_t*part; +ngx_table_elt_t*header; +ngx_http_chunked_filter_ctx_t *ctx; + +len = 0; + +part = &r->headers_out.trailers.part; +header = part->elts; + +for (i = 0; /* void */; i++) { + +if (i >= part->nelts) { +if (part->next == NULL) { +break; +} + +part = part->next; +header = part->elts; +i = 0; +} + +if (header[i].hash == 0) { +continue; +} + +len += header[i].key.len + sizeof(": ") - 1 + + header[i].value.len + sizeof(CRLF) - 1; +} + +if (len == 0) { +return NULL; +} + +len += sizeof(CRLF) - 1; + +ctx = ngx_http_get_module_ctx(r, ngx_http_chunked_filter_module); + +cl = ngx_chain_get_free_buf(r->pool, &ctx->free); +if (cl == NULL) { +return NULL; +} + +b = cl->buf; + +b->tag = (ngx_buf_tag_t) &ngx_http_chunked_filter_module; +b->temporary = 0; +b->memory = 1; +b->last_buf = 1; + +b->start = ngx_palloc(r->pool, len); +if (b->start == NULL) { +return NULL; +} + +b->end = b->last + len; +b->pos = b->start; +b->last = b->start; + +part = &r->headers_out.trailers.part; +header = part->elts; + +for (i = 0; /* void */; i++) { + +
[PATCH] HTTP/2: emit PROTOCOL_ERROR on padding errors
# HG changeset patch # User Piotr Sikora # Date 1490516705 25200 # Sun Mar 26 01:25:05 2017 -0700 # Node ID 8d3fb456411018e286345ba92a855ca42ca8af2f # Parent 22be63bf21edaa1b8ea916c7d8cd4e5fe4892061 HTTP/2: emit PROTOCOL_ERROR on padding errors. Signed-off-by: Piotr Sikora diff -r 22be63bf21ed -r 8d3fb4564110 src/http/v2/ngx_http_v2.c --- a/src/http/v2/ngx_http_v2.c +++ b/src/http/v2/ngx_http_v2.c @@ -810,7 +810,8 @@ ngx_http_v2_state_data(ngx_http_v2_conne "with incorrect length: %uz, padding: %uz", h2c->state.length, h2c->state.padding); -return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_SIZE_ERROR); +return ngx_http_v2_connection_error(h2c, +NGX_HTTP_V2_PROTOCOL_ERROR); } h2c->state.length -= h2c->state.padding; @@ -1053,7 +1054,8 @@ ngx_http_v2_state_headers(ngx_http_v2_co "with incorrect length: %uz, padding: %uz", h2c->state.length, h2c->state.padding); -return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_SIZE_ERROR); +return ngx_http_v2_connection_error(h2c, +NGX_HTTP_V2_PROTOCOL_ERROR); } h2c->state.length -= h2c->state.padding; ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH 2 of 2] HTTP/2: fix $body_bytes_sent variable
# HG changeset patch # User Piotr Sikora # Date 1490516703 25200 # Sun Mar 26 01:25:03 2017 -0700 # Node ID dd6c656ed7a327641b2ddfc34768f9551e44bb0f # Parent 74ee816e712ee3b731437947470383555653338d HTTP/2: fix $body_bytes_sent variable. Previously, its value included payloads and frame headers of HEADERS and CONTINUATION frames. Signed-off-by: Piotr Sikora diff -r 74ee816e712e -r dd6c656ed7a3 src/http/v2/ngx_http_v2_filter_module.c --- a/src/http/v2/ngx_http_v2_filter_module.c +++ b/src/http/v2/ngx_http_v2_filter_module.c @@ -1211,6 +1211,9 @@ ngx_http_v2_headers_frame_handler(ngx_ht "http2:%ui HEADERS frame %p was sent", stream->node->id, frame); +stream->request->header_size += NGX_HTTP_V2_FRAME_HEADER_SIZE ++ frame->length; + ngx_http_v2_handle_frame(stream, frame); ngx_http_v2_handle_stream(h2c, stream); ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] HTTP/2: emit PROTOCOL_ERROR on invalid ENABLE_PUSH setting value
# HG changeset patch # User Piotr Sikora # Date 1490516707 25200 # Sun Mar 26 01:25:07 2017 -0700 # Node ID 705897a463205ba00dce296ff49866c6b78fc6ee # Parent 22be63bf21edaa1b8ea916c7d8cd4e5fe4892061 HTTP/2: emit PROTOCOL_ERROR on invalid ENABLE_PUSH setting value. Signed-off-by: Piotr Sikora diff -r 22be63bf21ed -r 705897a46320 src/http/v2/ngx_http_v2.c --- a/src/http/v2/ngx_http_v2.c +++ b/src/http/v2/ngx_http_v2.c @@ -40,6 +40,7 @@ /* settings fields */ #define NGX_HTTP_V2_HEADER_TABLE_SIZE_SETTING0x1 +#define NGX_HTTP_V2_ENABLE_PUSH_SETTING 0x2 #define NGX_HTTP_V2_MAX_STREAMS_SETTING 0x3 #define NGX_HTTP_V2_INIT_WINDOW_SIZE_SETTING 0x4 #define NGX_HTTP_V2_MAX_FRAME_SIZE_SETTING 0x5 @@ -1993,6 +1994,19 @@ ngx_http_v2_state_settings_params(ngx_ht switch (id) { +case NGX_HTTP_V2_ENABLE_PUSH_SETTING: + +if (value != 0 && value != 1) { +ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0, + "client sent SETTINGS frame with incorrect " + "ENABLE_PUSH value %ui", value); + +return ngx_http_v2_connection_error(h2c, + NGX_HTTP_V2_PROTOCOL_ERROR); +} + +break; + case NGX_HTTP_V2_INIT_WINDOW_SIZE_SETTING: if (value > NGX_HTTP_V2_MAX_WINDOW) { ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH 1 of 2] HTTP/2: fix $bytes_sent variable
# HG changeset patch # User Piotr Sikora # Date 1490516702 25200 # Sun Mar 26 01:25:02 2017 -0700 # Node ID 74ee816e712ee3b731437947470383555653338d # Parent 22be63bf21edaa1b8ea916c7d8cd4e5fe4892061 HTTP/2: fix $bytes_sent variable. Previously, its value accounted for payloads of HEADERS, CONTINUATION and DATA frames, as well as frame headers of HEADERS and DATA frames, but it didn't account for frame headers of CONTINUATION frames. Signed-off-by: Piotr Sikora diff -r 22be63bf21ed -r 74ee816e712e src/http/v2/ngx_http_v2_filter_module.c --- a/src/http/v2/ngx_http_v2_filter_module.c +++ b/src/http/v2/ngx_http_v2_filter_module.c @@ -769,6 +769,8 @@ ngx_http_v2_create_headers_frame(ngx_htt rest -= frame_size; if (rest) { +frame->length += NGX_HTTP_V2_FRAME_HEADER_SIZE; + type = NGX_HTTP_V2_CONTINUATION_FRAME; flags = NGX_HTTP_V2_NO_FLAG; continue; ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] HTTP/2: style
# HG changeset patch # User Piotr Sikora # Date 1490516701 25200 # Sun Mar 26 01:25:01 2017 -0700 # Node ID a79510a00969e97331e8dbc8a423e0265115b2e4 # Parent 22be63bf21edaa1b8ea916c7d8cd4e5fe4892061 HTTP/2: style. Signed-off-by: Piotr Sikora diff -r 22be63bf21ed -r a79510a00969 src/http/v2/ngx_http_v2.c --- a/src/http/v2/ngx_http_v2.c +++ b/src/http/v2/ngx_http_v2.c @@ -938,7 +938,7 @@ ngx_http_v2_state_read_data(ngx_http_v2_ if (size >= h2c->state.length) { size = h2c->state.length; -stream->in_closed = h2c->state.flags & NGX_HTTP_V2_END_STREAM_FLAG; +stream->in_closed = h2c->state.flags & NGX_HTTP_V2_END_STREAM_FLAG; } r = stream->request; @@ -1901,7 +1901,7 @@ ngx_http_v2_state_rst_stream(ngx_http_v2 if (node == NULL || node->stream == NULL) { ngx_log_debug0(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0, -"unknown http2 stream"); + "unknown http2 stream"); return ngx_http_v2_state_complete(h2c, pos, end); } @@ -2015,6 +2015,7 @@ ngx_http_v2_state_settings_params(ngx_ht break; case NGX_HTTP_V2_MAX_FRAME_SIZE_SETTING: + if (value > NGX_HTTP_V2_MAX_FRAME_SIZE || value < NGX_HTTP_V2_DEFAULT_FRAME_SIZE) { diff -r 22be63bf21ed -r a79510a00969 src/http/v2/ngx_http_v2.h --- a/src/http/v2/ngx_http_v2.h +++ b/src/http/v2/ngx_http_v2.h @@ -249,8 +249,8 @@ ngx_http_v2_queue_blocked_frame(ngx_http { ngx_http_v2_out_frame_t **out; -for (out = &h2c->last_out; *out; out = &(*out)->next) -{ +for (out = &h2c->last_out; *out; out = &(*out)->next) { + if ((*out)->blocked || (*out)->stream == NULL) { break; } ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] HTTP/2: emit PROTOCOL_ERROR on invalid WINDOW_UPDATE increments
# HG changeset patch # User Piotr Sikora # Date 1490516706 25200 # Sun Mar 26 01:25:06 2017 -0700 # Node ID 9bbcacbdf6bd858a34a9dfd1ac2185eb8fc8c82f # Parent 22be63bf21edaa1b8ea916c7d8cd4e5fe4892061 HTTP/2: emit PROTOCOL_ERROR on invalid WINDOW_UPDATE increments. Signed-off-by: Piotr Sikora diff -r 22be63bf21ed -r 9bbcacbdf6bd src/http/v2/ngx_http_v2.c --- a/src/http/v2/ngx_http_v2.c +++ b/src/http/v2/ngx_http_v2.c @@ -2173,6 +2173,22 @@ ngx_http_v2_state_window_update(ngx_http stream = node->stream; +if (window == 0) { +ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0, + "client sent WINDOW_UPDATE frame for stream %ui " + "with incorrect window increment 0", h2c->state.sid); + +if (ngx_http_v2_terminate_stream(h2c, stream, + NGX_HTTP_V2_PROTOCOL_ERROR) +== NGX_ERROR) +{ +return ngx_http_v2_connection_error(h2c, + NGX_HTTP_V2_INTERNAL_ERROR); +} + +return ngx_http_v2_state_complete(h2c, pos, end); +} + if (window > (size_t) (NGX_HTTP_V2_MAX_WINDOW - stream->send_window)) { ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0, @@ -2211,6 +2227,14 @@ ngx_http_v2_state_window_update(ngx_http return ngx_http_v2_state_complete(h2c, pos, end); } +if (window == 0) { +ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0, + "client sent WINDOW_UPDATE frame " + "with incorrect window increment 0"); + +return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_PROTOCOL_ERROR); +} + if (window > NGX_HTTP_V2_MAX_WINDOW - h2c->send_window) { ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0, "client violated connection flow control: " ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] HTTP/2: fix flow control with padded DATA frames
# HG changeset patch # User Piotr Sikora # Date 1490516704 25200 # Sun Mar 26 01:25:04 2017 -0700 # Node ID 899a53d2789b8c6bafdd5e40d78b4e92dd32dd10 # Parent 22be63bf21edaa1b8ea916c7d8cd4e5fe4892061 HTTP/2: fix flow control with padded DATA frames. Previously, flow control didn't account for padding in DATA frames, which meant that its view of the world could drift from peer's view by up to 256 bytes per received padded DATA frame, which could lead to a deadlock. Signed-off-by: Piotr Sikora diff -r 22be63bf21ed -r 899a53d2789b src/http/v2/ngx_http_v2.c --- a/src/http/v2/ngx_http_v2.c +++ b/src/http/v2/ngx_http_v2.c @@ -783,9 +783,12 @@ ngx_http_v2_state_head(ngx_http_v2_conne static u_char * ngx_http_v2_state_data(ngx_http_v2_connection_t *h2c, u_char *pos, u_char *end) { +size_t size; ngx_http_v2_node_t*node; ngx_http_v2_stream_t *stream; +size = h2c->state.length; + if (h2c->state.flags & NGX_HTTP_V2_PADDED_FLAG) { if (h2c->state.length == 0) { @@ -802,33 +805,32 @@ ngx_http_v2_state_data(ngx_http_v2_conne } h2c->state.padding = *pos++; -h2c->state.length--; - -if (h2c->state.padding > h2c->state.length) { + +if (h2c->state.padding >= size) { ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0, "client sent padded DATA frame " "with incorrect length: %uz, padding: %uz", - h2c->state.length, h2c->state.padding); + size, h2c->state.padding); return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_SIZE_ERROR); } -h2c->state.length -= h2c->state.padding; +h2c->state.length -= 1 + h2c->state.padding; } ngx_log_debug0(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0, "http2 DATA frame"); -if (h2c->state.length > h2c->recv_window) { +if (size > h2c->recv_window) { ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0, "client violated connection flow control: " "received DATA frame length %uz, available window %uz", - h2c->state.length, h2c->recv_window); + size, h2c->recv_window); return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_FLOW_CTRL_ERROR); } -h2c->recv_window -= h2c->state.length; +h2c->recv_window -= size; if (h2c->recv_window < NGX_HTTP_V2_MAX_WINDOW / 4) { @@ -854,11 +856,11 @@ ngx_http_v2_state_data(ngx_http_v2_conne stream = node->stream; -if (h2c->state.length > stream->recv_window) { +if (size > stream->recv_window) { ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0, "client violated flow control for stream %ui: " "received DATA frame length %uz, available window %uz", - node->id, h2c->state.length, stream->recv_window); + node->id, size, stream->recv_window); if (ngx_http_v2_terminate_stream(h2c, stream, NGX_HTTP_V2_FLOW_CTRL_ERROR) @@ -871,7 +873,7 @@ ngx_http_v2_state_data(ngx_http_v2_conne return ngx_http_v2_state_skip_padded(h2c, pos, end); } -stream->recv_window -= h2c->state.length; +stream->recv_window -= size; if (stream->no_flow_control && stream->recv_window < NGX_HTTP_V2_MAX_WINDOW / 4) ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] HTTP/2: add logging of RST_STREAM frames with NO_ERROR code
# HG changeset patch # User Piotr Sikora # Date 1490516710 25200 # Sun Mar 26 01:25:10 2017 -0700 # Node ID 31dfcde3ea2ccf1a2dbd2601ebe8f4306887fc0f # Parent 22be63bf21edaa1b8ea916c7d8cd4e5fe4892061 HTTP/2: add logging of RST_STREAM frames with NO_ERROR code. Signed-off-by: Piotr Sikora diff -r 22be63bf21ed -r 31dfcde3ea2c src/http/v2/ngx_http_v2.c --- a/src/http/v2/ngx_http_v2.c +++ b/src/http/v2/ngx_http_v2.c @@ -1916,6 +1916,11 @@ ngx_http_v2_state_rst_stream(ngx_http_v2 switch (status) { +case NGX_HTTP_V2_NO_ERROR: +ngx_log_error(NGX_LOG_INFO, fc->log, 0, + "client closed stream %ui", h2c->state.sid); +break; + case NGX_HTTP_V2_CANCEL: ngx_log_error(NGX_LOG_INFO, fc->log, 0, "client canceled stream %ui", h2c->state.sid); ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] HTTP/2: reject HTTP/2 requests without ":scheme" pseudo-header
# HG changeset patch # User Piotr Sikora # Date 1490516708 25200 # Sun Mar 26 01:25:08 2017 -0700 # Node ID 6bb029b1df11662ba11e190490cf1ed175fcfaa6 # Parent 22be63bf21edaa1b8ea916c7d8cd4e5fe4892061 HTTP/2: reject HTTP/2 requests without ":scheme" pseudo-header. While there, fix typo in error logs about it. Signed-off-by: Piotr Sikora diff -r 22be63bf21ed -r 6bb029b1df11 src/http/v2/ngx_http_v2.c --- a/src/http/v2/ngx_http_v2.c +++ b/src/http/v2/ngx_http_v2.c @@ -3219,14 +3219,14 @@ ngx_http_v2_parse_scheme(ngx_http_reques { if (r->schema_start) { ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent duplicate :schema header"); + "client sent duplicate :scheme header"); return NGX_DECLINED; } if (header->value.len == 0) { ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent empty :schema header"); + "client sent empty :scheme header"); return NGX_DECLINED; } @@ -3291,6 +3291,7 @@ ngx_http_v2_construct_request_line(ngx_h static const u_char ending[] = " HTTP/2.0"; if (r->method_name.len == 0 +|| r->schema_start == NULL || r->unparsed_uri.len == 0) { ngx_http_finalize_request(r, NGX_HTTP_BAD_REQUEST); ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] HTTP/2: add debug logging of pseudo-headers and control frames
# HG changeset patch # User Piotr Sikora # Date 1490516711 25200 # Sun Mar 26 01:25:11 2017 -0700 # Node ID 6990fb6463ce47705e06ff6d0fbd9ae6696aeb37 # Parent 22be63bf21edaa1b8ea916c7d8cd4e5fe4892061 HTTP/2: add debug logging of pseudo-headers and control frames. Signed-off-by: Piotr Sikora diff -r 22be63bf21ed -r 6990fb6463ce src/http/v2/ngx_http_v2.c --- a/src/http/v2/ngx_http_v2.c +++ b/src/http/v2/ngx_http_v2.c @@ -1585,6 +1585,10 @@ ngx_http_v2_state_process_header(ngx_htt rc = ngx_http_v2_pseudo_header(r, header); if (rc == NGX_OK) { +ngx_log_debug2(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, + "http2 http header: \":%V: %V\"", + &header->name, &header->value); + return ngx_http_v2_state_header_complete(h2c, pos, end); } @@ -1955,6 +1959,9 @@ ngx_http_v2_state_settings(ngx_http_v2_c return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_SIZE_ERROR); } +ngx_log_debug0(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0, + "http2 SETTINGS frame ack:1"); + h2c->settings_ack = 1; return ngx_http_v2_state_complete(h2c, pos, end); @@ -1968,6 +1975,9 @@ ngx_http_v2_state_settings(ngx_http_v2_c return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_SIZE_ERROR); } +ngx_log_debug0(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0, + "http2 SETTINGS frame ack:0"); + ngx_http_v2_send_settings(h2c, 1); return ngx_http_v2_state_settings_params(h2c, pos, end); @@ -2070,12 +2080,16 @@ ngx_http_v2_state_ping(ngx_http_v2_conne } ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0, - "http2 PING frame, flags: %ud", h2c->state.flags); + "http2 PING frame ack:%ud", + h2c->state.flags & NGX_HTTP_V2_ACK_FLAG ? 1 : 0); if (h2c->state.flags & NGX_HTTP_V2_ACK_FLAG) { return ngx_http_v2_state_skip(h2c, pos, end); } +ngx_log_debug0(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0, + "http2 send PING frame ack:1"); + frame = ngx_http_v2_get_frame(h2c, NGX_HTTP_V2_PING_SIZE, NGX_HTTP_V2_PING_FRAME, NGX_HTTP_V2_ACK_FLAG, 0); ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] HTTP/2: add fast-path for HTTP/2 requests without request body
# HG changeset patch # User Piotr Sikora # Date 1490516712 25200 # Sun Mar 26 01:25:12 2017 -0700 # Node ID f9fd6a8babce9f57f038d304dc1eef82284dde8b # Parent 22be63bf21edaa1b8ea916c7d8cd4e5fe4892061 HTTP/2: add fast-path for HTTP/2 requests without request body. Signed-off-by: Piotr Sikora diff -r 22be63bf21ed -r f9fd6a8babce src/http/v2/ngx_http_v2.c --- a/src/http/v2/ngx_http_v2.c +++ b/src/http/v2/ngx_http_v2.c @@ -3494,7 +3494,9 @@ ngx_http_v2_read_request_body(ngx_http_r stream = r->stream; -if (stream->skip_data) { +if (stream->skip_data +|| (stream->in_closed && stream->preread == NULL)) +{ r->request_body_no_buffering = 0; post_handler(r); return NGX_OK; ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] HTTP/2: reject HTTP/2 requests with "Connection" header
# HG changeset patch # User Piotr Sikora # Date 1490516709 25200 # Sun Mar 26 01:25:09 2017 -0700 # Node ID b8daccea5fde213d4b7a10fa9f57070ab3b6a1ec # Parent 22be63bf21edaa1b8ea916c7d8cd4e5fe4892061 HTTP/2: reject HTTP/2 requests with "Connection" header. While there, populate r->headers_in.connection. Signed-off-by: Piotr Sikora diff -r 22be63bf21ed -r b8daccea5fde src/http/ngx_http_request.c --- a/src/http/ngx_http_request.c +++ b/src/http/ngx_http_request.c @@ -1659,6 +1659,22 @@ static ngx_int_t ngx_http_process_connection(ngx_http_request_t *r, ngx_table_elt_t *h, ngx_uint_t offset) { +if (r->headers_in.connection == NULL) { +r->headers_in.connection = h; +} + +#if (NGX_HTTP_V2) + +if (r->http_version >= NGX_HTTP_VERSION_20) { +ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, + "client sent HTTP/2 request with \"Connection\" header"); + +ngx_http_finalize_request(r, NGX_HTTP_BAD_REQUEST); +return NGX_ERROR; +} + +#endif + if (ngx_strcasestrn(h->value.data, "close", 5 - 1)) { r->headers_in.connection_type = NGX_HTTP_CONNECTION_CLOSE; ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] HTTP/2: fix flow control with padded DATA frames
Hey Valentin, >> @@ -802,33 +805,32 @@ ngx_http_v2_state_data(ngx_http_v2_conne >> } >> >> h2c->state.padding = *pos++; >> -h2c->state.length--; >> - >> -if (h2c->state.padding > h2c->state.length) { >> + >> +if (h2c->state.padding >= size) { >> ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0, >>"client sent padded DATA frame " >>"with incorrect length: %uz, padding: %uz", >> - h2c->state.length, h2c->state.padding); >> + size, h2c->state.padding); >> >> return ngx_http_v2_connection_error(h2c, >> NGX_HTTP_V2_SIZE_ERROR); >> } >> >> -h2c->state.length -= h2c->state.padding; >> +h2c->state.length -= 1 + h2c->state.padding; >> } > > IMHO, the previous version of this fragment with explicit h2c->state.length > decrement right after reading the padding size and "> h2c->state.length" > condition is more readable. > > YMMV. As was pointed out during internal code review, "h2c->state.padding >= size" follows wording from RFC7540, i.e. If the length of the padding is the length of the frame payload or greater, the recipient MUST treat this as a connection error (Section 5.4.1) of type PROTOCOL_ERROR. and matching code makes it easier to avoid off-by-one errors while mentally translating this logic to the code. Also, doing "h2c->state.padding > h2c->state.length" check and using "size" value in the error log feels kind of weird. Having said that, it's a matter of preference, so please explicitly ask if you want me to change it back, and I'll send updated patch shortly. Best regards, Piotr Sikora ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] HTTP/2: emit PROTOCOL_ERROR on invalid WINDOW_UPDATE increments
Hey Valentin, > I'm not sure that strictly following RFC here is worth the effort. > > It seems there's no other "harm" from zero window updates except that it > allows to reset timers without any progress. That's only slightly worse > than 1-bytes window updates. Flow control interoperability and deadlocks between various HTTP/2 implementations are the biggest issues with the protocol, so while there is no real harm in allowing 0 window updates, they indicate broken client, and resetting stream and/or connection as soon as such thing happens makes it much easier to find issues. > The downside is additional code and intolerance to potential client bugs. Catching client bugs early on is the upside, IMHO. > Also note that in your implementation if zero window update is received > for unknown stream then it's silently ignored. Good catch, thanks! I'll send fixed version shortly. Best regards, Piotr Sikora ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] HTTP/2: emit PROTOCOL_ERROR on invalid ENABLE_PUSH setting value
Hey Valentin, > Is there any practical reason for the check considering that > the value is ignored anyway? None, other than following RFC and providing early detection of broken clients. Best regards, Piotr Sikora ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] HTTP/2: reject HTTP/2 requests without ":scheme" pseudo-header
Hey Valentin, > This part of patch can be added to the style one: > > "HTTP/2: style and typos." Assuming that this patch gets dropped or even if it gets committed? > Is there any practical reason to force this restriction? None, other than following RFC and providing early detection of broken clients. Also, it looks that you have a test for it marked as TODO: http://hg.nginx.org/nginx-tests/annotate/a6abbfed42c0/h2_headers.t#l972 Best regards, Piotr Sikora ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] HTTP/2: emit PROTOCOL_ERROR on invalid WINDOW_UPDATE increments
# HG changeset patch # User Piotr Sikora # Date 1490516706 25200 # Sun Mar 26 01:25:06 2017 -0700 # Node ID ccb36c87291e38d1a63224d143cbeaa4ee4a4287 # Parent 22be63bf21edaa1b8ea916c7d8cd4e5fe4892061 HTTP/2: emit PROTOCOL_ERROR on invalid WINDOW_UPDATE increments. Signed-off-by: Piotr Sikora diff -r 22be63bf21ed -r ccb36c87291e src/http/v2/ngx_http_v2.c --- a/src/http/v2/ngx_http_v2.c +++ b/src/http/v2/ngx_http_v2.c @@ -2168,11 +2168,42 @@ ngx_http_v2_state_window_update(ngx_http ngx_log_debug0(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0, "unknown http2 stream"); +if (window == 0) { +ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0, + "client sent WINDOW_UPDATE frame for unknown " + "stream %ui with incorrect window increment 0", + h2c->state.sid); + +if (ngx_http_v2_send_rst_stream(h2c, h2c->state.sid, +NGX_HTTP_V2_PROTOCOL_ERROR) +== NGX_ERROR) +{ +return ngx_http_v2_connection_error(h2c, + NGX_HTTP_V2_INTERNAL_ERROR); +} +} + return ngx_http_v2_state_complete(h2c, pos, end); } stream = node->stream; +if (window == 0) { +ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0, + "client sent WINDOW_UPDATE frame for stream %ui " + "with incorrect window increment 0", h2c->state.sid); + +if (ngx_http_v2_terminate_stream(h2c, stream, + NGX_HTTP_V2_PROTOCOL_ERROR) +== NGX_ERROR) +{ +return ngx_http_v2_connection_error(h2c, + NGX_HTTP_V2_INTERNAL_ERROR); +} + +return ngx_http_v2_state_complete(h2c, pos, end); +} + if (window > (size_t) (NGX_HTTP_V2_MAX_WINDOW - stream->send_window)) { ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0, @@ -2211,6 +2242,14 @@ ngx_http_v2_state_window_update(ngx_http return ngx_http_v2_state_complete(h2c, pos, end); } +if (window == 0) { +ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0, + "client sent WINDOW_UPDATE frame " + "with incorrect window increment 0"); + +return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_PROTOCOL_ERROR); +} + if (window > NGX_HTTP_V2_MAX_WINDOW - h2c->send_window) { ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0, "client violated connection flow control: " ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] HTTP/2: emit PROTOCOL_ERROR on invalid WINDOW_UPDATE increments
Hey Valentin, > Here's my version of the patch. > It's made similar to ngx_http_v2_state_priority(). > > # HG changeset patch > # User Valentin Bartenev > # Date 1490721720 -10800 > # Tue Mar 28 20:22:00 2017 +0300 > # Node ID 3e798c552767068056c0251d7b6bd9ffd2587fc0 > # Parent ce37362a7a70c0acd14ba01c8c2223b366b62233 > HTTP/2: rejecting zero WINDOW_UPDATE with PROTOCOL_ERROR. > > It's required by RFC 7540. While there is no real harm from such frames, > that should help to detect broken clients. > > Prodded by Piotr Sikora. s/Prodded/Based on a patch/. > diff -r ce37362a7a70 -r 3e798c552767 src/http/v2/ngx_http_v2.c > --- a/src/http/v2/ngx_http_v2.c Tue Mar 28 18:15:42 2017 +0300 > +++ b/src/http/v2/ngx_http_v2.c Tue Mar 28 20:22:00 2017 +0300 > @@ -2161,6 +2161,40 @@ ngx_http_v2_state_window_update(ngx_http > "http2 WINDOW_UPDATE frame sid:%ui window:%uz", > h2c->state.sid, window); > > +if (window == 0) { > +ngx_log_error(NGX_LOG_INFO, h2c->connection->log, 0, > + "client sent WINDOW_UPDATE frame " > + "with incorrect window increment 0"); I don't think that omitting Stream ID is a good idea. Looks good, otherwise. Best regards, Piotr Sikora ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] HTTP/2: add logging of RST_STREAM frames with NO_ERROR code
Hey Valentin, > Currently such frames are logged with message: > > "client terminated stream %ui with status 0" > > Could you explain why NO_ERROR needs special handling here? The same reason that "client canceled stream %ui" does, it's more appropriate error message. > I haven't found in RFC mentions about cases in which clients can > send RST_STREAM with NO_ERROR. I believe that clients can send RST_STREAM with NO_ERROR when they are satisfied with the response they received, but didn't receive END_STREAM flag yet. One example of such behavior might be a client that received all DATA frames (as indicated by Content-Length or other application-level mechanism) but is still waiting on HEADERS frame (with trailers), that it doesn't care about. Note that this is more theoretical scenario and I'm not aware of any popular clients doing this right now. Best regards, Piotr Sikora ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] HTTP/2: reject HTTP/2 requests without ":scheme" pseudo-header
Hey Valentin, > IMHO it's not a good idea to combine style fixes with behavior changes. > Behavior changing commits are occasionally reverted. Fair enough, I'll update both patches shortly. > That's why it's still TODO (in other words intentionally skipped). > We discussed it with QA and decided to be more tolerant here. I disagree. Forgiving implementations that allow broken clients to seemingly "work", even when said clients are not obeying the specification, are the reason why we have broken clients in the first place. Best regards, Piotr Sikora ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] HTTP/2: style and typos
# HG changeset patch # User Piotr Sikora # Date 1490516701 25200 # Sun Mar 26 01:25:01 2017 -0700 # Node ID c76c2cedb2b2a1af16d77448e81801954713961f # Parent 22be63bf21edaa1b8ea916c7d8cd4e5fe4892061 HTTP/2: style and typos. Signed-off-by: Piotr Sikora diff -r 22be63bf21ed -r c76c2cedb2b2 src/http/v2/ngx_http_v2.c --- a/src/http/v2/ngx_http_v2.c +++ b/src/http/v2/ngx_http_v2.c @@ -938,7 +938,7 @@ ngx_http_v2_state_read_data(ngx_http_v2_ if (size >= h2c->state.length) { size = h2c->state.length; -stream->in_closed = h2c->state.flags & NGX_HTTP_V2_END_STREAM_FLAG; +stream->in_closed = h2c->state.flags & NGX_HTTP_V2_END_STREAM_FLAG; } r = stream->request; @@ -1901,7 +1901,7 @@ ngx_http_v2_state_rst_stream(ngx_http_v2 if (node == NULL || node->stream == NULL) { ngx_log_debug0(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0, -"unknown http2 stream"); + "unknown http2 stream"); return ngx_http_v2_state_complete(h2c, pos, end); } @@ -2015,6 +2015,7 @@ ngx_http_v2_state_settings_params(ngx_ht break; case NGX_HTTP_V2_MAX_FRAME_SIZE_SETTING: + if (value > NGX_HTTP_V2_MAX_FRAME_SIZE || value < NGX_HTTP_V2_DEFAULT_FRAME_SIZE) { @@ -3072,7 +3073,7 @@ ngx_http_v2_pseudo_header(ngx_http_reque } ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent unknown pseudo header \"%V\"", + "client sent unknown pseudo-header \":%V\"", &header->name); return NGX_DECLINED; @@ -3219,14 +3220,14 @@ ngx_http_v2_parse_scheme(ngx_http_reques { if (r->schema_start) { ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent duplicate :schema header"); + "client sent duplicate :scheme header"); return NGX_DECLINED; } if (header->value.len == 0) { ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, - "client sent empty :schema header"); + "client sent empty :scheme header"); return NGX_DECLINED; } diff -r 22be63bf21ed -r c76c2cedb2b2 src/http/v2/ngx_http_v2.h --- a/src/http/v2/ngx_http_v2.h +++ b/src/http/v2/ngx_http_v2.h @@ -249,8 +249,8 @@ ngx_http_v2_queue_blocked_frame(ngx_http { ngx_http_v2_out_frame_t **out; -for (out = &h2c->last_out; *out; out = &(*out)->next) -{ +for (out = &h2c->last_out; *out; out = &(*out)->next) { + if ((*out)->blocked || (*out)->stream == NULL) { break; } ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] HTTP/2: reject HTTP/2 requests without ":scheme" pseudo-header
# HG changeset patch # User Piotr Sikora # Date 1490516708 25200 # Sun Mar 26 01:25:08 2017 -0700 # Node ID 970f063ddc7aa1faa646514418abd6a9b2eff889 # Parent 22be63bf21edaa1b8ea916c7d8cd4e5fe4892061 HTTP/2: reject HTTP/2 requests without ":scheme" pseudo-header. Signed-off-by: Piotr Sikora diff -r 22be63bf21ed -r 970f063ddc7a src/http/v2/ngx_http_v2.c --- a/src/http/v2/ngx_http_v2.c +++ b/src/http/v2/ngx_http_v2.c @@ -3291,6 +3291,7 @@ ngx_http_v2_construct_request_line(ngx_h static const u_char ending[] = " HTTP/2.0"; if (r->method_name.len == 0 +|| r->schema_start == NULL || r->unparsed_uri.len == 0) { ngx_http_finalize_request(r, NGX_HTTP_BAD_REQUEST); ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] HTTP/2: add fast-path for HTTP/2 requests without request body
Hey Valentin, > That doesn't look like a correct patch to me as it changes behavior > of ngx_http_read_client_request_body() specifically for HTTP/2 case. Well, the behavior is already different, which is what this patch is trying to mitigate. In case of HTTP/1.x, a single buffer with headers is produced. In case of HTTP/2, two buffers are produced: one with headers and one empty with last_buf = 1. > Note, that in case of HTTP/1.x it always allocates r->request_body > for the main request unless r->discard_body is set. > > Even if it doesn't break something in nginx at the first glance, > there's always a chance that some 3rd-party modules can depend > on this. > > Anyway, this change should be made either for both protocols or > for none of them. Fair enough, I'll update patch to always allocate r->request_body. Best regards, Piotr Sikora ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] HTTP/2: add fast-path for HTTP/2 requests without request body
# HG changeset patch # User Piotr Sikora # Date 1490516712 25200 # Sun Mar 26 01:25:12 2017 -0700 # Node ID 630a8209defe25add7094dfc7b9bc9bcabe0933d # Parent 22be63bf21edaa1b8ea916c7d8cd4e5fe4892061 HTTP/2: add fast-path for HTTP/2 requests without request body. Signed-off-by: Piotr Sikora diff -r 22be63bf21ed -r 630a8209defe src/http/v2/ngx_http_v2.c --- a/src/http/v2/ngx_http_v2.c +++ b/src/http/v2/ngx_http_v2.c @@ -3520,6 +3520,12 @@ ngx_http_v2_read_request_body(ngx_http_r r->request_body = rb; +if (stream->in_closed && stream->preread == NULL) { +r->request_body_no_buffering = 0; +post_handler(r); +return NGX_OK; +} + h2scf = ngx_http_get_module_srv_conf(r, ngx_http_v2_module); clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module); ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] HTTP/2: reject HTTP/2 requests without ":scheme" pseudo-header
Hey Maxim, > How does google.com as a service behave with such clients? It sends RST_STREAM with PROTOCOL_ERROR. Best regards, Piotr Sikora ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH 1 of 3] HTTP: add support for trailers in HTTP responses
# HG changeset patch # User Piotr Sikora # Date 1490351854 25200 # Fri Mar 24 03:37:34 2017 -0700 # Node ID 8af81a0d66c0f69bcf501edcf10deed4c8f7fbd4 # Parent 39ff6939266e913e8bfd400e60f9520e70725a4d HTTP: add support for trailers in HTTP responses. Example: ngx_table_elt_t *h; h = ngx_list_push(&r->headers_out.trailers); if (h == NULL) { return NGX_ERROR; } ngx_str_set(&h->key, "Fun"); ngx_str_set(&h->value, "with trailers"); h->hash = ngx_hash_key_lc(h->key.data, h->key.len); The code above adds "Fun: with trailers" trailer to the response to the request with "TE: trailers" header (which indicates support for trailers). Modules that want to emit trailers must set r->expect_trailers = 1, otherwise they are going to be ignored. This change also adds $sent_trailer_* variables. Signed-off-by: Piotr Sikora diff -r 39ff6939266e -r 8af81a0d66c0 src/http/modules/ngx_http_chunked_filter_module.c --- a/src/http/modules/ngx_http_chunked_filter_module.c +++ b/src/http/modules/ngx_http_chunked_filter_module.c @@ -17,6 +17,7 @@ typedef struct { static ngx_int_t ngx_http_chunked_filter_init(ngx_conf_t *cf); +static ngx_chain_t *ngx_http_chunked_create_trailers(ngx_http_request_t *r); static ngx_http_module_t ngx_http_chunked_filter_module_ctx = { @@ -69,28 +70,33 @@ ngx_http_chunked_header_filter(ngx_http_ return ngx_http_next_header_filter(r); } -if (r->headers_out.content_length_n == -1) { +clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module); + +if (clcf->chunked_transfer_encoding +&& r->allow_trailers && r->expect_trailers) +{ +ngx_http_clear_content_length(r); +r->chunked = 1; + +} else if (r->headers_out.content_length_n == -1) { if (r->http_version < NGX_HTTP_VERSION_11) { r->keepalive = 0; +} else if (clcf->chunked_transfer_encoding) { +r->chunked = 1; + } else { -clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module); +r->keepalive = 0; +} +} -if (clcf->chunked_transfer_encoding) { -r->chunked = 1; +if (r->chunked) { +ctx = ngx_pcalloc(r->pool, sizeof(ngx_http_chunked_filter_ctx_t)); +if (ctx == NULL) { +return NGX_ERROR; +} -ctx = ngx_pcalloc(r->pool, - sizeof(ngx_http_chunked_filter_ctx_t)); -if (ctx == NULL) { -return NGX_ERROR; -} - -ngx_http_set_ctx(r, ctx, ngx_http_chunked_filter_module); - -} else { -r->keepalive = 0; -} -} +ngx_http_set_ctx(r, ctx, ngx_http_chunked_filter_module); } return ngx_http_next_header_filter(r); @@ -201,6 +207,15 @@ ngx_http_chunked_body_filter(ngx_http_re b->pos += 2; } +if (r->allow_trailers && r->expect_trailers) { +tl->next = ngx_http_chunked_create_trailers(r); + +if (tl->next != NULL) { +b->last -= 2; +b->last_buf = 0; +} +} + } else if (size > 0) { tl = ngx_chain_get_free_buf(r->pool, &ctx->free); if (tl == NULL) { @@ -230,6 +245,108 @@ ngx_http_chunked_body_filter(ngx_http_re } +static ngx_chain_t * +ngx_http_chunked_create_trailers(ngx_http_request_t *r) +{ +size_t len; +ngx_buf_t *b; +ngx_uint_t i; +ngx_chain_t*cl; +ngx_list_part_t*part; +ngx_table_elt_t*header; +ngx_http_chunked_filter_ctx_t *ctx; + +len = 0; + +part = &r->headers_out.trailers.part; +header = part->elts; + +for (i = 0; /* void */; i++) { + +if (i >= part->nelts) { +if (part->next == NULL) { +break; +} + +part = part->next; +header = part->elts; +i = 0; +} + +if (header[i].hash == 0) { +continue; +} + +len += header[i].key.len + sizeof(": ") - 1 + + header[i].value.len + sizeof(CRLF) - 1; +} + +if (len == 0) { +return NULL; +} + +len += sizeof(CRLF) - 1; + +ctx = ngx_http_get_module_ctx(r, ngx_http_chunked_filter_module); + +cl = ngx_chain_get_free_buf(r->pool, &ctx->free); +if (cl == NULL) { +return NULL; +} + +b = cl->buf; + +b->tag = (ngx_buf_tag_t) &ngx_http_chunked_filter_module; +b->temporary = 0; +b->memory = 1; +b->last_buf = 1; + +b->start = ngx_palloc(r->pool, len); +if (b->start == NULL) { +return NULL; +} + +b->end = b->last + len; +b->pos = b->start; +b->last = b->start; + +part = &r->headers_out.trailers.part; +header = part->elts; + +for (i = 0; /* void */; i++) { + +
[PATCH 2 of 3] Headers filter: add "add_trailer" directive
# HG changeset patch # User Piotr Sikora # Date 1490351854 25200 # Fri Mar 24 03:37:34 2017 -0700 # Node ID 5bab17ebe2b1f8ec42cf069bf484489c2a92c7a8 # Parent 8af81a0d66c0f69bcf501edcf10deed4c8f7fbd4 Headers filter: add "add_trailer" directive. Trailers added using this directive are evaluated after response body is processed by output filters (but before it's written to the wire), so it's possible to use variables calculated from the response body as the trailer value. Signed-off-by: Piotr Sikora diff -r 8af81a0d66c0 -r 5bab17ebe2b1 src/http/modules/ngx_http_chunked_filter_module.c --- a/src/http/modules/ngx_http_chunked_filter_module.c +++ b/src/http/modules/ngx_http_chunked_filter_module.c @@ -256,6 +256,10 @@ ngx_http_chunked_create_trailers(ngx_htt ngx_table_elt_t*header; ngx_http_chunked_filter_ctx_t *ctx; +if (ngx_http_eval_trailers(r) != NGX_OK) { +return NULL; +} + len = 0; part = &r->headers_out.trailers.part; diff -r 8af81a0d66c0 -r 5bab17ebe2b1 src/http/modules/ngx_http_headers_filter_module.c --- a/src/http/modules/ngx_http_headers_filter_module.c +++ b/src/http/modules/ngx_http_headers_filter_module.c @@ -48,6 +48,7 @@ typedef struct { time_t expires_time; ngx_http_complex_value_t *expires_value; ngx_array_t *headers; +ngx_array_t *trailers; } ngx_http_headers_conf_t; @@ -72,6 +73,8 @@ static char *ngx_http_headers_expires(ng void *conf); static char *ngx_http_headers_add(ngx_conf_t *cf, ngx_command_t *cmd, void *conf); +static char *ngx_http_headers_add_trailer(ngx_conf_t *cf, ngx_command_t *cmd, +void *conf); static ngx_http_set_header_t ngx_http_set_headers[] = { @@ -108,6 +111,14 @@ static ngx_command_t ngx_http_headers_f 0, NULL}, +{ ngx_string("add_trailer"), + NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_HTTP_LIF_CONF +|NGX_CONF_TAKE23, + ngx_http_headers_add_trailer, + NGX_HTTP_LOC_CONF_OFFSET, + 0, + NULL}, + ngx_null_command }; @@ -149,15 +160,24 @@ static ngx_http_output_header_filter_pt static ngx_int_t ngx_http_headers_filter(ngx_http_request_t *r) { -ngx_str_t value; -ngx_uint_ti, safe_status; -ngx_http_header_val_t*h; -ngx_http_headers_conf_t *conf; +u_char*p, *data; +size_t len; +ngx_str_t value; +ngx_uint_t i, safe_status; +ngx_table_elt_t *t; +ngx_http_header_val_t *h; +ngx_http_headers_conf_t *conf; +ngx_http_core_loc_conf_t *clcf; + +if (r != r->main) { +return ngx_http_next_header_filter(r); +} conf = ngx_http_get_module_loc_conf(r, ngx_http_headers_filter_module); -if ((conf->expires == NGX_HTTP_EXPIRES_OFF && conf->headers == NULL) -|| r != r->main) +if (conf->expires == NGX_HTTP_EXPIRES_OFF +&& conf->headers == NULL +&& conf->trailers == NULL) { return ngx_http_next_header_filter(r); } @@ -205,6 +225,84 @@ ngx_http_headers_filter(ngx_http_request } } +if (conf->trailers && r->allow_trailers) { + +if (r->http_version < NGX_HTTP_VERSION_20) { +if (r->header_only +|| r->headers_out.status == NGX_HTTP_NOT_MODIFIED +|| r->headers_out.status == NGX_HTTP_NO_CONTENT +|| r->headers_out.status < NGX_HTTP_OK +|| r->method == NGX_HTTP_HEAD) +{ + return ngx_http_next_header_filter(r); +} + +clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module); + +if (!clcf->chunked_transfer_encoding) { +return ngx_http_next_header_filter(r); +} +} + +len = 0; + +h = conf->trailers->elts; +for (i = 0; i < conf->trailers->nelts; i++) { + +if (!safe_status && !h[i].always) { +continue; +} + +if (h[i].value.value.len) { +len += h[i].key.len + sizeof(", ") - 1; +} +} + +if (len == 0) { +return ngx_http_next_header_filter(r); +} + +len -= sizeof(", ") - 1; + +t = ngx_list_push(&r->headers_out.headers); +if (t == NULL) { +return NGX_ERROR; +} + +data = ngx_pnalloc(r->pool, len); +if (data == NULL) { +return NGX_ERROR; +} + +p = data; + +h = conf->trailers->elts; +for (i = 0; i < conf->trailers->nelts; i++) { + +if (!safe_status && !h[i].always) { +continue; +} + +if (h[i].value.value.len) { +p = ngx_copy(p, h[i].key.data, h[i].key.len); + +if (p == data + len) { +
[PATCH 3 of 3] Upstream: add support for trailers in HTTP responses
# HG changeset patch # User Piotr Sikora # Date 1490351854 25200 # Fri Mar 24 03:37:34 2017 -0700 # Node ID 488c59bd49dcb1503144fe4d712165b69d1a5945 # Parent 5bab17ebe2b1f8ec42cf069bf484489c2a92c7a8 Upstream: add support for trailers in HTTP responses. Please note that due to how upstream module terminates processing of responses that cannot have message body (responses to HEAD requests, and responses with 1xx, 204 and 304 status codes), trailers of those responses won't be passed to the downstream. This change also adds $upstream_trailer_* variables. Signed-off-by: Piotr Sikora diff -r 5bab17ebe2b1 -r 488c59bd49dc src/http/modules/ngx_http_fastcgi_module.c --- a/src/http/modules/ngx_http_fastcgi_module.c +++ b/src/http/modules/ngx_http_fastcgi_module.c @@ -2784,10 +2784,10 @@ ngx_http_fastcgi_create_loc_conf(ngx_con conf->upstream.intercept_errors = NGX_CONF_UNSET; -/* "fastcgi_cyclic_temp_file" is disabled */ +/* the hardcoded values */ conf->upstream.cyclic_temp_file = 0; - conf->upstream.change_buffering = 1; +conf->upstream.pass_trailers = 0; conf->catch_stderr = NGX_CONF_UNSET_PTR; diff -r 5bab17ebe2b1 -r 488c59bd49dc src/http/modules/ngx_http_memcached_module.c --- a/src/http/modules/ngx_http_memcached_module.c +++ b/src/http/modules/ngx_http_memcached_module.c @@ -619,6 +619,7 @@ ngx_http_memcached_create_loc_conf(ngx_c conf->upstream.pass_request_headers = 0; conf->upstream.pass_request_body = 0; conf->upstream.force_ranges = 1; +conf->upstream.pass_trailers = 0; conf->index = NGX_CONF_UNSET; conf->gzip_flag = NGX_CONF_UNSET_UINT; diff -r 5bab17ebe2b1 -r 488c59bd49dc src/http/modules/ngx_http_proxy_module.c --- a/src/http/modules/ngx_http_proxy_module.c +++ b/src/http/modules/ngx_http_proxy_module.c @@ -2886,11 +2886,12 @@ ngx_http_proxy_create_loc_conf(ngx_conf_ conf->ssl_passwords = NGX_CONF_UNSET_PTR; #endif -/* "proxy_cyclic_temp_file" is disabled */ +/* the hardcoded values */ conf->upstream.cyclic_temp_file = 0; +conf->upstream.change_buffering = 1; +conf->upstream.pass_trailers = 0; conf->redirect = NGX_CONF_UNSET; -conf->upstream.change_buffering = 1; conf->cookie_domains = NGX_CONF_UNSET_PTR; conf->cookie_paths = NGX_CONF_UNSET_PTR; diff -r 5bab17ebe2b1 -r 488c59bd49dc src/http/modules/ngx_http_scgi_module.c --- a/src/http/modules/ngx_http_scgi_module.c +++ b/src/http/modules/ngx_http_scgi_module.c @@ -1234,10 +1234,10 @@ ngx_http_scgi_create_loc_conf(ngx_conf_t conf->upstream.intercept_errors = NGX_CONF_UNSET; -/* "scgi_cyclic_temp_file" is disabled */ +/* the hardcoded values */ conf->upstream.cyclic_temp_file = 0; - conf->upstream.change_buffering = 1; +conf->upstream.pass_trailers = 0; ngx_str_set(&conf->upstream.module, "scgi"); diff -r 5bab17ebe2b1 -r 488c59bd49dc src/http/modules/ngx_http_uwsgi_module.c --- a/src/http/modules/ngx_http_uwsgi_module.c +++ b/src/http/modules/ngx_http_uwsgi_module.c @@ -1448,10 +1448,10 @@ ngx_http_uwsgi_create_loc_conf(ngx_conf_ conf->ssl_passwords = NGX_CONF_UNSET_PTR; #endif -/* "uwsgi_cyclic_temp_file" is disabled */ +/* the hardcoded values */ conf->upstream.cyclic_temp_file = 0; - conf->upstream.change_buffering = 1; +conf->upstream.pass_trailers = 0; ngx_str_set(&conf->upstream.module, "uwsgi"); diff -r 5bab17ebe2b1 -r 488c59bd49dc src/http/ngx_http_upstream.c --- a/src/http/ngx_http_upstream.c +++ b/src/http/ngx_http_upstream.c @@ -55,6 +55,8 @@ static ngx_int_t ngx_http_upstream_inter static ngx_int_t ngx_http_upstream_test_connect(ngx_connection_t *c); static ngx_int_t ngx_http_upstream_process_headers(ngx_http_request_t *r, ngx_http_upstream_t *u); +static ngx_int_t ngx_http_upstream_process_trailers(ngx_http_request_t *r, +ngx_http_upstream_t *u); static void ngx_http_upstream_process_body_in_memory(ngx_http_request_t *r, ngx_http_upstream_t *u); static void ngx_http_upstream_send_response(ngx_http_request_t *r, @@ -149,6 +151,8 @@ static ngx_int_t ngx_http_upstream_rewri ngx_table_elt_t *h, ngx_uint_t offset); static ngx_int_t ngx_http_upstream_copy_allow_ranges(ngx_http_request_t *r, ngx_table_elt_t *h, ngx_uint_t offset); +static ngx_int_t ngx_http_upstream_copy_trailer(ngx_http_request_t *r, +ngx_table_elt_t *h, ngx_uint_t offset); #if (NGX_HTTP_GZIP) static ngx_int_t ngx_http_upstream_copy_content_encoding(ngx_http_request_t *r, @@ -166,6 +170,8 @@ static ngx_int_t ngx_http_upstream_respo ngx_http_request_t *r, ngx_http_variable_value_t *v, uintptr_t data); static ngx_int_t ngx_http_upstream_header_variable(ngx_http_request_t *r, ngx_http_variable_value_t *v, uintptr_t data); +static ngx_int_t ngx_http_upstream_trailer_variable(ngx_http_request_t *r, +ngx_http_variable_value_t *v, uintptr_t data); static ngx_int_t ngx_http_upstream_cookie_variable(ngx_http_request_t *r,
Re: [PATCH 3 of 3] Upstream: add support for trailers in HTTP responses
Hey, > +static ngx_int_t > +ngx_http_upstream_copy_trailer(ngx_http_request_t *r, > +ngx_table_elt_t *h, ngx_uint_t offset) > +{ > +ngx_table_elt_t *ho; > + > +if (!r->upstream->conf->pass_trailers > +|| !r->allow_trailers || !r->expect_trailers) > +{ > +return NGX_OK; > +} > + > +ho = ngx_list_push(&r->headers_out.headers); > +if (ho == NULL) { > +return NGX_ERROR; > +} > + > +*ho = *h; > + > +return NGX_OK; > +} Patch updated so that "Trailer" header is only forwarded with trailers. Best regards, Piotr Sikora ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] HTTP/2: add debug logging of control frames
# HG changeset patch # User Piotr Sikora # Date 1490516711 25200 # Sun Mar 26 01:25:11 2017 -0700 # Node ID 06d6418afe6e73604aea707ef9c5802f5bf27bf4 # Parent 22be63bf21edaa1b8ea916c7d8cd4e5fe4892061 HTTP/2: add debug logging of control frames. Signed-off-by: Piotr Sikora diff -r 22be63bf21ed -r 06d6418afe6e src/http/v2/ngx_http_v2.c --- a/src/http/v2/ngx_http_v2.c +++ b/src/http/v2/ngx_http_v2.c @@ -1955,6 +1955,9 @@ ngx_http_v2_state_settings(ngx_http_v2_c return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_SIZE_ERROR); } +ngx_log_debug0(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0, + "http2 SETTINGS frame ack:1"); + h2c->settings_ack = 1; return ngx_http_v2_state_complete(h2c, pos, end); @@ -1968,6 +1971,10 @@ ngx_http_v2_state_settings(ngx_http_v2_c return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_SIZE_ERROR); } +ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0, + "http2 SETTINGS frame params:%uz", + h2c->state.length / NGX_HTTP_V2_SETTINGS_PARAM_SIZE); + ngx_http_v2_send_settings(h2c, 1); return ngx_http_v2_state_settings_params(h2c, pos, end); @@ -2004,6 +2011,10 @@ ngx_http_v2_state_settings_params(ngx_ht NGX_HTTP_V2_FLOW_CTRL_ERROR); } +ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0, + "http2 SETTINGS frame INITIAL_WINDOW_SIZE:%ui", + value); + if (ngx_http_v2_adjust_windows(h2c, value - h2c->init_window) != NGX_OK) { @@ -2026,6 +2037,10 @@ ngx_http_v2_state_settings_params(ngx_ht NGX_HTTP_V2_PROTOCOL_ERROR); } +ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0, + "http2 SETTINGS frame MAX_FRAME_SIZE:%ui", + value); + h2c->frame_size = value; break; @@ -2070,12 +2085,16 @@ ngx_http_v2_state_ping(ngx_http_v2_conne } ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0, - "http2 PING frame, flags: %ud", h2c->state.flags); + "http2 PING frame ack:%ud", + h2c->state.flags & NGX_HTTP_V2_ACK_FLAG ? 1 : 0); if (h2c->state.flags & NGX_HTTP_V2_ACK_FLAG) { return ngx_http_v2_state_skip(h2c, pos, end); } +ngx_log_debug0(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0, + "http2 send PING frame ack:1"); + frame = ngx_http_v2_get_frame(h2c, NGX_HTTP_V2_PING_SIZE, NGX_HTTP_V2_PING_FRAME, NGX_HTTP_V2_ACK_FLAG, 0); @@ -2449,8 +2468,18 @@ ngx_http_v2_send_settings(ngx_http_v2_co ngx_http_v2_srv_conf_t *h2scf; ngx_http_v2_out_frame_t *frame; -ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0, - "http2 send SETTINGS frame ack:%ui", ack); +if (ack) { +ngx_log_debug0(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0, + "http2 send SETTINGS frame ack:1"); + +len = 0; + +} else { +ngx_log_debug0(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0, + "http2 send SETTINGS frame params:3"); + +len = NGX_HTTP_V2_SETTINGS_PARAM_SIZE * 3; +} frame = ngx_palloc(h2c->pool, sizeof(ngx_http_v2_out_frame_t)); if (frame == NULL) { @@ -2462,8 +2491,6 @@ ngx_http_v2_send_settings(ngx_http_v2_co return NGX_ERROR; } -len = ack ? 0 : (sizeof(uint16_t) + sizeof(uint32_t)) * 3; - buf = ngx_create_temp_buf(h2c->pool, NGX_HTTP_V2_FRAME_HEADER_SIZE + len); if (buf == NULL) { return NGX_ERROR; @@ -2494,15 +2521,27 @@ ngx_http_v2_send_settings(ngx_http_v2_co h2scf = ngx_http_get_module_srv_conf(h2c->http_connection->conf_ctx, ngx_http_v2_module); +ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0, + "http2 send SETTINGS frame MAX_CONCURRENT_STREAMS:%ui", + h2scf->concurrent_streams); + buf->last = ngx_http_v2_write_uint16(buf->last, NGX_HTTP_V2_MAX_STREAMS_SETTING); buf->last = ngx_http_v2_write_uint32(buf->last, h2scf->concurrent_streams); +ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0, + "http2 send SETTINGS frame INITIAL_WINDOW_SIZE:%uz", + h2scf->preread_size); + buf->last = ngx_http_v2_write_uint16(buf->last, NGX_HTTP_V2_INIT_WINDOW_SIZE_SETTING); buf->last = ngx_http_v2_write_uint32(buf->last, h2scf->preread_size); +ngx
[PATCH] HTTP/2: add debug logging of pseudo-headers
# HG changeset patch # User Piotr Sikora # Date 1490516711 25200 # Sun Mar 26 01:25:11 2017 -0700 # Node ID 3d72ae17c41990774721a678c50b8307ecb684c8 # Parent 22be63bf21edaa1b8ea916c7d8cd4e5fe4892061 HTTP/2: add debug logging of pseudo-headers. Signed-off-by: Piotr Sikora diff -r 22be63bf21ed -r 3d72ae17c419 src/http/v2/ngx_http_v2.c --- a/src/http/v2/ngx_http_v2.c +++ b/src/http/v2/ngx_http_v2.c @@ -1585,6 +1585,10 @@ ngx_http_v2_state_process_header(ngx_htt rc = ngx_http_v2_pseudo_header(r, header); if (rc == NGX_OK) { +ngx_log_debug2(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, + "http2 http header: \":%V: %V\"", + &header->name, &header->value); + return ngx_http_v2_state_header_complete(h2c, pos, end); } ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] HTTP/2: add debug logging of pseudo-headers and control frames
Hey, > # HG changeset patch > # User Piotr Sikora > # Date 1490516711 25200 > # Sun Mar 26 01:25:11 2017 -0700 > # Node ID 6990fb6463ce47705e06ff6d0fbd9ae6696aeb37 > # Parent 22be63bf21edaa1b8ea916c7d8cd4e5fe4892061 > HTTP/2: add debug logging of pseudo-headers and control frames. > > Signed-off-by: Piotr Sikora This was split into: http://mailman.nginx.org/pipermail/nginx-devel/2017-April/009767.html http://mailman.nginx.org/pipermail/nginx-devel/2017-April/009768.html Best regards, Piotr Sikora ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] HTTP/2: add fast-path for HTTP/2 requests without request body
Hey Valentin, > # HG changeset patch > # User Piotr Sikora > # Date 1490516712 25200 > # Sun Mar 26 01:25:12 2017 -0700 > # Node ID 630a8209defe25add7094dfc7b9bc9bcabe0933d > # Parent 22be63bf21edaa1b8ea916c7d8cd4e5fe4892061 > HTTP/2: add fast-path for HTTP/2 requests without request body. > > Signed-off-by: Piotr Sikora > > diff -r 22be63bf21ed -r 630a8209defe src/http/v2/ngx_http_v2.c > --- a/src/http/v2/ngx_http_v2.c > +++ b/src/http/v2/ngx_http_v2.c > @@ -3520,6 +3520,12 @@ ngx_http_v2_read_request_body(ngx_http_r > > r->request_body = rb; > > +if (stream->in_closed && stream->preread == NULL) { > +r->request_body_no_buffering = 0; > +post_handler(r); > +return NGX_OK; > +} > + > h2scf = ngx_http_get_module_srv_conf(r, ngx_http_v2_module); > clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module); > Any thoughts on the updated patch? Thanks! Best regards, Piotr Sikora ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH 1 of 3] HTTP: add support for trailers in HTTP responses
Hey, > # HG changeset patch > # User Piotr Sikora > # Date 1490351854 25200 > # Fri Mar 24 03:37:34 2017 -0700 > # Node ID 8af81a0d66c0f69bcf501edcf10deed4c8f7fbd4 > # Parent 39ff6939266e913e8bfd400e60f9520e70725a4d > HTTP: add support for trailers in HTTP responses. > > Example: > >ngx_table_elt_t *h; > >h = ngx_list_push(&r->headers_out.trailers); >if (h == NULL) { >return NGX_ERROR; >} > >ngx_str_set(&h->key, "Fun"); >ngx_str_set(&h->value, "with trailers"); >h->hash = ngx_hash_key_lc(h->key.data, h->key.len); > > The code above adds "Fun: with trailers" trailer to the response to > the request with "TE: trailers" header (which indicates support for > trailers). > > Modules that want to emit trailers must set r->expect_trailers = 1, > otherwise they are going to be ignored. > > This change also adds $sent_trailer_* variables. > > Signed-off-by: Piotr Sikora Ping. Best regards, Piotr Sikora ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] HTTP/2: add debug logging of pseudo-headers
Hey Valentin, > Maybe "http2 pseudo-header: \":%V: %V\""? > Because it doesn't look like a valid "http header". I was going back and forth between different versions, but I ended up using "http header", which matches rest of the headers and has the same alignment, because then pseudo-headers look like part of the same logical block, i.e. [debug] 1#1: *1 http2 http header: ":method: GET" [debug] 1#1: *1 http2 http header: ":path: /h2x" [debug] 1#1: *1 http2 http header: ":scheme: http" [debug] 1#1: *1 http2 http header: ":authority: 127.0.0.1" [debug] 1#1: *1 http2 http header: "accept: */*" [debug] 1#1: *1 http2 http header: "accept-encoding: gzip, deflate" [debug] 1#1: *1 http2 http header: "user-agent: nghttp2/1.21.0-DEV" vs [debug] 1#1: *1 http2 pseudo-header: ":method: GET" [debug] 1#1: *1 http2 pseudo-header: ":path: /h2x" [debug] 1#1: *1 http2 pseudo-header: ":scheme: http" [debug] 1#1: *1 http2 pseudo-header: ":authority: 127.0.0.1" [debug] 1#1: *1 http2 http header: "accept: */*" [debug] 1#1: *1 http2 http header: "accept-encoding: gzip, deflate" [debug] 1#1: *1 http2 http header: "user-agent: nghttp2/1.21.0-DEV" vs [debug] 1#1: *1 http2 http pseudo-header: ":method: GET" [debug] 1#1: *1 http2 http pseudo-header: ":path: /h2x" [debug] 1#1: *1 http2 http pseudo-header: ":scheme: http" [debug] 1#1: *1 http2 http pseudo-header: ":authority: 127.0.0.1" [debug] 1#1: *1 http2 http header: "accept: */*" [debug] 1#1: *1 http2 http header: "accept-encoding: gzip, deflate" [debug] 1#1: *1 http2 http header: "user-agent: nghttp2/1.21.0-DEV" The ":" prefix already indicates those are pseudo-headers, and because of different alignment, all the alternatives make them look like unrelated things. This might be just a matter of taste, though, so if you feel strongly about one of the alternatives, then I can change it, just please be explicit about it. Best regards, Piotr Sikora ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH] HTTP/2: add debug logging of control frames
Hey Valentin, > You can always find these values in configuration, and I can't remember a > case where > I've ever needed them. On the contrary, there's always a problem with the > huge size > of typical http/2 debug log. So it's not a good idea to add something just > because > we can. As someone who spent the last few days debugging HTTP/2 interoperability issues, looking at those debug logs, I strongly disagree. Also, NGINX's HTTP/2 debug log is really nice (other than the lack of "recv" prefix [1]) and can be used as a transcript of HTTP/2 exchange, which can be shared with people that don't use NGINX, in which case having SETTINGS values there is critical. Actually, I have local patch that adds even more debugging for SETTINGS frame (I'll squash it into this one and send to the mailing list shortly). [1] btw: Would you accept patch that adds "recv" prefix to all received frames? This would allow for easy "http2 send|http2 recv" grep for HTTP/2 transcript, and would align frame recv/send messages. Best regards, Piotr Sikora ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] HTTP/2: add debug logging of control frames
# HG changeset patch # User Piotr Sikora # Date 1490516711 25200 # Sun Mar 26 01:25:11 2017 -0700 # Node ID 7414a1467d0684a73d091c508834973b944890cd # Parent 22be63bf21edaa1b8ea916c7d8cd4e5fe4892061 HTTP/2: add debug logging of control frames. Signed-off-by: Piotr Sikora diff -r 22be63bf21ed -r 7414a1467d06 src/http/v2/ngx_http_v2.c --- a/src/http/v2/ngx_http_v2.c +++ b/src/http/v2/ngx_http_v2.c @@ -40,9 +40,11 @@ /* settings fields */ #define NGX_HTTP_V2_HEADER_TABLE_SIZE_SETTING0x1 +#define NGX_HTTP_V2_ENABLE_PUSH_SETTING 0x2 #define NGX_HTTP_V2_MAX_STREAMS_SETTING 0x3 #define NGX_HTTP_V2_INIT_WINDOW_SIZE_SETTING 0x4 #define NGX_HTTP_V2_MAX_FRAME_SIZE_SETTING 0x5 +#define NGX_HTTP_V2_HEADER_LIST_SIZE_SETTING 0x6 #define NGX_HTTP_V2_FRAME_BUFFER_SIZE24 @@ -1955,6 +1957,9 @@ ngx_http_v2_state_settings(ngx_http_v2_c return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_SIZE_ERROR); } +ngx_log_debug0(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0, + "http2 SETTINGS frame ack:1"); + h2c->settings_ack = 1; return ngx_http_v2_state_complete(h2c, pos, end); @@ -1968,6 +1973,10 @@ ngx_http_v2_state_settings(ngx_http_v2_c return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_SIZE_ERROR); } +ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0, + "http2 SETTINGS frame params:%uz", + h2c->state.length / NGX_HTTP_V2_SETTINGS_PARAM_SIZE); + ngx_http_v2_send_settings(h2c, 1); return ngx_http_v2_state_settings_params(h2c, pos, end); @@ -1993,6 +2002,27 @@ ngx_http_v2_state_settings_params(ngx_ht switch (id) { +case NGX_HTTP_V2_HEADER_TABLE_SIZE_SETTING: + +ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0, + "http2 SETTINGS frame HEADER_TABLE_SIZE:%ui " + "(ignored)", value); +break; + +case NGX_HTTP_V2_ENABLE_PUSH_SETTING: + +ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0, + "http2 SETTINGS frame ENABLE_PUSH:%ui " + "(ignored)", value); +break; + +case NGX_HTTP_V2_MAX_STREAMS_SETTING: + +ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0, + "http2 SETTINGS frame MAX_CONCURRENT_STREAMS:%ui " + "(ignored)", value); +break; + case NGX_HTTP_V2_INIT_WINDOW_SIZE_SETTING: if (value > NGX_HTTP_V2_MAX_WINDOW) { @@ -2004,6 +2034,10 @@ ngx_http_v2_state_settings_params(ngx_ht NGX_HTTP_V2_FLOW_CTRL_ERROR); } +ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0, + "http2 SETTINGS frame INITIAL_WINDOW_SIZE:%ui", + value); + if (ngx_http_v2_adjust_windows(h2c, value - h2c->init_window) != NGX_OK) { @@ -2026,10 +2060,25 @@ ngx_http_v2_state_settings_params(ngx_ht NGX_HTTP_V2_PROTOCOL_ERROR); } +ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0, + "http2 SETTINGS frame MAX_FRAME_SIZE:%ui", + value); + h2c->frame_size = value; break; +case NGX_HTTP_V2_HEADER_LIST_SIZE_SETTING: + +ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0, + "http2 SETTINGS frame MAX_HEADER_LIST_SIZE:%ui " + "(ignored)", value); +break; + default: + +ngx_log_debug2(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0, + "http2 SETTINGS frame 0x%Xi:%ui " + "(ignored)", id, value); break; } @@ -2070,12 +2119,16 @@ ngx_http_v2_state_ping(ngx_http_v2_conne } ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0, - "http2 PING frame, flags: %ud", h2c->state.flags); + "http2 PING frame ack:%ud", + h2c->state.flags & NGX_HTTP_V2_ACK_FLAG ? 1 : 0); if (h2c->state.flags & NGX_HTTP_V2_ACK_FLAG) { return ngx_http_v2_state_skip(h2c, pos, end); } +ngx_log_debug0(NGX_LOG_DEBUG_HTTP, h2c->connection->log, 0, + "http2 send PING frame ack:1"); + frame = ngx_http_v2_get_frame(h2c, NGX_HTTP_V2_PING_SIZE, NGX_HTTP_V2_PING_FRAME, NGX_HTTP_V2_ACK_FLAG, 0); @@ -2449,8 +2502,18 @@ ngx_http_v2_send_settings(ngx_http_v2_co ngx_http_v2_srv_conf_t *h2scf; ngx_http_v2_out_frame_t *frame; -ngx_log_debug1(NGX_LOG_DEBUG_HTTP, h2c->connection->log,
Re: [PATCH] HTTP/2: add fast-path for HTTP/2 requests without request body
Hey Valentin, > With your patch the behavior is different in these cases: > > GET / HTTP/1.1 > Host: example.com > Transfer-Encoding: chunked > > 0 > > and > > HEADERS > DATA length:0 END_STREAM That wasn't really the case I was optimizing for, but that's a good point. > Moreover, it depends on the time when DATA frame is processed: > before ngx_http_read_request_body() call or after. Does it? Won't content handlers (and thus ngx_http_v2_read_request_body()) be always called right after HEADERS frame is processed? Anyway, both of your patches work as well, so feel free to commit any of those. Thanks! Best regards, Piotr Sikora ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH] HTTP/2: add debug logging of pseudo-headers and cookies
# HG changeset patch # User Piotr Sikora # Date 1490516711 25200 # Sun Mar 26 01:25:11 2017 -0700 # Node ID 7e98d1dbb9ffc83a4ae621e05f8ebdc23fdf3b70 # Parent 22be63bf21edaa1b8ea916c7d8cd4e5fe4892061 HTTP/2: add debug logging of pseudo-headers and cookies. Signed-off-by: Piotr Sikora diff -r 22be63bf21ed -r 7e98d1dbb9ff src/http/v2/ngx_http_v2.c --- a/src/http/v2/ngx_http_v2.c +++ b/src/http/v2/ngx_http_v2.c @@ -1585,6 +1585,10 @@ ngx_http_v2_state_process_header(ngx_htt rc = ngx_http_v2_pseudo_header(r, header); if (rc == NGX_OK) { +ngx_log_debug2(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, + "http2 http header: \":%V: %V\"", + &header->name, &header->value); + return ngx_http_v2_state_header_complete(h2c, pos, end); } @@ -1626,6 +1630,10 @@ ngx_http_v2_state_process_header(ngx_htt NGX_HTTP_V2_INTERNAL_ERROR); } +ngx_log_debug2(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, + "http2 http header: \"%V: %V\"", + &header->name, &header->value); + return ngx_http_v2_state_header_complete(h2c, pos, end); } ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH 1 of 4] HTTP/2: emit new frames only after applying all SETTINGS params
# HG changeset patch # User Piotr Sikora # Date 1493073310 25200 # Mon Apr 24 15:35:10 2017 -0700 # Node ID 07adf0a7009c3244de4b795c0c06927f4316a87f # Parent 2c4dbcd6f2e4c9c2a1eb8dc1f0d39c99975ae208 HTTP/2: emit new frames only after applying all SETTINGS params. Previously, new frames could be emitted in the middle of applying new (and already acknowledged) SETTINGS params, which is illegal. Signed-off-by: Piotr Sikora diff -r 2c4dbcd6f2e4 -r 07adf0a7009c src/http/v2/ngx_http_v2.c --- a/src/http/v2/ngx_http_v2.c +++ b/src/http/v2/ngx_http_v2.c @@ -1982,7 +1982,9 @@ static u_char * ngx_http_v2_state_settings_params(ngx_http_v2_connection_t *h2c, u_char *pos, u_char *end) { -ngx_uint_t id, value; +ngx_uint_t id, value, adjustment; + +adjustment = 0; while (h2c->state.length) { if (end - pos < NGX_HTTP_V2_SETTINGS_PARAM_SIZE) { @@ -2008,13 +2010,7 @@ ngx_http_v2_state_settings_params(ngx_ht NGX_HTTP_V2_FLOW_CTRL_ERROR); } -if (ngx_http_v2_adjust_windows(h2c, value - h2c->init_window) -!= NGX_OK) -{ -return ngx_http_v2_connection_error(h2c, - NGX_HTTP_V2_INTERNAL_ERROR); -} - +adjustment = value - h2c->init_window; h2c->init_window = value; break; @@ -2041,6 +2037,13 @@ ngx_http_v2_state_settings_params(ngx_ht pos += NGX_HTTP_V2_SETTINGS_PARAM_SIZE; } +if (adjustment) { +if (ngx_http_v2_adjust_windows(h2c, adjustment) != NGX_OK) { +return ngx_http_v2_connection_error(h2c, +NGX_HTTP_V2_INTERNAL_ERROR); +} +} + return ngx_http_v2_state_complete(h2c, pos, end); } ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel
[PATCH 2 of 4] HTTP/2: send SETTINGS ACK after applying all SETTINGS params
# HG changeset patch # User Piotr Sikora # Date 1493073310 25200 # Mon Apr 24 15:35:10 2017 -0700 # Node ID a8cfd4c454ff5433629bfd16444c6c71ee932fa1 # Parent 07adf0a7009c3244de4b795c0c06927f4316a87f HTTP/2: send SETTINGS ACK after applying all SETTINGS params. This avoids sending unnecessary SETTINGS ACK in case of PROTOCOL_ERROR. Signed-off-by: Piotr Sikora diff -r 07adf0a7009c -r a8cfd4c454ff src/http/v2/ngx_http_v2.c --- a/src/http/v2/ngx_http_v2.c +++ b/src/http/v2/ngx_http_v2.c @@ -1972,8 +1972,6 @@ ngx_http_v2_state_settings(ngx_http_v2_c return ngx_http_v2_connection_error(h2c, NGX_HTTP_V2_SIZE_ERROR); } -ngx_http_v2_send_settings(h2c, 1); - return ngx_http_v2_state_settings_params(h2c, pos, end); } @@ -2037,6 +2035,8 @@ ngx_http_v2_state_settings_params(ngx_ht pos += NGX_HTTP_V2_SETTINGS_PARAM_SIZE; } +ngx_http_v2_send_settings(h2c, 1); + if (adjustment) { if (ngx_http_v2_adjust_windows(h2c, adjustment) != NGX_OK) { return ngx_http_v2_connection_error(h2c, ___ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel