Copilot commented on code in PR #3845:
URL: https://github.com/apache/avro/pull/3845#discussion_r3564363500
##########
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(avro.errors.AvroWarning(f"Ignoring invalid
{MAX_COLLECTION_ITEMS_ENV} value: {value!r}"))
+ return DEFAULT_MAX_COLLECTION_ITEMS
+ if parsed < 0:
+ warnings.warn(avro.errors.AvroWarning(f"Ignoring negative
{MAX_COLLECTION_ITEMS_ENV} value: {value!r}"))
+ return DEFAULT_MAX_COLLECTION_ITEMS
+ return parsed
+
+
+def check_max_collection_items(existing: int, items: int, limit: int) -> None:
+ """Guard against unbounded allocation when decoding an array or map.
+
+ :param existing: the number of items already read for the collection.
+ :param items: the number of items in the next block to be read. A negative
+ value indicates malformed input.
+ :param limit: the maximum total number of items permitted.
+ :raises avro.errors.AvroCollectionSizeException: if the block count is
+ negative or the running total would exceed ``limit``.
+ """
+ if items < 0:
+ raise avro.errors.AvroCollectionSizeException(f"Malformed data. Block
count is negative: {items}")
Review Comment:
This error message states “Malformed data. Block count is negative”, but
negative block counts are valid on the wire; hitting this branch really means
the check was called with an un-normalized count. Consider a message that
describes an invalid/unsupported value without implying that any negative count
is always malformed.
##########
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
Review Comment:
The environment variable override (`AVRO_MAX_COLLECTION_ITEMS`) is part of
the new behavior, but there are no unit tests verifying it (including
invalid/negative values falling back to the default). Adding tests would help
prevent regressions in the override parsing and warning behavior.
##########
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(avro.errors.AvroWarning(f"Ignoring invalid
{MAX_COLLECTION_ITEMS_ENV} value: {value!r}"))
+ return DEFAULT_MAX_COLLECTION_ITEMS
+ if parsed < 0:
+ warnings.warn(avro.errors.AvroWarning(f"Ignoring negative
{MAX_COLLECTION_ITEMS_ENV} value: {value!r}"))
+ return DEFAULT_MAX_COLLECTION_ITEMS
+ return parsed
+
+
+def check_max_collection_items(existing: int, items: int, limit: int) -> None:
+ """Guard against unbounded allocation when decoding an array or map.
+
+ :param existing: the number of items already read for the collection.
+ :param items: the number of items in the next block to be read. A negative
+ value indicates malformed input.
+ :param limit: the maximum total number of items permitted.
+ :raises avro.errors.AvroCollectionSizeException: if the block count is
+ negative or the running total would exceed ``limit``.
Review Comment:
The docstring implies a negative block count is malformed, but in Avro
encoding a negative count is valid (it indicates the “count + block size”
form). Since callers normalize negative counts before calling this helper,
document `items` as a non-negative count (e.g., `abs(count)` after consuming
block size) and update the `:raises` clause accordingly to avoid misleading
future maintainers.
--
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]
