From 82763183d2da5fe652d66dd482bbe7efedc2dbd9 Mon Sep 17 00:00:00 2001 From: Romain Derie Date: Thu, 12 Jan 2023 16:32:54 +0000 Subject: [PATCH] [FIX] website: redirect to case insensitive URL if not exact match Before this commit, if a link to a page was not correct because of a case mismatch, it would simply land on a 404 page. While it's correct, as URL are case sensitive, it leads to a few bad UX flow at the admin/editor level: - Create a link in your page (on a text or a button eg), type an URL which does not exists (to create it after) like /Page - Click on the link/button you just made, you are redirected to /Page which display a 404 with the "Create page" option (correct) - When you click on that button, it will actually create a page with /page URL, leading to a mismatch between the URL you created and the page URL. Your link/button will still lead to a 404 URL as it points to /Page. Since it's just a fallback when an exact URL match is not found, it should not break anything and should not have bad impact at any level (seo/speed etc). Indeed: - It's done through a 302 redirect - `_serve_page()` is already a fallback case, so it will only make the `website.redirect` and 404 cases a bit slower due to the extra search query. The only possible scenario seems to be if the user (mind the uppercase): - Created a /Page page - Created a redirect from /page to /another-page In this case, /page won't land on /another-page but on /Page. This flow seems unlikely and is not actually wrong either way. At least, it certainly is less important than ensuring a case insensitive fallback. Finally, note that another solution would have been to either: - Force page URL to lower case. -> This is not stable friendly, people might be relying on this to create pages with different casing: `/Batman-VII-The-Dark-Knight-Whatevers`, while not recommended, doesn't sounds idiot. On top of not being stable friendly, we probably want to keep offering this possibility - Redirect all URLs to lowercase endpoints. -> This is obviously not stable and not Odoo's jobs. It should be something decided by the sysadmin and done at nginx (etc) level. task-3110294 opw-3104030 closes odoo/odoo#111736 X-original-commit: f05491105f93939490cbeb078cb7653c38685644 Signed-off-by: Quentin Smetz (qsm) --- addons/website/models/ir_http.py | 17 +++++++++++++---- addons/website/tests/test_page.py | 7 +++++++ 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/addons/website/models/ir_http.py b/addons/website/models/ir_http.py index f110da79145..ea24ddabcd9 100644 --- a/addons/website/models/ir_http.py +++ b/addons/website/models/ir_http.py @@ -265,13 +265,22 @@ class Http(models.AbstractModel): @classmethod def _serve_page(cls): req_page = request.httprequest.path - page_domain = [('url', '=', req_page)] + request.website.website_domain() - published_domain = page_domain + def _search_page(comparator='='): + page_domain = [('url', comparator, req_page)] + request.website.website_domain() + return request.env['website.page'].sudo().search(page_domain, order='website_id asc', limit=1) + # specific page first - page = request.env['website.page'].sudo().search(published_domain, order='website_id asc', limit=1) + page = _search_page() - # redirect withtout trailing / + # case insensitive search + if not page: + page = _search_page('=ilike') + if page: + logger.info("Page %r not found, redirecting to existing page %r", req_page, page.url) + return request.redirect(page.url) + + # redirect without trailing / if not page and req_page != "/" and req_page.endswith("/"): # mimick `_postprocess_args()` redirect path = request.httprequest.path[:-1] diff --git a/addons/website/tests/test_page.py b/addons/website/tests/test_page.py index e3673cfbabc..fdb121489ae 100644 --- a/addons/website/tests/test_page.py +++ b/addons/website/tests/test_page.py @@ -483,3 +483,10 @@ class WithContext(HttpCase): # Check that is is rendered as a website page. self.assertEqual(403, r.status_code, "Must fail with 403") self.assertTrue('id="wrap"' in r.text, "Must be rendered as a website page") + + def test_page_url_case_insensitive_match(self): + r = self.url_open('/page_1') + self.assertEqual(r.status_code, 200, "Reaching page URL, common case") + r2 = self.url_open('/Page_1', allow_redirects=False) + self.assertEqual(r2.status_code, 303, "URL exists only in different casing, should redirect to it") + self.assertTrue(r2.headers.get('Location').endswith('/page_1'), "Should redirect /Page_1 to /page_1")