From 1d87710fdc8738d188ae3b2f6a795119dc00a6e2 Mon Sep 17 00:00:00 2001 From: Mathieu Walravens 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#120180 X-original-commit: ac59ef0668122ad71dffbb5575250c767a0a56ec Signed-off-by: Julien Castiaux (juc) --- odoo/addons/test_http/controllers.py | 25 ++++++++++++++++++++++++ odoo/addons/test_http/tests/test_misc.py | 8 ++++++++ odoo/service/model.py | 6 ++++++ 3 files changed, 39 insertions(+) diff --git a/odoo/addons/test_http/controllers.py b/odoo/addons/test_http/controllers.py index 9933c0862f6..dba5f5299e1 100644 --- a/odoo/addons/test_http/controllers.py +++ b/odoo/addons/test_http/controllers.py @@ -1,7 +1,11 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. import json import logging + import werkzeug +from psycopg2.errorcodes import SERIALIZATION_FAILURE +from psycopg2 import OperationalError + from odoo import http from odoo.exceptions import AccessError, UserError from odoo.http import request @@ -16,6 +20,14 @@ CT_JSON = {'Content-Type': 'application/json; charset=utf-8'} WSGI_SAFE_KEYS = {'PATH_INFO', 'QUERY_STRING', 'RAW_URI', 'SCRIPT_NAME', 'wsgi.url_scheme'} +# Force serialization errors. Patched in some tests. +should_fail = None + + +class SerializationFailureError(OperationalError): + pgcode = SERIALIZATION_FAILURE + + class TestHttp(http.Controller): # ===================================================== @@ -177,3 +189,16 @@ class TestHttp(http.Controller): raise AccessError("Wrong iris code") if error == 'UserError': raise UserError("Walter is AFK") + + @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/test_misc.py b/odoo/addons/test_http/tests/test_misc.py index 577bafd2907..2efddb9f8e0 100644 --- a/odoo/addons/test_http/tests/test_misc.py +++ b/odoo/addons/test_http/tests/test_misc.py @@ -1,6 +1,7 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. import json +from io import StringIO from socket import gethostbyname from unittest.mock import patch from urllib.parse import urlparse @@ -127,6 +128,13 @@ class TestHttpMisc(TestHttpBase): 'time_zone': 'Europe/Paris', }) + def test_misc6_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()) @tagged('post_install', '-at_install') class TestHttpCors(TestHttpBase): diff --git a/odoo/service/model.py b/odoo/service/model.py index e368d5bfcf1..c31a14977d5 100644 --- a/odoo/service/model.py +++ b/odoo/service/model.py @@ -142,6 +142,12 @@ def retrying(func, env): env.registry.reset_changes() if request: request.session = request._get_session_and_dbname()[0] + # Rewind files in case of failure + for filename, file in request.httprequest.files.items(): + if hasattr(file, "seekable") and file.seekable(): + file.seek(0) + else: + raise RuntimeError(f"Cannot retry request on input file {filename!r} after serialization failure") from exc if isinstance(exc, IntegrityError): raise _as_validation_error(env, exc) from exc if exc.pgcode not in PG_CONCURRENCY_ERRORS_TO_RETRY: