From 4111f02fb3cc4cdf3dd94adae0ca80cfe874170c Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Thu, 13 Jul 2023 07:21:56 +0000 Subject: [PATCH 1/3] [ADD] core: block `requests` calls during tests Non-external tests should not be performing requests to external websites or services. Add a mock to handle such requests: - if the test is not tagged external - and the request is not to localhost - and the request is not to `file:` (it's possible to install a FileAdapter in a session to resolve file URLs) - raise a connection error (and log, both with some details of the blocked request for debugging) The mock is layered on the lowest possible level (`Session.send`), so tests can either: - reconfigure the mock to handle cases differently (the mock is re-created on every test) - layer their own mock at a higher level of the library Eventually we might also built-in a routing / dispatch mechanism so it's easier to declare external services you want to mock, somewhat similar to what's available client-side. Whitelist `file:` because e.g. zeep performs `file:` request, using a bespoke adapter installed in its session. Part-of: odoo/odoo#128497 --- odoo/tests/common.py | 30 +++++++++++++++++++++++++++++- 1 file changed, 29 insertions(+), 1 deletion(-) diff --git a/odoo/tests/common.py b/odoo/tests/common.py index 2fa3849de7f..6c6644a4f70 100644 --- a/odoo/tests/common.py +++ b/odoo/tests/common.py @@ -43,6 +43,7 @@ from xmlrpc import client as xmlrpclib import requests import werkzeug.urls from lxml import etree, html +from requests import PreparedRequest, Session import odoo from odoo import api @@ -243,7 +244,7 @@ def _normalize_arch_for_assert(arch_string, parser_method="xml"): arch_string = etree.fromstring(arch_string, parser=parser) return etree.tostring(arch_string, pretty_print=True, encoding='unicode') - +_super_send = requests.Session.send class BaseCase(case.TestCase, metaclass=MetaCase): """ Subclass of TestCase for Odoo-specific code. This class is abstract and expects self.registry, self.cr and self.uid to be initialized by subclasses. @@ -257,6 +258,33 @@ class BaseCase(case.TestCase, metaclass=MetaCase): super().__init__(methodName) self.addTypeEqualityFunc(etree._Element, self.assertTreesEqual) self.addTypeEqualityFunc(html.HtmlElement, self.assertTreesEqual) + self.request_mock = None + + def setUp(self): + super().setUp() + if 'external' not in self.test_tags: + # if the method is passed directly `patch` discards the session + # object which we need + # pylint: disable=unnecessary-lambda + self.patch( + requests.sessions.Session, + 'send', + lambda s, r, **kwargs: self._request_handler(s, r, **kwargs), + ) + + def _request_handler(self, s: Session, r: PreparedRequest, /, **kw): + # allow localhost requests + # TODO: also check port? + url = werkzeug.urls.url_parse(r.url) + if url.host in (HOST, 'localhost'): + return _super_send(s, r, **kw) + if url.scheme == 'file': + return _super_send(s, r, **kw) + + _logger.getChild('requests').info( + "Blocking un-mocked external HTTP request %s %s", r.method, r.url) + raise requests.exceptions.ConnectionError( + f"External requests verboten (was {r.method} {r.url})") def run(self, result): testMethod = getattr(self, self._testMethodName) From 0892923bcff081b2545731cee04da31ada5ec907 Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Thu, 13 Jul 2023 13:31:06 +0000 Subject: [PATCH 2/3] [FIX] mass_mailing: test_mailing_controller Add a handler for the fake URL we track, otherwise after visiting the short URL we get redirected and the request blows up. Part-of: odoo/odoo#128497 --- addons/mass_mailing/tests/test_mailing_controllers.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/addons/mass_mailing/tests/test_mailing_controllers.py b/addons/mass_mailing/tests/test_mailing_controllers.py index 14000b65947..a7777616ab8 100644 --- a/addons/mass_mailing/tests/test_mailing_controllers.py +++ b/addons/mass_mailing/tests/test_mailing_controllers.py @@ -7,6 +7,8 @@ import datetime import werkzeug from odoo.addons.mass_mailing.tests.common import MassMailCommon +from requests import Session, PreparedRequest, Response + from odoo.tests.common import HttpCase from odoo.tests import tagged @@ -33,6 +35,13 @@ class TestMailingControllers(MassMailCommon, HttpCase): # freeze time base value cls._reference_now = datetime.datetime(2022, 6, 14, 10, 0, 0) + def _request_handler(self, s: Session, r: PreparedRequest, /, **kw): + if r.url.startswith('https://www.example.com/foo/bar'): + r = Response() + r.status_code = 200 + return r + return super()._request_handler(s, r, **kw) + def test_tracking_short_code(self): """ Test opening short code linked to a mailing trace: should set the trace as opened and clicked, create a click record. """ From 2a92852d5c3cf5332eb554eab911b03cf24cbb84 Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Thu, 13 Jul 2023 13:37:44 +0000 Subject: [PATCH 3/3] [FIX] web_editor: suppress exceptions in get_video_thumbnail Error responses are already ignored, but connection errors would blow up the entire thing, which breaks `test_resequence_images` (and possibly others). Part-of: odoo/odoo#128497 --- addons/web_editor/tools.py | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/addons/web_editor/tools.py b/addons/web_editor/tools.py index 4a66907e0d2..f9da1b9a8bc 100644 --- a/addons/web_editor/tools.py +++ b/addons/web_editor/tools.py @@ -2,6 +2,7 @@ # Part of Odoo. See LICENSE file for full copyright and licensing details. import base64 +import contextlib import re import requests @@ -137,17 +138,18 @@ def get_video_thumbnail(video_url): response = None platform, video_id = source[:2] - if platform == 'youtube': - response = requests.get(f'https://img.youtube.com/vi/{video_id}/0.jpg', timeout=10) - elif platform == 'vimeo': - res = requests.get(f'http://vimeo.com/api/oembed.json?url={video_url}', timeout=10) - if res.ok: - data = res.json() - response = requests.get(data['thumbnail_url'], timeout=10) - elif platform == 'dailymotion': - response = requests.get(f'https://www.dailymotion.com/thumbnail/video/{video_id}', timeout=10) - elif platform == 'instagram': - response = requests.get(f'https://www.instagram.com/p/{video_id}/media/?size=t', timeout=10) + with contextlib.suppress(requests.exceptions.RequestException): + if platform == 'youtube': + response = requests.get(f'https://img.youtube.com/vi/{video_id}/0.jpg', timeout=10) + elif platform == 'vimeo': + res = requests.get(f'http://vimeo.com/api/oembed.json?url={video_url}', timeout=10) + if res.ok: + data = res.json() + response = requests.get(data['thumbnail_url'], timeout=10) + elif platform == 'dailymotion': + response = requests.get(f'https://www.dailymotion.com/thumbnail/video/{video_id}', timeout=10) + elif platform == 'instagram': + response = requests.get(f'https://www.instagram.com/p/{video_id}/media/?size=t', timeout=10) if response and response.ok: return image_process(response.content)