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

[4.2] Removing MCrypt class #38527

Closed
wants to merge 1 commit into from
Closed

Conversation

Hackwar
Copy link
Member

@Hackwar Hackwar commented Aug 18, 2022

Summary of Changes

I'm currently checking our codebase for validity against PHP 7.2 and there are a few things which have come up. One of them is that the mcrypt library has been removed in PHP 7.2.0 completely.
https://www.php.net/manual/en/function.mcrypt-get-iv-size.php
This library does not exist for any version of PHP that Joomla supports and thus this class has not been possible to be used with MCrypt instead of OpenSSL. Since that means no one has been using this, we can remove this now without having to wait until 5.0.

Copy link
Contributor

@wilsonge wilsonge left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No this allows compatibility of extensions running over multiple joomla versions. This is wrong and the deprecation notice stands as is

@brianteeman
Copy link
Contributor

exactly what @wilsonge said and I'm pretty sure this has come up before

@Hackwar
Copy link
Member Author

Hackwar commented Aug 18, 2022

I'm wondering what situation could come up where this would still work. Yes, someone might develop an extension across 3.x and 4.x which uses this, but the moment the user updates to PHP 7.2, it wont work anymore. Also, if they update to Joomla 4, this code also wont work anymore. That person also wont be able to decrypt old encrypted data with this code again after updating to 4.x. If a dev uses the class directly instead of the AES class, I really doubt that person is going to check the support prior. In that case it will still fail, regardless of us having this class here or them running into the problem of the missing mcrypt extension. I would still vote to remove this.

@HLeithner
Copy link
Member

this is for combat and there is no reason to remove support in the bugfix release.
mcrypt still works with php 8.2.0 https://pecl.php.net/package/mcrypt

@HLeithner HLeithner closed this Aug 18, 2022
@HLeithner
Copy link
Member

@Hackwar would you please so kind an recreate the PR for 5.0?

@Hackwar Hackwar deleted the 4.2-mcrypt branch April 17, 2023 07:37
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

Successfully merging this pull request may close these issues.

None yet

5 participants