diff --git a/fdroidserver/net.py b/fdroidserver/net.py index cf01395a..49d67f2c 100644 --- a/fdroidserver/net.py +++ b/fdroidserver/net.py @@ -29,12 +29,20 @@ HEADERS = {'User-Agent': 'F-Droid'} def download_file(url, local_filename=None, dldir='tmp', retries=3, backoff_factor=0.1): + """Try hard to download the file, including retrying on failures. + + This has two retry cycles, one inside of the requests session, the + other provided by this function. The requests retry logic applies + to failed DNS lookups, socket connections and connection timeouts, + never to requests where data has made it to the server. This + handles ChunkedEncodingError during transfer in its own retry + loop. This can result in more retries than are specified in the + retries parameter. + + """ filename = urllib.parse.urlparse(url).path.split('/')[-1] if local_filename is None: local_filename = os.path.join(dldir, filename) - # Retry applies to failed DNS lookups, socket connections and connection - # timeouts, never to requests where data has made it to the server; so we - # handle ChunkedEncodingError during transfer ourselves. for i in range(retries + 1): if retries: max_retries = Retry(total=retries - i, backoff_factor=backoff_factor) diff --git a/tests/net.TestCase b/tests/net.TestCase index 48844a40..68c1c935 100755 --- a/tests/net.TestCase +++ b/tests/net.TestCase @@ -4,8 +4,13 @@ import inspect import logging import optparse import os +import random +import requests +import socket import sys import tempfile +import threading +import time import unittest from unittest.mock import MagicMock, patch @@ -20,6 +25,57 @@ from fdroidserver import common, net from pathlib import Path +class RetryServer: + """A stupid simple HTTP server that can fail to connect""" + + def __init__(self, port=None, failures=3): + self.port = port + if self.port is None: + self.port = random.randint(1024, 65535) + self.failures = failures + self.stop_event = threading.Event() + threading.Thread(target=self.run_fake_server).start() + + def stop(self): + self.stop_event.set() + + def run_fake_server(self): + server_sock = socket.socket() + server_sock.setsockopt(socket.SOL_SOCKET, socket.SO_REUSEADDR, 1) + server_sock.bind(('127.0.0.1', self.port)) + server_sock.listen(5) + server_sock.settimeout(5) + time.sleep(0.001) # wait for it to start + + while not self.stop_event.is_set(): + self.failures -= 1 + conn = None + try: + conn, address = server_sock.accept() + conn.settimeout(5) + except TimeoutError: + break + if self.failures > 0: + conn.close() + continue + conn.recv(8192) # request ignored + self.reply = b"""HTTP/1.1 200 OK + Date: Mon, 26 Feb 2024 09:00:14 GMT + Connection: close + Content-Type: text/html + + Hello World! + """ + self.reply = self.reply.replace(b' ', b'') # dedent + conn.sendall(self.reply) + conn.shutdown(socket.SHUT_RDWR) + conn.close() + + self.stop_event.wait(timeout=1) + server_sock.shutdown(socket.SHUT_RDWR) + server_sock.close() + + class NetTest(unittest.TestCase): basedir = Path(__file__).resolve().parent @@ -52,6 +108,20 @@ class NetTest(unittest.TestCase): self.assertTrue(os.path.exists(f)) self.assertEqual('tmp/com.downloader.aegis-3175421.apk', f) + def test_download_file_retries(self): + server = RetryServer() + f = net.download_file('http://localhost:%d/f.txt' % server.port) + # strip the HTTP headers and compare the reply + self.assertEqual(server.reply.split(b'\n\n')[1], Path(f).read_bytes()) + server.stop() + + def test_download_file_retries_not_forever(self): + """The retry logic should eventually exit with an error.""" + server = RetryServer(failures=5) + with self.assertRaises(requests.exceptions.ConnectionError): + net.download_file('http://localhost:%d/f.txt' % server.port) + server.stop() + if __name__ == "__main__": os.chdir(os.path.dirname(__file__))