Hello!

On Wed, May 08, 2019 at 02:30:28PM +0900, Sangdeuk Kwon wrote:

> # HG changeset patch
> # User Sangdeuk Kwon <dark264sh@gmail.com>
> # Date 1557288704 -32400
> # Wed May 08 13:11:44 2019 +0900
> # Node ID c4e1be885640a8302dcbf97162b3cbdc21710a84
> # Parent 16a1adadf43751f59257ba419f6bacd530dd19d3
> reset r->cache->valid_sec for following the redirect
>
> this config:
>
> http {
> proxy_intercept_errors on;
>
> proxy_cache_path /cache0 levels=1:2 keys_zone=STATIC:10m max_size=10g
> inactive=60m use_temp_path=off;
> proxy_cache_key "$host$uri";
> proxy_cache STATIC;
>
> upstream up_servers {
> server 127.0.0.2:8080;
> }
>
> server {
> listen 127.0.0.1:8000;
>
> location / {
> proxy_pass http://up_servers;
> proxy_set_header Host $host;
> proxy_http_version 1.1;
> add_header X-Cached $upstream_cache_status;
>
> proxy_intercept_errors on;
> error_page 301 302 303 307 308 = @handle_redirects;
> }
>
> location @handle_redirects {
> set $orig_loc $upstream_http_location;
> proxy_pass $orig_loc;
> }
> }
> }
>
> When origin server does not send cache time header (max-age or expires),
> nginx can follow the redirect with location header.
> But, if origin server sends cache time header,
> nginx can't follow the redirect and nginx send 302 response without
> location header to a client.
> So the client also can't follow the redirect because of absenting location
> header.
>
> diff -r 16a1adadf437 -r c4e1be885640 src/http/ngx_http_upstream.c
> --- a/src/http/ngx_http_upstream.c Wed Apr 24 16:38:56 2019 +0300
> +++ b/src/http/ngx_http_upstream.c Wed May 08 13:11:44 2019 +0900
> @@ -2645,6 +2645,15 @@
> if (valid) {
> r->cache->error = status;
> }
> +
> + if (status == NGX_HTTP_MOVED_PERMANENTLY
> + || status == NGX_HTTP_MOVED_TEMPORARILY
> + || status == NGX_HTTP_SEE_OTHER
> + || status == NGX_HTTP_TEMPORARY_REDIRECT
> + || status == NGX_HTTP_PERMANENT_REDIRECT)
> + {
> + r->cache->valid_sec = 0;
> + }
> }
>
> ngx_http_file_cache_free(r->cache, u->pipe->temp_file);

Thanks for the patch.

This seems to be a negative effect of dbd77a638eb7 (nginx 1.13.6,
http://hg.nginx.org/nginx/rev/dbd77a638eb7), which manifests
itself in the configuration in question. As long as the response
is cached, the particular configuration cannot work, since
$upstream_http_location is not available.

An obvious workaround is to ensure that such configuration do not
allow caching of responses, for example, by using "proxy_cache
off;" in such locations.

Using "r->cache->valid_sec = 0;" based on the status code,
however, looks wrong to me. First of all, it doesn't actually
disable caching, but rather preserves the existing response
validity as is - and this may result in more or less arbitrary
behaviour. Second, it doesn't clear to me if this is something to
be done unconditionally - there can be configuration when caching
is desired, despite the fact that $upstream_http_location will not
be available.

--
Maxim Dounin
http://mdounin.ru/
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel