[FIX] *: block external HTTP requests during non-external tests
A recent discovery has led us to understand that actual HTTP requests to external services are performed even during non-external tests (even during at-install tests in fact). This PR puts a stop to that, by installing a requests mock which results in a ConnectionError for any request to an other host than localhost / 127.0.0.1. May add richer routing support in the future, but currently the easiest way to handle is to override `_request_handler` and return custom responses as needed when matching expected endpoints. Issues found so far: - `/link_tracker:TestMailRenderMixin` performs requests to all sorts of domains real and fake during testing, originally fixed in #128249 but re-fixed more generically here (the link tracker handles connection failure gracefully). - `partner_autocomplete` tests cause a request to clearbit to resolve the logo of the mock partner, failure is handled silently so nothing more is needed, this probably occurs in a few more locations (I didn't check every single log). - `/mass_mailing:TestMailingControllers.test_tracking_short_code` creates shortlinks to example.com, then opens the shortlink. While the shortlink is local, it redirects to an external site and the test is not happy when that fails. Intercept requests to that endpoint and returns successes. - `/website_sale:TestProductPictureController.test_resequence_images` touches the video_url of a product, which causes a call to `get_video_thumbnail`, which did handle (ignore) non-200 responses but not complete request failure. Add support for that (ignore `RequestException`). closes odoo/odoo#128497 Signed-off-by: Xavier Morel (xmo) <xmo@odoo.com>
This commit is contained in:
@@ -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. """
|
||||
|
||||
+13
-11
@@ -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)
|
||||
|
||||
+29
-1
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user