ngclient: Prevent automatic decoding of gzip files

tests/simple_server.py

@@ -6,13 +6,14 @@
 """Simple HTTP server for python-tuf tests"""
 
 import socketserver
-from http.server import SimpleHTTPRequestHandler
+
+from tests.utils import CustomHTTPRequestHandler
 
 # Allow re-use so you can re-run tests as often as you want even if the
 # tests re-use ports. Otherwise TCP TIME-WAIT prevents reuse for ~1 minute
 socketserver.TCPServer.allow_reuse_address = True
 
-httpd = socketserver.TCPServer(("localhost", 0), SimpleHTTPRequestHandler)
+httpd = socketserver.TCPServer(("localhost", 0), CustomHTTPRequestHandler)
 port_message = "bind succeeded, server port is: " + str(httpd.server_address[1])
 print(port_message)
 httpd.serve_forever()

tests/test_fetcher_ng.py

@@ -7,16 +7,19 @@
 """
 
 import io
+import json
 import logging
 import math
 import os
 import sys
 import tempfile
 import unittest
+from functools import partialmethod
 from typing import Any, ClassVar, Iterator
 from unittest.mock import Mock, patch
 
 import requests
+from urllib3.exceptions import ReadTimeoutError
 
 from tests import utils
 from tuf.api import exceptions
@@ -94,6 +97,40 @@ def test_fetch_in_chunks(self) -> None:
  # Check that we calculate chunks as expected
  self.assertEqual(chunks_count, expected_chunks_count)
 
+ # Fetch data with Content-Encoding gzip (or deflate)
+ def test_fetch_content_encoding(self) -> None:
+ """
+ Regression test for issue #2047
+
+ By default, requests/urllib3 will automatically try to decode data
+ that is served with a "Content-Encoding: gzip" header (or "deflate").
+ As the length of the decoded data typically would be different from
+ the expected number of bytes specified in the TUF targets metadata,
+ this would give rise to a DownloadLengthMismatchError later on. Thus,
+ we need to ensure that a file served with the "Content-Encoding"
+ header is *not* decoded by the RequestsFetcher.
+ """
+ # Serve dummy file with "Content-Encoding: gzip" header
+ content_encoding_header = json.dumps({"Content-Encoding": "gzip"})
+ headers = {utils.DESIRED_RESPONSE_HEADERS: content_encoding_header}
+ get_with_headers = partialmethod(requests.Session.get, headers=headers)
+ target = "tuf.ngclient._internal.requests_fetcher.requests.Session.get"
+ # The test file content does not represent a real gzip file,
+ # so we can expect an error to be raised if requests/urllib3 tries to
+ # decode the file (urllib3 uses zlib for this).
+ try:
+ with tempfile.TemporaryFile() as temp_file:
+ with patch(target, get_with_headers):
+ for chunk in self.fetcher.fetch(self.url):
+ temp_file.write(chunk)
+ temp_file.seek(0)
+ fetched_data = temp_file.read()
+ except requests.exceptions.ContentDecodingError as e:
+ self.fail(f"fetch() raised unexpected decoding error: {e}")
+ # If all is well, decoding has *not* been attempted, and the fetched
+ # data matches the original file contents
+ self.assertEqual(self.file_contents, fetched_data)
+
  # Incorrect URL parsing
  def test_url_parsing(self) -> None:
  with self.assertRaises(exceptions.DownloadError):
@@ -109,18 +146,18 @@ def test_http_error(self) -> None:
  # Response read timeout error
  @patch.object(requests.Session, "get")
  def test_response_read_timeout(self, mock_session_get: Any) -> None:
- mock_response = Mock()
+ mock_response = Mock(raw=Mock())
  attr = {
- "iter_content.side_effect": requests.exceptions.ConnectionError(
- "Simulated timeout"
+ "read.side_effect": ReadTimeoutError(
+None, None, "Simulated timeout"
  )
  }
- mock_response.configure_mock(**attr)
+ mock_response.raw.configure_mock(**attr)
  mock_session_get.return_value = mock_response
 
  with self.assertRaises(exceptions.SlowRetrievalError):
  next(self.fetcher.fetch(self.url))
- mock_response.iter_content.assert_called_once()
+ mock_response.raw.read.assert_called_once()
 
  # Read/connect session timeout error
  @patch.object(

tests/test_utils.py

@@ -20,10 +20,14 @@
  Provide tests for some of the functions in utils.py module.
 """
 
+import io
+import json
 import logging
 import socket
 import sys
 import unittest
+from typing import List
+from unittest import mock
 
 from tests import utils
 
@@ -76,6 +80,44 @@ def test_server_exit_before_timeout(self) -> None:
  utils.TestServerProcess(logger, server="fast_server_exit.py")
 
 
+class CustomHTTPRequestHandlerTests(unittest.TestCase):
+ """Test functionality provided by CustomHTTPRequestHandler"""
+
+ def setUp(self) -> None:
+ # Based on cpython tests SocketlessRequestHandler:
+ # https://github.com/python/cpython/blob/main/Lib/test/test_httpservers.py#L921
+ request = mock.Mock()
+ request.makefile.return_value = io.BytesIO()
+ self.handler = utils.CustomHTTPRequestHandler(
+ request, None, None, directory=None # type: ignore
+ )
+ self.handler.get_called = False
+ self.handler.protocol_version = "HTTP/1.1"
+ self.handler.client_address = ("localhost", 0)
+
+ def send_request(self, message: bytes) -> List[bytes]:
+ # Based on cpython tests BaseHTTPRequestHandlerTestCase:
+ # https://github.com/python/cpython/blob/main/Lib/test/test_httpservers.py#L973
+ self.handler.rfile = io.BytesIO(message)
+ self.handler.wfile = io.BytesIO()
+ self.handler.handle_one_request()
+ self.handler.wfile.seek(0)
+ return self.handler.wfile.readlines()
+
+ def test_custom_response_headers(self) -> None:
+ header_name = "Some-Header"
+ header_value = "some value"
+ req_header = utils.DESIRED_RESPONSE_HEADERS
+ resp_headers = json.dumps({header_name: header_value})
+ raw_header = f"{req_header}: {resp_headers}\r\n".encode("utf-8")
+ raw_request = b"GET / HTTP/1.1\r\n"
+ raw_request += raw_header
+ raw_request += b"\r\n"
+ raw_response = b"".join(self.send_request(message=raw_request))
+ self.assertIn(header_name.encode("utf-8"), raw_response)
+ self.assertIn(header_value.encode("utf-8"), raw_response)
+
+
 if __name__ == "__main__":
  utils.configure_test_logging(sys.argv)
  unittest.main()

tests/utils.py

@@ -22,6 +22,7 @@
 
 import argparse
 import errno
+import json
 import logging
 import os
 import queue
@@ -33,6 +34,7 @@
 import unittest
 import warnings
 from contextlib import contextmanager
+from http.server import SimpleHTTPRequestHandler
 from typing import IO, Any, Callable, Dict, Iterator, List, Optional
 
 logger = logging.getLogger(__name__)
@@ -46,6 +48,12 @@
 # DataSet is only here so type hints can be used.
 DataSet = Dict[str, Any]
 
+# If the test-server receives an HTTP-request with this custom header,
+# it will add the specified headers to the HTTP-response. The corresponding
+# value must be a JSON object with header name/value pairs.
+DESIRED_RESPONSE_HEADERS = "Desired-Response-Headers"
+
+
 # Test runner decorator: Runs the test as a set of N SubTests,
 # (where N is number of items in dataset), feeding the actual test
 # function one test case at a time
@@ -364,3 +372,15 @@ def is_process_running(self) -> bool:
  assert isinstance(self.__server_process, subprocess.Popen)
  # pylint: disable=simplifiable-if-expression
  return True if self.__server_process.poll() is None else False
+
+
+class CustomHTTPRequestHandler(SimpleHTTPRequestHandler):
+ def end_headers(self) -> None:
+ # If DESIRED_RESPONSE_HEADERS is found in the request headers, add the
+ # specified headers to the response.
+ desired_headers = self.headers.get(DESIRED_RESPONSE_HEADERS)
+ if desired_headers:
+ headers = json.loads(desired_headers)
+ for key, value in headers.items():
+ self.send_header(keyword=key, value=value)
+ super().end_headers()

tuf/ngclient/_internal/requests_fetcher.py

@@ -11,6 +11,7 @@
 
 # Imports
 import requests
+from urllib3.exceptions import ReadTimeoutError
 
 import tuf
 from tuf.api import exceptions
@@ -19,6 +20,7 @@
 # Globals
 logger = logging.getLogger(__name__)
 
+
 # Classes
 class RequestsFetcher(FetcherInterface):
  """An implementation of ``FetcherInterface`` based on the requests library.
@@ -96,12 +98,16 @@ def _chunks(self, response: "requests.Response") -> Iterator[bytes]:
  download."""
 
  try:
- for data in response.iter_content(self.chunk_size):
+ while True:
+ # Requests already calls `urlopen(..., decode_content=False)`,
+ # but we explicitly disable decode_content here, to be safe.
+ data = response.raw.read(
+ amt=self.chunk_size, decode_content=False
+ )
+ if not data:
+ break
  yield data
- except (
- requests.exceptions.ConnectionError,
- requests.exceptions.Timeout,
- ) as e:
+ except ReadTimeoutError as e:
  raise exceptions.SlowRetrievalError from e
 
  finally: