From 20ac5338ac6bebfce4721f8bc2c9b7d5226bd779 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?S=C3=A9bastien=20Theys?= <seb@odoo.com>
Date: Tue, 15 Oct 2019 14:11:31 +0000
Subject: [PATCH] [FIX] fields: verify image resolution on create/write

Before this commit, if `max_width` and `max_height` are 0, the image would not
be opened by Pillow at all (to gain a bit of CPU) however this was actually a
bad idea in this case because then we don't ensure the given value is actually a
valid image when we save it, only later at display we notice it and it crashes.

This optimization only made sense when `image_process` is called many times,
from the route displaying images every time a visitor is requesting an image,
for example. It doesn't really have an impact for one time operations such as
creating/writing.

So when size parameters are 0, which is the default, another parameter must be
passed to `image_process` to ensure image validity, and `verify_resolution` is
actually built for this, as it does the minimal amount of processing:
- loading the image
- making sure it is valid
- and ensuring the resolution is not completely crazy (`IMAGE_MAX_RESOLUTION`).
It doesn't alter the original image if no other operation was requested.

Pr: #38292
X-original-commit: e16c36bb4bfa3ae1739712c69c72849665a057dd
---
 odoo/addons/test_new_api/tests/test_new_fields.py |  6 ++++++
 odoo/fields.py                                    | 15 +++++++++++----
 2 files changed, 17 insertions(+), 4 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 674dd0a029f..c485bb21252 100644
--- a/odoo/addons/test_new_api/tests/test_new_fields.py
+++ b/odoo/addons/test_new_api/tests/test_new_fields.py
@@ -1508,6 +1508,12 @@ class TestFields(common.TransactionCase):
         self.assertEqual(record.image_256[:8], b'iVBORw0K')
         self.assertEqual(image_data_uri(record.image_256)[:30], 'data:image/png;base64,iVBORw0K')
 
+        # ensure invalid image raises
+        with self.assertRaises(UserError):
+            record.write({
+                'image': 'invalid image',
+            })
+
     def test_95_binary_bin_size(self):
         binary_value = base64.b64encode(b'content')
         binary_size = b'7.00 bytes'
diff --git a/odoo/fields.py b/odoo/fields.py
index f753371719a..fdb56fc898a 100644
--- a/odoo/fields.py
+++ b/odoo/fields.py
@@ -2022,6 +2022,7 @@ class Image(Binary):
     _slots = {
         'max_width': 0,
         'max_height': 0,
+        'verify_resolution': True,
     }
 
     def create(self, record_values):
@@ -2042,13 +2043,19 @@ class Image(Binary):
         records.env.cache.update(records, self, [cache_value] * len(records))
 
     def _image_process(self, value):
-        if value and (self.max_width or self.max_height):
-            value = image_process(value, size=(self.max_width, self.max_height))
-        return value
+        return image_process(value,
+            size=(self.max_width, self.max_height),
+            verify_resolution=self.verify_resolution,
+        )
 
     def _process_related(self, value):
         """Override to resize the related value before saving it on self."""
-        return self._image_process(super()._process_related(value))
+        try:
+            return self._image_process(super()._process_related(value))
+        except UserError:
+            # Avoid the following `write` to fail if the related image was saved
+            # invalid, which can happen for pre-existing databases.
+            return False
 
 
 class Selection(Field):