magento-hackathon / HoneySpam

Spam protection module for customer registration, product review form and contact form.
Open Software License 3.0
102 stars 36 forks source link

prepareForward not working in Observer.php #66

Closed addison74 closed 3 years ago

addison74 commented 3 years ago

In order to generate a SPAM event in frontend fill up honeyspam input text box with a value and an email address. Check the URL of the page after redirection. The extension redirects to /newsletter/subscriber/new instead of /honeyspam/error. I got the same results testing with Magento 1.9.2.4, 1.3,9.10, 1.9.4.5 and OpenMage 20.0.4.

This issue is coming from prepareForward method used in Observer.php file. Here is one example:

    /**
     * validate honeypot field
     * @throws Mage_Core_Controller_Varien_Exception
     */
    protected function _checkHoneypot()
    {
        /* @var Hackathon_HoneySpam_Helper_Data $helper */
        $helper = Mage::helper('hackathon_honeyspam');
        if (strlen(Mage::app()->getRequest()->getParam($helper->getHoneypotName()))) {
            $helper->log('Honeypot Input filled. Aborted.', Zend_Log::WARN);

        $e = new Mage_Core_Controller_Varien_Exception();
            $e->prepareForward('index', 'error', 'honeyspam');
            throw $e;
        }
    }

I did a change in the code as follows in order to get honeyspam URL:

    /**
     * validate honeypot field
     * @throws Mage_Core_Controller_Varien_Exception
     */
    protected function _checkHoneypot()
    {
        /* @var Hackathon_HoneySpam_Helper_Data $helper */
        $helper = Mage::helper('hackathon_honeyspam');
        if (strlen(Mage::app()->getRequest()->getParam($helper->getHoneypotName()))) {
            $helper->log('Honeypot Input filled. Aborted.', Zend_Log::WARN);

        /*$e = new Mage_Core_Controller_Varien_Exception();
            $e->prepareForward('index', 'error', 'honeyspam');
            throw $e;*/

            $newURL = rtrim(Mage::getBaseUrl(), '/') . strtolower('/honeyspam/error');
             Mage::app()->getFrontController()->getResponse()->setRedirect($newURL);
            Mage::app()->getResponse()->sendResponse();
            exit;
        }
    }

Any thougths?

addison74 commented 3 years ago

I suggest the following change into the code:

Replace

$e->prepareForward('index', 'error', 'honeyspam');

with

$e->prepareRedirect('honeyspam/error');

I have tested and it is working as expected.

Schrank commented 3 years ago

It shouldn't work 🤔 😂 Because Magento has still a bug in their prepareRedirect method:

public function prepareRedirect($path, $arguments = array())
{
    $this->_resultCallback = self::RESULT_REDIRECT;
    $this->_resultCallbackParams($path, $arguments);
    return $this;
}

    $this->_resultCallbackParams($path, $arguments);

needs to be

    $this->_resultCallbackParams = array($path, $arguments);

I'm heavily wondering why this works.

But imho is that the way to go. Extend Mage_Core_Controller_Varien_Exception, fix the bug in prepareRedirect and then prepareRedirect() and throw this new implemented exception

Schrank commented 3 years ago

@ADDISON74 Thanks fir digging into this!

addison74 commented 3 years ago

I checked a lot of posts over the years related to prepareForward issue. In most of the cases the solution was the one I mentioned in the first post, but I checked the core code to understand what means Forward and Redirect.

Probably it worked in my case without changing the core code because there were no arguments in the method. I will check if OpenMage team has fixed the issue you mentioned.

Schrank commented 3 years ago

Yes they did 😅 If you are using OpenMage, this explains the working <3

addison74 commented 3 years ago

You guessed right. I made the change in OpenMage. It is a blessing the M1 project is led before that kind team.

addison74 commented 3 years ago

You can make a change in the code without extending any class. If it is Magento by default then you use the code from the first post (or other more elegant). If it is OpenMage then the redirection made through a line of code works like a charm.