Hello!

On Tue, Nov 02, 2010 at 06:57:33AM +0300, Maxim Dounin wrote:

> # HG changeset patch
> # User Maxim Dounin <mdounin@mdounin.ru>
> # Date 1288669813 -10800
> # Node ID 7ec15c9b1a6e7046334aabc09c907c7601ddbfdc
> # Parent 49670f58d178cbef2108a76676b8d79cf2cb648b
> Fix double content when return is used in error_page redirection.
>
> Test case:
>
> location / {
> error_page 405 /nope;
> return 405;
> }
>
> location /nope {
> return 200;
> }
>
> This is expected to return 405 with empty body, but in 0.8.42+ will return
> builtin 405 error page as well (though not counted in Content-Length, thus
> breaking protocol).
>
> Note that this patch also changes behaviour for "return 302 ..." and
> "rewrite ... redirect" used as error handler. E.g.
>
> location / {
> error_page 405 /redirect;
> return 405;
> }
>
> location /redirect {
> rewrite ^ http://example.com/;
> }
>
> will actually return redirect to "http://example.com/" instead of builtin 405
> error page with meaningless Location header. This looks like correct change
> and it's in line with what happens on e.g. directory redirects in error
> handlers.
>
> diff --git a/src/http/modules/ngx_http_rewrite_module.c b/src/http/modules/ngx_http_rewrite_module.c
> --- a/src/http/modules/ngx_http_rewrite_module.c
> +++ b/src/http/modules/ngx_http_rewrite_module.c
> @@ -167,15 +167,7 @@ ngx_http_rewrite_handler(ngx_http_reques
> code(e);
> }
>
> - if (e->status == NGX_DECLINED) {
> - return NGX_DECLINED;
> - }
> -
> - if (r->err_status == 0) {
> - return e->status;
> - }
> -
> - return r->err_status;
> + return e->status;
> }

Forget this one, it's wrong as it breaks

location /return405 {
return 405;
}

location /error404return405 {
error_page 404 /return405;
return 404;
}

test case (should return 404 status code for
"/error404return405").

Updated patch:

# HG changeset patch
# User Maxim Dounin <mdounin@mdounin.ru>
# Date 1288675107 -10800
# Node ID 39827c7c5c0191aec623b632aacf07e6783be1ce
# Parent 49670f58d178cbef2108a76676b8d79cf2cb648b
Fix double content when return is used in error_page redirection.

Test case:

location / {
error_page 405 /nope;
return 405;
}

location /nope {
return 200;
}

This is expected to return 405 with empty body, but in 0.8.42+ will return
builtin 405 error page as well (though not counted in Content-Length, thus
breaking protocol).

Note that this patch also changes behaviour for "return 302 ..." and
"rewrite ... redirect" used as error handler. E.g.

location / {
error_page 405 /redirect;
return 405;
}

location /redirect {
rewrite ^ http://example.com/;
}

will actually return redirect to "http://example.com/" instead of builtin 405
error page with meaningless Location header. This looks like correct change
and it's in line with what happens on e.g. directory redirects in error
handlers.

diff --git a/src/http/modules/ngx_http_rewrite_module.c b/src/http/modules/ngx_http_rewrite_module.c
--- a/src/http/modules/ngx_http_rewrite_module.c
+++ b/src/http/modules/ngx_http_rewrite_module.c
@@ -167,8 +167,8 @@ ngx_http_rewrite_handler(ngx_http_reques
code(e);
}

- if (e->status == NGX_DECLINED) {
- return NGX_DECLINED;
+ if (e->status < NGX_HTTP_BAD_REQUEST) {
+ return e->status;
}

if (r->err_status == 0) {


Maxim Dounin

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