[PATCH] fix ngx_realpath() for win32

[Maxim Dounin]

Hello!

On Wed, Nov 02, 2022 at 03:43:50PM +0900, Won-Kyu Park wrote:

> > Hello!
> >
> > On Tue, Nov 01, 2022 at 08:15:48PM +0900, Won-Kyu Park wrote:
> >
> > ...
> >
> > Could you please clarify "as expected"?
> >
> > It doesn't look like _fullpath() resolves symbolic links, which is expected behaviour for $realpath_root.
> >
> 
> You are right but for most situations, translating a relative path to
> fullpath is enough, at least for me. (please see use case)

First of all, $document_root is already absolute, not a relative.  
It can contain some relative references, such as "/../", if you've 
wrote them in the configuration, but it's still absolute.

Resolving relative references is not what the variable is expected 
to do.  If you need an absolute path without any relative elements 
it is always possible to write one in the configuration.

What the $realpath_root variable is expected to do is symlink 
resolution, providing a way to switch to a different site version 
atomically by changing a symlink to a directory with the 
particular site version.  For example, this makes it possible to 
arbitrary change included files in php code between different site 
versions: since symlinks are resolved, all included files are 
loaded from the particular site version, and changing the symlink 
does not create a race condition.  Further, it is also resolves 
opcode cache issues in such deployment schemes, as opcode cache 
essentially makes the race window longer.  See, for example, 
https://serverfault.com/questions/848503/nginx-caching-symlinks 
and https://deployer.org/docs/7.x/avoid-php-fpm-reloading for some 
practical details.

[...]

> > Rather, it looks like GetFinalPathNameByHandle() should be 
> > used if we want to actually support $realpath_root as it is 
> > expected to work, that is, providing path with symlinks 
> > resolved. It might
> > be tricky to use it though, as it is only available in Windows 
> > Vista or later.
> 
> yes. GetFinalPathNameByHandle() is not available in pre-Vista.
> full implementation might be preferred but this simple oneliner solve
> most use cases I guess.

Well, the problem is that the $realpath_root variable with your 
patch applied does not become better regarding its main property, 
that is, symbolic links resolution.  Rather, the relevant code is 
no longer marked to be a stub and therefore becomes silently 
broken rather than not implemented.

Arguably, _fullpath() can be seen as a better stub than the one 
which is currently used.  But I don't think that $realpath_root 
actually guarantees resolution of relative references in the path, 
it is only documented to resolve symlinks.

> 
> - win32/mingw32 only support _fullpath()
> - PHP7 use GetFinalPathNameByHandleW()
> -------------
> 
> use case:
> 
> server {
>         location / {
>             root   ../to/html; # works nicely without problem
>             index  index.php index.html index.htm;
>         }
> 
>         location ~ \.php$ {
>             root           ../to/html; # not work. $realpath_root
> gives wrong value (c:\foobar\nginxroot\..\wiki), $document_root
> returns relative path
>             # for this case, $realpath_root gives expected value with
> this _fullpath fix.
>             #root c:/to/my/absolute/path/html; # works fine
>             fastcgi_index  index.php;
> 
>             # fix to use realpath_root
>             #fastcgi_param SCRIPT_FILENAME
> $document_root$fastcgi_script_name; # not work.
>             fastcgi_param SCRIPT_FILENAME
> $realpath_root$fastcgi_script_name; # works fine with _fullpath fix.
>             ...
>             fastcgi_pass   127.0.0.1:9000;
>         }
> }

The "c:\foobar\nginxroot\..\wiki" and "c:\foobar\wiki" paths are 
equivalent as long as nginxroot exists (and both are absolute).  
The problem with your configuration is that PHP rejects paths with 
".." for security reasons 
(https://github.com/php/php-src/blob/master/sapi/cgi/cgi_main.c#L1048).

While using $realpath_root on Unix systems resolves this, this is 
more like a side effect, and not a guaranteed behaviour.

A better solution might be to avoid using ".." in paths sent to 
PHP.  This might be done either by providing appropriate absolute 
paths in nginx configuration or by changing things to keep the 
php code under nginx prefix (by changing nginx prefix in effect, 
by actually moving directories, or by creating a symlink).

-- 
Maxim Dounin
http://mdounin.ru/