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

mcrypt_list_algorithms is deprecated #1629

Closed
Frittenfett opened this issue Mar 19, 2021 · 4 comments
Closed

mcrypt_list_algorithms is deprecated #1629

Frittenfett opened this issue Mar 19, 2021 · 4 comments

Comments

@Frittenfett
Copy link

Frittenfett commented Mar 19, 2021

Hey,
file: phpseclib/phpseclib/Crypt/Common/SymmetricKey.php

            case self::ENGINE_MCRYPT:
                return $this->cipher_name_mcrypt &&
                       extension_loaded('mcrypt') &&
                       in_array($this->cipher_name_mcrypt, @mcrypt_list_algorithms());

This won't work on php 7.4 in strict mode:
Function mcrypt_list_algorithms() is deprecated

You should just use your own internal function for this LUL:
in_array($this->cipher_name_mcrypt, phpseclib_mcrypt_list_algorithms());

@terrafrost
Copy link
Member

terrafrost commented Mar 19, 2021

When mcrypt is available it offers such a SIGNIFICANT speed increase that it is impossible to ignore. I'm talking 45x faster than ANY pure-PHP implementation could EVER hope to achieve. And you want me to just ignore that so I can get a warm fuzzy feeling on the inside that deprecated methods aren't being used? Sorry, but ain't going to happen. See #1028 (comment) for a more in depth elaboration.

This is also a dupe of #1512, #1500, #1368, #1229, #1228, #1148, #1134, #1067, #1028, etc

@terrafrost
Copy link
Member

You're also, in all likelihood, getting that error because you're using a custom error handler.

Overall, this leaves us at a bit of an impasse. The two options that I see are:

  • Remove mcrypt support all together and let phpseclib take HUUUGE performance hit
  • Let it stay, which breaks things for people using custom error handlers

Both options are suboptimal.

@terrafrost
Copy link
Member

In doing some testing it looks like phpseclib implementing it's own custom error handler might be a very good solution. The old error handler can be restored after the phpseclib error handler has been enabled.

Sample code:

<?php

set_error_handler(function($errno, $errstr) { throw new Exception($errstr); });

$old = set_error_handler(function() {});

user_error('zzz');

set_error_handler($old);

try {
    user_error('zzz');
} catch (Exception $e) {
    echo "exception caught";
}

I'll try to implement something like this. That should give us the best of both. People still get the performance boost of mcrypt whilst also enabling custom error handlers.

@terrafrost
Copy link
Member

e209db7

Custom error handlers are now used, which I believe should "fix" the issue for those encountering it.

(for the record, I hadn't pursued this before because I didn't know you can restore the old error handler until just a few days ago)

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

2 participants