Change blank uwsgi_cache_key default, or log warning

[Maxim Dounin]

Hello!

On Tue, Sep 09, 2014 at 02:25:15PM -0400, gthb wrote:

> Hello,
> 
> Because uwsgi_cache_key has no default value (or rather, has the empty
> string as its default value), a configuration with uwsgi_cache set but
> uwsgi_cache_key not set behaves in a way that is very unlikely to be
> desired: Nginx caches the first publicly cacheable response it gets from
> upstream (for any request), and serves that cached response to *any* request
> mapping to the same location. The internals of that can be seen in the debug
> log, where the cache key is empty for all requests:
> 
> 2014/09/09 17:41:02 [debug] 91211#0: *13 http cache key: ""
> 
> This is in contrast to a configuration with proxy_cache enabled but
> proxy_cache_key not set; that behaves OK because proxy_cache_key has a
> useful default.
> 
> Because of the *general* correspondence between the http_proxy and
> http_uwsgi modules, it's easy to fall into this trap, defining uwsgi_cache
> but not uwsgi_cache_key. When that happens, and unexpected responses start
> coming back, the first place one looks is error.log, and there's nothing
> there.
> 
> To get rid of this gotcha, I suggest either:
> 
> 1. log a warning whenever a location/server/http block has uwsgi_cache set
> but no uwsgi_cache_key set.

Yes, this is mostly trivial and certainly makes sense.
Patch below.

# HG changeset patch
# User Maxim Dounin <mdounin at mdounin.ru>
# Date 1410371072 -14400
#      Wed Sep 10 21:44:32 2014 +0400
# Node ID bc4ee0b7cf2643fdcea310638302b9cadc7ac939
# Parent  aaa82dc56c9460db1b4233fc1d4559fdd07ff7ed
Added warning about unset cache keys.

In fastcgi, scgi and uwsgi modules there are no default cache keys, and
using a cache without a cache key set is likely meaningless.

diff --git a/src/http/modules/ngx_http_fastcgi_module.c b/src/http/modules/ngx_http_fastcgi_module.c
--- a/src/http/modules/ngx_http_fastcgi_module.c
+++ b/src/http/modules/ngx_http_fastcgi_module.c
@@ -2582,6 +2582,11 @@ ngx_http_fastcgi_merge_loc_conf(ngx_conf
         conf->cache_key = prev->cache_key;
     }
 
+    if (conf->upstream.cache && conf->cache_key.value.data == NULL) {
+        ngx_conf_log_error(NGX_LOG_WARN, cf, 0,
+                           "no \"fastcgi_cache_key\" for \"fastcgi_cache\"");
+    }
+
     ngx_conf_merge_value(conf->upstream.cache_lock,
                               prev->upstream.cache_lock, 0);
 
diff --git a/src/http/modules/ngx_http_scgi_module.c b/src/http/modules/ngx_http_scgi_module.c
--- a/src/http/modules/ngx_http_scgi_module.c
+++ b/src/http/modules/ngx_http_scgi_module.c
@@ -1337,6 +1337,11 @@ ngx_http_scgi_merge_loc_conf(ngx_conf_t 
         conf->cache_key = prev->cache_key;
     }
 
+    if (conf->upstream.cache && conf->cache_key.value.data == NULL) {
+        ngx_conf_log_error(NGX_LOG_WARN, cf, 0,
+                           "no \"scgi_cache_key\" for \"scgi_cache\"");
+    }
+
     ngx_conf_merge_value(conf->upstream.cache_lock,
                               prev->upstream.cache_lock, 0);
 
diff --git a/src/http/modules/ngx_http_uwsgi_module.c b/src/http/modules/ngx_http_uwsgi_module.c
--- a/src/http/modules/ngx_http_uwsgi_module.c
+++ b/src/http/modules/ngx_http_uwsgi_module.c
@@ -1524,6 +1524,11 @@ ngx_http_uwsgi_merge_loc_conf(ngx_conf_t
         conf->cache_key = prev->cache_key;
     }
 
+    if (conf->upstream.cache && conf->cache_key.value.data == NULL) {
+        ngx_conf_log_error(NGX_LOG_WARN, cf, 0,
+                           "no \"uwsgi_cache_key\" for \"uwsgi_cache\"");
+    }
+
     ngx_conf_merge_value(conf->upstream.cache_lock,
                               prev->upstream.cache_lock, 0);
 

-- 
Maxim Dounin
http://nginx.org/