Copilot commented on code in PR #3845:
URL: https://github.com/apache/avro/pull/3845#discussion_r3564319190
##########
lang/py/avro/io.py:
##########
@@ -103,6 +104,53 @@
STRUCT_SIGNED_INT = struct.Struct(">i") # big-endian signed int
STRUCT_SIGNED_LONG = struct.Struct(">q") # big-endian signed long
+# Name of the environment variable used to override the default maximum number
+# of items permitted in a single decoded array or map.
+MAX_COLLECTION_ITEMS_ENV = "AVRO_MAX_COLLECTION_ITEMS"
+
+# Default upper bound on the number of items in a single array or map decoded
+# from a stream. When decoding, the block count of an array or map is read from
+# the (potentially untrusted or truncated) input and drives the allocation of
+# the resulting collection. Reading a collection that declares more items than
+# this limit raises an :class:`avro.errors.AvroCollectionSizeException` instead
+# of attempting a potentially huge allocation. This mirrors the Java SDK's
+# ``org.apache.avro.limits.collectionItems.maxLength`` limit. The default may
be
+# overridden with the ``AVRO_MAX_COLLECTION_ITEMS`` environment variable.
+DEFAULT_MAX_COLLECTION_ITEMS = (1 << 31) - 8
+
+
+def _default_max_collection_items() -> int:
+ """Return the default collection item limit, honoring the environment
override."""
+ value = os.environ.get(MAX_COLLECTION_ITEMS_ENV)
+ if value is None:
+ return DEFAULT_MAX_COLLECTION_ITEMS
+ try:
+ parsed = int(value)
+ except ValueError:
+ warnings.warn(f"Ignoring invalid {MAX_COLLECTION_ITEMS_ENV} value:
{value!r}", avro.errors.AvroWarning)
Review Comment:
`warnings.warn` is called with a string message + category class here, but
the rest of the codebase consistently passes an `AvroWarning` instance (e.g.,
`warnings.warn(avro.errors.AvroWarning(...))`). Using the instance form keeps
warning construction consistent across modules.
##########
lang/py/avro/io.py:
##########
@@ -834,13 +887,19 @@ def read_map(self, writers_schema: avro.schema.MapSchema,
readers_schema: avro.s
"""
read_items = {}
block_count = decoder.read_long()
+ # Track the number of pairs decoded rather than len(read_items):
repeated
+ # keys collapse in the dict and would otherwise let the cumulative
check
+ # be bypassed by a stream that keeps rewriting the same key.
+ items_read = 0
while block_count != 0:
if block_count < 0:
block_count = -block_count
decoder.skip_long()
+ check_max_collection_items(items_read, block_count,
self.max_collection_items)
Review Comment:
`read_map` now enforces `max_collection_items`, but `skip_map` can still be
driven by an untrusted `block_count` into an unbounded loop while skipping
writer-only data (CPU DoS). Mirroring the same cumulative limit check in
`skip_map` keeps the protection effective even when fields are skipped during
schema resolution.
##########
lang/py/avro/io.py:
##########
@@ -103,6 +104,53 @@
STRUCT_SIGNED_INT = struct.Struct(">i") # big-endian signed int
STRUCT_SIGNED_LONG = struct.Struct(">q") # big-endian signed long
+# Name of the environment variable used to override the default maximum number
+# of items permitted in a single decoded array or map.
+MAX_COLLECTION_ITEMS_ENV = "AVRO_MAX_COLLECTION_ITEMS"
+
+# Default upper bound on the number of items in a single array or map decoded
+# from a stream. When decoding, the block count of an array or map is read from
+# the (potentially untrusted or truncated) input and drives the allocation of
+# the resulting collection. Reading a collection that declares more items than
+# this limit raises an :class:`avro.errors.AvroCollectionSizeException` instead
+# of attempting a potentially huge allocation. This mirrors the Java SDK's
+# ``org.apache.avro.limits.collectionItems.maxLength`` limit. The default may
be
+# overridden with the ``AVRO_MAX_COLLECTION_ITEMS`` environment variable.
+DEFAULT_MAX_COLLECTION_ITEMS = (1 << 31) - 8
+
+
+def _default_max_collection_items() -> int:
+ """Return the default collection item limit, honoring the environment
override."""
+ value = os.environ.get(MAX_COLLECTION_ITEMS_ENV)
+ if value is None:
+ return DEFAULT_MAX_COLLECTION_ITEMS
+ try:
+ parsed = int(value)
+ except ValueError:
+ warnings.warn(f"Ignoring invalid {MAX_COLLECTION_ITEMS_ENV} value:
{value!r}", avro.errors.AvroWarning)
+ return DEFAULT_MAX_COLLECTION_ITEMS
+ if parsed < 0:
+ warnings.warn(f"Ignoring negative {MAX_COLLECTION_ITEMS_ENV} value:
{value!r}", avro.errors.AvroWarning)
Review Comment:
`warnings.warn` is called with a string message + category class here, but
the codebase consistently passes an `AvroWarning` instance (e.g.,
`warnings.warn(avro.errors.AvroWarning(...))`). This keeps warning construction
consistent and avoids mixing the two call styles.
##########
lang/py/avro/io.py:
##########
@@ -795,12 +847,13 @@ def read_array(self, writers_schema:
avro.schema.ArraySchema, readers_schema: av
The actual count in this case
is the absolute value of the count written.
"""
- read_items = []
+ read_items: List[object] = []
block_count = decoder.read_long()
while block_count != 0:
if block_count < 0:
block_count = -block_count
decoder.skip_long()
+ check_max_collection_items(len(read_items), block_count,
self.max_collection_items)
Review Comment:
`read_array` now enforces `max_collection_items`, but `skip_array` can still
be driven by an untrusted `block_count` into an unbounded per-item loop (CPU
DoS), especially for schemas where items are cheap to skip. Applying the same
cumulative limit check in `skip_array` prevents resource exhaustion when the
reader is skipping writer-only fields.
--
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]
