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

Can we also have old PHP behavior? #38

Closed
afilina opened this issue Feb 19, 2024 · 11 comments
Closed

Can we also have old PHP behavior? #38

afilina opened this issue Feb 19, 2024 · 11 comments

Comments

@afilina
Copy link

afilina commented Feb 19, 2024

I know it's a long shot, but doesn't hurt to ask. Can we have multiple versions of this package so that we can have PHP 5 behavior?

For example, in PHP 5, one could pass an incorrect key size to mcrypt_encrypt and it triggered a warning. In PHP 7, it triggered an error, which is what this package does too. This makes patching legacy a much bigger project than it needs to be. Thanks!

And before anyone schools me, yes, I do want to refactor the code to not do dumb things like that. However, the codebase is huge and it's urgent to get it off PHP 5.3 as a first step.

P.S.: This package rocks and saved me headaches on quite a few legacy upgrades.

@terrafrost
Copy link
Member

terrafrost commented Feb 19, 2024

In principal I'm perfectly okay with doing that but I'm not really sure how the versioning would work.

mcrypt_compat 1.0 requires phpseclib 2.0, mcrypt_compat 2.0 requires phpseclib 3.0. Having mcrypt_compat 3.0 require phpseclib 3.0 but emulating PHP 5 vs PHP 7 would be kinda weird.

In theory maybe something like 2.0-PHP5 and 2.0-PHP7 could be done but idk of another composer package that does stuff like that. If there is one that'd be news to me. Like I'd kinda like there to be precedent for my doing something like that...

@terrafrost
Copy link
Member

Did you speak at Longhorn PHP a few years back btw?

I've attended every Longhorn PHP since they started them!

@afilina
Copy link
Author

afilina commented Feb 19, 2024

Good point. Perhaps a simpler approach could be a runtime check using version_compare() to switch to older behavior. Then the same branch could service multiple PHP versions. I'm not sure whether phpseclib would significantly affect the outcome. I know that mine is a niche need.

And yes, I spoke in Austin a couple of years ago. You can DM me on social media if you want to reminisce :)

@terrafrost
Copy link
Member

I try to accommodate niche needs lol.

version_compare() is a good idea altho I could also envision scenarios wherein you're running PHP 8 and want to emulate mcrypt for PHP 5 vs mcrypt for PHP 7. What if I made it so that if you wanted to emulate a specific version of mcrypt you could do define('PHPSECLIB_MCRYPT_TARGET_VERSION', '5.5')? If I did that then what 5.x versions would you prefer I do? If you'd like multiple ones which ones would you prioritize?

I may hit you up on DM if you're on FB at some point (that's the only social network I'm on lol)! Also, thanks for the patreon sponsorship! I'll try add you to BACKERS.md this evening after I get home from work, time permitting!

@afilina
Copy link
Author

afilina commented Feb 19, 2024

Yes, that's what I meant. Not sure why I said version_compare. Defining a constant with a target version is a very good idea. I prepend a bootstrap file to all scripts, so I can specify it there. My current legacy project is on PHP 5.3.

@terrafrost
Copy link
Member

Added you to BACKERS.md! I'll try to have this feature done in the next few days and will do a release when that's done.

Thanks!

@terrafrost
Copy link
Member

See terrafrost@86d851b

I'm now working on switching mcrypt_compat from using Travis CI to using GitHub Actions. Once I do that I'll add some unit tests and then I'll push those changes to phpseclib/mcrypt_compat vs terrafrost/mcrypt_compat and then I'll do a release!

@terrafrost
Copy link
Member

So to target a specific version you'd do define('PHPSECLIB_MCRYPT_TARGET_VERSION', '5.3.0'); before including vendor/autoload.php.

I got the PHP versions that things changed from the PHP changelogs. I did not look at the PHP 4 changelogs and I did not make it so that targeting versions where functions were deprecated issues actual deprecated warnings.

@terrafrost
Copy link
Member

1.0.16 and 2.0.5 have been released that implement this feature!

Thanks!

@afilina
Copy link
Author

afilina commented Feb 28, 2024

Amazing! Sorry for the late reply. I was away for over a week. I sent you a one-time donation.

@terrafrost
Copy link
Member

I sent you a one-time donation.

Got it - much appreciated - thank you! 😊

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