GzipDecompressor: decompress concatenated gzip members instead of dropping them

tornado/httputil.py

@@ -325,6 +325,14 @@ def parse(cls, headers: str, *, _chars_are_bytes: bool = True) -> HTTPHeaders:
     # MutableMapping abstract method implementations.
 
     def __setitem__(self, name: str, value: str) -> None:
+        # `MutableMapping` lets any object reach the indexer; non-string keys
+        # used to leak an `AttributeError` from inside `_normalize_header`
+        # (which is `lru_cache`d and splits on `-`). Reject them up-front with a
+        # clear message; `__contains__` already short-circuits the same way.
+        if not isinstance(name, str):
+            raise TypeError(
+                "HTTPHeaders keys must be str, not %s" % type(name).__name__
+            )
         norm_name = _normalize_header(name)
         self._combined_cache[norm_name] = value
         self._as_list[norm_name] = [value]
@@ -338,12 +346,20 @@ def __contains__(self, name: object) -> bool:
         return norm_name in self._as_list
 
     def __getitem__(self, name: str) -> str:
+        if not isinstance(name, str):
+            raise TypeError(
+                "HTTPHeaders keys must be str, not %s" % type(name).__name__
+            )
         header = _normalize_header(name)
         if header not in self._combined_cache:
             self._combined_cache[header] = ",".join(self._as_list[header])
         return self._combined_cache[header]
 
     def __delitem__(self, name: str) -> None:
+        if not isinstance(name, str):
+            raise TypeError(
+                "HTTPHeaders keys must be str, not %s" % type(name).__name__
+            )
         norm_name = _normalize_header(name)
         del self._combined_cache[norm_name]
         del self._as_list[norm_name]

tornado/test/httputil_test.py

@@ -499,6 +499,48 @@ def test_setdefault(self):
         self.assertEqual(headers["quux"], "xyzzy")
         self.assertEqual(sorted(headers.get_all()), [("Foo", "bar"), ("Quux", "xyzzy")])
 
+    def test_non_string_key_setitem_raises_type_error(self):
+        # HTTPHeaders indexes via _normalize_header, which is decorated with
+        # @lru_cache and only accepts str. Non-string keys used to leak an
+        # AttributeError out of the cache wrapper; now they raise TypeError
+        # at the call site so callers get a useful error.
+        headers = HTTPHeaders()
+        for bad in (1, 1.5, None, b"Foo", ("Foo",), object()):
+            with self.assertRaises(TypeError):
+                headers[bad] = "value"
+            # __setitem__ must not silently store partial state for the bad
+            # key, and must not corrupt the lru_cache for valid lookups.
+            self.assertEqual(len(headers), 0)
+            self.assertNotIn(bad, headers)
+
+    def test_non_string_key_getitem_raises_type_error(self):
+        headers = HTTPHeaders()
+        headers["Foo"] = "bar"
+        for bad in (1, 1.5, None, b"Foo", ("Foo",), object()):
+            with self.assertRaises(TypeError):
+                headers[bad]
+        # Pre-existing string key still reads back.
+        self.assertEqual(headers["Foo"], "bar")
+
+    def test_non_string_key_delitem_raises_type_error(self):
+        headers = HTTPHeaders()
+        headers["Foo"] = "bar"
+        for bad in (1, 1.5, None, b"Foo", ("Foo",), object()):
+            with self.assertRaises(TypeError):
+                del headers[bad]
+        # Pre-existing string entry must still be intact.
+        self.assertEqual(headers["Foo"], "bar")
+
+    def test_non_string_key_contains_returns_false(self):
+        # __contains__ already guarded against non-strings; this pins the
+        # behaviour so a future refactor of __setitem__/__getitem__ cannot
+        # regress it.
+        headers = HTTPHeaders()
+        headers["Foo"] = "bar"
+        for bad in (1, 1.5, None, b"Foo", ("Foo",), object()):
+            self.assertFalse(bad in headers)
+        self.assertIn("Foo", headers)
+
     def test_string(self):
         headers = HTTPHeaders()
         headers.add("Foo", "1")

tornado/test/util_test.py

@@ -366,3 +366,42 @@ def test_version_info_compatible(self):
 
     def test_current_version(self):
         self.assert_version_info_compatible(tornado.version, tornado.version_info)
