[PATCH 2 of 3] HTTP/2: added support for trailers in HTTP responses

[Valentin V. Bartenev]

On Tuesday 13 June 2017 05:19:54 Piotr Sikora via nginx-devel wrote:
> # HG changeset patch
> # User Piotr Sikora <piotrsikora at 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 <piotrsikora at 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 <vbart at 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;