PemFTP Failing to Fall Back to Pure PHP Implementation

[merty]

In /wp-admin/includes/class-ftp.php on line 902:

if(!@dl($prefix . 'sockets.' . PHP_SHLIB_SUFFIX)) $mod_sockets=FALSE;

This is the if-check PemFTP uses to try loading php_sockets extension if it is not enabled already, and to fall back to pure PHP implementation of PemFTP when it fails to load it.

However, if dl() is not available either because the PHP is running in Safe Mode or PHP is built with ZTS support, this if-check silently fails and does not update the $mod_sockets variable with the value FALSE.

To fix that, we can first check whether dl() is callable or not and if it is not callable, we can immediately set $mod_sockets to FALSE and make PemFTP use the pure PHP implementation instead of the one that uses php_sockets.

We just need to replace the line above, with the following:

if(!is_callable('dl') || !@dl($prefix . 'sockets.' . PHP_SHLIB_SUFFIX)) $mod_sockets=FALSE;

[kurtpayne]

Alternate patch

[kurtpayne]

I like the idea of checking that dl() is available before calling it. That function could be disabled by the server admin.

When setting the value of $mod_sockets, is there a downside to checking for the existence of the socket functions directly?

$mod_sockets = function_exists( 'socket_set_option' );

[merty]

Technically there is no difference but extension_loaded('sockets') looks like a better coding practice to me.

The actual issue, however, is not detecting the availability of php_sockets. The issue is, wherever dl() is not available, the if-check silently fails and therefore it does not set $mod_sockets's value to FALSE even though it should.

dl() is deprecated in PHP 5.3 and completely removed in PHP6. Either we can completely remove the part where it tries to load the extension in the runtime, or we can just check the availability of dl() and prevent unexpected behaviors.

[dd32]

. If the functionality of loading modules is not available or has been disabled (either by setting enable_dl off or by enabling safe mode in php.ini) an E_ERROR is emitted and execution is stopped. If dl() fails because the specified library couldn't be loaded, in addition to FALSE an E_WARNING message is emitted.

​This makes it sound like the Execution is halted if loading modules is disabled.. which doesn't sound right to me.. btu failing with a true-ey value when it's disabled doesn't sound right either..

As for the code in PemFTP, as it's an external library, anything here should also be raised as a ticket/issue for the library - although i seem to recall that development of PemFTP doesn't have any active tracker.

As for the code here in WordPress, I'm all for changing to making sure it's callable, and checking capabilities rather than the result of dl(). although, I think we should keep to using extension_loaded() as in the if block that surounds that code, rather than function_exists().

For reference, I believe this is the only location in WordPress that we actually try using dl(), We don't do similar things for MySQL or GD (although, they'd benefit more than FTP would from it's usage).

[merty]

Unfortunately PemFTP is no longer being developed by its developer. The last change made on the code is 3.5 years old. I'm thinking of forking it to keep it up-to-date.

Until WordPress completely drops support for PHP5, which is something we are years away I believe, using is_callable() is the best solution I think. It harms no one but may benefit someone.

[nacin]

We should probably commit this. This causes execution to terminate otherwise.

[dd32]

For what it's worth, I couldn't cause the script to die.

If dl is disabled via safemode or enable_dl = Off, the function simply returns false. If the function is disabled via disable_functions, it returns null. False or Null would trigger setting it to false.

in the above cases, is_callable() would return true, as it could be called. function_exists() would return false however. This may differ in a binary that it was specifically removed in though, which would cause it to die.

Instead, I'll commit a version which should avoid any issues..

[dd32]

In [20311]:

WP_Filesystem: PemFTP: Detect dl() being disabled in order to avoid any fatals which may be produced when the Sockets extension is not available. Props to kurtpayne for initial patch, Fixes #20187

[nacin]

Per the docs on php.net/dl:

If the functionality of loading modules is not available or has been disabled (either by setting enable_dl off or by enabling safe mode in php.ini) an E_ERROR is emitted and execution is stopped.

I was able to reproduce this even with the error suppression operator.

[dd32]

Replying to nacin:

Per the docs on php.net/dl:

If the functionality of loading modules is not available or has been disabled (either by setting enable_dl off or by enabling safe mode in php.ini) an E_ERROR is emitted and execution is stopped.

I was able to reproduce this even with the error suppression operator.

In both of those cases, for me, an E_ERROR was raised, execution of the dl() operation ceased, and script execution continued, although I'm using php-fpm which may act differently than the apache module.

[merty]

Replying to dd32:

Replying to nacin:

Per the docs on php.net/dl:

If the functionality of loading modules is not available or has been disabled (either by setting enable_dl off or by enabling safe mode in php.ini) an E_ERROR is emitted and execution is stopped.

I was able to reproduce this even with the error suppression operator.

In both of those cases, for me, an E_ERROR was raised, execution of the dl() operation ceased, and script execution continued, although I'm using php-fpm which may act differently than the apache module.

The problem is, it does not update the mod_sockets variable with the value FALSE when it raises an error. At least, it did not when we tried on three different servers.