[IMP] web, website: add validation for /web/assets

Increase the validation of assets url especially when parsing extra to
avoid aving to much possible url for the same asset.

Part-of: odoo/odoo#131353

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

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:

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,
+        )

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'],