Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use extension_loaded() instead of checking for a constant #8

Open
cweagans opened this issue Jul 31, 2017 · 4 comments
Open

Use extension_loaded() instead of checking for a constant #8

cweagans opened this issue Jul 31, 2017 · 4 comments

Comments

@cweagans
Copy link

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

@terrafrost
Copy link
Member

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

phpseclib/phpseclib#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.

@ivanweiler
Copy link

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 !!

@terrafrost
Copy link
Member

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?

@glensc
Copy link

glensc commented May 26, 2019

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants