From 3b19a53191f5f144f2987c11b86d6e0632e49753 Mon Sep 17 00:00:00 2001 From: Romain Derie Date: Fri, 25 Aug 2023 11:50:52 +0000 Subject: [PATCH] [IMP] test_website: add a test for 308 redirect with multi url route This commit introduces a test in Odoo 15 about a broken behavior in Odoo 16 which was not covered by a test. A fix will be shipped alongside this commit forward-port in Odoo 16. The behavior involved here is about: - Having a multi-url defined route - One of those URL not having a model converter - One of those URL having a model converter - Creating a 308 redirect for the URL without model converter Something like this route (real example from the code): ```py @http.route([ '''/shop''', '''/shop/page/''', '''/shop/category/''', '''/shop/category//page/''' ], type='http', auth="public", website=True, sitemap=sitemap_shop) ``` And this redirect (real usual example from clients): ```py self.env['website.rewrite'].create({ 'name': 'Test Multi URL 308', 'redirect_type': '308', 'url_from': '/shop', 'url_to': '/magasin', }) ``` In Odoo 15, everything will work as expected: 1. Accessing /shop will redirect to /magasin 2. Accessing /shop/category/categ-1 will not redirect and user will be on /shop/category/categ-1 still But in Odoo 16: In the case described above, acessing the URL with model converter from the route will fail and redirect to a non slugified URL in the form of `/some-url?param=test.model%287%2C%29` which is basically the stringified encoded version of the record `test.model(7,)`. Note: - It doesn't matter if the route is contained in the other, the error will still occur even if `'''/shop'''` was `'''/abc'''`. - Even if all URL from the route have their own 308 redirect, the error will still occur. - It won't have any issue if there is a 308 on the route with model converter: both the one without model converter and other url with model converter will work and won't be impacted. This might be because of a concept of "merging URL" from httpocalypse, see https://github.com/odoo/odoo/commit/347a3ccf763cc6958d5dc1df3168c9c65245736c opw-3450054 opw-3466498 opw-3477380 opw-3475694 X-original-commit: 22ca1c48f78cdf575f9a2b0823f9487f942bc7cd Part-of: odoo/odoo#134206 --- addons/test_website/controllers/main.py | 7 +++++++ addons/test_website/tests/test_redirect.py | 21 +++++++++++++++++++++ 2 files changed, 28 insertions(+) diff --git a/addons/test_website/controllers/main.py b/addons/test_website/controllers/main.py index b683eee4c52..ef74add8380 100644 --- a/addons/test_website/controllers/main.py +++ b/addons/test_website/controllers/main.py @@ -143,6 +143,13 @@ class WebsiteTest(Home): def test_redirect_view_qs(self, **kw): return request.render('test_website.test_redirect_view_qs') + @http.route([ + '/test_countries_308', + '/test_countries_308/', + ], type='http', auth='public', website=True, sitemap=False) + def test_countries_308(self, **kwargs): + return request.make_response('ok') + # Test Sitemap def sitemap_test(env, rule, qs): if not qs or qs.lower() in '/test_website_sitemap': diff --git a/addons/test_website/tests/test_redirect.py b/addons/test_website/tests/test_redirect.py index f707f7ef971..4949e19e965 100644 --- a/addons/test_website/tests/test_redirect.py +++ b/addons/test_website/tests/test_redirect.py @@ -231,3 +231,24 @@ class TestRedirect(HttpCase): r = self.url_open(url_rec1, allow_redirects=True) self.assertEqual(r.status_code, 200) self.assertTrue(r.url.endswith(redirect.url_to)) + + def test_redirect_308_multiple_url_endpoint(self): + self.env['website.rewrite'].create({ + 'name': 'Test Multi URL 308', + 'redirect_type': '308', + 'url_from': '/test_countries_308', + 'url_to': '/test_countries_308_redirected', + }) + rec1 = self.env['test.model'].create({ + 'name': '301 test record', + 'is_published': True, + }) + url_rec1 = f"/test_countries_308/{slug(rec1)}" + + resp = self.url_open("/test_countries_308", allow_redirects=False) + self.assertEqual(resp.status_code, 308) + self.assertEqual(resp.headers.get('Location'), self.base_url() + "/test_countries_308_redirected") + + resp = self.url_open(url_rec1) + self.assertEqual(resp.status_code, 200) + self.assertTrue(resp.url.endswith(url_rec1))