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

Fixes Mage_Core_Helper_Url::removeRequestParam() #4295

Merged
merged 4 commits into from
Oct 23, 2024

Conversation

sreichel
Copy link
Contributor

Description (*)

Mage_Core_Helper_Url::removeRequestParam() uses preg_replace or str_replace that can lead to malformed urls for partial matches.

Fixed Issues (if relevant)

  1. Fixes removeRequestParam called by setBeforeAuthUrl messed up Query if ___SID is used #4294

Manual testing scenarios (*)

$url = 'https://example.com?___SID=S&SID=S&foo=bar&boo=baz';
Mage::helper('core/url')->removeRequestParam(
    $url,
    Mage::getSingleton('core/session')->getSessionIdQueryParam()
);

getSessionIdQueryParam() returns SID, that should be removed.

Expected

https://example.com?___SID=S&foo=bar&boo=baz

Current

https://example.com?___foo=bar&boo=baz

@github-actions github-actions bot added the Component: Core Relates to Mage_Core label Oct 19, 2024
@ADDISON74
Copy link
Contributor

@Hanmac - is this PR solving your issue? I tested and it works.

@Hanmac
Copy link
Contributor

Hanmac commented Oct 20, 2024

Maybe return early if the URL doesn't have '?'

@sreichel
Copy link
Contributor Author

Yep. Updated.

@kiatng kiatng merged commit 5207750 into OpenMage:main Oct 23, 2024
18 checks passed
@sreichel sreichel changed the title Fixes Mage_Core_Helper_Url::removeRequestParam() for ___SID/SID Fixes Mage_Core_Helper_Url::removeRequestParam() Oct 23, 2024
@sreichel sreichel deleted the fix-4294 branch October 23, 2024 02:44
fballiano added a commit to MahoCommerce/maho that referenced this pull request Oct 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Core Relates to Mage_Core
Projects
None yet
Development

Successfully merging this pull request may close these issues.

removeRequestParam called by setBeforeAuthUrl messed up Query if ___SID is used
4 participants