[BUG] url not properly handled for try_files and error_page redirect in ngx_http_internal_redirect

[Maxim Dounin]

Hello!

On Sat, Dec 17, 2011 at 12:34:15AM +0800, lanshun zhou wrote:

> url is changed in ngx_http_internal_redirect, but some flags like
> r->valid_unparsed_uri
> are not always reset. Then in ngx_http_proxy_create_request the
> original uri is sent to
> backends instead of the new one.
> 
> This affects the redirects in ngx_http_core_try_files_phase and
> ngx_http_send_error_page.

For others, from http://trac.nginx.org/nginx/ticket/70:

Historically, such behaviour of the ngx_http_internal_redirect() 
was a feature: it allowed to pass the same request to (another) 
upstream server via error_page redirection. Since then named 
locations appeared though, and it's probably time to start 
resetting r->valid_unparsed_uri on internal redirects. (The 
try_files behaviour outlined above is certainly a bug.)

> 
> simple fix:
> 
> diff -ruN nginx-1.1.11/src/http/ngx_http_core_module.c
> nginx-1.1.11_zls/src/http/ngx_http_core_module.c
> --- nginx-1.1.11/src/http/ngx_http_core_module.c        2011-12-06
> 21:23:37.000000000 +0800
> +++ nginx-1.1.11_zls/src/http/ngx_http_core_module.c    2011-12-16
> 16:31:27.751927635 +0800
> @@ -2481,6 +2481,8 @@
> 
>      r->uri = *uri;
> 
> +    r->valid_unparsed_uri = 0;
> +
>      if (args) {
>          r->args = *args;
> 
> diff -ruN nginx-1.1.11/src/http/ngx_http_upstream.c
> nginx-1.1.11_zls/src/http/ngx_http_upstream.c
> --- nginx-1.1.11/src/http/ngx_http_upstream.c   2011-12-09
> 21:19:57.000000000 +0800
> +++ nginx-1.1.11_zls/src/http/ngx_http_upstream.c       2011-12-16
> 16:30:25.399218885 +0800
> @@ -1895,8 +1895,6 @@
>              r->method = NGX_HTTP_GET;
>          }
> 
> -        r->valid_unparsed_uri = 0;
> -
>          ngx_http_internal_redirect(r, uri, &args);
>          ngx_http_finalize_request(r, NGX_DONE);
>          return NGX_DONE;

I think the patch is right and should be committed.

If somebody is still using old-style error_page + proxy_pass 
without uri, it's time to start using named locations instead.

(Not sure if we're going to merge it into 1.0.x stable branch 
though.)

Maxim Dounin