WordPress / Requests

Requests for PHP is a humble HTTP request library. It simplifies how you interact with other sites and takes away all your worries.
https://requests.ryanmccue.info/
Other
3.57k stars 498 forks source link

PHP Warning when curl_exec() is disabled #269

Open montaniasystemab opened 7 years ago

montaniasystemab commented 7 years ago

We're getting this warning from WordPress from time to time:

curl_exec() has been disabled for security reasons

It seems to me that this test is not enough to determine if curl is available:

https://github.com/rmccue/Requests/blob/master/library/Requests/Transport/cURL.php#L528

This simple test shows that function_exists() still returns true when curl_exec is disabled using disable_functions:

var_dump(ini_get('disable_functions'));
var_dump(function_exists('curl_exec'));
string(241) "… curl_exec …" 
bool(true)

I'm thinking that an additional test for if curl_exec or curl_init is present in the disable_functions ini setting should solve this issue.

Would you consider a PR with such a change? Thanks

rmccue commented 7 years ago

On recent versions of PHP (5.3+ maybe), function_exists acts correctly, so this only affects systems a) running 5.2, b) with curl_exec disabled, and c) also doesn't disable ini_get. In practice, it seems like hosts which disable curl_exec also disable ini_get in any case.

In https://core.trac.wordpress.org/ticket/37680, calls to function_exists were specifically added, since it does check disable_functions.

I'm personally 👎 on something so edge-case-y.

@dd32 Thoughts? (Seeing as you also happened to be the one who committed 37680.)

montaniasystemab commented 7 years ago

In our case function_exists does not seem to behave correctly. Maybe it's because disable_functions is configured per PHP-FPM pool in the pool definition file?

var_dump(
    strpos(ini_get('disable_functions'), 'curl_exec'), 
    phpversion(), 
    function_exists('curl_exec'), 
    php_sapi_name()
);
int(12) 
string(6) "5.6.30" 
bool(true) 
string(8) "fpm-fcgi"
dd32 commented 7 years ago

I'd always assumed that the reason function_exists() didn't work correctly in these cases was due to suhosin which has it's own blocklist (separate from disable_functions) and disables them in some other way (I believe).

If configuring it per-fpm-pool also triggers it, that doesn't surprise me one bit..

Technically, I'm against adding workarounds for function_exists() not working, realistically, I came to the conclusion that PHP is weird and hosts-be-hosts and the workaround is actually needed.