Fix "header too long" error

[Sergey Kandaurov]

> On 16 Jan 2018, at 14:57, Vadim Fedorenko <vadimjunk at gmail.com> wrote:
> 
> # HG changeset patch
> # User Vadim Fedorenko <vfedorenko at yandex-team.ru>
> # Date 1516103689 -10800
> #      Tue Jan 16 14:54:49 2018 +0300
> # Node ID deaa364977488f3390d48306c34dc80961e54e14
> # Parent  fbf6a421212b291cbacfcfc503173c0168449165
> Fix “header too long" error

Style: missing “Cache: “ prefix and a dot after the sentence.

> 
> This error occurs in rare cases when cached file with different "Vary"
> header value have headers length more than main cache file and main
> cache file can be used without revalidation (ngx_file_cache_exists
> finds file node and rewrites c->body_start on first read). Fix saves
> buffer size derived from proxy_buffer parameter.
> 
> diff -r fbf6a421212b -r deaa36497748 src/http/ngx_http_file_cache.c
> --- a/src/http/ngx_http_file_cache.c    Fri Jan 12 02:27:18 2018 +0300
> +++ b/src/http/ngx_http_file_cache.c    Tue Jan 16 14:54:49 2018 +0300
> @@ -271,6 +271,7 @@
>      ngx_open_file_info_t       of;
>      ngx_http_file_cache_t     *cache;
>      ngx_http_core_loc_conf_t  *clcf;
> +    size_t                     buffer_size;

buffer_size is unsorted.

>  
>      c = r->cache;
>  
> @@ -294,6 +295,12 @@
>          cln->data = c;
>      }
>  
> +    /* save buffer_size because ngx_http_file_cache_exists
> +     * can overwrite c->body_start
> +     */

Multi-line comments should look like real sentences.

> +    
> +    buffer_size = c->body_start;
> +    
>      rc = ngx_http_file_cache_exists(cache, c);
>  
>      ngx_log_debug2(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
> @@ -382,7 +389,7 @@
>      c->length = of.size;
>      c->fs_size = (of.fs_size + cache->bsize - 1) / cache->bsize;
>  
> -    c->buf = ngx_create_temp_buf(r->pool, c->body_start);
> +    c->buf = ngx_create_temp_buf(r->pool, buffer_size);
>      if (c->buf == NULL) {
>          return NGX_ERROR;
>      }

The patch isn't optimal as it disregards c->body_start knowledge preserved
in file cache node and used as a cache file buffer size.  Thus, the cache
buffer would be overallocated by the default size, as specified by the user
of ngx_http_file_cache_open(), that's ngx_http_upstream_cache() buffer_size.

Please try this patch instead.

# HG changeset patch
# User Sergey Kandaurov <pluknet at nginx.com>
# Date 1501864657 -10800
#      Fri Aug 04 19:37:37 2017 +0300
# Node ID f727ed0e9f2f3e4706fa6444e8e3df0a21f8fa3a
# Parent  93abb5a855d6534f0356882f45be49f8c6a95a8b
Cache: reset c->body_start when reading a variant on vary mismatch.

Previously, a variant not present in shared memory and stored on disk using a
secondary key was read with c->body_start from a variant stored with a primary
key.  This could result in critical errors "cache file .. has too long header".

diff --git a/src/http/ngx_http_cache.h b/src/http/ngx_http_cache.h
--- a/src/http/ngx_http_cache.h
+++ b/src/http/ngx_http_cache.h
@@ -80,6 +80,7 @@ struct ngx_http_cache_s {
     ngx_str_t                        vary;
     u_char                           variant[NGX_HTTP_CACHE_KEY_LEN];
 
+    size_t                           buffer_size;
     size_t                           header_start;
     size_t                           body_start;
     off_t                            length;
diff --git a/src/http/ngx_http_file_cache.c b/src/http/ngx_http_file_cache.c
--- a/src/http/ngx_http_file_cache.c
+++ b/src/http/ngx_http_file_cache.c
@@ -294,6 +294,8 @@ ngx_http_file_cache_open(ngx_http_reques
         cln->data = c;
     }
 
+    c->buffer_size = c->body_start;
+
     rc = ngx_http_file_cache_exists(cache, c);
 
     ngx_log_debug2(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
@@ -1230,7 +1232,7 @@ ngx_http_file_cache_reopen(ngx_http_requ
 
     c->secondary = 1;
     c->file.name.len = 0;
-    c->body_start = c->buf->end - c->buf->start;
+    c->body_start = c->buffer_size;
 
     ngx_memcpy(c->key, c->variant, NGX_HTTP_CACHE_KEY_LEN);
 

-- 
Sergey Kandaurov