diff --git a/deluge/conftest.py b/deluge/conftest.py index 4266938fa..55c50a42d 100644 --- a/deluge/conftest.py +++ b/deluge/conftest.py @@ -4,8 +4,9 @@ # See LICENSE for more details. # -import unittest.mock +import tempfile import warnings +from unittest.mock import Mock, patch import pytest import pytest_twisted @@ -49,7 +50,7 @@ def mock_callback(): mock.side_effect = lambda *args, **kw: deferred.callback((args, kw)) mock.deferred = deferred - mock = unittest.mock.Mock() + mock = Mock() original_reset_mock = mock.reset_mock mock.reset_mock = reset mock.reset_mock() @@ -181,3 +182,11 @@ class BaseTestCase: have finished. """ + + +@pytest.fixture +def mock_mkstemp(tmp_path): + """Return known tempfile location to verify file deleted""" + tmp_file = tempfile.mkstemp(dir=tmp_path) + with patch('tempfile.mkstemp', return_value=tmp_file): + yield tmp_file diff --git a/deluge/tests/test_tracker_icons.py b/deluge/tests/test_tracker_icons.py index 1645c29c8..2f793d12e 100644 --- a/deluge/tests/test_tracker_icons.py +++ b/deluge/tests/test_tracker_icons.py @@ -3,6 +3,7 @@ # the additional special exception to link portions of this program with the OpenSSL library. # See LICENSE for more details. # +import os.path import pytest import pytest_twisted @@ -28,11 +29,12 @@ class TestTrackerIcons(BaseTestCase): return component.shutdown() @pytest_twisted.ensureDeferred - async def test_get_deluge_png(self): + async def test_get_deluge_png(self, mock_mkstemp): # Deluge has a png favicon link icon = TrackerIcon(common.get_test_data_file('deluge.png')) result = await self.icons.fetch('deluge-torrent.org') assert result == icon + assert not os.path.isfile(mock_mkstemp[1]) @pytest_twisted.ensureDeferred async def test_get_google_ico(self): @@ -68,3 +70,10 @@ class TestTrackerIcons(BaseTestCase): async def test_get_empty_string_tracker(self): result = await self.icons.fetch('') assert result is None + + @pytest_twisted.ensureDeferred + async def test_invalid_host(self, mock_mkstemp): + """Test that TrackerIcon can handle invalid hostname""" + result = await self.icons.fetch('deluge.example.com') + assert not result + assert not os.path.isfile(mock_mkstemp[1]) diff --git a/deluge/ui/tracker_icons.py b/deluge/ui/tracker_icons.py index 7f2cb7a6b..5f619af63 100644 --- a/deluge/ui/tracker_icons.py +++ b/deluge/ui/tracker_icons.py @@ -8,8 +8,8 @@ import logging import os +import tempfile from html.parser import HTMLParser -from tempfile import mkstemp from urllib.parse import urljoin, urlparse from twisted.internet import defer, threads @@ -203,8 +203,10 @@ class TrackerIcons(Component): else: # We need to fetch it self.pending[host] = [] + tmp_file = tempfile.mkstemp(prefix='deluge_trackericon_html.') + filename = tmp_file[1] # Start callback chain - d = self.download_page(host) + d = self.download_page(host, filename) d.addCallbacks( self.on_download_page_complete, self.on_download_page_fail, @@ -213,6 +215,7 @@ class TrackerIcons(Component): d.addCallbacks( self.on_parse_complete, self.on_parse_fail, callbackArgs=(host,) ) + d.addBoth(self.del_tmp_file, tmp_file) d.addCallback(self.download_icon, host) d.addCallbacks( self.on_download_icon_complete, @@ -224,24 +227,38 @@ class TrackerIcons(Component): d.addCallback(self.store_icon, host) return d - def download_page(self, host, url=None): - """ - Downloads a tracker host's page + @staticmethod + def del_tmp_file(result, tmp_file): + """Remove tmp_file created when downloading tracker page""" + fd, filename = tmp_file + try: + os.close(fd) + os.remove(filename) + except OSError: + log.debug(f'Unable to delete temporary file: {filename}') + + return result + + def download_page( + self, host: str, filename: str, url: str = None + ) -> 'defer.Deferred[str]': + """Downloads a tracker host's page + If no url is provided, it bases the url on the host - :param host: the tracker host - :type host: string - :param url: the (optional) url of the host - :type url: string - :returns: the filename of the tracker host's page - :rtype: Deferred + Args: + host: The tracker host + filename: Location to download page + url: The url of the host + + Returns: + The filename of the tracker host's page """ if not url: url = self.host_to_url(host) - log.debug('Downloading %s %s', host, url) - tmp_fd, tmp_file = mkstemp(prefix='deluge_ticon.') - os.close(tmp_fd) - return download_file(url, tmp_file, force_filename=True) + + log.debug(f'Downloading {host} {url} to {filename}') + return download_file(url, filename, force_filename=True) def on_download_page_complete(self, page): """ @@ -291,10 +308,6 @@ class TrackerIcons(Component): if parser.left_head: break parser.close() - try: - os.remove(page) - except OSError as ex: - log.warning('Could not remove temp file: %s', ex) return parser.get_icons()