Bug #69472 [Com]: php_sys_readlink ignores misc errors from GetFinalPathNameByHandleA
| From: | jan dot starke at t-systems dot com | Date: | Thu, 23 Apr 2015 13:46:05 +0000 |
| Subject: | Bug #69472 [Com]: php_sys_readlink ignores misc errors from GetFinalPathNameByHandleA | ||
| References: | 1 | Groups: | php.bugs |
| Request: | Send a blank email to php-bugs+get-192295@lists.php.net to get a copy of this message | ||
Edit report at https://bugs.php.net/bug.php?id=69472&edit=1
ID: 69472
Comment by: jan dot starke at t-systems dot com
Reported by: jan dot starke at t-systems dot com
Summary: php_sys_readlink ignores misc errors from
GetFinalPathNameByHandleA
Status: Feedback
Type: Bug
Package: Filesystem function related
Operating System: Windows
PHP Version: Irrelevant
Assigned To: ab
Block user comment: N
Private report: N
New Comment:
In case the error code is returned by NtQueryObject, it would map to STATUS_OBJECT_NAME_COLLISION
...
I don't know why NtQueryObject should return this error code.
Previous Comments:
------------------------------------------------------------------------
[2015-04-23 13:28:09] jan dot starke at t-systems dot com
No, we are not using the patched versions. Our policies enforce us to use official releases only.
But we have some testing environments where we can test such patches.
However, during our test we did not see something interesting in our logs. Could you give me a hint?
Today, I added some debug code:
dwRet = pGetFinalPathNameByHandle(hFile, target, MAXPATHLEN, VOLUME_NAME_DOS);
if (dwRet == 0) {
FILE* teeshop_fh;
teeshop_fh = fopen("C:\\Temp\\GetFinalPathNameByHandle.log", "a");
fprintf(teeshop_fh, "GetFinalPathNameByHandle('%s') returned 0x%08x\n", link,
GetLastError());
fclose(teeshop_fh);
return -1;
}
if(dwRet >= MAXPATHLEN) {
return -1;
}
We observed that GetFinalPathNameByHandle returns 0xb7 (ERROR_ALREADY_EXISTS). Unfortunately, this
error should never be raised by this function (according to Microsoft's documentation). I
assume this error is returned by NtQueryObject, but I do not really know. Does it make sense to post
a bug to Microsoft?
In my eyes it is impossible to create a working testcase, because we cannot reliably create the
error context. We should assume that GetFinalPathNameByHandle may return 0 in any case, without
knowing the cause.
Regards, Jan
------------------------------------------------------------------------
[2015-04-22 08:11:26] ab@php.net
Thanks for the follow up.
Please explain why the usage of GetFileInformationByHandleEx would be better. From tho doc, both
support the same technologies.
Regarding your patch - yeah, you made a good catch. Are you already using the patched version? If
so, you could see something interesting in the error logs.
Btw. what kind of shares are used on those hosts?
Thanks.
------------------------------------------------------------------------
[2015-04-20 21:02:19] jan dot starke at t-systems dot com
Not really. We currently do not know the reason why GetFinalPathNameByHandle returns 0. We host a
large number of microsites, but we observe this behaviour only with a very small number of sites.
Additionally, this problems seems to be IIS/Fast-CGI related. It did not occur with php.exe until
now, only with php-cgi.exe when run within IIS.
To write a test, we must construct a case where GetFinalPathNameByHandle reliably returns 0. As soon
as we do not understand the reason for the behaviour of GetFinalPathNameByHandle, we cannot create a
working test.
As soon as we know more about the root cause of our specific problem, I will let you know.
BTW; I suppose there is another problem with GetFinalPathNameByHandle itself: We observed that
simply renaming one of the folders in open_basedir (and changing the setting accordingly) made the
problem go away. Possibly, a better fix was to not use GetFinalPathNameByHandle at all, but switch
to GetFileInformationByHandleEx...
Regards, Jan
------------------------------------------------------------------------
[2015-04-20 08:15:10] ab@php.net
Thanks for the patch. Could you please add a test for this case as well (check any of the current
*.phpt cases for an example, maybe in ext\standard\tests\file)?
Thanks.
------------------------------------------------------------------------
[2015-04-16 18:15:58] jan dot starke at t-systems dot com
Description:
------------
The Windows API functions GetFinalPathNameByHandleA, which is used by php_sys_readlink, has two
different ways of returning an error:
(1) the return value is different from cchFilePath (which is MAXPATHLEN in php_sys_readlink). This
error is raised if the buffer to store the pathname is not large enough to hold the pathname.
(2) the return value is zero. This error can be caused by one of ERROR_PATH_NOT_FOUND,
ERROR_NOT_ENOUGH_MEMORY or ERROR_INVALID_PARAMETER.
Unfortunately, the second case is not handled by php_sys_readlink. As a result, php_sys_readlink
writes an empty string to target and returns 0 as its length, which both is incorrect.
Expected result:
----------------
php_sys_readlink should return -1 if GetFinalPathNameByHandleA returns 0:
<code>
if (dwRet >= MAXPATHLEN || dwRet == 0) {
return -1;
}
</code>
Actual result:
--------------
if GetFinalPathNameByHandleA returns 0, php_sys_readlink returns 0 :-(
This brakes php_check_specific_open_basedir, which in our case creates some problems
<code>
if (dwRet >= MAXPATHLEN) {
return -1;
}
</code>
------------------------------------------------------------------------
--
Edit this bug report at https://bugs.php.net/bug.php?id=69472&edit=1