[FIX] base, web: set 304 status in binary_content

Before this commit, the No Change status wasn't properly set because
the `filehash` wasn't passed to `_binary_set_headers` and because
the status was not changed to 304 if it was already set to 200 earlier.

This commit fixes this issue and also prevents images to be processed
in `content_image` if the status is 304.

opw-2008426

closes odoo/odoo#34032

Signed-off-by: Christophe Simonis <chs@odoo.com>

addons/web/controllers/main.py

@@ -1046,7 +1046,7 @@ class Binary(http.Controller):
             filename_field=filename_field, download=download, mimetype=mimetype,
             default_mimetype='image/png', access_token=access_token)
 
-        if status == 301 or (status != 200 and download):
+        if status in [301, 304] or (status != 200 and download):
             return request.env['ir.http']._response_by_status(status, headers, content)
         if not content:
             content = base64.b64encode(self.placeholder(image='placeholder.png'))

addons/web/tests/test_image.py

@@ -2,6 +2,7 @@
 # Part of Odoo. See LICENSE file for full copyright and licensing details.
 
 import io
+import base64
 
 from PIL import Image
 
@@ -42,3 +43,23 @@ class TestImage(HttpCase):
         response = self.url_open('/web/image/fake/0/image_no_size')
         image = Image.open(io.BytesIO(response.content))
         self.assertEqual(image.size, (256, 256))
+
+    def test_02_content_image_Etag_304(self):
+        """This test makes sure that the 304 response is properly returned if the ETag is properly set"""
+
+        attachment = self.env['ir.attachment'].create({
+            'datas': b"R0lGODdhAQABAIAAAP///////ywAAAAAAQABAAACAkQBADs=",
+            'name': 'testEtag.gif',
+            'public': True,
+            'datas_fname': 'testEtag.gif',
+            'mimetype': 'image/gif',
+        })
+        response = self.url_open('/web/image/%s' % attachment.id)
+        self.assertEqual(response.status_code, 200)
+        self.assertEqual(base64.b64encode(response.content), attachment.datas)
+
+        etag = response.headers.get('ETag')
+
+        response2 = self.url_open('/web/image/%s' % attachment.id, headers={"If-None-Match": etag})
+        self.assertEqual(response2.status_code, 304)
+        self.assertEqual(len(response2.content), 0)

odoo/addons/base/models/ir_http.py

@@ -345,9 +345,11 @@ class IrHttp(models.AbstractModel):
         headers = [('Content-Type', mimetype), ('X-Content-Type-Options', 'nosniff')]
         # cache
         etag = bool(request) and request.httprequest.headers.get('If-None-Match')
-        status = status or (304 if filehash and etag == filehash else 200)
+        status = status or 200
         if filehash:
             headers.append(('ETag', filehash))
+            if etag == filehash and status == 200:
+                status = 304
         headers.append(('Cache-Control', 'max-age=%s' % (STATIC_CACHE if unique else 0)))
         # content-disposition default name
         if download:
@@ -390,11 +392,11 @@ class IrHttp(models.AbstractModel):
             status, content, filename, mimetype, filehash = self._binary_ir_attachment_redirect_content(record, default_mimetype=default_mimetype)
         if not content:
             status, content, filename, mimetype, filehash = self._binary_record_content(
-                record, field=field, filename=None, filename_field=filename_field,
+                record, field=field, filename=filename, filename_field=filename_field,
                 default_mimetype='application/octet-stream')
 
         status, headers, content = self._binary_set_headers(
-            status, content, filename, mimetype, unique, filehash=False, download=download)
+            status, content, filename, mimetype, unique, filehash=filehash, download=download)
 
         return status, headers, content
 

odoo/tests/common.py

@@ -822,12 +822,12 @@ class HttpCase(TransactionCase):
         self.opener = requests.Session()
         self.opener.cookies['session_id'] = self.session_id
 
-    def url_open(self, url, data=None, timeout=10):
+    def url_open(self, url, data=None, timeout=10, headers=None):
         if url.startswith('/'):
             url = "http://%s:%s%s" % (HOST, PORT, url)
         if data:
-            return self.opener.post(url, data=data, timeout=timeout)
-        return self.opener.get(url, timeout=timeout)
+            return self.opener.post(url, data=data, timeout=timeout, headers=headers)
+        return self.opener.get(url, timeout=timeout, headers=headers)
 
     def _wait_remaining_requests(self):
         t0 = int(time.time())