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

Don't use HTTP_REFERER for Dummy_Request to prevent (logged) exceptions. #1234

Open
wants to merge 1 commit into
base: devel
Choose a base branch
from

Conversation

possi
Copy link
Contributor

@possi possi commented Jul 20, 2016

Scenario to create an Invalid Dummy-Request-Url (which throws an exception in Debug-Mode):
0. Assume a global, public ESI-Block on page (e.g. "menu")

  1. Search for "" (without quotes) -> /catalogsearch/result/?q=\
  2. Click any page
  3. "Invalid URI supplied"-Zend_Uri_Exception is thrown

Disabling the debug mode suppresses the exception, but spams the logs.

Reason:
For global blocks no /r64/.../-Param is passed to the ESI-Url. Mage_Core_Controller_Varien_Action::_getRefererUrl() uses $_SERVER['HTTP_REFERER'] if no r64-Param is in the Url. This Http-Header is never filtered, and the \ within the Url isn't allowed by Zend_Uri_http::validateQuery()s regexp.

Solution:
There is no reason to use the referrer as the Dummy_Request for global blocks, as they shouldn't rely on any url other than the front-page. If else, they currently wouldn't function either, as the referrer-url contains a different url then the page the user is currently visiting.

@miguelbalparda
Copy link
Contributor

Have you seen #1235? It seems to be pretty similar to this, thoughts?

@possi
Copy link
Contributor Author

possi commented Oct 3, 2016

Indeed. But my approach fixes the root-cause instead of just silently catch the exception.

@miguelbalparda
Copy link
Contributor

@magedev thoughts?

@jeroenvermeulen
Copy link
Contributor

Looks good (did not test)

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.

3 participants