Hello!

On Tue, Jan 31, 2012 at 03:39:03PM -0800, Dave Bailey wrote:

> Hi,
>
> In ngx_http_upstream_rewrite_location(), r->upstream->rewrite_redirect may
> replace the Location header value with a new value. This value ends up
> going through ngx_http_header_filter(), and if it's a relative URL, the
> header filter adds a scheme and host to it. However, if
> r->upstream->rewrite_redirect declines (rc = NGX_DECLINED) to rewrite the
> redirect, the value does not go through ngx_http_header_filter() even if it
> is a relative URL.
>
> Is this intended behavior? If it is not, I attached a patch that sets
> r->headers_out.location consistently.

I believe the intended behaviour is:

1. If we got relative redirect from a backend and are going to pass it
to a client - don't touch it. It's backend which breaks RFC, so
we don't care.

In the code path without upstream->rewrite_redirect set you may even see
special code to protect such redirects:

if (ho->value.data[0] != '/') {
r->headers_out.location = ho;
}

/*
* we do not set r->headers_out.location here to avoid the handling
* the local redirects without a host name by ngx_http_header_filter()
*/

return NGX_OK;

2. If we are about to produce our own relative redirect - make
sure it will be properly converted to absolute one by the header
filter.

The code looks correct here.

The problem I see is that if rewrite_redirect() returns
NGX_DECLINED, the r->headers_out.location won't be set and it
won't be possible to clear it in case we'll need to.

The same applies to relative redirects from a backend though, and
requires reconsidering of the "don't set r->headers_out.location
to avoid handling by ngx_http_header_filter()" policy to make
mentioned clearing indeed possible.

Maxim Dounin

_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel