Copilot commented on code in PR #3861:
URL: https://github.com/apache/avro/pull/3861#discussion_r3567393889
##########
lang/py/avro/io.py:
##########
@@ -832,23 +1016,37 @@ def read_map(self, writers_schema:
avro.schema.MapSchema, readers_schema: avro.s
The actual count in this case
is the absolute value of the count written.
"""
- read_items = {}
+ read_items: Dict[str, object] = {}
+ # Map keys are strings (>= 1 byte length prefix) plus the value.
+ min_bytes = 1 + _min_bytes_per_element(writers_schema.values)
+ zero_byte_limit, structural_limit = _collection_limits()
block_count = decoder.read_long()
while block_count != 0:
if block_count < 0:
block_count = -block_count
decoder.skip_long()
+ self._ensure_collection_available(decoder, len(read_items),
block_count, min_bytes, zero_byte_limit, structural_limit)
Review Comment:
`read_map()` uses `len(read_items)` as the cumulative element count for
`_ensure_collection_available()`. Because map keys are not guaranteed unique,
duplicate keys can keep `len(read_items)` small while still decoding a very
large number of entries, bypassing the intended cumulative structural/zero-byte
caps across blocks. Track the number of decoded entries separately from the
dictionary size and use that for the limit check.
##########
lang/py/avro/io.py:
##########
@@ -599,6 +644,90 @@ def write_timestamp_micros_long(self, datum:
datetime.datetime) -> None:
#
# DatumReader/Writer
#
+#: Environment variable overriding the maximum number of zero-byte-encoded
+#: collection elements (e.g. an array of nulls) to allocate from a single
decode.
+MAX_COLLECTION_ITEMS_ENV = "AVRO_MAX_COLLECTION_ITEMS"
+
+#: Default maximum number of zero-byte-encoded collection elements to allocate.
+#: Elements whose schema encodes to zero bytes (``null``, a zero-length
``fixed``,
+#: or a record with only zero-byte fields) consume no input, so the
bytes-remaining
+#: check cannot bound their count; without a cap a tiny payload can declare a
huge
+#: block count and exhaust memory. A legitimate collection of zero-byte
elements is
+#: small, so this default is generous while still rejecting pathological
input. It
+#: can be raised (or lowered) with the ``AVRO_MAX_COLLECTION_ITEMS``
environment
+#: variable.
+DEFAULT_MAX_COLLECTION_ITEMS = 10_000_000
+
+#: Default structural cap on the number of elements in any array or map (a
+#: collection larger than this is treated as malformed regardless of element
+#: type). Matches the historical ``Integer.MAX_VALUE - 8`` limit. Elements
with a
+#: positive on-wire size are also bounded by the bytes remaining; this cap is
an
+#: additional overflow/defense-in-depth guard. ``AVRO_MAX_COLLECTION_ITEMS``,
when
+#: set, overrides both this and :data:`DEFAULT_MAX_COLLECTION_ITEMS`.
+DEFAULT_MAX_COLLECTION_STRUCTURAL = (1 << 31) - 8
Review Comment:
The docstring says this cap matches ``Integer.MAX_VALUE - 8``, but the value
is computed as `(1 << 31) - 8` (i.e., `2**31 - 8`). Please align the comment
with the actual constant to avoid confusion about the intended limit.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
