On Tuesday 13 June 2017 05:19:54 Piotr Sikora via nginx-devel wrote: > # HG changeset patch > # User Piotr Sikora <piotrsik...@google.com> > # Date 1490351854 25200 > # Fri Mar 24 03:37:34 2017 -0700 > # Node ID 73f67e06ab103e0368d1810c6f8cac5c70c4e246 > # Parent 07a5d26b49f04425ff54cc998f885aa987b7823f > HTTP/2: added support for trailers in HTTP responses. > > Signed-off-by: Piotr Sikora <piotrsik...@google.com> > [..] > + > + if (header[i].key.len > NGX_HTTP_V2_MAX_FIELD) { > + ngx_log_error(NGX_LOG_WARN, r->connection->log, 0, > + "too long response trailer name: \"%V\"", > + &header[i].key); > + return NULL; > + } > + > + if (header[i].value.len > NGX_HTTP_V2_MAX_FIELD) { > + ngx_log_error(NGX_LOG_WARN, r->connection->log, 0, > + "too long response trailer value: \"%V: %V\"", > + &header[i].key, &header[i].value); > + return NULL; > + } [..]
I've overlooked this while doing previous review, but it looks strange. Why do you use NGX_LOG_WARN for trailers headers? It results in finalizing request with an error (in case of HTTP/2 it means RST_STREAM). For main headers the NGX_LOG_CRIT level is used. It looks too serious, but the WARN level is too low. It seems the right log level for both cases is NGX_LOG_ERR. So I'm going to commit the patch below: # HG changeset patch # User Valentin Bartenev <vb...@nginx.com> # Date 1497453034 -10800 # Wed Jun 14 18:10:34 2017 +0300 # Node ID 460ed2f4e160bc6b0953456d1218381cdfc7e40b # Parent 3c55863e6887ee764265fd43a878b3f793e0a7b9 HTTP/2: adjusted log level for too long response headers. It's likely an upstream fault and the CRIT log level looks too high. diff -r 3c55863e6887 -r 460ed2f4e160 src/http/v2/ngx_http_v2_filter_module.c --- a/src/http/v2/ngx_http_v2_filter_module.c Tue Jun 13 17:01:08 2017 +0300 +++ b/src/http/v2/ngx_http_v2_filter_module.c Wed Jun 14 18:10:34 2017 +0300 @@ -385,14 +385,14 @@ ngx_http_v2_header_filter(ngx_http_reque } if (header[i].key.len > NGX_HTTP_V2_MAX_FIELD) { - ngx_log_error(NGX_LOG_CRIT, fc->log, 0, + ngx_log_error(NGX_LOG_ERR, fc->log, 0, "too long response header name: \"%V\"", &header[i].key); return NGX_ERROR; } if (header[i].value.len > NGX_HTTP_V2_MAX_FIELD) { - ngx_log_error(NGX_LOG_CRIT, fc->log, 0, + ngx_log_error(NGX_LOG_ERR, fc->log, 0, "too long response header value: \"%V: %V\"", &header[i].key, &header[i].value); return NGX_ERROR; _______________________________________________ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel