On Thu, Apr 21, 2022 at 01:18:57AM +0300, Maxim Dounin wrote:
> # HG changeset patch
> # User Maxim Dounin <mdounin@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@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;
}
_______________________________________________
nginx-devel mailing list -- nginx-devel@nginx.org
To unsubscribe send an email to nginx-devel-leave@nginx.org