String garbling when using nested regex locations

[launchpad.net/~blubberdiblub]

In certain circumstances with certain configurations (I'm not sure about the exact conditions), the string passed as SCRIPT_NAME to phm-fpm gets garbled (it may have been garbled earlier, tho, for instance during fastcgi_split_path_info or during assignment to the fastcgi_param).

We used to have this problem with an earlier version, too (1.4.1 or 1.6.3, I don't remember), in a different config file, which we rewrote (and which is now uglier).

Now it showed up in this config file which I present an excerpt of here when we upgraded from 1.6.3 to 1.8.0.

In the fastcgi hexdump note the variable SCRIPT_NAME, which gets a value of ^/pm/recommended/index.php assigned by nginx. However, the expected value is /recommended/index.php. It appears as if nginx mixed one of the regexes with the path to be parsed.

The request for which that happened was GET /pma/recommended/ HTTP/1.0.

[launchpad.net/~blubberdiblub]

hexdump of what nginx sent to php-fpm

[Maxim Dounin]

Simplified configuration to trigger this:

location /pma {
    alias /opt/dezem/var/www/sites/pma/htdocs;

    location ~ ^/pma/.*/$ {
        try_files /recommended/index.php =405;
    }
}

Similar configuration is already in ticket #97:

# bug: request "/foo/test.gif" will try "/tmp//foo/test.gif"
location /foo/ {
    alias /tmp/;
    location ~ gif {
        try_files $uri =405;
    }
}

It tests a wrong file in try_files (in your case, you compensate it by a regex in fastcgi_split_path_info). Additionally, if a file is nevertheless found - it uses wrong URI after try_files. Wrong URI seems to be a regression introduced by c985d90a8d1f (nginx 1.7.1).

I'll take a look how to fix this properly.

[Maxim Dounin]

Here is a patch which fixes the problem (both problems, actually: with incorrect file tried and the regression with wrong URI used). Testing is appreciated.

# HG changeset patch
# User Maxim Dounin <mdounin@mdounin.ru>
# Date 1438038209 -10800
#      Tue Jul 28 02:03:29 2015 +0300
# Node ID ab2f7ea48b3accd880163f7785185a9bd44e9a29
# Parent  701a1596905f5a07c6b487e46a4da4ace7656d5c
Fixed wrong URI after try_files in nested location (ticket #97).

The following configuration with alias, nested location and try_files
resulted in wrong file being used.  Request "/foo/test.gif" tried to
use "/tmp//foo/test.gif" instead of "/tmp/test.gif":

    location /foo/ {
        alias /tmp/;
        location ~ gif {
            try_files $uri =405;
        }
    }

Additionally, rev. c985d90a8d1f introduced a regression if
the "/tmp//foo/test.gif" file was found (ticket #768).  Resulting URI
was set to "gif?/foo/test.gif", as the code used clcf->name of current
location ("location ~ gif") instead of parent one ("location /foo/").

Fix is to use r->uri instead of clcf->name in all cases in the
ngx_http_core_try_files_phase() function.  It is expected to be
already matched and identical to the clcf->name of the right
location.

diff --git a/src/http/ngx_http_core_module.c b/src/http/ngx_http_core_module.c
--- a/src/http/ngx_http_core_module.c
+++ b/src/http/ngx_http_core_module.c
@@ -1240,7 +1240,7 @@ ngx_http_core_try_files_phase(ngx_http_r
             *e.pos = '\0';
 
             if (alias && alias != NGX_MAX_SIZE_T_VALUE
-                && ngx_strncmp(name, clcf->name.data, alias) == 0)
+                && ngx_strncmp(name, r->uri.data, alias) == 0)
             {
                 ngx_memmove(name, name + alias, len - alias);
                 path.len -= alias;
@@ -1324,6 +1324,8 @@ ngx_http_core_try_files_phase(ngx_http_r
             }
 
         } else {
+            name = r->uri.data;
+
             r->uri.len = alias + path.len;
             r->uri.data = ngx_pnalloc(r->pool, r->uri.len);
             if (r->uri.data == NULL) {
@@ -1331,8 +1333,8 @@ ngx_http_core_try_files_phase(ngx_http_r
                 return NGX_OK;
             }
 
-            p = ngx_copy(r->uri.data, clcf->name.data, alias);
-            ngx_memcpy(p, name, path.len);
+            p = ngx_copy(r->uri.data, name, alias);
+            ngx_memcpy(p, path.data, path.len);
         }
 
         ngx_http_set_exten(r);

[Maxim Dounin <mdounin@…>]

In bd55d75a1410502a9b4ac6fb44ec4528437b530a/nginx:

Fixed wrong URI after try_files in nested location (ticket #97).

The following configuration with alias, nested location and try_files
resulted in wrong file being used. Request "/foo/test.gif" tried to
use "/tmpfoo/test.gif" instead of "/tmp/test.gif":

location /foo/ {

alias /tmp/;
location ~ gif {

try_files $uri =405;

}

}

Additionally, rev. c985d90a8d1f introduced a regression if
the "/tmpfoo/test.gif" file was found (ticket #768). Resulting URI
was set to "gif?/foo/test.gif", as the code used clcf->name of current
location ("location ~ gif") instead of parent one ("location /foo/").

Fix is to use r->uri instead of clcf->name in all cases in the
ngx_http_core_try_files_phase() function. It is expected to be
already matched and identical to the clcf->name of the right
location.

[Maxim Dounin]

Fix committed, thank you.

[Maxim Dounin <mdounin@…>]

In 6341:2797b4347a2a/nginx:

Fixed wrong URI after try_files in nested location (ticket #97).

The following configuration with alias, nested location and try_files
resulted in wrong file being used. Request "/foo/test.gif" tried to
use "/tmpfoo/test.gif" instead of "/tmp/test.gif":

location /foo/ {

alias /tmp/;
location ~ gif {

try_files $uri =405;

}

}

Additionally, rev. c985d90a8d1f introduced a regression if
the "/tmpfoo/test.gif" file was found (ticket #768). Resulting URI
was set to "gif?/foo/test.gif", as the code used clcf->name of current
location ("location ~ gif") instead of parent one ("location /foo/").

Fix is to use r->uri instead of clcf->name in all cases in the
ngx_http_core_try_files_phase() function. It is expected to be
already matched and identical to the clcf->name of the right
location.