set realistic timeouts, scale file/message download timeouts using file size metadata

securedrop_client/api_jobs/downloads.py

@@ -1,6 +1,7 @@
 import binascii
 import hashlib
 import logging
+import math
 import os
 import shutil
 
@@ -40,6 +41,24 @@ def __init__(self, data_dir: str) -> None:
         super().__init__()
         self.data_dir = data_dir
 
+    def _get_realistic_timeout(self, size_in_bytes: int) -> int:
+        """
+        Returns a realistic timeout in seconds for a file, message, or reply based on its size.
+
+        Note that:
+        * the size of the file provided by server is in bytes (the server computes it using
+        os.stat.ST_SIZE)
+        * A reasonable time to download a 1 MB file download time according to OnionPerf was
+        ~7.5s:
+        https://metrics.torproject.org/torperf.html?start=2019-05-02&end=2019-07-31&server=onion&filesize=1mb
+
+        This simply scales the timeouts per file, so that a small file times out much faster than a
+        large file, which is given a long time to complete.
+        """
+
+        SCALING_FACTOR_FILE_DL_TIME_SECS_PER_MB = 7.5
+        return math.ceil(size_in_bytes * SCALING_FACTOR_FILE_DL_TIME_SECS_PER_MB)
+
     def call_download_api(self, api: API,
                           db_object: Union[File, Message, Reply]) -> Tuple[str, str]:
         '''
@@ -228,7 +247,8 @@ def call_download_api(self, api: API, db_object: Message) -> Tuple[str, str]:
         sdk_object = SdkSubmission(uuid=db_object.uuid)
         sdk_object.source_uuid = db_object.source.uuid
         sdk_object.filename = db_object.filename
-        return api.download_submission(sdk_object)
+        return api.download_submission(sdk_object,
+                                       timeout=self._get_realistic_timeout(db_object.size))
 
     def call_decrypt(self, filepath: str, session: Session = None) -> str:
         '''
@@ -274,7 +294,8 @@ def call_download_api(self, api: API, db_object: File) -> Tuple[str, str]:
         sdk_object = SdkSubmission(uuid=db_object.uuid)
         sdk_object.source_uuid = db_object.source.uuid
         sdk_object.filename = db_object.filename
-        return api.download_submission(sdk_object)
+        return api.download_submission(sdk_object,
+                                       timeout=self._get_realistic_timeout(db_object.size))
 
     def call_decrypt(self, filepath: str, session: Session = None) -> str:
         '''

securedrop_client/queue.py

@@ -112,6 +112,11 @@ def logout(self) -> None:
 
     def login(self, api_client: API) -> None:
         logger.debug('Passing API token to queues')
+
+        # Setting realistic (shorter) timeout for general requests so that user feedback
+        # is faster
+        api_client.default_request_timeout = 5
+
         self.main_queue.api_client = api_client
         self.download_file_queue.api_client = api_client
         self.start_queues()

tests/api_jobs/test_downloads.py

@@ -318,7 +318,7 @@ def test_FileDownloadJob_happy_path_no_etag(mocker, homedir, session, session_ma
     gpg = GpgHelper(homedir, session_maker, is_qubes=False)
     mock_decrypt = patch_decrypt(mocker, homedir, gpg, file_.filename)
 
-    def fake_download(sdk_obj: SdkSubmission) -> Tuple[str, str]:
+    def fake_download(sdk_obj: SdkSubmission, timeout: int) -> Tuple[str, str]:
         '''
         :return: (etag, path_to_dl)
         '''
@@ -357,7 +357,7 @@ def test_FileDownloadJob_happy_path_sha256_etag(mocker, homedir, session, sessio
     gpg = GpgHelper(homedir, session_maker, is_qubes=False)
     mock_decrypt = patch_decrypt(mocker, homedir, gpg, file_.filename)
 
-    def fake_download(sdk_obj: SdkSubmission) -> Tuple[str, str]:
+    def fake_download(sdk_obj: SdkSubmission, timeout: int) -> Tuple[str, str]:
         '''
         :return: (etag, path_to_dl)
         '''
@@ -393,7 +393,7 @@ def test_FileDownloadJob_bad_sha256_etag(mocker, homedir, session, session_maker
 
     gpg = GpgHelper(homedir, session_maker, is_qubes=False)
 
-    def fake_download(sdk_obj: SdkSubmission) -> Tuple[str, str]:
+    def fake_download(sdk_obj: SdkSubmission, timeout: int) -> Tuple[str, str]:
         '''
         :return: (etag, path_to_dl)
         '''
@@ -426,7 +426,7 @@ def test_FileDownloadJob_happy_path_unknown_etag(mocker, homedir, session, sessi
 
     gpg = GpgHelper(homedir, session_maker, is_qubes=False)
 
-    def fake_download(sdk_obj: SdkSubmission) -> Tuple[str, str]:
+    def fake_download(sdk_obj: SdkSubmission, timeout: int) -> Tuple[str, str]:
         '''
         :return: (etag, path_to_dl)
         '''
@@ -467,7 +467,7 @@ def test_FileDownloadJob_decryption_error(mocker, homedir, session, session_make
     gpg = GpgHelper(homedir, session_maker, is_qubes=False)
     mock_decrypt = mocker.patch.object(gpg, 'decrypt_submission_or_reply', side_effect=CryptoError)
 
-    def fake_download(sdk_obj: SdkSubmission) -> Tuple[str, str]:
+    def fake_download(sdk_obj: SdkSubmission, timeout: int) -> Tuple[str, str]:
         '''
         :return: (etag, path_to_dl)
         '''
@@ -498,3 +498,34 @@ def fake_download(sdk_obj: SdkSubmission) -> Tuple[str, str]:
 
     # ensure mocks aren't stale
     assert mock_decrypt.called
+
+
+def test_timeout_length_of_file_downloads(mocker, homedir, session, session_maker):
+    """
+    Ensure that files downloads have timeouts scaled by the size of the file.
+    """
+    source = factory.Source()
+    small_file = factory.File(source=source, is_downloaded=None, is_decrypted=None, size=1)
+    large_file = factory.File(source=source, is_downloaded=None, is_decrypted=None, size=100)
+    session.add(source)
+    session.add(small_file)
+    session.add(large_file)
+    session.commit()
+
+    gpg = GpgHelper(homedir, session_maker, is_qubes=False)
+
+    small_job = FileDownloadJob(
+        small_file.uuid,
+        os.path.join(homedir, 'data'),
+        gpg,
+    )
+    large_job = FileDownloadJob(
+        small_file.uuid,
+        os.path.join(homedir, 'data'),
+        gpg,
+    )
+
+    small_file_timeout = small_job._get_realistic_timeout(small_file.size)
+    large_file_timeout = large_job._get_realistic_timeout(large_file.size)
+
+    assert small_file_timeout < large_file_timeout