iemejia commented on code in PR #3845:
URL: https://github.com/apache/avro/pull/3845#discussion_r3564349186
##########
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:
Fixed in fdfc426206 — both `warnings.warn` calls now pass an
`avro.errors.AvroWarning` instance, matching the codebase convention.
##########
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:
Fixed in fdfc426206 (same change — instance form for both warnings).
##########
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:
Good catch — fixed in fdfc426206. `skip_array` now applies the same
cumulative `max_collection_items` check on the positive per-item skip loop (the
negative path is already bounded by `skip(block_size)`). Added
`test_skip_array_respects_limit`.
##########
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:
Fixed in fdfc426206 — `skip_map` bounds the positive per-item skip loop the
same way. Added `test_skip_map_respects_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]
