Bug #69472 [Fbk]: php_sys_readlink ignores misc errors from GetFinalPathNameByHandleA

Edit report at https://bugs.php.net/bug.php?id=69472&edit=1 ID: 69472 Updated by: ab@php.net 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: Hi Jan, I guess the ERROR_ALREADY_EXISTS you see comes from the fopen call. GetLastError() needs to be saved straight after the GetFinalPathNameByHandleA() (or in general, after the function of interest). There are also cases where a function doesn't touch the errno, so if not reset it stays from one of the previous calls (but obviously not the case here). Also, it'd be still interesting whether the files/links involved are on network shares. That could be an additional factor for a reproduce. Maybe you need some more load or break the connectivity on the share. I think we should apply this patch anyway, but it were still useful to know to investigate. IMHO. Thanks. Previous Comments: ------------------------------------------------------------------------ [2015-04-23 13:46:04] jan dot starke at t-systems dot com 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. ------------------------------------------------------------------------ [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. ------------------------------------------------------------------------ The remainder of the comments for this report are too long. To view the rest of the comments, please view the bug report online at https://bugs.php.net/bug.php?id=69472 -- Edit this bug report at https://bugs.php.net/bug.php?id=69472&edit=1