From 289f318f64e8006bf0b551902b42b87af6c3e5a8 Mon Sep 17 00:00:00 2001 From: Zo Bot Date: Fri, 26 Jun 2026 15:38:18 +0000 Subject: [PATCH 1/2] HTTPHeaders: raise TypeError on non-string keys in set/get/delitem --- tornado/httputil.py | 16 +++++++++++++ tornado/test/httputil_test.py | 42 +++++++++++++++++++++++++++++++++++ 2 files changed, 58 insertions(+) diff --git a/tornado/httputil.py b/tornado/httputil.py index 4bd17786d..4d0d388f8 100644 --- a/tornado/httputil.py +++ b/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] diff --git a/tornado/test/httputil_test.py b/tornado/test/httputil_test.py index 4e966eb50..d1bbce7d2 100644 --- a/tornado/test/httputil_test.py +++ b/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") From e6ff06788f77f4baabbec75e357c76b4af78a4f6 Mon Sep 17 00:00:00 2001 From: Zo Bot Date: Tue, 30 Jun 2026 03:49:19 +0000 Subject: [PATCH 2/2] GzipDecompressor: decompress concatenated gzip members instead of dropping them Issue #3560. The previous implementation called zlib.decompressobj.decompress once per chunk and returned whatever the inner decompressor produced. When the wrapped HTTP response contains two or more gzip members in sequence (some servers split a gzipped response across members for obfuscation or to defeat naive transport encoders), the inner decompressor reached EOF on the first member and declared itself complete. Any bytes past the first trailer sat in the inner unused_data and were never read, so the second member and every member after it were silently dropped on the floor. Rework GzipDecompressor so callers that pass max_length=0 (the default, and the only mode used by HTTP1Connection) transparently advance across consecutive members: after each member finishes, any leftover bytes in the inner decompressobj's unused_data are folded back into the input and a fresh zlib.decompressobj is wired up for the next member. The public unconsumed_tail property continues to report what a caller should re-feed on the next call (now backed by an explicit _pending buffer combined with the inner tail). When max_length is non-zero the single-call behaviour callers depend on is preserved and unused_data is staged in _pending for the next call. flush advances across all remaining members as well. Added GzipDecompressorTest covering a single member (baseline), two concatenated members fed in one decompress call, and four members split across alternating decompress calls to exercise the between-call buffering path. --- tornado/test/util_test.py | 39 ++++++++++++++++++++++ tornado/util.py | 70 ++++++++++++++++++++++++++++++++++++--- 2 files changed, 105 insertions(+), 4 deletions(-) diff --git a/tornado/test/util_test.py b/tornado/test/util_test.py index 83b78e5cf..45992e177 100644 --- a/tornado/test/util_test.py +++ b/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"") diff --git a/tornado/util.py b/tornado/util.py index 37b595e0b..6246dbf66 100644 --- a/tornado/util.py +++ b/tornado/util.py @@ -62,7 +62,13 @@ 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: @@ -70,6 +76,10 @@ def __init__(self) -> None: # 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: