From 49548e9a8c5925df759d85d86a010db2dbe59427 Mon Sep 17 00:00:00 2001
From: Jorge Pinna Puissant <jpp@odoo.com>
Date: Thu, 26 Oct 2023 09:28:36 +0200
Subject: [PATCH] [IMP] base, test_assetsbundle: avoid parsing broken XML
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Before this commit, the `recover` parameter is used when parsing the xml
files. The issue with this, is that it will allow and parse broken XML
files, and the error messages in JS will not be related to the issue on
the broken XML.

Now, the parser is stricter, and the broken template will be replaced
with an error template with a compressive error message of the parsing
error.

closes odoo/odoo#139818

Task-id: 3420400
Signed-off-by: Xavier Dollé (xdo) <xdo@odoo.com>
---
 odoo/addons/base/models/assetsbundle.py        |  7 +++++--
 odoo/addons/test_assetsbundle/__manifest__.py  |  3 +++
 .../static/invalid_src/xml/invalid_xml.xml     |  6 ++++++
 .../tests/test_assetsbundle.py                 | 18 ++++++++++++++++++
 4 files changed, 32 insertions(+), 2 deletions(-)
 create mode 100644 odoo/addons/test_assetsbundle/static/invalid_src/xml/invalid_xml.xml

diff --git a/odoo/addons/base/models/assetsbundle.py b/odoo/addons/base/models/assetsbundle.py
index 55f23a4b9a4..86141a82a4c 100644
--- a/odoo/addons/base/models/assetsbundle.py
+++ b/odoo/addons/base/models/assetsbundle.py
@@ -898,8 +898,11 @@ class XMLAsset(WebAsset):
         except AssetError as e:
             return u"console.error(%s);" % json.dumps(to_text(e))
 
-        parser = etree.XMLParser(ns_clean=True, recover=True, remove_comments=True)
-        root = etree.parse(io.BytesIO(content.encode('utf-8')), parser=parser).getroot()
+        parser = etree.XMLParser(ns_clean=True, remove_comments=True, resolve_entities=False)
+        try:
+            root = etree.fromstring(content.encode('utf-8'), parser=parser)
+        except etree.XMLSyntaxError as e:
+            return f'<t t-name="parsing_error"><parsererror>Invalid XML template: {self.url} \n {e.msg} </parsererror></t>'
         if root.tag in ('templates', 'template'):
             return ''.join(etree.tostring(el, encoding='unicode') for el in root)
         return etree.tostring(root, encoding='unicode')
diff --git a/odoo/addons/test_assetsbundle/__manifest__.py b/odoo/addons/test_assetsbundle/__manifest__.py
index 1ab5561d25b..632291e2ef7 100644
--- a/odoo/addons/test_assetsbundle/__manifest__.py
+++ b/odoo/addons/test_assetsbundle/__manifest__.py
@@ -62,6 +62,9 @@
         'test_assetsbundle.lazy_test_component': [
             'test_assetsbundle/static/tests/lazy_test_component/**/*',
         ],
+        'test_assetsbundle.broken_xml': [
+            'test_assetsbundle/static/invalid_src/xml/invalid_xml.xml',
+        ],
     },
     'license': 'LGPL-3',
 }
diff --git a/odoo/addons/test_assetsbundle/static/invalid_src/xml/invalid_xml.xml b/odoo/addons/test_assetsbundle/static/invalid_src/xml/invalid_xml.xml
new file mode 100644
index 00000000000..51408a14070
--- /dev/null
+++ b/odoo/addons/test_assetsbundle/static/invalid_src/xml/invalid_xml.xml
@@ -0,0 +1,6 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<templates xml:space="preserve">
+  <t t-name="test_assetsvundle.invalid_xml">
+    <SomeComponent>content<SomeComponent/>
+  </t>
+</templates>
diff --git a/odoo/addons/test_assetsbundle/tests/test_assetsbundle.py b/odoo/addons/test_assetsbundle/tests/test_assetsbundle.py
index 64642924f14..754a71469bd 100644
--- a/odoo/addons/test_assetsbundle/tests/test_assetsbundle.py
+++ b/odoo/addons/test_assetsbundle/tests/test_assetsbundle.py
@@ -626,6 +626,24 @@ class TestJavascriptAssetsBundle(FileTouchable):
     </body>
 </html>"""))
 
+class TestXMLAssetsBundle(FileTouchable):
+
+    def _get_asset(self, bundle, rtl=False, debug_assets=False):
+        files, _ = self.env['ir.qweb']._get_asset_content(bundle)
+        return AssetsBundle(bundle, files, env=self.env, debug_assets=debug_assets, rtl=rtl)
+
+    def test_01_broken_xml(self):
+        """ Checks that a bundle don't try hard to parse broken xml, and returns a comprehensive
+        error message.
+        """
+        self.bundle = self._get_asset('test_assetsbundle.broken_xml')
+
+        # there shouldn't be any test_assetsvundle.invalid_xml template.
+        # there should be an parsing_error template with the parsing error message.
+        self.assertEqual(self.bundle.xml(),
+                         '<t t-name="parsing_error"><parsererror>Invalid XML template: /test_assetsbundle/static/invalid_src/xml/invalid_xml.xml \n Opening and ending tag mismatch: SomeComponent line 4 and t, line 5, column 7 </parsererror></t>',
+                         "the parsing error should be shown")
+
 
 @tagged('-at_install', 'post_install')
 class TestAssetsBundleInBrowser(HttpCase):