+
+
+class GzipDecompressorTest(unittest.TestCase):
+    def _compress(self, data: bytes) -> bytes:
+        import gzip
+
+        return gzip.compress(data)
+
+    def test_single_member(self) -> None:
+        data = b"hello world"
+        d = tornado.util.GzipDecompressor()
+        out = d.decompress(self._compress(data))
+        out += d.flush()
+        self.assertEqual(out, data)
+        self.assertEqual(d.unconsumed_tail, b"")
+
+    def test_concatenated_members_in_single_decompress(self) -> None:
+        members = [b"first message", b"second message", b"third message"]
+        blob = b"".join(self._compress(m) for m in members)
+        d = tornado.util.GzipDecompressor()
+        out = d.decompress(blob)
+        out += d.flush()
+        self.assertEqual(out, b"".join(members))
+
+    def test_concatenated_members_across_decompress_calls(self) -> None:
+        members = [b"alpha", b"beta", b"gamma", b"delta"]
+        blobs = [self._compress(m) for m in members]
+        d = tornado.util.GzipDecompressor()
+        out = bytearray()
+        # Split each member's bytes across separate decompress calls so we
+        # exercise the buffering path between calls.
+        for blob in blobs:
+            chunk_a = blob[: len(blob) // 2]
+            chunk_b = blob[len(blob) // 2 :]
+            out.extend(d.decompress(chunk_a))
+            out.extend(d.decompress(chunk_b))
+        out.extend(d.flush())
+        self.assertEqual(bytes(out), b"".join(members))
+        self.assertEqual(d.unconsumed_tail, b"")

tornado/util.py

@@ -62,14 +62,24 @@ class GzipDecompressor:
     """Streaming gzip decompressor.
 
     The interface is like that of `zlib.decompressobj` (without some of the
-    optional arguments, but it understands gzip headers and checksums.
+    optional arguments, but it understands gzip headers and checksums).
+
+    .. versionchanged:: 6.6
+       Concatenated gzip members are now decompressed: after one member is
+       fully flushed, any remaining bytes in the stream are automatically
+       fed to a fresh decompressor for the next member. Previously, the
+       trailing members were silently dropped.
     """
 
     def __init__(self) -> None:
         # Magic parameter makes zlib module understand gzip header
         # http://stackoverflow.com/questions/1838699/how-can-i-decompress-a-gzip-stream-with-zlib
         # This works on cpython and pypy, but not jython.
         self.decompressobj = zlib.decompressobj(16 + zlib.MAX_WBITS)
+        # Buffer for bytes that come in after the current member has been
+        # flushed; they belong to the next gzip member and are processed
+        # lazily on the next call to `decompress`.
+        self._pending: bytes = b""
 
     def decompress(self, value: bytes, max_length: int = 0) -> bytes:
         """Decompress a chunk, returning newly-available data.
@@ -81,21 +91,73 @@ def decompress(self, value: bytes, max_length: int = 0) -> bytes:
         If ``max_length`` is given, some input data may be left over
         in ``unconsumed_tail``; you must retrieve this value and pass
         it back to a future call to `decompress` if it is not empty.
+
+        .. versionchanged:: 6.6
+           If the previous member ended and ``value`` contains the start of
+           a new gzip member, the new member is decompressed transparently
+           rather than being buffered until the next call.
         """
-        return self.decompressobj.decompress(value, max_length)
+        # Combine any bytes left over from a previous call with the new
+        # input so we can advance through consecutive members in one pass.
+        if self._pending:
+            value = self._pending + value
+            self._pending = b""
+        result = bytearray()
+        if max_length == 0:
+            # Drain the entire stream, advancing across concatenated
+            # members transparently. This is the legacy behaviour that
+            # callers get when they don't impose a per-call cap.
+            while True:
+                chunk = self.decompressobj.decompress(value)
+                result.extend(chunk)
+                value = self.decompressobj.unconsumed_tail
+                if not self.decompressobj.eof:
+                    break
+                next_member = self.decompressobj.unused_data + value
+                if not next_member:
+                    break
+                self.decompressobj = zlib.decompressobj(16 + zlib.MAX_WBITS)
+                value = next_member
+            self._pending = b""
+            return bytes(result)
+        # When max_length is set the caller drives the loop via
+        # `unconsumed_tail`, so we return after a single inner call.
+        chunk = self.decompressobj.decompress(value, max_length)
+        result.extend(chunk)
+        trailing = self.decompressobj.unconsumed_tail
+        if self.decompressobj.eof:
+            # `unused_data` holds bytes past the current member's
+            # trailer that belong to the next concatenated member.
+            trailing = self.decompressobj.unused_data + trailing
+            self.decompressobj = zlib.decompressobj(16 + zlib.MAX_WBITS)
+        self._pending = trailing
+        return bytes(result)
 
     @property
     def unconsumed_tail(self) -> bytes:
         """Returns the unconsumed portion left over"""
-        return self.decompressobj.unconsumed_tail
+        return self._pending + self.decompressobj.unconsumed_tail
 
     def flush(self) -> bytes:
         """Return any remaining buffered data not yet returned by decompress.
 
         Also checks for errors such as truncated input.
         No other methods may be called on this object after `flush`.
+
+        .. versionchanged:: 6.6
+           If the stream contains concatenated gzip members, all members
+           are flushed and concatenated into the returned bytes.
         """
-        return self.decompressobj.flush()
+        result = bytearray()
+        while True:
+            result.extend(self.decompressobj.flush())
+            next_member = self.decompressobj.unused_data
+            self.decompressobj = zlib.decompressobj(16 + zlib.MAX_WBITS)
+            if not next_member:
+                break
+            # Drain the next member fully and append its output.
+            self.decompressobj.decompress(next_member)
+        return bytes(result)
 
 
 def import_object(name: str) -> Any: