AVRO-4282: [python] Bound collection size when decoding arrays and maps by iemejia · Pull Request #3845 · apache/avro

iemejia · 2026-07-11T12:14:23Z

What is the purpose of the change

When decoding an array or map, the block count is read from the input and drives
allocation of the resulting collection. A very large or malformed block count
(for example from truncated or hostile input) could request an unbounded
allocation, since items such as null occupy no bytes on the wire.

This validates the block count — per block and cumulatively — against a
configurable maximum before allocating, mirroring the Java SDK's collection item
limit (org.apache.avro.limits.collectionItems.maxLength, AVRO-3819). The limit
defaults to 2^31 - 8 items and can be overridden with the
AVRO_MAX_COLLECTION_ITEMS environment variable, or per reader via
DatumReader.max_collection_items. Exceeding it raises
avro.errors.AvroCollectionSizeException.

This is part of the umbrella issue AVRO-4277.

Verifying this change

This change added tests and can be verified as follows:

Added TestCollectionSizeLimit in lang/py/avro/test/test_io.py covering:
- a single block count above the default limit (array and map)
- a per-reader configured limit (array and map)
- a cumulative limit across multiple blocks
- a negative block count bounded by its absolute value
- a within-limit collection still decoding correctly
Run: cd lang/py && python3 -m unittest avro.test.test_io

Documentation

Does this pull request introduce a new feature? (no — hardening/robustness)
If yes, how is the feature documented? (not applicable; the new
AVRO_MAX_COLLECTION_ITEMS environment variable is documented in code comments)

The block count of an array or map is read from the input and drives allocation of the resulting collection. A very large or malformed block count (for example from truncated input) could request an unbounded allocation. Validate the block count per block and cumulatively against a configurable maximum (AVRO_MAX_COLLECTION_ITEMS), mirroring the Java SDK's collection item limit, and raise AvroCollectionSizeException when exceeded. Assisted-by: GitHub Copilot:claude-opus-4.8

Use a running pair count rather than len(read_items) for the cumulative collection check in read_map: repeated keys collapse in the dict and would otherwise let a stream rewriting the same key bypass the configured maximum. Assisted-by: GitHub Copilot:claude-opus-4.8

iemejia · 2026-07-11T13:35:38Z

Pushed a small hardening follow-up (eb6a031): read_map now tracks a running pair count instead of len(read_items) for the cumulative check. Repeated keys collapse in the dict, so counting dict size would let a stream that keeps rewriting the same key bypass the configured maximum. This mirrors the same fix applied to the PHP and Perl decoders after review. Added test_map_cumulative_limit_with_repeated_keys.

Add the second blank line after check_max_collection_items so the module is correctly formatted per ruff (two blank lines before the following top-level statement). Assisted-by: GitHub Copilot:claude-opus-4.8

The cumulative-limit check calls len(read_items) before the first append, so mypy (strict) can no longer infer the element type. Annotate the list as List[object]. Assisted-by: GitHub Copilot:claude-opus-4.8

Copilot

Pull request overview

This PR hardens the Python Avro decoder against unbounded allocations when reading arrays and maps from potentially malformed or hostile input, by enforcing a configurable maximum collection size and raising a dedicated exception when exceeded.

Changes:

Introduces a configurable max-items limit for decoded arrays/maps (defaulting to 2^31 - 8, overrideable via AVRO_MAX_COLLECTION_ITEMS or DatumReader.max_collection_items).
Enforces the limit during array/map decoding (including cumulative multi-block limits and repeated-key map cases).
Adds unit tests covering default-limit rejection, per-reader overrides, cumulative limits, negative-count handling, and within-limit decoding.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.

File	Description
lang/py/avro/io.py	Adds configurable collection-size limit and enforces it during array/map decoding.
lang/py/avro/errors.py	Adds `AvroCollectionSizeException` for size-limit violations.
lang/py/avro/test/test_io.py	Adds `TestCollectionSizeLimit` and wires it into `load_tests`.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

iemejia · 2026-07-11T14:15:59Z

+    try:
+        parsed = int(value)
+    except ValueError:
+        warnings.warn(f"Ignoring invalid {MAX_COLLECTION_ITEMS_ENV} value: {value!r}", avro.errors.AvroWarning)


Fixed in fdfc426 — both warnings.warn calls now pass an avro.errors.AvroWarning instance, matching the codebase convention.

iemejia · 2026-07-11T14:16:00Z

+        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)


Fixed in fdfc426 (same change — instance form for both warnings).

iemejia · 2026-07-11T14:16:01Z

        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)


