From 6614c8910afce3bfc1732f6d7ce67744d1190543 Mon Sep 17 00:00:00 2001 From: drew2a Date: Fri, 24 May 2024 12:17:11 +0200 Subject: [PATCH] Refactor proxy settings and add error handling --- .../download_manager/download_manager.py | 42 ++++++---- .../libtorrent/tests/test_download_manager.py | 82 +++++++++++++------ 2 files changed, 87 insertions(+), 37 deletions(-) diff --git a/src/tribler/core/components/libtorrent/download_manager/download_manager.py b/src/tribler/core/components/libtorrent/download_manager/download_manager.py index 06620faf6b0..dfa0ba0964a 100644 --- a/src/tribler/core/components/libtorrent/download_manager/download_manager.py +++ b/src/tribler/core/components/libtorrent/download_manager/download_manager.py @@ -8,11 +8,11 @@ import os import time as timemod from asyncio import CancelledError, gather, iscoroutine, shield, sleep, wait_for -from binascii import unhexlify from copy import deepcopy from shutil import rmtree -from typing import Callable, Dict, List, Optional +from typing import Callable, Dict, List, Optional, Tuple +from binascii import unhexlify from ipv8.taskmanager import TaskManager from tribler.core import notifications @@ -362,21 +362,35 @@ def get_session(self, hops=0): return self.ltsessions[hops] - def set_proxy_settings(self, ltsession, ptype, server=None, auth=None): + def set_proxy_settings(self, session, ptype, server: Optional[Tuple[str, str]] = None, + auth: Optional[Tuple[str, str]] = None): """ Apply the proxy settings to a libtorrent session. This mechanism changed significantly in libtorrent 1.1.0. """ - settings = {} - settings["proxy_type"] = ptype - settings["proxy_hostnames"] = True - settings["proxy_peer_connections"] = True - if server and server[0] and server[1]: - settings["proxy_hostname"] = server[0] - settings["proxy_port"] = int(server[1]) - if auth: - settings["proxy_username"] = auth[0] - settings["proxy_password"] = auth[1] - self.set_session_settings(ltsession, settings) + settings = {"proxy_type": ptype, "proxy_hostnames": True, "proxy_peer_connections": True} + + try: + if not all(v is not None for v in server): + raise ValueError('Host and port should not be None') + + host, port = server + port = int(port) + settings["proxy_hostname"] = host + settings["proxy_port"] = port + except (ValueError, TypeError) as e: + self._logger.exception(e) + + try: + if not all(v is not None for v in auth): + raise ValueError('Username and password should not be None') + + username, proxy_password = auth + settings["proxy_username"] = username + settings["proxy_password"] = proxy_password + except (ValueError, TypeError) as e: + self._logger.exception(e) + + self.set_session_settings(session, settings) def set_upload_rate_limit(self, rate, hops=None): # Rate conversion due to the fact that we had a different system with Swift diff --git a/src/tribler/core/components/libtorrent/tests/test_download_manager.py b/src/tribler/core/components/libtorrent/tests/test_download_manager.py index c7f8c24f56a..4ce6aa98b41 100644 --- a/src/tribler/core/components/libtorrent/tests/test_download_manager.py +++ b/src/tribler/core/components/libtorrent/tests/test_download_manager.py @@ -1,5 +1,4 @@ import asyncio -import functools import itertools from asyncio import Future from unittest.mock import AsyncMock, MagicMock, Mock @@ -347,28 +346,15 @@ def test_set_proxy_settings(fake_dlmgr): """ Test setting the proxy settings """ - - def on_proxy_set(settings): - assert settings - assert settings.hostname == 'a' - assert settings.port == 1234 - assert settings.username == 'abc' - assert settings.password == 'def' - - def on_set_settings(settings): - assert settings - assert settings['proxy_hostname'] == 'a' - assert settings['proxy_port'] == 1234 - assert settings['proxy_username'] == 'abc' - assert settings['proxy_password'] == 'def' - assert settings['proxy_peer_connections'] - assert settings['proxy_hostnames'] - - mock_lt_session = MagicMock() - mock_lt_session.get_settings = dict - mock_lt_session.set_settings = on_set_settings - mock_lt_session.set_proxy = on_proxy_set # Libtorrent < 1.1.0 uses set_proxy to set proxy settings - fake_dlmgr.set_proxy_settings(mock_lt_session, 0, ('a', "1234"), ('abc', 'def')) + session = Mock() + fake_dlmgr.set_proxy_settings(session, 0, ('a', "1234"), ('abc', 'def')) + settings, = session.method_calls[-1].args + assert settings['proxy_hostname'] == 'a' + assert settings['proxy_port'] == 1234 + assert settings['proxy_username'] == 'abc' + assert settings['proxy_password'] == 'def' + assert settings['proxy_peer_connections'] + assert settings['proxy_hostnames'] def test_payout_on_disconnect(fake_dlmgr): @@ -575,3 +561,53 @@ def test_update_trackers_list_append(fake_dlmgr) -> None: actual_trackers = set(itertools.chain.from_iterable(tracker_list)) assert actual_trackers == {fake_tracker1, fake_tracker2} + +def test_set_proxy_settings_invalid_port(fake_dlmgr): + # Test setting the proxy settings for an invalid port number. In this case port and host should not be set. + session = Mock() + proxy_type = 2 + + fake_dlmgr.set_proxy_settings(session, proxy_type, ('host name', 'invalid port')) + + settings, = session.method_calls[-1].args + assert 'proxy_port' not in settings + assert 'proxy_hostname' not in settings + assert settings['proxy_type'] == proxy_type + + +def test_set_proxy_defaults(fake_dlmgr): + # Test setting the proxy settings with default values + session = Mock() + proxy_type = 2 + + fake_dlmgr.set_proxy_settings(session, proxy_type) + settings, = session.method_calls[-1].args + assert 'proxy_port' not in settings + assert 'proxy_hostname' not in settings + assert settings['proxy_type'] == proxy_type + + +def test_set_proxy_corner_case(fake_dlmgr): + # Test setting the proxy settings with None values + session = Mock() + + fake_dlmgr.set_proxy_settings(session, 2, (None, None)) + settings, = session.method_calls[-1].args + assert settings['proxy_type'] == 2 + assert 'proxy_port' not in settings + + fake_dlmgr.set_proxy_settings(session, 3, (None,)) + settings, = session.method_calls[-1].args + assert settings['proxy_type'] == 3 + assert 'proxy_port' not in settings + + fake_dlmgr.set_proxy_settings(session, 3, (None, 123)) + settings, = session.method_calls[-1].args + assert settings['proxy_type'] == 3 + assert 'proxy_port' not in settings + + fake_dlmgr.set_proxy_settings(session, 3, (None, 123), ('name', None)) + settings, = session.method_calls[-1].args + assert settings['proxy_type'] == 3 + assert 'proxy_port' not in settings + assert 'proxy_username' not in settings