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

Avoid double decoding of URL #97

Merged
merged 4 commits into from
Jul 10, 2018
Merged

Avoid double decoding of URL #97

merged 4 commits into from
Jul 10, 2018

Conversation

pkamps
Copy link
Member

@pkamps pkamps commented May 19, 2018

eZSYS::requestURI is already decoding the URL. The eZURI class does it a 2nd time.

Example of the problem. Given URL:
http://dev.lovestack.mugo.ca/(foo)/bar%2Bbar

Double decoded version is is /(foo)/bar bar (that's a space in between)
But the correct value is /(foo)/bar+bar

You can test this by adding following into your pagelayout.tpl:
{$view_parameters|dump()}

Use following URL and look at the dump output:
http://dev.lovestack.mugo.ca/(foo)/bar%2Bbar

The correct value is " /(foo)/bar+bar" - the pull request should produce the correct output.

eZSYS::requestURI is already decoding the URL. The eZURI class does it a 2nd time.

Example of the problem. Given URL:
http://dev.lovestack.mugo.ca/(foo)/bar%2Bbar

Double decoded version is is /(foo)/bar bar   (that's a space in between)
But the correct value is /(foo)/bar+bar
@ernestob
Copy link
Member

👍 This is ready to merge.

On my test on the master branch.

$view_parameters was getting 'bar bar', but the $module_result.uri has the correct encoding

uri | string | '/(foo)/bar+bar'

On the pkamps-double-decoding-fixed branch both $view_parameters and $module_result.uri are matching and showing the correct encoding.

@pkamps pkamps merged commit f435c2f into master Jul 10, 2018
@pkamps pkamps deleted the pkamps-double-decoding-fixed branch July 10, 2018 17:26
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.

2 participants