From d412fafd03f037bbd91b36a48ce41a6d52a5f6d6 Mon Sep 17 00:00:00 2001 From: drew2a Date: Fri, 21 Jan 2022 14:28:28 +0100 Subject: [PATCH] Fix downloads endpoint --- .../tribler_common/rest_constants.py | 3 + .../download_manager/download_manager.py | 12 +-- .../libtorrent/restapi/downloads_endpoint.py | 16 ++-- .../tests/test_torrentinfo_endpoint.py | 74 +++++++++++-------- .../restapi/torrentinfo_endpoint.py | 19 ++--- .../libtorrent/tests/test_download_api.py | 18 ++++- .../dialogs/startdownloaddialog.py | 18 ++--- 7 files changed, 89 insertions(+), 71 deletions(-) create mode 100644 src/tribler-common/tribler_common/rest_constants.py diff --git a/src/tribler-common/tribler_common/rest_constants.py b/src/tribler-common/tribler_common/rest_constants.py new file mode 100644 index 00000000000..7363685b245 --- /dev/null +++ b/src/tribler-common/tribler_common/rest_constants.py @@ -0,0 +1,3 @@ +MAGNET_PREFIX = 'magnet' +HTTP_PREFIX = 'http' +FILE_PREFIX = 'file' \ No newline at end of file diff --git a/src/tribler-core/tribler_core/components/libtorrent/download_manager/download_manager.py b/src/tribler-core/tribler_core/components/libtorrent/download_manager/download_manager.py index 7b8a15b4dbe..42eb6bdb25b 100644 --- a/src/tribler-core/tribler_core/components/libtorrent/download_manager/download_manager.py +++ b/src/tribler-core/tribler_core/components/libtorrent/download_manager/download_manager.py @@ -16,8 +16,8 @@ from ipv8.taskmanager import TaskManager, task from tribler_common.network_utils import default_network_utils +from tribler_common.rest_constants import FILE_PREFIX, HTTP_PREFIX, MAGNET_PREFIX from tribler_common.simpledefs import DLSTATUS_SEEDING, MAX_LIBTORRENT_RATE_LIMIT, NTFY, STATEDIR_CHECKPOINT_DIR -from tribler_common.utilities import uri_to_path from tribler_core.components.libtorrent.download_manager.dht_health_manager import DHTHealthManager from tribler_core.components.libtorrent.download_manager.download import Download @@ -505,10 +505,10 @@ def _map_call_on_ltsessions(self, hops, funcname, *args, **kwargs): getattr(self.get_session(hops), funcname)(*args, **kwargs) async def start_download_from_uri(self, uri, config=None): - if uri.startswith("http"): + if uri.startswith(HTTP_PREFIX): tdef = await TorrentDef.load_from_url(uri) return self.start_download(tdef=tdef, config=config) - if uri.startswith("magnet:"): + if uri.startswith(MAGNET_PREFIX): name, infohash, _ = parse_magnetlink(uri) if infohash is None: raise RuntimeError("Missing infohash") @@ -517,9 +517,9 @@ async def start_download_from_uri(self, uri, config=None): else: tdef = TorrentDefNoMetainfo(infohash, "Unknown name" if name is None else name, url=uri) return self.start_download(tdef=tdef, config=config) - if uri.startswith("file:"): - argument = uri_to_path(uri) - return self.start_download(torrent_file=argument, config=config) + if uri.startswith(FILE_PREFIX): + file = uri[len(FILE_PREFIX) + 1:] + return self.start_download(torrent_file=file, config=config) raise Exception("invalid uri") def start_download(self, torrent_file=None, tdef=None, config=None, checkpoint_disabled=False, hidden=False): diff --git a/src/tribler-core/tribler_core/components/libtorrent/restapi/downloads_endpoint.py b/src/tribler-core/tribler_core/components/libtorrent/restapi/downloads_endpoint.py index eebadf063ab..2bbc7ee284d 100644 --- a/src/tribler-core/tribler_core/components/libtorrent/restapi/downloads_endpoint.py +++ b/src/tribler-core/tribler_core/components/libtorrent/restapi/downloads_endpoint.py @@ -233,9 +233,10 @@ def get_files_info_json(download): "and should only be used in situations where this data is required. " ) async def get_downloads(self, request): - get_peers = request.query.get('get_peers', '0') == '1' - get_pieces = request.query.get('get_pieces', '0') == '1' - get_files = request.query.get('get_files', '0') == '1' + params = request.query + get_peers = params.get('get_peers', '0') == '1' + get_pieces = params.get('get_pieces', '0') == '1' + get_files = params.get('get_files', '0') == '1' downloads_json = [] downloads = self.download_manager.get_downloads() @@ -373,16 +374,17 @@ async def get_downloads(self, request): 'location, a magnet link or a HTTP(S) url.'), })) async def add_download(self, request): - parameters = await request.json() - if not parameters.get('uri'): + params = await request.json() + uri = params.get('uri') + if not uri: return RESTResponse({"error": "uri parameter missing"}, status=HTTP_BAD_REQUEST) - download_config, error = DownloadsEndpoint.create_dconfig_from_params(parameters) + download_config, error = DownloadsEndpoint.create_dconfig_from_params(params) if error: return RESTResponse({"error": error}, status=HTTP_BAD_REQUEST) try: - download = await self.download_manager.start_download_from_uri(parameters['uri'], config=download_config) + download = await self.download_manager.start_download_from_uri(uri, config=download_config) except Exception as e: return RESTResponse({"error": str(e)}, status=HTTP_INTERNAL_SERVER_ERROR) diff --git a/src/tribler-core/tribler_core/components/libtorrent/restapi/tests/test_torrentinfo_endpoint.py b/src/tribler-core/tribler_core/components/libtorrent/restapi/tests/test_torrentinfo_endpoint.py index 6ebae3946f8..9c253a8275b 100644 --- a/src/tribler-core/tribler_core/components/libtorrent/restapi/tests/test_torrentinfo_endpoint.py +++ b/src/tribler-core/tribler_core/components/libtorrent/restapi/tests/test_torrentinfo_endpoint.py @@ -10,9 +10,11 @@ import pytest +from tribler_common.rest_constants import FILE_PREFIX from tribler_common.simpledefs import NTFY from tribler_core.components.libtorrent.restapi.torrentinfo_endpoint import TorrentInfoEndpoint +from tribler_core.components.libtorrent.settings import LibtorrentSettings from tribler_core.components.libtorrent.torrentdef import TorrentDef from tribler_core.components.metadata_store.db.orm_bindings.torrent_metadata import tdef_to_metadata_dict from tribler_core.components.restapi.rest.base_api_test import do_request @@ -24,8 +26,26 @@ @pytest.fixture -def endpoint(mock_dlmgr): - return TorrentInfoEndpoint(mock_dlmgr) +def download_manager(state_dir): + dlmgr = Mock() + dlmgr.config = LibtorrentSettings() + dlmgr.shutdown = lambda: succeed(None) + checkpoints_dir = state_dir / 'dlcheckpoints' + checkpoints_dir.mkdir() + dlmgr.get_checkpoint_dir = lambda: checkpoints_dir + dlmgr.state_dir = state_dir + dlmgr.get_downloads = lambda: [] + dlmgr.downloads = {} + dlmgr.metainfo_requests = {} + dlmgr.get_channel_downloads = lambda: [] + dlmgr.shutdown = lambda: succeed(None) + dlmgr.notifier = Mock() + return dlmgr + + +@pytest.fixture +def endpoint(download_manager): + return TorrentInfoEndpoint(download_manager) @pytest.fixture @@ -35,15 +55,23 @@ def rest_api(loop, aiohttp_client, endpoint): # pylint: disable=unused-argument return loop.run_until_complete(aiohttp_client(app)) -async def test_get_torrentinfo(mock_dlmgr, tmp_path, rest_api, endpoint): +async def test_get_torrentinfo_escaped_characters(tmp_path, rest_api): + # test for the bug fix: https://github.com/Tribler/tribler/issues/6700 + source = TORRENT_UBUNTU_FILE + destination = tmp_path / 'ubuntu%20%21 15.04.torrent' + shutil.copyfile(source, destination) + + response = await do_request(rest_api, url='torrentinfo', params={'uri': f'{FILE_PREFIX}:{destination}'}, expected_code=200) + assert 'metainfo' in response + + +async def test_get_torrentinfo(tmp_path, rest_api, endpoint: TorrentInfoEndpoint): """ Testing whether the API returns a correct dictionary with torrent info. """ def _path(file): - return f'file:{TESTS_DATA_DIR / file}' - - endpoint.download_manager = mock_dlmgr + return f'{FILE_PREFIX}:{TESTS_DATA_DIR / file}' shutil.copyfile(TORRENT_UBUNTU_FILE, tmp_path / 'ubuntu.torrent') @@ -51,12 +79,6 @@ def verify_valid_dict(json_data): metainfo_dict = json.loads(unhexlify(json_data['metainfo'])) assert 'info' in metainfo_dict - mock_dlmgr.downloads = {} - mock_dlmgr.metainfo_requests = {} - mock_dlmgr.get_channel_downloads = lambda: [] - mock_dlmgr.shutdown = lambda: succeed(None) - mock_dlmgr.notifier = Mock() - url = 'torrentinfo' await do_request(rest_api, url, expected_code=400) await do_request(rest_api, url, params={'uri': 'def'}, expected_code=400) @@ -93,20 +115,20 @@ def get_metainfo(infohash, timeout=20, hops=None, url=None): assert url == unquote_plus(path) return succeed(tdef.get_metainfo()) - mock_dlmgr.get_metainfo = get_metainfo + endpoint.download_manager.get_metainfo = get_metainfo verify_valid_dict(await do_request(rest_api, f'torrentinfo?uri={path}', expected_code=200)) path = 'magnet:?xt=urn:ed2k:354B15E68FB8F36D7CD88FF94116CDC1' # No infohash await do_request(rest_api, f'torrentinfo?uri={path}', expected_code=400) path = quote_plus(f"magnet:?xt=urn:btih:{'a' * 40}&dn=test torrent") - mock_dlmgr.get_metainfo = lambda *_, **__: succeed(None) + endpoint.download_manager.get_metainfo = lambda *_, **__: succeed(None) await do_request(rest_api, f'torrentinfo?uri={path}', expected_code=500) # Ensure that correct torrent metadata was sent through notifier (to MetadataStore) - mock_dlmgr.notifier.notify.assert_called_with(NTFY.TORRENT_METADATA_ADDED.value, metainfo_dict) + endpoint.download_manager.notifier.notify.assert_called_with(NTFY.TORRENT_METADATA_ADDED.value, metainfo_dict) - mock_dlmgr.get_metainfo = get_metainfo + endpoint.download_manager.get_metainfo = get_metainfo verify_valid_dict(await do_request(rest_api, f'torrentinfo?uri={path}', expected_code=200)) await do_request(rest_api, f'torrentinfo?uri={path}&hops=0', expected_code=200) @@ -119,37 +141,29 @@ def get_metainfo(infohash, timeout=20, hops=None, url=None): mock_download = Mock() path = quote_plus(f'magnet:?xt=urn:btih:{hexlify(UBUNTU_1504_INFOHASH)}&dn=test torrent') - mock_dlmgr.downloads = {UBUNTU_1504_INFOHASH: mock_download} + endpoint.download_manager.downloads = {UBUNTU_1504_INFOHASH: mock_download} result = await do_request(rest_api, f'torrentinfo?uri={path}', expected_code=200) assert result["download_exists"] # Check that we do not return "downloads_exists" if the download is metainfo only download - mock_dlmgr.downloads = {UBUNTU_1504_INFOHASH: mock_download} - mock_dlmgr.metainfo_requests = {UBUNTU_1504_INFOHASH: [mock_download]} + endpoint.download_manager.downloads = {UBUNTU_1504_INFOHASH: mock_download} + endpoint.download_manager.metainfo_requests = {UBUNTU_1504_INFOHASH: [mock_download]} result = await do_request(rest_api, f'torrentinfo?uri={path}', expected_code=200) assert not result["download_exists"] # Check that we return "downloads_exists" if there is a metainfo download for the infohash, # but there is also a regular download for the same infohash - mock_dlmgr.downloads = {UBUNTU_1504_INFOHASH: mock_download} - mock_dlmgr.metainfo_requests = {UBUNTU_1504_INFOHASH: [Mock()]} + endpoint.download_manager.downloads = {UBUNTU_1504_INFOHASH: mock_download} + endpoint.download_manager.metainfo_requests = {UBUNTU_1504_INFOHASH: [Mock()]} result = await do_request(rest_api, f'torrentinfo?uri={path}', expected_code=200) assert result["download_exists"] -async def test_on_got_invalid_metainfo(mock_dlmgr, rest_api): +async def test_on_got_invalid_metainfo(rest_api): """ Test whether the right operations happen when we receive an invalid metainfo object """ - def get_metainfo(*_, **__): - return succeed("abcd") - - mock_dlmgr.get_metainfo = get_metainfo - mock_dlmgr.shutdown = lambda: succeed(None) - mock_dlmgr.shutdown_downloads = lambda: succeed(None) - mock_dlmgr.checkpoint_downloads = lambda: succeed(None) path = f"magnet:?xt=urn:btih:{hexlify(UBUNTU_1504_INFOHASH)}&dn={quote_plus('test torrent')}" - res = await do_request(rest_api, f'torrentinfo?uri={path}', expected_code=500) assert "error" in res diff --git a/src/tribler-core/tribler_core/components/libtorrent/restapi/torrentinfo_endpoint.py b/src/tribler-core/tribler_core/components/libtorrent/restapi/torrentinfo_endpoint.py index 6bae10eeb4a..6ffaf9be448 100644 --- a/src/tribler-core/tribler_core/components/libtorrent/restapi/torrentinfo_endpoint.py +++ b/src/tribler-core/tribler_core/components/libtorrent/restapi/torrentinfo_endpoint.py @@ -3,15 +3,12 @@ from copy import deepcopy from aiohttp import ClientResponseError, ClientSession, ServerConnectionError, web - from aiohttp_apispec import docs - -from ipv8.REST.schema import schema - from marshmallow.fields import String +from ipv8.REST.schema import schema +from tribler_common.rest_constants import FILE_PREFIX, HTTP_PREFIX, MAGNET_PREFIX from tribler_common.simpledefs import NTFY - from tribler_core.components.libtorrent.download_manager.download_manager import DownloadManager from tribler_core.components.libtorrent.torrentdef import TorrentDef from tribler_core.components.libtorrent.utils.libtorrent_helper import libtorrent as lt @@ -25,10 +22,6 @@ from tribler_core.utilities.unicode import hexlify, recursive_unicode from tribler_core.utilities.utilities import bdecode_compat, froze_it, parse_magnetlink -MAGNET = 'magnet' -HTTP = 'http' -FILE = 'file:' - async def query_http_uri(uri: str) -> bytes: # This is moved to a separate method to be able to patch it separately, @@ -85,15 +78,15 @@ async def get_torrent_info(self, request): return RESTResponse({"error": "uri parameter missing"}, status=HTTP_BAD_REQUEST) metainfo = None - if uri.startswith(FILE): - file = uri[len(FILE):] + if uri.startswith(FILE_PREFIX): + file = uri[len(FILE_PREFIX) + 1:] try: tdef = TorrentDef.load(file) metainfo = tdef.metainfo except (TypeError, RuntimeError): return RESTResponse({"error": f"error while decoding torrent file: {file}"}, status=HTTP_INTERNAL_SERVER_ERROR) - elif uri.startswith(HTTP): + elif uri.startswith(HTTP_PREFIX): try: response = await query_http_uri(uri) except (ServerConnectionError, ClientResponseError) as e: @@ -105,7 +98,7 @@ async def get_torrent_info(self, request): metainfo = await self.download_manager.get_metainfo(infohash, timeout=60, hops=hops, url=response) else: metainfo = bdecode_compat(response) - elif uri.startswith(MAGNET): + elif uri.startswith(MAGNET_PREFIX): infohash = parse_magnetlink(uri)[1] if infohash is None: return RESTResponse({"error": "missing infohash"}, status=HTTP_BAD_REQUEST) diff --git a/src/tribler-core/tribler_core/components/libtorrent/tests/test_download_api.py b/src/tribler-core/tribler_core/components/libtorrent/tests/test_download_api.py index 8f223c947eb..9665a26c4ac 100644 --- a/src/tribler-core/tribler_core/components/libtorrent/tests/test_download_api.py +++ b/src/tribler-core/tribler_core/components/libtorrent/tests/test_download_api.py @@ -2,19 +2,29 @@ import pytest +from tribler_common.rest_constants import HTTP_PREFIX from tribler_common.simpledefs import DLSTATUS_DOWNLOADING from tribler_core.tests.tools.common import TORRENT_UBUNTU_FILE +async def download_from_file(file_name, tmp_path, file_server, download_manager): + shutil.copyfile(TORRENT_UBUNTU_FILE, tmp_path / "ubuntu.torrent") + download = await download_manager.start_download_from_uri(f'{HTTP_PREFIX}://localhost:{file_server}/ubuntu.torrent') + await download.wait_for_status(DLSTATUS_DOWNLOADING) + + @pytest.mark.asyncio @pytest.mark.timeout(10) async def test_download_torrent_from_url(tmp_path, file_server, download_manager): + await download_from_file('ubuntu.torrent', tmp_path, file_server, download_manager) - # Setup file server to serve torrent file - shutil.copyfile(TORRENT_UBUNTU_FILE, tmp_path / "ubuntu.torrent") - download = await download_manager.start_download_from_uri(f'http://localhost:{file_server}/ubuntu.torrent') - await download.wait_for_status(DLSTATUS_DOWNLOADING) + +@pytest.mark.asyncio +@pytest.mark.timeout(10) +async def test_download_torrent_from_url_escaped_characters(tmp_path, file_server, download_manager): + # test for the bug fix: https://github.com/Tribler/tribler/issues/6700 + await download_from_file('ubuntu%20%21 15.04.torrent', tmp_path, file_server, download_manager) @pytest.mark.asyncio diff --git a/src/tribler-gui/tribler_gui/dialogs/startdownloaddialog.py b/src/tribler-gui/tribler_gui/dialogs/startdownloaddialog.py index 4fdf319e00d..b703b416386 100644 --- a/src/tribler-gui/tribler_gui/dialogs/startdownloaddialog.py +++ b/src/tribler-gui/tribler_gui/dialogs/startdownloaddialog.py @@ -8,7 +8,7 @@ from PyQt5.QtCore import QTimer, pyqtSignal from PyQt5.QtWidgets import QFileDialog, QSizePolicy -from tribler_common.utilities import uri_to_path +from tribler_common.rest_constants import FILE_PREFIX, MAGNET_PREFIX from tribler_gui.defs import METAINFO_MAX_RETRIES, METAINFO_TIMEOUT from tribler_gui.dialogs.confirmationdialog import ConfirmationDialog @@ -25,13 +25,8 @@ ) from tribler_gui.widgets.torrentfiletreewidget import TORRENT_FILES_TREE_STYLESHEET -MAGNET = 'magnet:' - -FILE = 'file:' - class StartDownloadDialog(DialogContainer): - button_clicked = pyqtSignal(int) received_metainfo = pyqtSignal(dict) @@ -39,9 +34,9 @@ def __init__(self, parent, download_uri): DialogContainer.__init__(self, parent) torrent_name = download_uri - if torrent_name.startswith(FILE): - torrent_name = torrent_name[len(FILE):] - elif torrent_name.startswith(MAGNET): + if torrent_name.startswith(FILE_PREFIX): + torrent_name = torrent_name[len(FILE_PREFIX) + 1 :] + elif torrent_name.startswith(MAGNET_PREFIX): torrent_name = unquote_plus(torrent_name) self.download_uri = download_uri @@ -148,8 +143,9 @@ def perform_files_request(self): params = {'uri': self.download_uri} if direct: params['hops'] = 0 - self.rest_request = TriblerNetworkRequest('torrentinfo', self.on_received_metainfo, capture_core_errors=False, - url_params=params) + self.rest_request = TriblerNetworkRequest( + 'torrentinfo', self.on_received_metainfo, capture_core_errors=False, url_params=params + ) if self.metainfo_retries <= METAINFO_MAX_RETRIES: fetch_mode = tr("directly") if direct else tr("anonymously")