Close GH-16718: Add SAPI module folder to DLL search path #16958
Conversation
On Windows, it is customary to put dependency DLLs in the main PHP folder, i.e. right besides php.exe. If the SAPI is implemented as .exe, dependency lookup works fine, since DLLs are always searched in the folder of the .exe. However, for Webserver modules, this usually does not work, since the Webserver's executable likely resides in a different folder. While there are obviously solutions to this, the Windows error messages if a module can't be loaded are confusing, so users and even popular AMP distributions often choose to put the DLLs in the folder of the Webserver (e.g. right besides httpd.exe). That can easily cause more confusion later, when users update to a newer PHP version, which requires more recent dependencies. Thus, to simplify setup and to bring the behavior more in line with CLI and CGI SAPIs, we add the SAPI module folder to the DLL search path. We keep that simple, and do not cater to long paths, and silently don't change the DLL search order in case of unexpected failures. Since we are using `SetDllDirectory()` for all SAPIs, that effectively also prevents DLLs being looked up in the current folder, what might break some scripts, but can also avoid confusion regarding ZTS builds, where the current directory of the process is not necessarily what is reported by `getcwd()`. It is also a small security measure, even stronger than safe DLL search mode[1]. Note that this certainly does not solve all problems related to DLL lookup, e.g. that the application folder is still searched first, and that already loaded DLLs will not be searched and loaded again. But it appears to be a sensible improvement, and hopefully prevents others to put DLLs in folders where they don't belong, in the future. [1] <https://learn.microsoft.com/en-us/windows/win32/dlls/dynamic-link-library-search-order>
cmb69
changed the title
Switching to a static nghttp2_a.lib is certainly preferable. I always dislike dependent dll’s, especially if they are linked in a single php_x.dll. In my own builds there is no need for a nghttp2.dll. I compile nghttp2 using a custom GNU makefile. For https://github.com/winlibs/nghttp2 a more Windows native compiling method would be the best way to go. |
|
Thanks @Jan-E!
See winlibs/winlib-builder#31 (and winlibs/winlib-builder#32 for the changes regarding cURL). Do you have any thoughts on this PR in general ("Add SAPI module folder to DLL search path")? |
|
FWIW it looks reasonable to me but I don't use Windows much so will leave it up to you to decide... |
On Windows, it is customary to put dependency DLLs in the main PHP folder, i.e. right besides php.exe. If the SAPI is implemented as .exe, dependency lookup works fine, since DLLs are always searched in the folder of the .exe. However, for Webserver modules, this usually does not work, since the Webserver's executable likely resides in a different folder. While there are obviously solutions to this, the Windows error messages if a module can't be loaded are confusing, so users and even popular AMP distributions often choose to put the DLLs in the folder of the Webserver (e.g. right besides httpd.exe). That can easily cause more confusion later, when users update to a newer PHP version, which requires more recent dependencies.
Thus, to simplify setup and to bring the behavior more in line with CLI and CGI SAPIs, we add the SAPI module folder to the DLL search path. We keep that simple, and do not cater to long paths, and silently don't change the DLL search order in case of unexpected failures.
Since we are using
SetDllDirectory()for all SAPIs, that effectively also prevents DLLs being looked up in the current folder, what might break some scripts, but can also avoid confusion regarding ZTS builds, where the current directory of the process is not necessarily what is reported bygetcwd(). It is also a small security measure, even stronger than safe DLL search mode[1].Note that this certainly does not solve all problems related to DLL lookup, e.g. that the application folder is still searched first, and that already loaded DLLs will not be searched and loaded again. But it appears to be a sensible improvement, and hopefully prevents others to put DLLs in folders where they don't belong, in the future.
[1] https://learn.microsoft.com/en-us/windows/win32/dlls/dynamic-link-library-search-order
Note that I obviously can't check for all SAPIs, but some testing with an older Apache (2.4.39) from apachelounge.com worked nicely with a full snapshot build (the only extension that couldn't be loaded was php_curl.dll, since Apache also ships nghttp2.dll, which is version 1.40.0.0 here, while the current master builds use 1.63.0.0; winlibs should probably consider to ship a static nghttp2 library to avoid this problem).
This may be of interest to @Jan-E and maybe to @dunglas (FrankenPHP).