Inchoo / Inchoo_PHP7

PHP 7 compatibility extension for Magento 1 (DEPRECATED!!)
MIT License
352 stars 112 forks source link

mcrypt is removed in PHP 7.2 #126

Open csdougliss opened 7 years ago

csdougliss commented 7 years ago

https://wiki.php.net/rfc/mcrypt-viking-funeral

https://wiki.php.net/rfc/libsodium

I haven't tested anything in 7.2 yet..

https://github.com/Inchoo/Inchoo_PHP7/issues/98

mklooss commented 6 years ago

you will also some problems with the customer area in the backend:

PHP Fatal error: Declaration of Mage_Customer_Model_Resource_Address_Attribute_Source_Country::getAllOptions() must be compatible with Mage_Eav_Model_Entity_Attribute_Source_Table::getAllOptions($withEmpty = true, $defaultValues = false) in magento/app/code/core/Mage/Customer/Model/Resource/Address/Attribute/Source/Country.php on line 0 Testet in Magento 1.9.3.7

i've fixed this with "rewrites"

https://github.com/mklooss/Loewenstark_Php72

postadelmaga commented 6 years ago

Hi guys, Since I was planing a switch to PHP 7.X I am also interested into this fix. .. And at this point makes sense to look at PHP 7.2 issues before doing such a switch from PHP 5.6 ...

Personally I don't know much about Mcrypt, I suppose it used mostly to crypt password stored in DB but there is probably something more ( otherwise why just switch to a different cryptography library all the password ? I mean an extension, to be installed before switching to PHP 7.2 that migrate the passwords from mcrypt to the new lib ? )

At the moment I can only give the following contribution:

I have experience on M1 but not much with this part of the PHP libraries anyway I am glad to help anyone that has a better understanding of the issue, just let me know how :) ( for sure I can test any patch/extension on my codebase and help to fix any problem that arise ... after 6 yrs experience on M1 I am actually quite good at debugging code :)

durzel commented 6 years ago

Out of interest - what is the goal by upgrading to 7.2? There is a tangible speed benefit from going from 5.6.x to 7.0, and this extension works just fine with 7.0.

hostep commented 6 years ago

@durzel: PHP 7.0 is EOL at the end of 2018, which is not so far away anymore.

sr972 commented 6 years ago

So still PHP 7.1 left to use with MCrypt. BTW, MCrypt is still available in 7.2 via PECL. If this is the best choice? Don't know.

ivanweiler commented 6 years ago

Related to #98
I wrote small status update there

csdougliss commented 6 years ago

@ivanweiler looking forward to the release, when is it expected?

mitcht commented 6 years ago

Update... As of today, the Ubuntu 18 Daily build has dropped all built in support for packages for php7.1.

mitcht commented 6 years ago

The development branch is seeing some activity as of 8 days ago. https://github.com/Inchoo/Inchoo_PHP7/tree/develop

csdougliss commented 6 years ago

Any updates on this? I've now installed php7.2 locally and upgraded my OS to Ubuntu 18.04 due to some hardware support issues.

mitcht commented 6 years ago

I've found that installing mcrypt via pecl is still a viable solution at this time. I find it better than throwing 2 month old code from a repo dealing with security into production.

postadelmaga commented 6 years ago

Magento 2 community has created a project to fix all problems related PHP 7.2:

I am pretty sure we can backport some of the solution adopted there ... in case there are enough folks here that want to keep Magento 1 alive ...

ivanweiler commented 6 years ago

Last time I checked (a while ago) M2 was still using mcrypt, with suppressed warnings, so they're not yet 7.2 ready: https://github.com/magento-engcom/php-7.2-support/blob/2.3-develop/lib/internal/Magento/Framework/Encryption/Crypt.php#L59

But we don't mind if they backport some of the solutions from us ;)

Our M1 solution can be seen in our current develop branch, feel free to test, more info here https://github.com/Inchoo/Inchoo_PHP7/issues/98#issuecomment-363050865

We just can't find some free time to do final release, so if anyone can test a bit it would be really helpful !! We are concentrating on php 7.1, but everything implemented will solve most of 7.2 issues.

mitcht commented 6 years ago

Trust me... there’s interest. Now that magento 1.x has the rwd theme, some of the magic of magento2 has been brought back to the smaller dev teams (like us) that have a fairly well tested magento to crm / warehousing system that needs to be carefully rewritten and tested. We can offer the mobile experience now without the tech debt of redoing that system. (It may seem like we are putting it off but the primary goal was move to the “cloud” and “be mobile friendly”)

piotrekkaminski commented 6 years ago

Anyone having luck using this with 7.2? What is your experience? What types of warnings get generated?

csdougliss commented 6 years ago

Any updates on this? Is there going to be an official Magento 1.x release? as per this:

We're currently working with Magento directly on releasing the official PHP 7.2 patch. This will include some changes and lot of additional testing across versions, so we ask for a bit more of your patience. Hopefully, version 3.0 of this extension won't be needed anymore :)
mrlerch commented 6 years ago

That is absolutely awesome! Great work and keep it up.

Martin

On Aug 29, 2018, at 2:42 AM, Craig Carnell notifications@github.com wrote:

Any updates on this? Is there going to be an official Magento 1.x release? as per this:

We're currently working with Magento directly on releasing the official PHP 7.2 patch. This will include some changes and lot of additional testing across versions, so we ask for a bit more of your patience. Hopefully, version 3.0 of this extension won't be needed anymore :) — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/Inchoo/Inchoo_PHP7/issues/126#issuecomment-416892124, or mute the thread https://github.com/notifications/unsubscribe-auth/AL69GbajcRacwA5wnwU5ZcvEYRiJ47m-ks5uVmH0gaJpZM4QyENu.

mitcht commented 6 years ago

Agreed. This is good news. It will simplify my automated deployment script to not require pecl and the required building.

durzel commented 6 years ago

I'm confused.. @craigcarnell asked for an update on developments, he wasn't saying there had been progress made?

mitcht commented 6 years ago

My thoughts taken from the comment:

We're currently working with Magento directly on releasing the official PHP 7.2 patch.

I take it inchoo is working with magento for a fix for mcrypt to go into core.

This will include some changes and lot of additional testing across versions, so we ask for a bit more of your patience. Hopefully, version 3.0 of this extension won't be needed anymore :)

Across versions = 1 & 2. The Inchoo_PHP7 won't be needed any longer. The Inchoo_PHP7 compatibility extension is for Magento 1, not 2. I could be wrong, but i'm just excited about the comment. It means something is happening.

durzel commented 6 years ago

The "We're currently working with Magento" comment is from here, posted back in June.

There may have been developments between then and now, which is what @craigcarnell was asking for an update on I think. To the best of my knowledge the develop branch (3.0.0) has fixes for PHP 7.1 and 7.2, but it's "untested" per @ivanweiler's earlier comment.

mitcht commented 6 years ago

I may be a candidate to do some testing soon. we are due to revisit our ecommerce site in the next few months. I may introduce V3.0 of Inchoo_PHP7 into our test cycle. I will need to review the code with our auditor to ensure there aren't any glaring security flaws but I don't see any myself. The thing is that I really would like to have the core fix first... Cart before the horse!