[Backport 2.2-develop] #11343 #11478 In admin, url params may get url encoded more than once, fix backend url model

[adrian-martinez-interactiv4]

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. Related with PR#11479

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)

[RomaKis]

This PR has backward incompatible changes. Method Magento\Backend\Model\Url::getUrl() now returns parent::getUrl($routePath, $routeParams) when don't use secret keys and after this PR it will return parent::getUrl("{$routeName}/{$controllerName}/{$actionName}", $routeParams). $routePath may contains more then "{$routeName}/{$controllerName}/{$actionName}".
For example: method Magento/SalesRule/Block/Adminhtml/Promo/Quote/Edit/Tab/Conditions::addTabToForm() try to get url with code:

$this->getUrl(
            'sales_rule/promo_quote/newConditionHtml/form/' . $conditionsFieldSetId,
            ['form_namespace' => $formName]
        );

so before this fix it returned http://some_url/admin/sales_rule/promo_quote/newConditionHtml/form/sales_rule_formrule_conditions_fieldset_/form_namespace/sales_rule_form/
and after
http://some_url/admin/sales_rule/promo_quote/newConditionHtml/form_namespace/sales_rule_form/

[adrian-martinez-interactiv4]

@RomaKis ok, let me have a look at this

[adrian-martinez-interactiv4]

hi @RomaKis , I've updated the PR to preserve BC, the problem was not in the parent call itself, but in not retrieving the route params parsed by \Magento\Framework\Url::_setRoutePath. It should work now as expected.

[adrian-martinez-interactiv4]

@RomaKis tests are failing, I'm checking them

[adrian-martinez-interactiv4]

@okorshenko @RomaKis fixed, tests passed

[AVoskoboinikov]

I've noticed that after this fixes each Logout request in admin panel has 2 redirects instead of 1 (that we have now). Is that intended behavior?
@okorshenko, you can check it on PAT bamboo build MCCE22-PAT247-SSEM-1 log

(Edited to remove Magento system internal urls, no need to expose them)

[adrian-martinez-interactiv4]

@AZVO @okorshenko that behaviour was not intended on purpose, let me check

[adrian-martinez-interactiv4]

@AZVO @okorshenko Using 2.2-develop branch, without changes proposed here, I'm obtaining the same results (using form key).

Upper right Magento icon, and after logout, use \Magento\Backend\Helper\Data::getHomePageUrl method to retrieve home page url. Clicking in the Magento icon, causes to be redirected to dashboard with proper url secret key:

Logout uses admin/auth/logout action, what also uses home page url for redirection, so its the same case as before, but with one extra step:

So it behaves the same after the changes applied, but I also was wondering why is \Magento\Framework\UrlInterface::getRouteUrl needed as public method, instead of centralizing url creation through \Magento\Framework\UrlInterface::getUrl, as getRouteUrl seems a subcase of getUrl; for example in \Magento\Backend\Helper\Data::getHomePageUrl:

    /**
     * Get backend start page URL
     *
     * @return string
     */
    public function getHomePageUrl()
    {
        return $this->_backendUrl->getRouteUrl('adminhtml');
    }

Why is using getRouteUrl, when that method does not care about url secret key? Shouldn't getRouteUrl method be deprecated from interface and only used internally if needed (as it is done in \Magento\Framework\Url::createUrl method), and centralize external url creation requests through getUrl method?

[adrian-martinez-interactiv4]

As agreed with @vkublytskyi and indirectly with @antonkril , I'm going to perform the following actions:

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

[adrian-martinez-interactiv4]

Done, ready to review

[okorshenko]

Hi @adrian-martinez-interactiv4
This PR breaks CE/EE and B2B functionality. Please take a looks at functional test results here https://github.com/magento-engcom/magento2ce/pull/1307

I'm closing this PR for now. If you would like to proceed with this fix, feel free to reopen this PR. Also I will close #11481 and #11479

Let's finalize the solution for one branch and then we can create backports/forwardports to other branches