[PATCH] Add $upstream_cache_key

[Maxim Dounin]

Hello!

On Thu, May 23, 2024 at 10:52:13AM +0100, Kirill A. Korinsky wrote:

> Greetings,
> 
> Here is a patch that exposes the constructed cache key as
> $upstream_cache_key variable.
> 
> Sometimes it's quite useful when debugging complicated setups and fighting
> some typos.

Thanks for the patch.

I have no objections in general, see below for some minor 
comments.

> 
> index 2ce9f2114..561108681 100644
> --- src/http/ngx_http_upstream.c
> +++ src/http/ngx_http_upstream.c
> @@ -23,6 +23,8 @@ static ngx_int_t ngx_http_upstream_cache_check_range(ngx_http_request_t *r,
>      ngx_http_upstream_t *u);
>  static ngx_int_t ngx_http_upstream_cache_status(ngx_http_request_t *r,
>      ngx_http_variable_value_t *v, uintptr_t data);
> +static ngx_int_t ngx_http_upstream_cache_key(ngx_http_request_t *r,
> +    ngx_http_variable_value_t *v, uintptr_t data);
>  static ngx_int_t ngx_http_upstream_cache_last_modified(ngx_http_request_t *r,
>      ngx_http_variable_value_t *v, uintptr_t data);
>  static ngx_int_t ngx_http_upstream_cache_etag(ngx_http_request_t *r,
> @@ -414,6 +416,10 @@ static ngx_http_variable_t  ngx_http_upstream_vars[] = {
>        ngx_http_upstream_cache_status, 0,
>        NGX_HTTP_VAR_NOCACHEABLE, 0 },
>  
> +    { ngx_string("upstream_cache_key"), NULL,
> +      ngx_http_upstream_cache_key, 0,
> +      NGX_HTTP_VAR_NOCACHEABLE, 0 },
> +
>      { ngx_string("upstream_cache_last_modified"), NULL,
>        ngx_http_upstream_cache_last_modified, 0,
>        NGX_HTTP_VAR_NOCACHEABLE|NGX_HTTP_VAR_NOHASH, 0 },
> @@ -5990,6 +5996,47 @@ ngx_http_upstream_cache_status(ngx_http_request_t *r,
>  }
>  
>  
> +static ngx_int_t
> +ngx_http_upstream_cache_key(ngx_http_request_t *r,
> +    ngx_http_variable_value_t *v, uintptr_t data)
> +{
> +    size_t             len;
> +    ngx_uint_t         i;
> +    ngx_str_t         *key;

Nitpicking: wrong variables order, should be sorted by type length.

> +    ngx_http_cache_t  *c;
> +
> +    if (r->cache == NULL || r->cache->keys.nelts == 0) {
> +        v->not_found = 1;
> +        return NGX_OK;
> +    }
> +
> +    c = r->cache;
> +
> +    key = c->keys.elts;
> +    len = 0;
> +    for (i = 0; i < c->keys.nelts; i++) {
> +        len += key[i].len;
> +    }
> +
> +    v->data = ngx_pcalloc(r->pool, len);

There is no need to use ngx_pcalloc() here: the memory is anyway 
to be written to, so just normal ngx_palloc(), without "c", would 
be enough.  Further, given the result is a string, no alignment is 
needed, so ngx_pnalloc() would be appropriate.

> +    if (v->data == 0) {

In [free]nginx code, NULL is used in pointer contexts.

> +        return NGX_ERROR;
> +    }
> +
> +    v->len = 0;
> +    for (i = 0; i < c->keys.nelts; i++) {
> +        memcpy(v->data + v->len, key[i].data, key[i].len);
> +        v->len += key[i].len;

Usual approach for such loops is doing something like

    p = ngx_cpymem(p, key[i].data, key[i].len);

This avoids an extra addition per iteration, and generally easier 
to read.

(Also, using memcpy() directly is discouraged, it should be either 
ngx_memcpy() or ngx_cpymem().)

> +    }
> +
> +    v->valid = 1;
> +    v->no_cacheable = 0;
> +    v->not_found = 0;
> +
> +    return NGX_OK;
> +}
> +
> +
>  static ngx_int_t
>  ngx_http_upstream_cache_last_modified(ngx_http_request_t *r,
>      ngx_http_variable_value_t *v, uintptr_t data)
> 

Patch with the above comments incorporated, please take a look:

# HG changeset patch
# User Kirill A. Korinsky <kirill at korins.ky>
# Date 1716479312 -10800
#      Thu May 23 18:48:32 2024 +0300
# Node ID bd920ccd6f1aa43fcb40507ba463e44548ca4676
# Parent  46ecad404a296042c0088e699f275a92758e5ab9
Upstream: $upstream_cache_key variable.

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
@@ -23,6 +23,8 @@ static ngx_int_t ngx_http_upstream_cache
     ngx_http_upstream_t *u);
 static ngx_int_t ngx_http_upstream_cache_status(ngx_http_request_t *r,
     ngx_http_variable_value_t *v, uintptr_t data);
+static ngx_int_t ngx_http_upstream_cache_key(ngx_http_request_t *r,
+    ngx_http_variable_value_t *v, uintptr_t data);
 static ngx_int_t ngx_http_upstream_cache_last_modified(ngx_http_request_t *r,
     ngx_http_variable_value_t *v, uintptr_t data);
 static ngx_int_t ngx_http_upstream_cache_etag(ngx_http_request_t *r,
@@ -414,6 +416,10 @@ static ngx_http_variable_t  ngx_http_ups
       ngx_http_upstream_cache_status, 0,
       NGX_HTTP_VAR_NOCACHEABLE, 0 },
 
+    { ngx_string("upstream_cache_key"), NULL,
+      ngx_http_upstream_cache_key, 0,
+      NGX_HTTP_VAR_NOCACHEABLE, 0 },
+
     { ngx_string("upstream_cache_last_modified"), NULL,
       ngx_http_upstream_cache_last_modified, 0,
       NGX_HTTP_VAR_NOCACHEABLE|NGX_HTTP_VAR_NOHASH, 0 },
@@ -6004,6 +6010,49 @@ ngx_http_upstream_cache_status(ngx_http_
 
 
 static ngx_int_t
+ngx_http_upstream_cache_key(ngx_http_request_t *r,
+    ngx_http_variable_value_t *v, uintptr_t data)
+{
+    u_char            *p;
+    size_t             len;
+    ngx_str_t         *key;
+    ngx_uint_t         i;
+    ngx_http_cache_t  *c;
+
+    if (r->cache == NULL || r->cache->keys.nelts == 0) {
+        v->not_found = 1;
+        return NGX_OK;
+    }
+
+    c = r->cache;
+
+    len = 0;
+    key = c->keys.elts;
+
+    for (i = 0; i < c->keys.nelts; i++) {
+        len += key[i].len;
+    }
+
+    p = ngx_pnalloc(r->pool, len);
+    if (p == NULL) {
+        return NGX_ERROR;
+    }
+
+    v->len = len;
+    v->valid = 1;
+    v->no_cacheable = 0;
+    v->not_found = 0;
+    v->data = p;
+
+    for (i = 0; i < c->keys.nelts; i++) {
+        p = ngx_cpymem(p, key[i].data, key[i].len);
+    }
+
+    return NGX_OK;
+}
+
+
+static ngx_int_t
 ngx_http_upstream_cache_last_modified(ngx_http_request_t *r,
     ngx_http_variable_value_t *v, uintptr_t data)
 {


-- 
Maxim Dounin
http://mdounin.ru/