HTTP_WebDAV_Server 1.0.0RC5 BC break; path confusion in different environments
| From: | Tim Jackson | Date: | Tue, 09 Nov 2010 14:46:49 +0000 |
| Subject: | HTTP_WebDAV_Server 1.0.0RC5 BC break; path confusion in different environments | ||
| Groups: | php.pear.dev | ||
| Request: | Send a blank email to pear-dev+get-53885@lists.php.net to get a copy of this message | ||
Hi,
My colleagues and I have been successfully using HTTP_WebDAV_Server for a
long time and it generally works. One of the things we've always had to
work around is that versions up to 1.0.0RC4 looked for
$_SERVER['PATH_INFO'], which is not defined in many environments and seems
to be a rather historic way of doing things; according to
http://php.net/manual/en/reserved.variables.server.php
it was for cases
where you had URLs like:
http://www.example.com/php/path_info.php/some/stuff?foo=bar
in which case PATH_INFO would be "/some/stuff"
In most modern environments many people (including us) just use something
like
RewriteRule /.*
/filesystem/path/to/some_front_controller_maybe_outside_the_docroot.php
and handle all the URLs in that front controller, in which case a request
for "/some/stuff?foo=bar" typically results in the following server
variables (these can also vary according to the use of mod_php vs CGI,
reverse proxies etc.; let's ignore that for now):
PATH_INFO is not defined
REQUEST_URI = /some/stuff?foo=bar
SCRIPT_NAME = /some/stuff
We always hacked around this by using a derived class of
HTTP_WebDAV_Server and setting $this->_SERVER['PATH_INFO'] to
$_SERVER['REQUEST_URI'] in the constructor (perhaps it should have been
SCRIPT_NAME although we don't use query strings in practice so it doesn't
matter). However, HTTP_WebDAV_Server 1.0.0RC5 has changed the way URLs are
handled and (on line 169) now does this:
$path_info = substr($this->_SERVER["REQUEST_URI"],
strlen($this->_SERVER["SCRIPT_NAME"]));
It's good that it no longer relies on PATH_INFO, but the new code means
that $path_info ends up set to the wrong thing (and doesn't work in at
least our environment), because if you take the above example,
strlen('/some/stuff') = 11;
$path_info = substr('/some/stuff?foo=bar', 11) = '?foo=bar'; // this
matches neither the definition of "path info" in the PHP manual nor the
previous usage.
Even in a situation such as that envisaged by the PHP manual
("/php/path_info.php/some/stuff?foo=bar"), I still don't think it would
work correctly, because with the RC5 code, $path_info would end up being
"/some/stuff?foo=bar", which also doesn't match the former usage of
$_SERVER['PATH_INFO'].
Similarly in a simpler case without a query string (e.g. request for
"/foo"), $path_info ends up as an empty string (strlen('/foo')=4;
substr('/foo',4)='') and is simply set to "/" as a result (line 173).
Now, we don't work in an environment where "path info" is used, but
looking at how $path_info is used in HTTP_WebDAV_Server and going back to
the example URLs of EITHER "/php/path_info.php/some/stuff?foo=bar" or
"some/stuff?foo=bar", my colleagues and I think it is *supposed* to be
filled with "/some/stuff" (which indeed matches the previous use by
HTTP_WebDAV_Server of $_SERVER['PATH_INFO'] in the case of the URL
"/php/path_info.php/some/stuff?foo=bar"), but that's not what the new code
is doing. Surely then line 169 should be replaced with something like
this:
if (array_key_exists('PATH_INFO', $this->_SERVER)) {
// old-style PATH_INFO handling i.e. /php/path_info.php/some/stuff?foo=bar
$path_info = $this->_SERVER['PATH_INFO']; // /some/stuff
} else {
$path_info = $this->_SERVER['SCRIPT_NAME']; // /some/stuff
}
?
However, looking at the usage of $_SERVER['SCRIPT_NAME'] in the code,
there are several places where it appears that HTTP_WebDAV_Server tries to
reconstruct the "old style" paths, such as line 724[RC5]:
$href = $this->_mergePaths($this->_SERVER['SCRIPT_NAME'], $path);
This makes sense in the "old style" environments
(/php/path_info.php/some/stuff) but not for more modern environments. A
similar thing applies on lines 164, 177 and 872 [also RC5].
Therefore, it seems like the code needs to establish clear concepts of:
1. "an optional URL prefix" (/php/path_info.php),
2. "the 'real' path, excluding any prefix and the query string" (/some/stuff)
3. "the query string" (?foo=bar).
and use these consistently.
The problem is that the $_SERVER variables simply don't provide these in a
way which is consistent across environments. So, I suggest that they need
initially decoding into some consistent variables (handling environmental
differences at this stage), and those decoded variables (instead of
$this->_SERVER or $_SERVER) are used everywhere.
The query string is not used a lot, so to focus on the other two; the
original line 169 would then be replaced with something like:
if (array_key_exists('PATH_INFO', $this->_SERVER)) {
// old-style PATH_INFO handling i.e. /php/path_info.php/some/stuff?foo=bar
$path_info = $this->_SERVER['PATH_INFO']; // /some/stuff
$path_prefix = $this->_SERVER['SCRIPT_NAME']; // /php/path_info.php
} else {
$path_info = $this->_SERVER['SCRIPT_NAME'];
$path_prefix = '';
}
// N.B. other code would need to be changed as well
Then, line 724[RC5] would look something like (pseudo-code; ignoring
variable scope here):
$href = $this->_mergePaths($path_prefix, $path_info);
(N.B. line 1551[RC5] looks like it could also be cleaned up as a result)
There are some more potential environmental differences which should
probably be accounted for, but I thought I'd get some initial feedback.
Thanks,
Tim