Remove max_length parameter from fetch

If a file has a bigger size than expected, RequestFetcher.fetch
downloads it up to max_length without any errors. Only the
consecutive hash check raises exception. A better behaviour
in such case would be to raise a DownloadLengthMismatchError.

For this reason this commit:
- removes max_length from the fetch() definition,
- modifies download_file to check the downloaded length after
  each chunk and raise an error accordingly.

Signed-off-by: Teodora Sechkova <[email protected]>

Author: sechkova

tuf/ngclient/_internal/requests_fetcher.py

@@ -53,15 +53,11 @@ def __init__(self):
         self.chunk_size: int = 400000  # bytes
         self.sleep_before_round: Optional[int] = None
 
-    def fetch(self, url: str, max_length: int) -> Iterator[bytes]:
-        """Fetches the contents of HTTP/HTTPS url from a remote server.
-
-        Ensures the length of the downloaded data is up to 'max_length'.
+    def fetch(self, url: str) -> Iterator[bytes]:
+        """Fetches the contents of HTTP/HTTPS url from a remote server
 
         Arguments:
             url: A URL string that represents a file location.
-            max_length: An integer value representing the maximum
-                number of bytes to be downloaded.
 
         Raises:
             exceptions.SlowRetrievalError: A timeout occurs while receiving
@@ -90,17 +86,14 @@ def fetch(self, url: str, max_length: int) -> Iterator[bytes]:
             status = e.response.status_code
             raise exceptions.FetcherHTTPError(str(e), status)
 
-        return self._chunks(response, max_length)
+        return self._chunks(response)
 
-    def _chunks(
-        self, response: "requests.Response", max_length: int
-    ) -> Iterator[bytes]:
+    def _chunks(self, response: "requests.Response") -> Iterator[bytes]:
         """A generator function to be returned by fetch. This way the
         caller of fetch can differentiate between connection and actual data
         download."""
 
         try:
-            bytes_received = 0
             while True:
                 # We download a fixed chunk of data in every round. This is
                 # so that we can defend against slow retrieval attacks.
@@ -111,35 +104,19 @@ def _chunks(
                 if self.sleep_before_round:
                     time.sleep(self.sleep_before_round)
 
-                read_amount = min(
-                    self.chunk_size,
-                    max_length - bytes_received,
-                )
-
                 # NOTE: This may not handle some servers adding a
                 # Content-Encoding header, which may cause urllib3 to
                 #  misbehave:
                 # https://github.com/pypa/pip/blob/404838abcca467648180b358598c597b74d568c9/src/pip/_internal/download.py#L547-L582
-                data = response.raw.read(read_amount)
-                bytes_received += len(data)
+                data = response.raw.read(self.chunk_size)
 
-                # We might have no more data to read. Check number of bytes
-                # downloaded.
+                # We might have no more data to read, we signal
+                # that the download is complete.
                 if not data:
-                    # Finally, we signal that the download is complete.
                     break
 
                 yield data
 
-                if bytes_received >= max_length:
-                    break
-
-            logger.debug(
-                "Downloaded %d out of %d bytes",
-                bytes_received,
-                max_length,
-            )
-
         except urllib3.exceptions.ReadTimeoutError as e:
             raise exceptions.SlowRetrievalError(str(e))
 

tuf/ngclient/fetcher.py

@@ -29,15 +29,11 @@ class FetcherInterface:
     __metaclass__ = abc.ABCMeta
 
     @abc.abstractmethod
-    def fetch(self, url: str, max_length: int) -> Iterator[bytes]:
+    def fetch(self, url: str) -> Iterator[bytes]:
         """Fetches the contents of HTTP/HTTPS url from a remote server.
 
-        Ensures the length of the downloaded data is up to 'max_length'.
-
         Arguments:
             url: A URL string that represents a file location.
-            max_length: An integer value representing the maximum
-                number of bytes to be downloaded.
 
         Raises:
             tuf.exceptions.SlowRetrievalError: A timeout occurs while receiving
@@ -77,14 +73,22 @@ def download_file(self, url: str, max_length: int) -> Iterator[IO]:
         number_of_bytes_received = 0
 
         with tempfile.TemporaryFile() as temp_file:
-            chunks = self.fetch(url, max_length)
+            chunks = self.fetch(url)
             for chunk in chunks:
-                temp_file.write(chunk)
                 number_of_bytes_received += len(chunk)
-            if number_of_bytes_received > max_length:
-                raise exceptions.DownloadLengthMismatchError(
-                    max_length, number_of_bytes_received
-                )
+                if number_of_bytes_received > max_length:
+                    raise exceptions.DownloadLengthMismatchError(
+                        max_length, number_of_bytes_received
+                    )
+
+                temp_file.write(chunk)
+
+            logger.debug(
+                "Downloaded %d out of %d bytes",
+                number_of_bytes_received,
+                max_length,
+            )
+
             temp_file.seek(0)
             yield temp_file