From a856c4659558ef848a3b5a0cb37d5614b16b14b2 Mon Sep 17 00:00:00 2001 From: Mathieu Walravens <wama@odoo.com> Date: Tue, 4 Apr 2023 14:00:28 +0000 Subject: [PATCH] [FIX] http: rewind file upload on serialization failure Before this commit: When uploading a file, if the transaction fails due to a serialization failure, Odoo will retry the request. However, if a file upload is read during the transaction, the file pointer will be at the end of the file, and calling `.read()` again returns an empty bytes object. After this commit: Upon retrying the request, rewind uploads to the beginning of the file, if the file supports it. opw-3228200 closes odoo/odoo#117669 Signed-off-by: Julien Castiaux (juc) <juc@odoo.com> --- odoo/addons/test_http/__init__.py | 1 + odoo/addons/test_http/__manifest__.py | 11 ++++++++ odoo/addons/test_http/controllers.py | 29 ++++++++++++++++++++++ odoo/addons/test_http/tests/__init__.py | 1 + odoo/addons/test_http/tests/test_upload.py | 17 +++++++++++++ odoo/http.py | 9 +++++++ 6 files changed, 68 insertions(+) create mode 100644 odoo/addons/test_http/__init__.py create mode 100644 odoo/addons/test_http/__manifest__.py create mode 100644 odoo/addons/test_http/controllers.py create mode 100644 odoo/addons/test_http/tests/__init__.py create mode 100644 odoo/addons/test_http/tests/test_upload.py diff --git a/odoo/addons/test_http/__init__.py b/odoo/addons/test_http/__init__.py new file mode 100644 index 000000000000..e046e49fbe22 --- /dev/null +++ b/odoo/addons/test_http/__init__.py @@ -0,0 +1 @@ +from . import controllers diff --git a/odoo/addons/test_http/__manifest__.py b/odoo/addons/test_http/__manifest__.py new file mode 100644 index 000000000000..40fd94f5fd8f --- /dev/null +++ b/odoo/addons/test_http/__manifest__.py @@ -0,0 +1,11 @@ +# Part of Odoo. See LICENSE file for full copyright and licensing details. +{ + 'name': 'Test HTTP', + 'version': '1.0', + 'category': 'Hidden/Tests', + 'description': """A module to test HTTP""", + 'depends': ['base', 'web', 'web_tour'], + 'installable': True, + 'data': [], + 'license': 'LGPL-3', +} diff --git a/odoo/addons/test_http/controllers.py b/odoo/addons/test_http/controllers.py new file mode 100644 index 000000000000..05af6390a9bb --- /dev/null +++ b/odoo/addons/test_http/controllers.py @@ -0,0 +1,29 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +from psycopg2.errorcodes import SERIALIZATION_FAILURE +from psycopg2 import OperationalError + +from odoo import http + +# Force serialization errors. Patched in some tests. +should_fail = None + + +class SerializationFailureError(OperationalError): + pgcode = SERIALIZATION_FAILURE + + +class HttpTest(http.Controller): + @http.route("/test_http/upload_file", methods=["POST"], type="http", auth="none", csrf=False) + def upload_file_retry(self, ufile): + global should_fail # pylint: disable=W0603 + if should_fail is None: + raise ValueError("should_fail should be set.") + + data = ufile.read() + if should_fail: + should_fail = False # Fail once + raise SerializationFailureError() + + return data.decode() diff --git a/odoo/addons/test_http/tests/__init__.py b/odoo/addons/test_http/tests/__init__.py new file mode 100644 index 000000000000..1810163c7aad --- /dev/null +++ b/odoo/addons/test_http/tests/__init__.py @@ -0,0 +1 @@ +from . import test_upload diff --git a/odoo/addons/test_http/tests/test_upload.py b/odoo/addons/test_http/tests/test_upload.py new file mode 100644 index 000000000000..fe57b4a2c56f --- /dev/null +++ b/odoo/addons/test_http/tests/test_upload.py @@ -0,0 +1,17 @@ +# -*- coding: utf-8 -*- +# Part of Odoo. See LICENSE file for full copyright and licensing details. + +from io import StringIO +from unittest.mock import patch +from odoo.tests import common, tagged + + +@tagged('-at_install', 'post_install') +class TestHttpUpload(common.HttpCase): + def test_upload_file_retry(self): + from odoo.addons.test_http import controllers # pylint: disable=C0415 + + with patch.object(controllers, "should_fail", True), StringIO("Hello world!") as file: + res = self.url_open("/test_http/upload_file", files={"ufile": file}, timeout=None) + self.assertEqual(res.status_code, 200) + self.assertEqual(res.text, file.getvalue()) diff --git a/odoo/http.py b/odoo/http.py index af937cc263ff..27614315bc10 100644 --- a/odoo/http.py +++ b/odoo/http.py @@ -345,6 +345,15 @@ class WebRequest(object): if self._cr and not first_time: self._cr.rollback() self.env.clear() + + # Rewind files in case of failure + if not first_time: + for filename, file in self.httprequest.files.items(): + if hasattr(file, "seekable") and file.seekable(): + file.seek(0) + else: + raise RuntimeError("Cannot retry request on input file %r after serialization failure" % filename) + first_time = False result = self.endpoint(*a, **kw) if isinstance(result, Response) and result.is_qweb: -- GitLab