mcrypt_compat icon indicating copy to clipboard operation
mcrypt_compat copied to clipboard

Use extension_loaded() instead of checking for a constant

Open cweagans opened this issue 7 years ago • 4 comments

https://github.com/phpseclib/mcrypt_compat/blob/master/lib/mcrypt.php#L44 should be if (!extension_loaded('mcrypt')) {

cweagans avatar Jul 31 '17 02:07 cweagans

The concern I'd have with that is mcrypt_compat being loaded twice. phpseclib used to have issues with it being included twice:

https://github.com/phpseclib/phpseclib/pull/217

random_compat works similarly to mcrypt_compat:

https://github.com/paragonie/random_compat/blob/master/lib/random.php

Mind you, whereas mcrypt_compat just does one if check before defining everything random_compat does if checks for everything.

terrafrost avatar Jul 31 '17 13:07 terrafrost

Hello all, I want to add check in legacy app, something like:

if(extension_loaded('mcrypt') || $hasMcryptCompat) {
//use mcrypt functions
} else {
//do something else
}

I can also use function_exists() ofc, but detecting mcrypt_compat would be better solution. Anyway, maybe solution to both problems (how to detect mcrypt_compat, or not load it twice) would be to add something like define('PHPSECLIB_MCRYPT_COMPAT', true) to mcrypt_compat ?

This way mcrypt_compat can detect if it's already loaded, it can use extension_loaded('mcrypt') and we can detect that we have it.

Just a suggestion, thx for great polyfill !!

ivanweiler avatar Jan 24 '18 11:01 ivanweiler

phpseclib does three different tests in different places:

if (!defined('MCRYPT_MODE_ECB')) {
...
if (!function_exists('phpseclib_mcrypt_list_algorithms')) {
...
if (!function_exists('mcrypt_list_algorithms')) {

The only one that PHPSECLIB_MCRYPT_COMPAT would be a viable solution would be !function_exists('phpseclib_mcrypt_list_algorithms') and idk that it's any better than `PHPSECLIB_MCRYPT_COMPAT?

For your own check you could do !function_exists('phpseclib_mcrypt_list_algorithms') yourself?

terrafrost avatar Jan 24 '18 18:01 terrafrost

I agree with @terrafrost here, that you should anyway test the feature (function exists, constant exists), not implementation details (extension_loaded, polyfill loaded). however, the loaded twice is not a real concern, why extension_loaded should not be used.

glensc avatar May 26 '19 07:05 glensc