ProxiBlue / reCaptcha

Clean implementation of Google reCaptcha for magento
http://www.proxiblue.com.au/blog/magento-recaptcha/
GNU General Public License v3.0
86 stars 60 forks source link

Warning in PHP 8.0 #52

Closed norgeindian closed 3 years ago

norgeindian commented 3 years ago

We recently updated to PHP 8.0. When a customer now tries to log in, he gets the following warning:

Warning: Trying to access array offset on value of type null  in /var/www/html/app/code/core/Mage/Captcha/Model/Observer.php on line 290

#0 /var/www/html/app/code/core/Mage/Captcha/Model/Observer.php(290): mageCoreErrorHandler()
#1 /var/www/html/app/code/core/Mage/Captcha/Model/Observer.php(71): Mage_Captcha_Model_Observer->_getCaptchaString()
#2 /var/www/html/app/code/core/Mage/Core/Model/App.php(1374): Mage_Captcha_Model_Observer->checkUserLogin()
#3 /var/www/html/app/code/core/Mage/Core/Model/App.php(1353): Mage_Core_Model_App->_callObserverMethod()
#4 /var/www/html/app/Mage.php(451): Mage_Core_Model_App->dispatchEvent()
#5 /var/www/html/app/code/core/Mage/Core/Controller/Varien/Action.php(531): Mage::dispatchEvent()
#6 /var/www/html/app/code/core/Mage/Core/Controller/Front/Action.php(69): Mage_Core_Controller_Varien_Action->preDispatch()
#7 /var/www/html/app/code/core/Mage/Customer/controllers/AccountController.php(65): Mage_Core_Controller_Front_Action->preDispatch()
#8 /var/www/html/app/code/core/Mage/Core/Controller/Varien/Action.php(407): Mage_Customer_AccountController->preDispatch()
#9 /var/www/html/app/code/core/Mage/Core/Controller/Varien/Router/Standard.php(254): Mage_Core_Controller_Varien_Action->dispatch()
#10 /var/www/html/app/code/core/Mage/Core/Controller/Varien/Front.php(172): Mage_Core_Controller_Varien_Router_Standard->match()
#11 /var/www/html/app/code/core/Mage/Core/Model/App.php(381): Mage_Core_Controller_Varien_Front->dispatch()
#12 /var/www/html/app/Mage.php(686): Mage_Core_Model_App->run()
#13 /var/www/html/index.php(83): Mage::run()
#14 {main}

As soon as I disable the extension, the error is gone. Any idea what might go wrong here?

ProxiBlue commented 3 years ago

The module has not been tested with PHP 8 Which version of openmage are you using?

ProxiBlue commented 3 years ago

Out of interest. I see the trace involves all core code, which would be invoked by the recaptcha module as it uses mostly core magento code to effect work.

Have you tested using core magento captcha? Yes the ugly enter these characters and check if dame error happens?

This looks a bit luke some core magrnto code failing.

norgeindian commented 3 years ago

@ProxiBlue , we are using the latest Mage One patches, which now support PHP 8.0. I just tried again with default Magento Captcha and that worked fine. So even if we only see default Magento in the trace, this seems to be caused by ReCaptcha somehow.

ProxiBlue commented 3 years ago

Thanks for the update. I will need to get mage one serup and test.

However i won't get to it soon.

On Tue, 13 Apr 2021, 13:01 Ole Schäfer, @.***> wrote:

@ProxiBlue https://github.com/ProxiBlue , we are using the latest Mage One patches, which now support PHP 8.0. I just tried again with default Magento Captcha and that worked fine. So even if we only see default Magento in the trace, this seems to be caused by ReCaptcha somehow.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ProxiBlue/reCaptcha/issues/52#issuecomment-818438081, or unsubscribe https://github.com/notifications/unsubscribe-auth/ABGDJVDLKNUW3YVYG2B6UEDTIPF2PANCNFSM42VBQFUA .

norgeindian commented 3 years ago

@ProxiBlue , I shortly debugged a bit further into that issue and found out, that the notice is just now a warning in PHP 8.0 (see https://www.php.net/manual/de/migration80.incompatible.php#migration80.incompatible.core.other:~:text=Attempting%20to%20access%20an%20array%20index%20of%20a%20non%2Darray.) The behaviour has always been the same, it just never came up. So you don't need a Mage One instance, you can use your current version as well. As you use app/design/frontend/base/default/template/captcha/recaptcha.phtml instead of app/design/frontend/base/default/template/captcha/recaptcha.phtml, there is no input field called captcha. So $captchaParams is always null, but in PHP 7, that was not a big issue. What do you think about including such an input field, which then returns null at the correct index or do you see a way to skip this function call?

ProxiBlue commented 3 years ago

Hello,

Appreciate your effort and time here, I will have a look into it as soon as I have a moment (still recovering form surgery) as soon as I am cought up with paid client work.

ProxiBlue commented 3 years ago

What do you think about including such an input field, which then returns null at the correct index or do you see a way to skip this function call?

That looks like it would be the simplest solution. As cab be seen in the code flow, none of the proxiblue code even comes into play, and to skip that function will require additional rewrites onto the core code, which I don;t want to do.

So, if you want to add such a field, and submit a PR I will get that merged in.

I can get to it in a few days, as will need to test, and I don't have time right now, but checking code, just placing any value in teh field will fix this, as that is never used in the reCapctha flow.

norgeindian commented 3 years ago

@ProxiBlue , please be so kind and take a look at my pull request. Just tested it and it seems to work fine

ProxiBlue commented 3 years ago

Thanks for your effort. Appreciated.