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

Custom URL Rewrite where the request path ends with a forward slash is not matched #8264

Closed
hostep opened this issue Jan 24, 2017 · 6 comments
Labels
bug report Component: CatalogUrlRewrite Fixed in 2.1.x The issue has been fixed in 2.1 release line Fixed in 2.2.x The issue has been fixed in 2.2 release line Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development

Comments

@hostep
Copy link
Contributor

hostep commented Jan 24, 2017

Preconditions

  1. Magento CE 2.1.3
  2. PHP 7.0.15

Steps to reproduce

  1. Set up a clean Magento CE 2.1.3 store
  2. In the admin, go to Marketing => URL Rewrites
  3. Add a new Custom URL Rewrite like this:
    screen shot 2017-01-24 at 20 10 57

Expected result

In the frontend, going to http://mydomain/test/ redirects you to http://mydomain/contact/

Actual result

In the frontend, going to http://mydomain/test/ gives you a 404 Not Found

Solution

Change the trim to ltrim in Magento\UrlRewrite\Controller\Router::getRewrite, otherwise the trailing slash will be removed and the url will never match.
(You need to flush your cache after changing this for some reason.)

Discussion

We want to import a bunch of custom redirects since we want to migrate an old custom build shop to magento 2 and we want to redirect using 301 headers from their old url structure to the new structure. The old structure uses a lot of url's with trailing slashes and this bug prevents it from redirecting properly to the new url structure. If we don't fix it, we would lose a bunch of Link Juice (SEO term) from the old url structure and that would very be bad.

Credits

Thanks to @LoganGS who discovered this bug and the solution over here: #6700

@miakusha
Copy link
Contributor

miakusha commented Feb 8, 2017

Hi @hostep , thanks for report.
We've created internal ticket MAGETWO-64348 to address this issue.

@miakusha miakusha added the Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development label Feb 8, 2017
@miakusha miakusha removed their assignment Feb 8, 2017
@hostep
Copy link
Contributor Author

hostep commented Feb 15, 2017

@miakusha: ok if I close this issue, since it looks like this is a duplicate of #4876 and #7040, correct?

@vrann
Copy link
Contributor

vrann commented May 18, 2017

In order to make the complete fix next cases should be supported:

# redirect behaviour
1 "test" => "contact" http://magento.loc/test/ => http://magento.loc/contact
2 "test" => "contact" http://magento.loc/test => http://magento.loc/contact
3 "test/" => "contact/" http://magento.loc/test/ => http://magento.loc/contact/
4 "test/" => "contact/" http://magento.loc/test => http://magento.loc/contact/

@magento-team
Copy link
Contributor

Internal ticket to track issue progress: MAGETWO-70255

@magento-team
Copy link
Contributor

Internal ticket to track issue progress: MAGETWO-70505

@magento-engcom-team magento-engcom-team added 2.1.x Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development bug report Component: CatalogUrlRewrite and removed G1 Passed labels Sep 5, 2017
@magento-engcom-team magento-engcom-team added 2.1.x Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development bug report Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Component: CatalogUrlRewrite labels Sep 11, 2017
@magento-engcom-team
Copy link
Contributor

Fixed in 2.2.0 and 2.1 releases. @hostep thank you for report

@magento-engcom-team magento-engcom-team added Fixed in 2.1.x The issue has been fixed in 2.1 release line Fixed in 2.2.x The issue has been fixed in 2.2 release line labels Sep 20, 2017
magento-devops-reposync-svc pushed a commit that referenced this issue Jun 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug report Component: CatalogUrlRewrite Fixed in 2.1.x The issue has been fixed in 2.1 release line Fixed in 2.2.x The issue has been fixed in 2.2 release line Issue: Format is valid Gate 1 Passed. Automatic verification of issue format passed Issue: Ready for Work Gate 4. Acknowledged. Issue is added to backlog and ready for development
Projects
None yet
Development

No branches or pull requests

7 participants