From f3e10f698032407f8e6da9abcd37aa224e7c0c1e Mon Sep 17 00:00:00 2001 From: Thanh Dodeur <tso@odoo.com> Date: Tue, 11 Jun 2019 10:47:31 +0000 Subject: [PATCH] [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 | 2 +- addons/web/tests/test_image.py | 21 +++++++++++++++++++++ odoo/addons/base/models/ir_http.py | 8 +++++--- odoo/tests/common.py | 6 +++--- 4 files changed, 30 insertions(+), 7 deletions(-) diff --git a/addons/web/controllers/main.py b/addons/web/controllers/main.py index 040d13aad20a..6d382c92333f 100644 --- a/addons/web/controllers/main.py +++ b/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')) diff --git a/addons/web/tests/test_image.py b/addons/web/tests/test_image.py index e17073d0fb72..d26528b88e36 100644 --- a/addons/web/tests/test_image.py +++ b/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) diff --git a/odoo/addons/base/models/ir_http.py b/odoo/addons/base/models/ir_http.py index 293269d61d1f..8920c8e9a6b2 100644 --- a/odoo/addons/base/models/ir_http.py +++ b/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 diff --git a/odoo/tests/common.py b/odoo/tests/common.py index 1c4f87d75aca..92e6fe0addf7 100644 --- a/odoo/tests/common.py +++ b/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()) -- GitLab