[PATCH 17 of 20] Upstream: handling of multiple Vary headers (ticket #1423)

Sergey Kandaurov pluknet at nginx.com
Wed May 11 20:44:52 UTC 2022


On Thu, Apr 21, 2022 at 01:18:57AM +0300, Maxim Dounin wrote:
> # HG changeset patch
> # User Maxim Dounin <mdounin at mdounin.ru>
> # Date 1650492340 -10800
> #      Thu Apr 21 01:05:40 2022 +0300
> # Node ID 2027b85971d4b8a7e33c018548468057cb57eaf7
> # Parent  f460a2f9f88d264ef6c8588eb37bcb85c48010db
> Upstream: handling of multiple Vary headers (ticket #1423).
> 
> Previously, only the last header value was used when caching.
> 
> diff --git a/src/http/ngx_http_upstream.c b/src/http/ngx_http_upstream.c
> --- a/src/http/ngx_http_upstream.c
> +++ b/src/http/ngx_http_upstream.c
> @@ -5175,6 +5175,9 @@ static ngx_int_t
>  ngx_http_upstream_process_vary(ngx_http_request_t *r,
>      ngx_table_elt_t *h, ngx_uint_t offset)
>  {
> +    u_char                *p;
> +    size_t                 len;
> +    ngx_str_t              vary;
>      ngx_table_elt_t      **ph;
>      ngx_http_upstream_t   *u;
>  
> @@ -5192,17 +5195,47 @@ ngx_http_upstream_process_vary(ngx_http_
>          return NGX_OK;
>      }
>  
> -    if (r->cache == NULL) {
> +    if (r->cache == NULL || !u->cacheable) {
> +        return NGX_OK;
> +    }
> +
> +    if (h->value.len == 1 && h->value.data[0] == '*') {
> +        u->cacheable = 0;
>          return NGX_OK;
>      }
>  
> -    if (h->value.len > NGX_HTTP_CACHE_VARY_LEN
> -        || (h->value.len == 1 && h->value.data[0] == '*'))
> -    {
> +    if (u->headers_in.vary->next) {
> +
> +        len = 0;
> +
> +        for (h = u->headers_in.vary; h; h = h->next) {
> +            len += h->value.len + 2;
> +        }
> +
> +        len -= 2;
> +
> +        p = ngx_pnalloc(r->pool, len);
> +        if (p == NULL) {
> +            return NGX_ERROR;
> +        }
> +
> +        vary.len = len;
> +        vary.data = p;
> +
> +        for (h = u->headers_in.vary; h; h = h->next) {
> +            p = ngx_copy(p, h->value.data, h->value.len);

(h->next == NULL) check missing

> +            *p++ = ','; *p++ = ' ';
> +        }
> +
> +    } else {
> +        vary = h->value;
> +    }
> +
> +    if (vary.len > NGX_HTTP_CACHE_VARY_LEN) {
>          u->cacheable = 0;
>      }
>  
> -    r->cache->vary = h->value;
> +    r->cache->vary = vary;
>  
>  #endif
>  

Memory from the pool could be allocated just once among multiple values
if postpone it to somewhere else (but probably it's not worth it).

Combined patch on top off:

# HG changeset patch
# User Sergey Kandaurov <pluknet at nginx.com>
# Date 1652193477 -14400
#      Tue May 10 18:37:57 2022 +0400
# Node ID cffc4a23b8bb69143d4128b2249b0b87d5fd68cb
# Parent  facf698a696385b7b07abc0b2f7e5c1f6394a486
imported patch p17b

diff --git a/src/http/ngx_http_upstream.c b/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_vary_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_send_response(ngx_http_request_t *r,
@@ -2788,6 +2790,12 @@ ngx_http_upstream_process_headers(ngx_ht
 
     umcf = ngx_http_get_module_main_conf(r, ngx_http_upstream_module);
 
+    if (ngx_http_upstream_process_vary_headers(r, u) != NGX_OK) {
+        ngx_http_upstream_finalize_request(r, u,
+                                           NGX_HTTP_INTERNAL_SERVER_ERROR);
+        return NGX_DONE;
+    }
+
     if (u->headers_in.x_accel_redirect
         && !(u->conf->ignore_headers & NGX_HTTP_UPSTREAM_IGN_XA_REDIRECT))
     {
@@ -2928,6 +2936,73 @@ ngx_http_upstream_process_headers(ngx_ht
 
 
 static ngx_int_t
+ngx_http_upstream_process_vary_headers(ngx_http_request_t *r,
+    ngx_http_upstream_t *u)
+{
+    u_char           *p;
+    size_t            len;
+    ngx_str_t         vary;
+    ngx_table_elt_t  *h;
+
+#if (NGX_HTTP_CACHE)
+
+    if (u->conf->ignore_headers & NGX_HTTP_UPSTREAM_IGN_VARY) {
+        return NGX_OK;
+    }
+
+    if (r->cache == NULL || !u->cacheable) {
+        return NGX_OK;
+    }
+
+    if (u->headers_in.vary == NULL) {
+        return NGX_OK;
+    }
+
+    if (u->headers_in.vary->next) {
+
+        len = 0;
+
+        for (h = u->headers_in.vary; h; h = h->next) {
+            len += h->value.len + 2;
+        }
+
+        len -= 2;
+
+        p = ngx_pnalloc(r->pool, len);
+        if (p == NULL) {
+            return NGX_ERROR;
+        }
+
+        vary.len = len;
+        vary.data = p;
+
+        for (h = u->headers_in.vary; h; h = h->next) {
+            p = ngx_copy(p, h->value.data, h->value.len);
+
+            if (h->next == NULL) {
+                break;
+            }
+
+            *p++ = ','; *p++ = ' ';
+        }
+
+    } else {
+        vary = u->headers_in.vary->value;
+    }
+
+    if (vary.len > NGX_HTTP_CACHE_VARY_LEN) {
+        u->cacheable = 0;
+    }
+
+    r->cache->vary = vary;
+
+#endif
+
+    return NGX_OK;
+}
+
+
+static ngx_int_t
 ngx_http_upstream_process_trailers(ngx_http_request_t *r,
     ngx_http_upstream_t *u)
 {
@@ -5175,9 +5250,6 @@ static ngx_int_t
 ngx_http_upstream_process_vary(ngx_http_request_t *r,
     ngx_table_elt_t *h, ngx_uint_t offset)
 {
-    u_char                *p;
-    size_t                 len;
-    ngx_str_t              vary;
     ngx_table_elt_t      **ph;
     ngx_http_upstream_t   *u;
 
@@ -5189,60 +5261,9 @@ ngx_http_upstream_process_vary(ngx_http_
     *ph = h;
     h->next = NULL;
 
-#if (NGX_HTTP_CACHE)
-
-    if (u->conf->ignore_headers & NGX_HTTP_UPSTREAM_IGN_VARY) {
-        return NGX_OK;
-    }
-
-    if (r->cache == NULL || !u->cacheable) {
-        return NGX_OK;
-    }
-
     if (h->value.len == 1 && h->value.data[0] == '*') {
         u->cacheable = 0;
-        return NGX_OK;
-    }
-
-    if (u->headers_in.vary->next) {
-
-        len = 0;
-
-        for (h = u->headers_in.vary; h; h = h->next) {
-            len += h->value.len + 2;
-        }
-
-        len -= 2;
-
-        p = ngx_pnalloc(r->pool, len);
-        if (p == NULL) {
-            return NGX_ERROR;
-        }
-
-        vary.len = len;
-        vary.data = p;
-
-        for (h = u->headers_in.vary; h; h = h->next) {
-            p = ngx_copy(p, h->value.data, h->value.len);
-
-            if (h->next == NULL) {
-                break;
-            }
-
-            *p++ = ','; *p++ = ' ';
-        }
-
-    } else {
-        vary = h->value;
-    }
-
-    if (vary.len > NGX_HTTP_CACHE_VARY_LEN) {
-        u->cacheable = 0;
-    }
-
-    r->cache->vary = vary;
-
-#endif
+    }
 
     return NGX_OK;
 }



More information about the nginx-devel mailing list