Hello! On Wed, Jun 14, 2017 at 06:12:25PM +0300, Valentin V. Bartenev wrote:
> 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: [...] Given that the limit is about 2 megabytes, I don't think that this can be triggered in practice by a backend. As such, NGX_LOG_CRIT looks logical, as the only practically possible reason for the test to fail is a bug somewhere. -- Maxim Dounin http://nginx.org/ _______________________________________________ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel