Skip to content

#11343 #11478 In admin, url params may get url encoded more than once, fix backend url model #11479

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

Conversation

adrian-martinez-interactiv4
Copy link
Contributor

@adrian-martinez-interactiv4 adrian-martinez-interactiv4 commented Oct 16, 2017

Description

In admin, url params may get url encoded more than once, when \Magento\Backend\Model\Url::getUrl() is called many times within the same request. This can mess up parameters like grid filters, and others.

Fixed Issues

  1. Search term in admin grid column filter is replaced with unicode characters #11343: In admin, url params may get url encoded more than once
  2. In admin, url params may get url encoded more than once #11478: Search term in admin grid column filter is replaced with unicode characters

Manual testing scenarios

  1. As explained in In admin, url params may get url encoded more than once #11478

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (if applicable)
  • All automated tests passed successfully (all builds on Travis CI are green)

@vrann vrann self-assigned this Oct 16, 2017
@vrann vrann added this to the October 2017 milestone Oct 16, 2017
@vrann vrann added develop Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release labels Oct 16, 2017
@magento-engcom-team magento-engcom-team changed the base branch from develop to 2.3-develop October 20, 2017 15:32
@adrian-martinez-interactiv4
Copy link
Contributor Author

@vrann Needs update label has been added, but there is no comment or review, does this still need update?

@adrian-martinez-interactiv4
Copy link
Contributor Author

@okorshenko @RomaKis fixed, tests passed

@adrian-martinez-interactiv4 adrian-martinez-interactiv4 force-pushed the FR#11478-ADMIN-URL-MODEL-URLENCODE branch from e0322b5 to 736ab3b Compare December 29, 2017 12:52
@adrian-martinez-interactiv4
Copy link
Contributor Author

adrian-martinez-interactiv4 commented Dec 29, 2017

As agreed with @vkublytskyi and indirectly with @antonkril , following actions have been performed:

  1. Deprecate getRouteUrl
  2. Move code from getRouteUrl to some new private method of Magento\Framework\Url
  3. In getRouteUrl return result of new private method
  4. Replace usage of getRouteUrl to new private method inside Magento\Framework\Url
  5. Replace usage of getRouteUrl with getUrl in all other places

For further info check PR #11480

@okorshenko okorshenko modified the milestones: December 2017, January 2018 Jan 8, 2018
@okorshenko
Copy link
Contributor

see comment here #11480 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Progress: needs update Reproduced on 2.1.x The issue has been reproduced on latest 2.1 release Reproduced on 2.2.x The issue has been reproduced on latest 2.2 release Reproduced on 2.3.x The issue has been reproduced on latest 2.3 release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants