diff --git a/morango/sync/syncsession.py b/morango/sync/syncsession.py index 777004ec..dbc74101 100644 --- a/morango/sync/syncsession.py +++ b/morango/sync/syncsession.py @@ -69,15 +69,25 @@ def _get_server_ip(hostname): def _get_client_ip_for_server(server_host, server_port): - s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM) + default_ip = "127.0.0.1" + + # strict apparmor rules or sandboxing could prevent this + try: + s = socket.socket(socket.AF_INET, socket.SOCK_DGRAM) + except OSError: + logger.warning( + f"Could not create socket to determine external IP address to reach {server_host}" + ) + return default_ip + try: s.connect((server_host, server_port)) - IP = s.getsockname()[0] - except: # noqa: E722 - IP = "127.0.0.1" + return s.getsockname()[0] + except (OSError, ValueError, OverflowError): # ValueError covers UnicodeError + logger.warning(f"Could not determine external IP address to reach {server_host}") + return default_ip finally: s.close() - return IP def ignore_404(target): diff --git a/tests/testapp/tests/helpers.py b/tests/testapp/tests/helpers.py index 55bc9467..7ee07034 100644 --- a/tests/testapp/tests/helpers.py +++ b/tests/testapp/tests/helpers.py @@ -3,6 +3,7 @@ """ import json +import socket import uuid import factory @@ -35,6 +36,34 @@ from .compat import EnvironmentVarGuard +def get_free_tcp_port(): + # sandboxing can cause 0 to fail (which is to try for any open port), so fallback to a hardcoded + # list afterward + ports = [0, 3000, 8000, 8001, 8080, 8081, 8888, 10001, 10002, 10003] + for p in ports: + try: + with socket.socket(socket.AF_INET, socket.SOCK_STREAM) as tcp: + tcp.bind(("127.0.0.1", p)) + _, port = tcp.getsockname() + return port + except OSError: + pass + + raise RuntimeError("Could not find open port") + + +class FreePortLiveServerTestCase(LiveServerTestCase): + """ + LiveServerTestCase defaults to port 0, which sandboxing can deny, so pick an explicit port + for each test class instead + """ + + @classmethod + def setUpClass(cls): + cls.port = get_free_tcp_port() + super(FreePortLiveServerTestCase, cls).setUpClass() + + class FacilityModelFactory(factory.django.DjangoModelFactory): class Meta: model = Facility @@ -325,7 +354,7 @@ def create_buffer_and_store_dummy_data(transfer_session_id): return data -class BaseClientTestCase(LiveServerTestCase): +class BaseClientTestCase(FreePortLiveServerTestCase): profile = "facilitydata" def setUp(self): diff --git a/tests/testapp/tests/integration/test_syncsession.py b/tests/testapp/tests/integration/test_syncsession.py index cc019328..40f33a37 100644 --- a/tests/testapp/tests/integration/test_syncsession.py +++ b/tests/testapp/tests/integration/test_syncsession.py @@ -1,7 +1,6 @@ import contextlib import json import os -import socket import subprocess import sys import time @@ -29,6 +28,7 @@ from morango.sync.controller import MorangoProfileController from ..compat import EnvironmentVarGuard +from ..helpers import get_free_tcp_port SECOND_TEST_DATABASE = "default2" SECOND_SYSTEM_ID = "default2" @@ -45,14 +45,6 @@ def second_environment(): assert instance1.id != instance2.id -def get_free_tcp_port(): - tcp = socket.socket(socket.AF_INET, socket.SOCK_STREAM) - tcp.bind(("", 0)) - addr, port = tcp.getsockname() - tcp.close() - return port - - class LiveServer: def __init__(self): self.env = os.environ.copy() diff --git a/tests/testapp/tests/sync/test_syncsession.py b/tests/testapp/tests/sync/test_syncsession.py index e715329b..5e63612e 100644 --- a/tests/testapp/tests/sync/test_syncsession.py +++ b/tests/testapp/tests/sync/test_syncsession.py @@ -2,7 +2,7 @@ import uuid import mock -from django.test.testcases import LiveServerTestCase +from django.test import SimpleTestCase from django.test.utils import override_settings from requests.exceptions import ChunkedEncodingError from requests.exceptions import HTTPError @@ -27,6 +27,7 @@ from morango.sync.context import NetworkSessionContext from morango.sync.controller import MorangoProfileController from morango.sync.session import SessionWrapper +from morango.sync.syncsession import _get_client_ip_for_server from morango.sync.syncsession import NetworkSyncConnection from morango.sync.syncsession import PullClient from morango.sync.syncsession import PushClient @@ -35,6 +36,7 @@ from ..helpers import BaseClientTestCase from ..helpers import BaseTransferClientTestCase +from ..helpers import FreePortLiveServerTestCase def mock_patch_decorator(func): @@ -52,7 +54,31 @@ def wrapper(*args, **kwargs): return wrapper -class NetworkSyncConnectionTestCase(LiveServerTestCase): +class GetClientIPForServerTestCase(SimpleTestCase): + logger_name = "morango.sync.syncsession" + + @mock.patch("morango.sync.syncsession.socket.socket") + def test_socket_creation_fails(self, mock_socket): + mock_socket.side_effect = OSError("Too many open files") + with self.assertLogs(self.logger_name, "WARNING") as logs: + ip = _get_client_ip_for_server("example.com", 80) + self.assertEqual(ip, "127.0.0.1") + self.assertIn("Could not create socket", logs.output[0]) + + @mock.patch("morango.sync.syncsession.socket.socket") + def test_connect_fails(self, mock_socket): + for error in (OSError("unreachable"), UnicodeError("label too long"), OverflowError()): + with self.subTest(error=type(error).__name__): + mock_socket.reset_mock() + mock_socket.return_value.connect.side_effect = error + with self.assertLogs(self.logger_name, "WARNING") as logs: + ip = _get_client_ip_for_server("example.com", 80) + self.assertEqual(ip, "127.0.0.1") + self.assertIn("Could not determine external IP address", logs.output[0]) + mock_socket.return_value.close.assert_called_once_with() + + +class NetworkSyncConnectionTestCase(FreePortLiveServerTestCase): def setUp(self): super(NetworkSyncConnectionTestCase, self).setUp() self.profile = "facilitydata" diff --git a/tests/testapp/tests/test_uuid_utilities.py b/tests/testapp/tests/test_uuid_utilities.py index 5f9df151..58d7b393 100644 --- a/tests/testapp/tests/test_uuid_utilities.py +++ b/tests/testapp/tests/test_uuid_utilities.py @@ -144,6 +144,7 @@ def test_consistent_0_5_instance_id(self, *args): with EnvironmentVarGuard() as env: env["MORANGO_SYSTEM_ID"] = "magicsysid" + env["MORANGO_NODE_ID"] = "" # ensure envvar override does not kick in DatabaseIDModel.objects.all().update(current=False) DatabaseIDModel.objects.create(id="7fe445b75cea11858c00fb97bdee8878", current=True)