Change blank uwsgi_cache_key default, or log warning

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.

or

  1. change the default value of uwsgi_cache_key to a more useful default,
    such as $scheme$host$request_uri, similar to proxy_cache_key (not quite
    the
    same, because the proxy_cache_key has $proxy_host in its default, and
    there
    is no corresponding $uwsgi_host). You might not want to make such a
    default-behavior change in a subminor release — but as a
    counterargument,
    the current default seems quite unlikely to be relied on by anyone.

Cheers,

Gulli

Posted at Nginx Forum:

Hello!

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

2014/09/09 17:41:02 [debug] 91211#0: *13 http cache key: “”

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 D. [email protected]

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 D.
http://nginx.org/