From fd5c6a861cae4be5498f9bc5bb3ce9300bd3a003 Mon Sep 17 00:00:00 2001 From: Julien Castiaux Date: Fri, 17 Jun 2022 13:04:11 +0000 Subject: [PATCH] [FIX] http_routing: missing 301/302 on access err Install website and website_hr_recruitment, open /web with ?debug=1, go to website > configuration > redirect, create a temporary (302) redirection from `/jobs/detail/experienced-developer-4` to `/404`. Open the `/jobs/detail/experienced-developer-4` as admin and unpublish the page. Open the same URL via private browsing (so that you are not connected), you get the default 403 - Forbidden page, you were not redirected to the 404 - Not Found page. Custom 301 (permanent) and 302 (temporary) redirections are fallback redirections when the requested page does not exist or is not accessible to the current user. The HTTPocalypse broke the later case, it was not checking for existing redirection upon access error. The use case is the one supported with [1] where people want/need to display something better than a 403 when they unpublish a record like a job position for instance (most of the requested cases on opw). Indeed: - People have link to that record/job everywhere on the internet - The job position / record is no more relevant, and people need to unpublish it - People don't want to delete it (or can't sometimes due to record relations) - People don't want visitors to land on a 403, mainly because it is a non customizable advanced/technical page (it displays a technical message including the record name etc) - Their need is to either land a their customizable friendly 404 or sometimes on another record to promote it. [1]: https://github.com/odoo/odoo/commit/3b9cd536607b1631dd375ab2e5cc94eb814a6e9b closes odoo/odoo#93981 X-original-commit: eb7eecec976570ae3301c17a04adc9c110d5b14a Signed-off-by: Romain Derie (rde) Signed-off-by: Julien Castiaux Co-authored-by: Romain Derie --- addons/http_routing/models/ir_http.py | 7 +++- addons/test_website/tests/test_redirect.py | 37 ++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/addons/http_routing/models/ir_http.py b/addons/http_routing/models/ir_http.py index 9ebec6dca62..ebe37919fee 100644 --- a/addons/http_routing/models/ir_http.py +++ b/addons/http_routing/models/ir_http.py @@ -626,7 +626,12 @@ class IrHttp(models.AbstractModel): code, values = cls._get_exception_code_values(exception) request.cr.rollback() - if code == 500: + if code == 403: + response = cls._serve_fallback() + if response: + cls._post_dispatch(response) + return response + elif code == 500: values = cls._get_values_500_error(request.env, values, exception) try: code, html = cls._get_error_html(request.env, code, values) diff --git a/addons/test_website/tests/test_redirect.py b/addons/test_website/tests/test_redirect.py index 1080530ba0a..39840f9184d 100644 --- a/addons/test_website/tests/test_redirect.py +++ b/addons/test_website/tests/test_redirect.py @@ -172,3 +172,40 @@ class TestRedirect(HttpCase): 'href="/empty_controller_test_redirected?a=a"', r.text, "Redirection should have been applied, and query string should not have been duplicated.", ) + + @mute_logger('odoo.http') # mute 403 warning + def test_04_redirect_301_route_unpublished_record(self): + # 1. Accessing published record: Normal case, expecting 200 + rec1 = self.env['test.model'].create({ + 'name': '301 test record', + 'is_published': True, + }) + url_rec1 = '/test_website/200/' + slug(rec1) + r = self.url_open(url_rec1) + self.assertEqual(r.status_code, 200) + + # 2. Accessing unpublished record: expecting 403 by default + rec1.is_published = False + r = self.url_open(url_rec1) + self.assertEqual(r.status_code, 403) + + # 3. Accessing unpublished record with redirect to a 404: expecting 404 + redirect = self.env['website.rewrite'].create({ + 'name': 'Test 301 Redirect route unpublished record', + 'redirect_type': '301', + 'url_from': url_rec1, + 'url_to': '/404', + }) + r = self.url_open(url_rec1) + self.assertEqual(r.status_code, 404) + + # 4. Accessing unpublished record with redirect to another published + # record: expecting redirect to that record + rec2 = rec1.copy({'is_published': True}) + url_rec2 = '/test_website/200/' + slug(rec2) + redirect.url_to = url_rec2 + r = self.url_open(url_rec1) + self.assertEqual(r.status_code, 200) + self.assertTrue( + r.url.endswith(url_rec2), + "Unpublished record should redirect to published record set in redirect")