diff --git a/addons/web/controllers/binary.py b/addons/web/controllers/binary.py index 95e1cf2ce1a..794edfbd01f 100644 --- a/addons/web/controllers/binary.py +++ b/addons/web/controllers/binary.py @@ -9,7 +9,6 @@ import os import unicodedata import werkzeug.exceptions - try: from werkzeug.utils import send_file except ImportError: @@ -104,8 +103,6 @@ class Binary(http.Controller): if not attachment: # try to generate one with replace_exceptions(ValueError, by=werkzeug.exceptions.BadRequest): - extra_parts = extra.split('+') - params = request.env['ir.asset']._parse_assets_extra(extra_parts) if debug_assets: bundle_name, asset_type = filename.rsplit('.', 1) else: @@ -113,6 +110,11 @@ class Binary(http.Controller): if min_ != 'min': _logger.error("'min' expected in extension in non debug mode") raise request.not_found() + try: + params, rtl = request.env['ir.asset']._parse_assets_extra(extra.split('+'), asset_type) + except ValueError: + _logger.exception('Invalid assets') + raise request.not_found() css = asset_type == 'css' js = asset_type == 'js' if not js and not css: @@ -123,7 +125,7 @@ class Binary(http.Controller): css=css, js=js, debug_assets=debug_assets, - rtl='rtl' in extra_parts, + rtl=rtl, assets_params=params, ) # check if the version matches. If not, redirect to the last version diff --git a/addons/website/models/ir_asset.py b/addons/website/models/ir_asset.py index 90d8eec8cfe..e58fedaa57d 100644 --- a/addons/website/models/ir_asset.py +++ b/addons/website/models/ir_asset.py @@ -9,6 +9,7 @@ class IrAsset(models.Model): key = fields.Char(copy=False) # used to resolve multiple assets in a multi-website environment website_id = fields.Many2one('website', ondelete='cascade') + def _get_asset_params(self): params = super()._get_asset_params() params['website_id'] = self.env['website'].get_current_website(fallback=False).id @@ -21,12 +22,17 @@ class IrAsset(models.Model): website_id_path = website_id and ('website-%s+' % website_id) or '' return website_id_path + extra - def _parse_assets_extra(self, extra_parts): - params = super()._parse_assets_extra(extra_parts) - for extra_part in extra_parts: - if extra_part.startswith('w-'): - params['website_id'] = int(extra_part[2:]) - return params + def _parse_assets_extra(self, extra_parts, asset_type): + website_params = {} + if extra_parts[0].startswith('website-'): + website_extra, *extra_parts = extra_parts + website_id = int(website_extra[8:]) + if not self.env['website'].browse(website_id).exists(): + message = f'{website_extra} contains a non existing website' + raise ValueError(message) + website_params['website_id'] = website_id + params, rtl = super()._parse_assets_extra(extra_parts, asset_type) + return {**params, **website_params}, rtl def _get_related_assets(self, domain, website_id=None, **params): if website_id: diff --git a/addons/website/tests/test_assets.py b/addons/website/tests/test_assets.py index da8ad346a6a..db623469ca4 100644 --- a/addons/website/tests/test_assets.py +++ b/addons/website/tests/test_assets.py @@ -99,3 +99,84 @@ class TestWebsiteAssets(odoo.tests.HttpCase): new_public_assets_links = re.findall(r'(/web/assets/.+/web.assets_frontend\..+)"/>', page) self.assertEqual(new_admin_assets_links, new_public_assets_links, "t-cache should have been invalidated for public user too") + +@odoo.tests.tagged('-at_install', 'post_install') +class TestWebAssets(odoo.tests.HttpCase): + def test_assets_url_validation(self): + website_id = self.env['website'].search([], limit=1, order='id desc').id + # valid urls + self.assertEqual( + self.url_open(f'/web/assets/debug/website-{website_id}+ltr/web.assets_frontend.css').status_code, + 200, + ) + self.assertEqual( + self.url_open(f'/web/assets/debug/website-{website_id}+rtl/web.assets_frontend.css').status_code, + 200, + ) + self.assertEqual( + self.url_open(f'/web/assets/debug/website-{website_id}+-/web.assets_frontend.js').status_code, + 200, + ) + + # invalid urls + with odoo.tools.mute_logger('odoo.addons.web.controllers.binary'): + self.assertEqual( + self.url_open(f'/web/assets/debug/website-{website_id}+hello/web.assets_frontend.css').status_code, + 404, + "unexpected direction extra", + ) + self.assertEqual( + self.url_open(f'/web/assets/debug/website-{website_id}+-/web.assets_f_ontend.js').status_code, + 404, + "bundle name contains `_` and should be escaped wildcard", + ) + self.assertEqual( + self.url_open(f'/web/assets/debug/website-{website_id}+ltr/web.assets_frontend.js').status_code, + 404, + "js cannot have `ltr` has extra", + ) + self.assertEqual( + self.url_open(f'/web/assets/debug/website-{website_id}+rtl/web.assets_frontend.js').status_code, + 404, + "js cannot have `rtl` has extra", + ) + self.assertEqual( + self.url_open(f'/web/assets/debug/website-{website_id}+-/web.assets_frontend.css').status_code, + 404, + "css cannot have `-` has extra", + ) + self.assertEqual( + self.url_open(f'/web/assets/debug/website-{website_id+1}+ltr/web.assets_frontend.css').status_code, + 404, + "website_id does not exist", + ) + self.assertEqual( + self.url_open(f'/web/assets/debug/website-{website_id}+aa+ltr/web.assets_frontend.css').status_code, + 404, + "only two extra expected, website and direction", + ) + self.assertEqual( + self.url_open(f'/web/assets/debug/website-{website_id}+website-{website_id}+ltr/web.assets_frontend.css').status_code, + 404, + "website defined twice", + ) + self.assertEqual( + self.url_open(f'/web/assets/debug/ltr+website-{website_id}/web.assets_frontend.css').status_code, + 404, + "website and direction inverted", + ) + + # redirect urls + self.assertEqual( + self.url_open(f'/web/assets/invalid_version/website-{website_id}+ltr/web.assets_frontend.min.css', allow_redirects=False).status_code, + 303 + ) + self.assertEqual( + self.url_open(f'/web/assets/invalid_version/website-{website_id}+ltr/web.assets_frontend.min.css', allow_redirects=False).headers['location'].split('/web/assets/')[1], + self.env['ir.qweb']._get_asset_bundle('web.assets_frontend', assets_params={'website_id': website_id}).get_link('css').split('/web/assets/')[1], + ) + + self.assertEqual( + self.url_open('/web/assets///').status_code, + 404, + ) diff --git a/odoo/addons/base/models/ir_asset.py b/odoo/addons/base/models/ir_asset.py index 5daa171a9c8..df0ae314ea2 100644 --- a/odoo/addons/base/models/ir_asset.py +++ b/odoo/addons/base/models/ir_asset.py @@ -105,9 +105,18 @@ class IrAsset(models.Model): """ return extra - - def _parse_assets_extra(self, extra_parts): - return {} + def _parse_assets_extra(self, extra_parts, asset_type): + if len(extra_parts) != 1: + raise ValueError('Expect only one element in extra') + direction = extra_parts[0] + if asset_type == 'css': + if direction not in ['rtl', 'ltr']: + msg = f'Invalid extra {direction}' + raise ValueError(msg) + elif direction != '-': + msg = f'Invalid extra {direction}' + raise ValueError(msg) + return {}, direction == 'rtl' @tools.conditional( 'xml' not in tools.config['dev_mode'],