Skip to content

Commit

Permalink
Refactor proxy settings and add error handling
Browse files Browse the repository at this point in the history
Reworked the 'set_proxy_settings' function in the download manager to handle invalid port values more gracefully. The function now attempts to convert the port value to an integer and logs any ValueError exceptions that occur during this process. This prevents crashes when non-integer values are provided for the port. Also, reorganized import statements according to PEP8 guidelines.

Additionally, a new test case 'test_set_proxy_settings_invalid_port' has been added to verify this behavior. It checks if proxy settings are not set when an invalid port number is provided.
  • Loading branch information
drew2a committed May 23, 2024
1 parent 828bb79 commit ede809a
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -362,17 +362,25 @@ 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, ltsession, ptype, server: Optional[Tuple] = None, auth: Optional[List] = 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 server:
proxy_port = None

try:
proxy_port = int(server[1])
except ValueError as e:
self._logger.exception(e)

if server[0] and proxy_port:
settings["proxy_hostname"] = server[0]
settings["proxy_port"] = proxy_port
if auth:
settings["proxy_username"] = auth[0]
settings["proxy_password"] = auth[1]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import asyncio
import functools
import itertools
from asyncio import Future
from unittest.mock import AsyncMock, MagicMock, Mock
Expand Down Expand Up @@ -575,3 +574,27 @@ 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

0 comments on commit ede809a

Please sign in to comment.