Opened 13 years ago
Last modified 9 years ago
#97 accepted defect
try_files and alias problems
| Reported by: | Maxim Dounin | Owned by: | somebody |
|---|---|---|---|
| Priority: | minor | Milestone: | |
| Component: | nginx-core | Version: | |
| Keywords: | try_files alias | Cc: | |
| uname -a: | na | ||
| nginx -V: | na | ||
Description (last modified by )
# bug: request to "/test/x" will try "/tmp/x" (good) and
# "/tmp//test/y" (bad?)
location /test/ {
alias /tmp/;
try_files $uri /test/y =404;
}
# bug: request to "/test/x" will fallback to "fallback" instead of "/test/fallback"
location /test/ {
alias /tmp/;
try_files $uri /test/fallback?$args;
}
# bug: request to "/test/x" will try "/tmp/x/test/x" instead of "/tmp/x"
location ~ /test/(.*) {
alias /tmp/$1;
try_files $uri =403;
}
Or document special case for regexp locations with alias? See 3711bb1336c3.
# bug: request "/foo/test.gif" will try "/tmp//foo/test.gif"
location /foo/ {
alias /tmp/;
location ~ gif {
try_files $uri =405;
}
}
Change History (26)
comment:1 by , 13 years ago
| Description: | modified (diff) |
|---|---|
| Status: | new → accepted |
comment:2 by , 13 years ago
| Description: | modified (diff) |
|---|
comment:3 by , 12 years ago
| Description: | modified (diff) |
|---|
comment:5 by , 11 years ago
| Description: | modified (diff) |
|---|---|
| sensitive: | → 0 |
comment:7 by , 11 years ago
I still confirm Maxim's reported bug 1st example of bug as of today with 1.6.0.
alias core function is to do precisely that, and its combination with try_files is one of the simplest I can think of.
Could its severity be incremented?
comment:8 by , 10 years ago
This bug is really annoying, simple task to have few sites in different locations under one vhost becomes problem...
comment:9 by , 10 years ago
Could someone please fix this bug? I run into it fairly often. At this point it's one of the very oldest open tickets on this tracker.
comment:10 by , 10 years ago
Could you please try the following patch?
# HG changeset patch
# User Sergey Kandaurov <pluknet@nginx.com>
# Date 1401711096 -14400
# Mon Jun 02 16:11:36 2014 +0400
# Node ID 5dc95efa0ea15bfeb0caf17a916c40ad8817211c
# Parent 790ba7484bb67d2a1e1a4ce55eeb0fe149ebd176
Changed try_files/alias behavior in prefix location (ticket #97).
When location matches the beginning of the try_files parameter, the matched
part is cut from the path to a file and substituted with alias like it always
worked for try_files with variables (in particular, try_files $uri). Below,
the request to "/test/x" will try "/tmp/x" and "/tmp/y".
location /test {
alias /tmp;
try_files $uri /test/y =404;
}
The last try_files parameter now always ignores aliasing irrelevant to the
presence of variables in the directive configuration. In the example below,
an internal redirect to "/test/fallback" is made.
location /test/ {
alias /tmp/;
try_files $uri /test/fallback?$args;
}
diff -r 790ba7484bb6 -r 5dc95efa0ea1 src/http/ngx_http_core_module.c
--- a/src/http/ngx_http_core_module.c Wed May 28 20:18:05 2014 +0400
+++ b/src/http/ngx_http_core_module.c Mon Jun 02 16:11:36 2014 +0400
@@ -1286,17 +1286,19 @@ ngx_http_core_try_files_phase(ngx_http_r
path.len = e.pos - path.data;
*e.pos = '\0';
-
+ }
+
+ test_dir = tf->test_dir;
+
+ tf++;
+
+ if (tf->lengths || tf->name.len) {
if (alias && ngx_strncmp(name, clcf->name.data, alias) == 0) {
ngx_memmove(name, name + alias, len - alias);
path.len -= alias;
}
}
- test_dir = tf->test_dir;
-
- tf++;
-
ngx_log_debug3(NGX_LOG_DEBUG_HTTP, r->connection->log, 0,
"trying to use %s: \"%s\" \"%s\"",
test_dir ? "dir" : "file", name, path.data);
comment:12 by , 10 years ago
And did you ever try to test the proposed patch?
Due to the questionable nature of the behavior change this patch introduces,
it will not likely be committed without proper feedback and/or testing.
comment:13 by , 10 years ago
I can test it on my test project. But I don't have test suite large enough to fully test it.
comment:14 by , 10 years ago
I have a work around for the issue until the bug is fixed. It is using the 'evil' if statements, but it does handle looking for files and directories before routing to index.
Code highlighting:
location /api { ## URL string to use for api ## alias /home/api/site_files/; ## Site root for api code ## ## Check for file existing and if there, stop ## if (-f $request_filename) { break; } ## Check for file existing and if there, stop ## if (-d $request_filename) { break; } ## If we get here then there is no file or directory matching request_filename ## rewrite (.*) /api/index.php?$query_string; ## Normal php block for processing ## location ~ \.php$ { fastcgi_split_path_info ^(.+\.php)(/.+)$; fastcgi_pass unix:/var/run/php5-fpm.sock; fastcgi_index index.php; include fastcgi_params; } }
follow-up: 16 comment:15 by , 10 years ago
I also ran into this bug, but for me it only occurs when $is_args$args is in the try_files. If I leave off $is_args$args, the try_files properly routes where expected.
A workaround is simply to double the path such as:
location /test/ {
alias /tmp/;
try_files $uri /test/test/fallback?$args;
}
comment:16 by , 10 years ago
This still doesn't seem to address directories that are in the alias location. Even if I change your code to try_files $uri $uri/ /test/test/fallback?$args; I don't get directory access when needed. If I have a file rules.txt in /tmp/docs/ I should be able to access that file from http://example.com/test/docs/rules.txt, however an attempt here only brings me back to the fallback.
With my solution above you can access existing files in the root of your alias as well as in subdirectories.
Replying to Andrew Stoltz:
I also ran into this bug, but for me it only occurs when $is_args$args is in the try_files. If I leave off $is_args$args, the try_files properly routes where expected.
A workaround is simply to double the path such as:
location /test/ { alias /tmp/; try_files $uri /test/test/fallback?$args; }
follow-up: 18 comment:17 by , 10 years ago
Replying to Maxim Dounin:
# bug: request to "/test/x" will try "/tmp/x" (good) and
# "/tmp//test/y" (bad?)
location /test/ {
alias /tmp/;
try_files $uri /test/y =404;
}
I believe this is not a bug. It seems there is confusion with the URI path "test" in location and the directory "test" in try_files. These are not related, as the following rewrite shows:
# request to "/testlink/x" will try "/tmp/x" (good)
# and "/tmp/testdir/y" (also good IMHO)
location /testlink/ {
alias /tmp/;
try_files $uri testdir/y =404;
}
Only the last part (here =404) represents a status code or alternately, a link. Otherwise it is a file system path, relative to the directory specified by root or alias as clearly stated in the docs .
I also believe bug #176 suffers from the same confusion. This is the expected behaviour:
2012/06/14 14:17:52 [debug] 17647#0: *15 http script var: "/test/asdf"
2012/06/14 14:17:52 [debug] 17647#0: *15 trying to use dir: "/asdf" "/doesnt/exist/asdf"
2012/06/14 14:17:52 [debug] 17647#0: *15 http script copy: "/test/index.php?"
2012/06/14 14:17:52 [debug] 17647#0: *15 trying to use file: "/index.php?" "/doesnt/exist/index.php?"
Whereas this is actually an error:
2012/06/14 14:25:03 [debug] 17678#0: *17 trying to use file: "/test/index.php" "/doesnt/exist/test/index.php"
/test should here be aliased to /doesnt/exist, presumably that also is valid inside the location declaration for /test itself.
comment:18 by , 10 years ago
Replying to geir.aalberg.com:
Replying to Maxim Dounin:
# bug: request to "/test/x" will try "/tmp/x" (good) and # "/tmp//test/y" (bad?) location /test/ { alias /tmp/; try_files $uri /test/y =404; }I believe this is not a bug. It seems there is confusion with the URI path "test" in
locationand the directory "test" intry_files.
The problem is that $uri in the try_files above is /test/x. That is, try_files $uri /test/y =404 is expected to be equivalent to try_files /test/x /test/y =404, but it's not.
Only the last part (here
=404) represents a status code or alternately, a link. Otherwise it is a file system path, relative to the directory specified byrootoraliasas clearly stated in the docs .
Docs say that "the path to a file is constructed from the file parameter according to the root and alias directives", it doesn't try to say "relative to the directory specified by...".
follow-up: 20 comment:19 by , 10 years ago
The problem is that $uri in the
try_filesabove is/test/x. That is,try_files $uri /test/y =404is expected to be equivalent totry_files /test/x /test/y =404, but it's not.
I'm no expert on nginx, but coming from an Apache background I would certainly not expect this. The alias translation phase should come before the map_to_storage phase. That means $uri would have the /test/ part stripped out before further processing from URL to partial filepath, so the call is actually try_files x /test/y =404 ($uri, in the context of try_files is a misnomer).
Remember, the syntax is try_files file file =code; /test/y, being a file and not an URL is not subject to aliasing, only resolving in regard to documentroot. I'm sure there is some clever way to test the actual value of $uri at this point, hopefully without having to recompile.
Docs say that "the path to a file is constructed from the file parameter according to the root and alias directives", it doesn't try to say "relative to the directory specified by...".
Actually, the docs for root says "a path to the file is constructed by merely adding a URI to the value of the root directive", i.e. string concatenation. That is exactly how you resolve relative URLs and filepaths before normalization. Also, an alias "defines a replacement for the specified location", i.e. overrides the current value of root (hence they both live in the variable $document_root).
I'm not sure what you would expect to be the correct resolution for /test/y, but replace that with /etc/passwd and you'll see why ignoring the current documentroot is a bad idea. Remember, try_files must work (albeit differently) inside both location /test and location /test/, so you cannot have a leading slash treated as an absolute file path.
comment:20 by , 10 years ago
Replying to geir.aalberg.com:
The problem is that $uri in the
try_filesabove is/test/x. That is,try_files $uri /test/y =404is expected to be equivalent totry_files /test/x /test/y =404, but it's not.
I'm no expert on nginx, but coming from an Apache background I would certainly not expect this. The alias translation phase should come before the map_to_storage phase. That means $uri would have the
/test/part stripped out before further processing from URL to partial filepath, so the call is actuallytry_files x /test/y =404($uri, in the context of try_files is a misnomer).
The $uri is just a variable, and it's value is strictly defined: it's current URI of a request, that is, it is /test/x in this case. And any other variable will do the same, compare:
# will test /tmp/x, /tmp/foo and /tmp//test/y
location /test/ {
alias /tmp/;
set $foo "/test/foo";
try_files $uri $foo /test/y =404;
}
While try_files arguments are called "files" and specify files to try, they are expected to be in URI space. When trying these files, nginx converts each arguments from URI space to a file path. This works well when using root - because mapping from URI space to filesystem paths is trivial. But there are various problems related to handling this with alias. And the above problem is one of them.
follow-up: 22 comment:21 by , 10 years ago
The $uri is just a variable, and it's value is strictly defined [...]
Well, it also states that "the value of $uri may change during request processing".
The crucial question is whether $uri is aliased before calling try_files (which would be the correct behaviour IMHO), or that ngx_http_core_try_files_phase() performs aliasing on each of the parameters after concatenation and variable substitution. So I did some testing, compiled using the latest stable release; the results are available on | Pastebin. And as can be seen from the following line, it does indeed seem to be the latter case that is happening (see example below).
While try_files arguments are called "files" and specify files to try, they are expected to be in URI space.
Source for this? It is not the impression I get from reading the docs. One might possibly draw that conclusion from observing how nginx currently behaves; however we both agree that the current behaviour is inconsistent and buggy. The critical part lies in determining where the bug manifests itself and what the correct behaviour should be.
It makes no sense to identically modify a string constant for each request (why not write the modified value in the first place?). It could also make it impossible to hardcode file path values which would collide with the URI part being aliased, as there is no escaping syntax. Finally, aliasing using $uri in try_files bar/$uri $uri/baz =404 doesn't work unless $uri is anywhere but at the start of the parameter, viz:
*11 http script var: "/test/bar" *11 http script copy: "/baz" *11 trying to use file: "bar/baz" "/home/geira/scratch/bar/baz"
vs.
*12 http script copy: "bar/" *12 http script var: "/test/baz" *12 trying to use file: "bar//test/baz" "/home/geira/scratch/bar//test/baz"
As stated previously, I believe the correct procedure would be to rewrite $uri before concatenation in try_files, not afterwards as is currently the case. From this point of view, the current behaviour of try_files $uri /test/y =404 works just as expected. To implement this correctly however would require a major rewrite.
In the mean time I suggest avoiding the use of $uri in conjunction with alias in favour of regex captures. This way you can control the aliasing yourself and construct correct filepaths that work regardless of position in the parameter ... as long as the current functionality is not broken by changing the parts that actually work instead of those who need fixing.
comment:22 by , 10 years ago
Replying to geir.aalberg.com:
The $uri is just a variable, and it's value is strictly defined [...]
Well, it also states that "the value of $uri may change during request processing".
But it's not something what happens during location matching. Either way, as already shown - this is completely irrelevant to the problem discussed.
While try_files arguments are called "files" and specify files to try, they are expected to be in URI space.
Source for this? It is not the impression I get from reading the docs. One might possibly draw that conclusion from observing how nginx currently behaves; however we both agree that the current behaviour is inconsistent and buggy. The critical part lies in determining where the bug manifests itself and what the correct behaviour should be.
The behaviour is perfectly clear and consistent when using root.
It makes no sense to identically modify a string constant for each request (why not write the modified value in the first place?).
Yes, sure. But the problem is consistency. Any difference between static strings and strings with variables makes it harder to use and breaks POLA. Hence the bug.
It could also make it impossible to hardcode file path values which would collide with the URI part being aliased, as there is no escaping syntax.
As already said, arguments are expected to be in URI space, not file paths, so there is no collision problem. Hardcoding corresponding URI will do the trick.
Finally, aliasing using $uri in
try_files bar/$uri $uri/baz =404doesn't work unless $uri is anywhere but at the start of the parameter, viz:
*11 http script var: "/test/bar" *11 http script copy: "/baz" *11 trying to use file: "bar/baz" "/home/geira/scratch/bar/baz"vs.
*12 http script copy: "bar/" *12 http script var: "/test/baz" *12 trying to use file: "bar//test/baz" "/home/geira/scratch/bar//test/baz"
That's expected and specially checked in the code - if the try_files parameter doesn't starts with what location matched, we don't try to replace the part matched by a location but rather treat alias similar to root.
As stated previously, I believe the correct procedure would be to rewrite $uri before concatenation in
try_files, not afterwards as is currently the case. From this point of view, the current behaviour oftry_files $uri /test/y =404works just as expected. To implement this correctly however would require a major rewrite.
You assume that at some point in the example discussed $uri becomes "x". This is simply not true and never happens. The $uri variable remains "/test/x". It is file path which is changed as per alias directive, not $uri.
comment:23 by , 10 years ago
| Keywords: | try_files alias added |
|---|
Added case from ticket #176.