Good catch — fixed in fdfc426. 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.

iemejia · 2026-07-11T14:16:02Z

        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)


Fixed in fdfc426 — skip_map bounds the positive per-item skip loop the same way. Added test_skip_map_respects_limit.

- Apply the cumulative collection limit in skip_array/skip_map: the positive per-item skip loop can otherwise be driven unbounded by a large untrusted block count (CPU DoS), even when items are cheap to skip. - Pass AvroWarning instances to warnings.warn to match the codebase convention. - Add tests for the skip_array/skip_map limits. Assisted-by: GitHub Copilot:claude-opus-4.8

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

iemejia · 2026-07-11T14:32:34Z

+    :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``.


Fixed in b500066 — reworded the docstring: callers pass the normalized (non-negative) count, and a negative value there indicates an internal error rather than malformed wire data (negative block counts are valid in the Avro encoding).

iemejia · 2026-07-11T14:32:35Z

+        negative or the running total would exceed ``limit``.
+    """
+    if items < 0:
+        raise avro.errors.AvroCollectionSizeException(f"Malformed data. Block count is negative: {items}")


Fixed in b500066 — the message is now "Block count must be normalized before checking, got: ...", reflecting that reaching this branch means an un-normalized count was passed rather than malformed input.

iemejia · 2026-07-11T14:32:36Z

+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


Fixed in b500066 — added tests for the AVRO_MAX_COLLECTION_ITEMS override: a valid value is honored, invalid/negative/empty values fall back to the default, and a reader constructed with the env-set limit enforces it end to end.

… env tests - Reword the check_max_collection_items docstring and negative-count error: callers pass a normalized (non-negative) count, and a negative value there indicates an internal error, not malformed wire data (negative block counts are valid in the Avro encoding). - Add tests for the AVRO_MAX_COLLECTION_ITEMS override, including invalid and negative values falling back to the default and end-to-end enforcement. Assisted-by: GitHub Copilot:claude-opus-4.8

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

iemejia · 2026-07-11T14:42:24Z

            if block_count < 0:
                block_size = decoder.read_long()
                decoder.skip(block_size)


Good catch — fixed in c0ba2d8. skip_array now validates block_size >= 0 before calling decoder.skip() and raises InvalidAvroBinaryEncoding otherwise, so a negative block size can no longer seek the decoder backwards. Added a test.

iemejia · 2026-07-11T14:42:26Z

            if block_count < 0:
                block_size = decoder.read_long()
                decoder.skip(block_size)


Fixed in c0ba2d8 — same guard added to skip_map, with a test covering the negative block size case.

A negative block size read from untrusted input would make decoder.skip() seek backwards (BinaryDecoder.skip does not guard against negative n), corrupting decode state. Validate block_size >= 0 before skipping and raise InvalidAvroBinaryEncoding otherwise. Add tests. Assisted-by: GitHub Copilot:claude-opus-4.8

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.

github-actions Bot added the Python label Jul 11, 2026

iemejia added 2 commits July 11, 2026 15:50

AVRO-4282: [python] Fix ruff formatting

71b1a43

Add the second blank line after check_max_collection_items so the module is correctly formatted per ruff (two blank lines before the following top-level statement). Assisted-by: GitHub Copilot:claude-opus-4.8

AVRO-4282: [python] Annotate read_items for mypy

5956bc6

The cumulative-limit check calls len(read_items) before the first append, so mypy (strict) can no longer infer the element type. Annotate the list as List[object]. Assisted-by: GitHub Copilot:claude-opus-4.8

iemejia requested a review from Copilot July 11, 2026 13:57

Copilot started reviewing on behalf of iemejia July 11, 2026 13:58 View session

Copilot AI reviewed Jul 11, 2026

View reviewed changes

iemejia requested a review from Copilot July 11, 2026 14:20

Copilot started reviewing on behalf of iemejia July 11, 2026 14:21 View session

Copilot AI reviewed Jul 11, 2026

View reviewed changes

iemejia requested a review from Copilot July 11, 2026 14:35

Copilot started reviewing on behalf of iemejia July 11, 2026 14:36 View session

Copilot AI reviewed Jul 11, 2026

View reviewed changes

iemejia requested a review from Copilot July 11, 2026 14:45

Copilot started reviewing on behalf of iemejia July 11, 2026 14:45 View session

Copilot AI reviewed Jul 11, 2026

View reviewed changes

Uh oh!

Conversation

iemejia commented Jul 11, 2026

What is the purpose of the change

Verifying this change

Documentation

Uh oh!

iemejia commented Jul 11, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants