Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#184 closed defect (fixed)

Unspecified behavior when going through multiple named locations

Reported by: www.google.com/accounts/o8/id?id=AItOawnQxODTko53PmY0va90aNN76a_jMUVqVco Owned by: somebody
Priority: major Milestone: 1.3.3
Component: nginx-core Version: 1.3.x
Keywords: named locations error_page Cc:
uname -a: Linux 3.2.13
nginx -V: tested on 1.2.1 and 1.3.2

Description

I have complex configuration which demands multiple jumps between named locations to avoid config duplication. Config is like:

error_page 404 /404.htm;

location = / {

rewrite /showpage/;
error_page 418 = @contcommon;
recursive_error_pages on;
return 418;

}
location @contcommon { #common steps for several locations

if ( [some tests1] ) {

return 403;

}
if ( [some tests2] ) {

return 403;

}

if ( $request_method = POST ) {

error_page 418 = @cgipass; #post requests go directly to cgi
return 418;

}

recursive_error_pages on;
error_page 418 = @cgicache; #get requests go through memcache check
return 418;

}

location @cgicache {

set $memcached_key "$http_host$request_uri:blablabla";
error_page 404 502 504 = @cgipass;
recursive_error_pages on;
memcached_pass path/to/socket;

}

location @cgipass {

#...some checks and rewrites like
rewrite /([a-z]+)(?:/(.*))?$ /cgi-bin/$1.fcg/$2;
proxy_pass ..... #pass to apache

}

root /my/root;
location / {
}

Problem is that memcached_pass is never applied for "GET /", i.e. nginx goes to default location section and tries to open file "/my/root/showpage/index.html" and shows "/404.htm". Placing, e.g. "return 405", at the beginning of @cgipass gives error 405 as it must, but no content handler is executed whatever it was. I tried several which has content handlers like proxy_pass and stub_status. It looked like content_handler field of request is erased at some point. I looked through sources several times, trying to find such place, turned on debugging and carried out different experiments.

After several hours I've found solution which overcomes the problem: put "break" inside @cgicache section. So it began to look like:
location @cgicache {

set $memcached_key "$http_host$request_uri:blablabla";

break; #############that's it

error_page 404 502 504 = @cgipass;
recursive_error_pages on;
memcached_pass path/to/socket;

}

Now memcached_pass began to work and @cgipass is called when no key found in memcache.
BUT! One more strange thing. If memcached_pass is commented out request continues to work! But it shouldn't (it should show "/404.html" as it did without break). Error log says that attempt to read "/my/root/showpage/index.html" is happening and it returns 404 error, but for some unknown reason error_page from @cgicache still works (though location "/" is working at that point) and directs nginx to @cgipass location where final processing occurs.

It seems that script code of rewrite module becomes broken at some point. I'm not sure that my solution is universal, but it worked in such config with "GET /" request.

Change History (8)

comment:1 by Ruslan Ermilov, 12 years ago

I don't see anything wrong here.

For "/", when rewrite gets to @cgicache by following 418 returns, it executes the "set" directive and stops. Since no "if" selected another configuration, and no "break" was used, a changed uri then matches the "/" location and is processed with the static file content handler.

By putting a "break" inside @cgicache you tell rewrite to stop rewriting and assign the request the configuration of containing location. And in this location, the content handler is memcached.

Now, if you comment out memcached but retain "break", the content handler is inherited from the implicit server location, which happens to be "index" content handler (since uri ends with "/"). Index first checks the root+uri+index file, /my/root/showpage/index.html, fails, then tests if its parent directory exists (to return either 403 or 404), and since there's no directory, returns 404 which is then internally redirected to @cgipass. There, uri gets rewritten to /cgi-bin/showpage.fcg/, and rewrite module stops. The uri then matches the "/" location with the static file handler, which is probably not what you wanted. But you already know how to solve it.

The rules of how rewrite directives are processed can be found here:
http://nginx.org/en/docs/http/ngx_http_rewrite_module.html
We should probably emphasize more on the fact that only successful "if" and "break" can change request's configuration.

Please also consider reading http://wiki.nginx.org/IfIsEvil thoroughly.

Last edited 12 years ago by Ruslan Ermilov (previous) (diff)

comment:2 by Maxim Dounin, 12 years ago

Status: newaccepted

I think it's a bug, really. The is no rewrite directives in location @cgicache, and searching for a location after rewrite phase here shouldn't happend. Reduced test case:

server {
    listen 8080;

    location = / {
        rewrite ^ /foo/;
        error_page 418 = @named;
        return 418;
    }

    location @named {
        root /tmp/named;
    }

    location / {
        root /tmp/root;
    }
}

Request to "/" ends up in "location /" instead of "location @named" unless "break" is added to "location @named".

Please try the following patch, it should resolve the issue:

--- a/src/http/ngx_http_core_module.c
+++ b/src/http/ngx_http_core_module.c
@@ -2588,6 +2588,7 @@ ngx_http_named_location(ngx_http_request
 
             r->internal = 1;
             r->content_handler = NULL;
+            r->uri_changed = 0;
             r->loc_conf = (*clcfp)->loc_conf;
 
             /* clear the modules contexts */

Thank you for your report.

comment:3 by www.google.com/accounts/o8/id?id=AItOawnQxODTko53PmY0va90aNN76a_jMUVqVco, 12 years ago

Yes, now it works without break.
Спасибо.

comment:4 by www.google.com/accounts/o8/id?id=AItOawnQxODTko53PmY0va90aNN76a_jMUVqVco, 12 years ago

By the way. Previously i used patch from here http://catap.ru/patches/nginx/ngx_http_rewrite_named-0.1.patch to enable rewrite to named locations. It worked nicely in 0.7.63, but in 1.2.1 and later using of such rewrites in processing of first keepalive request leads to hanging of following requests of the same keepalive session. i.e. request to load main page runs successfully, but loading of CSS file hangs. Looking at tcpdump output showed that requests were in the same session, request for CSS is present, but no reply. During this access log shows successful processing of CSS-file request.
May be that patch can be corrected in some simple way to avoid such hanging?

comment:5 by Maxim Dounin, 12 years ago

In [4737/nginx]:

(The changeset message doesn't reference this ticket)

comment:6 by Maxim Dounin, 12 years ago

Fix committed, thanks.

As for Kirill's patch, it's probably broken by rewrite module changes in 0.8.42 to support "return CODE text". No idea if it will be easy to fix it, likely yes.

comment:7 by Maxim Dounin, 12 years ago

Resolution: fixed
Status: acceptedclosed

comment:8 by Maxim Dounin, 12 years ago

In 4788/nginx:

(The changeset message doesn't reference this ticket)

Note: See TracTickets for help on using tickets.