From b478c42beb0f939e34c5ebbfdb88e3a602b3114c Mon Sep 17 00:00:00 2001
From: nda <nda@odoo.com>
Date: Mon, 20 Nov 2023 23:55:47 +0000
Subject: [PATCH] [FIX] core: avoid infinite loop during HTML sanitize

During an onchange, fields read from database are validated again. This causes
an infinite loop with HTML fields since Odoo 16  because we're revalidating a
value we're reading from the database, which triggers a sanitisation check,
which fetches the original value, etc etc.

Even if the functional bug only appears in 16.0 and above, the root cause is
also present in 15.0 so this commit targets 15.0

steps to reproduce (in 16 or next versions):

- Change Marc Demos's access rights Website = Restricted Editor
- Bypass HTML Field Sanitize = Off
- Inventory / Products / Product Variants
- Studio on the Sales tab and add website_description under Website Sequence
- Go to product [E-COM07] Large Cabinet and fill in the newly added website description
- Log out and log in as marc demo
- Navigate back to [E-COM07] Large Cabinet product variant and try to re-order the vendors on the purchase tab
- Error

before this commit:
Error (infinite loop) during onchange

  File "/home/nda/dev/odoo/16.0/odoo/odoo/models.py", line 6489, in onchange
    record[parent_name]._update_cache({name: record[name]})
  File "/home/nda/dev/odoo/16.0/odoo/odoo/models.py", line 5310, in _update_cache
    value = field.convert_to_cache(value, self, validate)
  File "/home/nda/dev/odoo/16.0/odoo/odoo/fields.py", line 1977, in convert_to_cache
    return self._convert(value, record, validate)
  File "/home/nda/dev/odoo/16.0/odoo/odoo/fields.py", line 2000, in _convert
    original_value = record[self.name]
  File "/home/nda/dev/odoo/16.0/odoo/odoo/models.py", line 5897, in __getitem__
    return self._fields[key].__get__(self, type(self))
  File "/home/nda/dev/odoo/16.0/odoo/odoo/fields.py", line 1198, in __get__
    value = self.convert_to_cache(record._origin[self.name], record)
  File "/home/nda/dev/odoo/16.0/odoo/odoo/fields.py", line 1977, in convert_to_cache
    return self._convert(value, record, validate)
  File "/home/nda/dev/odoo/16.0/odoo/odoo/fields.py", line 2000, in _convert
    original_value = record[self.name]
  ...

after this commit:
no error

opw-3575865

X-original-commit: 24d804e
Part-of: odoo/odoo#143307
---
 .../test_new_api/tests/test_new_fields.py     | 30 +++++++++++++++++++
 odoo/fields.py                                |  8 ++---
 2 files changed, 32 insertions(+), 6 deletions(-)

diff --git a/odoo/addons/test_new_api/tests/test_new_fields.py b/odoo/addons/test_new_api/tests/test_new_fields.py
index 211d3b874a7..0b38546f1e2 100644
--- a/odoo/addons/test_new_api/tests/test_new_fields.py
+++ b/odoo/addons/test_new_api/tests/test_new_fields.py
@@ -3268,6 +3268,36 @@ class TestHtmlField(common.TransactionCase):
         new_record.invalidate_recordset()
         new_record.with_user(internal_user).comment5
 
+    @patch('odoo.fields.html_sanitize', return_value='<p>comment</p>')
+    def test_onchange_sanitize(self, patch):
+        self.assertTrue(self.registry['test_new_api.mixed'].comment2.sanitize)
+
+        record = self.env['test_new_api.mixed'].create({
+            'comment2': '<p>comment</p>',
+        })
+
+        # in a perfect world this should be 1, but at the moment the value is
+        # sanitized more than once during creation of the record
+        self.assertEqual(patch.call_count, 3)
+
+        # new value needs to be validated, so it is sanitized once more
+        record.comment2 = '<p>comment</p>'
+        self.assertEqual(patch.call_count, 4)
+
+        # the value is already sanitized for flushing
+        record.flush_recordset()
+        self.assertEqual(patch.call_count, 4)
+
+        # value coming from db does not need to be sanitized
+        record.invalidate_recordset()
+        record.comment2
+        self.assertEqual(patch.call_count, 4)
+
+        # value coming from db during an onchange does not need to be sanitized
+        new_record = record.new(origin=record)
+        new_record.comment2
+        self.assertEqual(patch.call_count, 4)
+
 
 class TestMagicFields(common.TransactionCase):
 
diff --git a/odoo/fields.py b/odoo/fields.py
index 661a986c246..d00e98d317d 100644
--- a/odoo/fields.py
+++ b/odoo/fields.py
@@ -1191,7 +1191,7 @@ class Field(MetaField('DummyField', (object,), {})):
 
         elif self.store and record._origin and not (self.compute and self.readonly):
             # new record with origin: fetch from origin
-            value = self.convert_to_cache(record._origin[self.name], record)
+            value = self.convert_to_cache(record._origin[self.name], record, validate=False)
             env.cache.set(record, self, value)
 
         elif self.compute: #pylint: disable=using-constant-test
@@ -2042,11 +2042,7 @@ class Html(_String):
             if record.user_has_groups('base.group_sanitize_override'):
                 return value
 
-            # This may cause an infinite recursion when accessing the field on a
-            # new record. Indeed, if record has no value in cache, we default
-            # on the field's value on record._origin and convert it to the
-            # cache format, which ends up here, accessing the field on record!
-            original_value = record[self.name] if record.id else None
+            original_value = record[self.name]
             if original_value:
                 # Note that sanitize also normalize
                 original_value_sanitized = html_sanitize(original_value, **sanitize_vals)