You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@avro.apache.org by "iemejia (via GitHub)" <gi...@apache.org> on 2026/08/07 16:27:10 UTC
[PR] AVRO-4328: [python] Bound decode recursion depth [avro]
iemejia opened a new pull request, #3929:
URL: https://github.com/apache/avro/pull/3929
## What is the purpose of the change
Python SDK implementation of AVRO-4302 (parent). Recursive schemas (e.g. a
linked list or tree) let a small, hostile payload drive arbitrarily deep
nesting during binary decoding, exhausting the Python call stack
(`RecursionError`, or a fatal interpreter crash once the C stack is exhausted)
before any allocation limit is reached.
This bounds the decode nesting depth by counting structural descents into
records, arrays, maps and unions in `DatumReader.read_data`, rejecting input
that nests deeper than the limit with a bounded `AvroException` instead of a
`RecursionError` / crash. The default is 100 (matching Protocol Buffers and the
Java SDK) and is configurable via the `AVRO_MAX_DECODE_DEPTH` environment
variable. The depth is tracked per `DatumReader` for the current datum, reset at
the start of each top-level `read()`, and restored on exit so a reader can be
reused.
## Verifying this change
This change added tests and can be verified as follows:
- Added `test_decode_recursion_depth.py`: a ~100k-deep recursive linked-list
payload is rejected with a bounded `AvroException` (not a `RecursionError`),
a moderately nested value within the limit still decodes, and the
`AVRO_MAX_DECODE_DEPTH` override is honored.
- `ruff check`, `ruff format` and `mypy --strict` pass.
## Documentation
- Does this pull request introduce a new feature? (no — DoS hardening)
- If yes, how is the feature documented? (docstrings — the new
`AVRO_MAX_DECODE_DEPTH` limit is documented alongside the existing collection
limits in `avro/io.py`)
--
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: dev-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AVRO-4328: [python] Bound decode recursion depth [avro]
Posted by "iemejia (via GitHub)" <gi...@apache.org>.
iemejia commented on code in PR #3929:
URL: https://github.com/apache/avro/pull/3929#discussion_r3737381539
##########
lang/py/avro/test/test_decode_recursion_depth.py:
##########
@@ -0,0 +1,84 @@
+#!/usr/bin/env python3
+
+##
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""AVRO-4302: bound decode recursion depth to prevent stack exhaustion."""
+
+import io
+import json
+import unittest
+
+import avro.errors
+import avro.io
+import avro.schema
+
+
+def _encode_long(value: int) -> bytes:
+ """Zig-zag + varint encode a long, matching BinaryEncoder.write_long."""
+ datum = (value << 1) ^ (value >> 63)
+ out = bytearray()
+ while (datum & ~0x7F) != 0:
+ out.append((datum & 0x7F) | 0x80)
+ datum >>= 7
+ out.append(datum)
+ return bytes(out)
+
+
+class TestDecodeRecursionDepth(unittest.TestCase):
+ # A self-referencing linked-list schema: the classic recursion-bomb shape.
+ NODE = avro.schema.parse(json.dumps({"type": "record", "name": "Node", "fields": [{"name": "next", "type": ["null", "Node"]}]}))
+
+ @staticmethod
+ def _linked_list(depth: int) -> bytes:
+ """Encode a Node linked list nested ``depth`` levels deep.
+
+ Each level selects the ``Node`` union branch (index 1); the final level
+ selects ``null`` (index 0) to terminate. ~1 byte per level.
+ """
+ return _encode_long(1) * depth + _encode_long(0)
+
+ def _read(self, data: bytes) -> object:
+ reader = avro.io.DatumReader(self.NODE, self.NODE)
+ return reader.read(avro.io.BinaryDecoder(io.BytesIO(data)))
+
+ def test_deeply_nested_input_rejected_with_bounded_error(self) -> None:
+ # ~100k levels: far beyond the default depth limit and enough to overflow
+ # the stack if it were left unbounded, yet only ~100kB of input. Must fail
+ # with a bounded AvroException rather than a RecursionError / crash.
+ bomb = self._linked_list(100_000)
+ self.assertRaises(avro.errors.AvroException, self._read, bomb)
+
+ def test_moderately_nested_input_within_limit_still_decodes(self) -> None:
+ # Two structural descents (union + record) are counted per list level, so
+ # keep the level count well under half the default limit.
+ result = self._read(self._linked_list(20))
+ self.assertIsInstance(result, dict)
+
+ def test_custom_depth_limit_env_is_honored(self) -> None:
+ import os
+
+ os.environ[avro.io.MAX_DECODE_DEPTH_ENV] = "6"
+ try:
+ # 6 allows only 3 list levels (2 descents each); 10 levels must fail.
+ self.assertRaises(avro.errors.AvroException, self._read, self._linked_list(10))
+ finally:
+ del os.environ[avro.io.MAX_DECODE_DEPTH_ENV]
Review Comment:
Fixed in 710eace: the test now saves the prior `AVRO_MAX_DECODE_DEPTH` value and restores it (only deleting when it was previously unset).
##########
lang/py/avro/io.py:
##########
@@ -845,8 +887,33 @@ def read(self, decoder: "BinaryDecoder") -> object:
# many small collection fields cannot bypass it (see
# _ensure_collection_available).
self._zero_byte_items_read = 0
+ # Start a fresh nesting-depth budget for this datum (see _nested_read).
+ self._read_depth = 0
return self.read_data(self.writers_schema, reader_schema, decoder)
+ @contextlib.contextmanager
+ def _nested_read(self) -> Generator[None, None, None]:
+ """Track one level of structural nesting while decoding.
+
+ Entering a record, array, map or union grows the (recursive) Python call
+ stack. Bounding the nesting depth turns a recursive schema fed a deeply
+ nested payload into a clean, catchable error instead of a RecursionError
+ or a fatal interpreter crash from C-stack exhaustion. The check runs
+ before incrementing so the counter stays balanced as the exception
+ unwinds the enclosing (already-entered) levels, and the depth is restored
+ on exit so a reader instance can be reused.
+ """
+ if self._read_depth >= _max_decode_depth():
+ raise avro.errors.AvroException(
+ f"Decode nesting depth exceeds the maximum allowed of {_max_decode_depth()} "
+ f"(configure with the {MAX_DECODE_DEPTH_ENV} environment variable)"
+ )
+ self._read_depth += 1
Review Comment:
Fixed in 710eace: `_nested_read()` now reads `_max_decode_depth()` once into a local and reuses it for both the check and the error message.
##########
lang/py/avro/io.py:
##########
@@ -918,12 +986,15 @@ def read_data(self, writers_schema: avro.schema.Schema, readers_schema: avro.sch
if isinstance(writers_schema, avro.schema.EnumSchema) and isinstance(readers_schema, avro.schema.EnumSchema):
return self.read_enum(writers_schema, readers_schema, decoder)
if isinstance(writers_schema, avro.schema.ArraySchema) and isinstance(readers_schema, avro.schema.ArraySchema):
- return self.read_array(writers_schema, readers_schema, decoder)
+ with self._nested_read():
+ return self.read_array(writers_schema, readers_schema, decoder)
if isinstance(writers_schema, avro.schema.MapSchema) and isinstance(readers_schema, avro.schema.MapSchema):
- return self.read_map(writers_schema, readers_schema, decoder)
+ with self._nested_read():
+ return self.read_map(writers_schema, readers_schema, decoder)
if isinstance(writers_schema, avro.schema.RecordSchema) and isinstance(readers_schema, avro.schema.RecordSchema):
# .type in ["record", "error", "request"]:
- return self.read_record(writers_schema, readers_schema, decoder)
+ with self._nested_read():
+ return self.read_record(writers_schema, readers_schema, decoder)
Review Comment:
Good catch. Guarded the structural skip path in 710eace: `skip_data` now wraps `skip_array`/`skip_map`/`skip_union`/`skip_record` in `_nested_read()`, so skipping a writer-only field for a recursive schema during resolution is bounded too. Added a regression test that a deeply nested payload is rejected when skipped.
--
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: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] AVRO-4328: [python] Bound decode recursion depth [avro]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #3929:
URL: https://github.com/apache/avro/pull/3929#discussion_r3737319497
##########
lang/py/avro/io.py:
##########
@@ -845,8 +887,33 @@ def read(self, decoder: "BinaryDecoder") -> object:
# many small collection fields cannot bypass it (see
# _ensure_collection_available).
self._zero_byte_items_read = 0
+ # Start a fresh nesting-depth budget for this datum (see _nested_read).
+ self._read_depth = 0
return self.read_data(self.writers_schema, reader_schema, decoder)
+ @contextlib.contextmanager
+ def _nested_read(self) -> Generator[None, None, None]:
+ """Track one level of structural nesting while decoding.
+
+ Entering a record, array, map or union grows the (recursive) Python call
+ stack. Bounding the nesting depth turns a recursive schema fed a deeply
+ nested payload into a clean, catchable error instead of a RecursionError
+ or a fatal interpreter crash from C-stack exhaustion. The check runs
+ before incrementing so the counter stays balanced as the exception
+ unwinds the enclosing (already-entered) levels, and the depth is restored
+ on exit so a reader instance can be reused.
+ """
+ if self._read_depth >= _max_decode_depth():
+ raise avro.errors.AvroException(
+ f"Decode nesting depth exceeds the maximum allowed of {_max_decode_depth()} "
+ f"(configure with the {MAX_DECODE_DEPTH_ENV} environment variable)"
+ )
+ self._read_depth += 1
Review Comment:
_nested_read() calls _max_decode_depth() twice inside the failure path, which re-reads/parses the env var and could produce an inconsistent error message if the environment changes between calls. Compute the limit once and reuse it for the check and message.
##########
lang/py/avro/test/test_decode_recursion_depth.py:
##########
@@ -0,0 +1,84 @@
+#!/usr/bin/env python3
+
+##
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# https://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+"""AVRO-4302: bound decode recursion depth to prevent stack exhaustion."""
+
+import io
+import json
+import unittest
+
+import avro.errors
+import avro.io
+import avro.schema
+
+
+def _encode_long(value: int) -> bytes:
+ """Zig-zag + varint encode a long, matching BinaryEncoder.write_long."""
+ datum = (value << 1) ^ (value >> 63)
+ out = bytearray()
+ while (datum & ~0x7F) != 0:
+ out.append((datum & 0x7F) | 0x80)
+ datum >>= 7
+ out.append(datum)
+ return bytes(out)
+
+
+class TestDecodeRecursionDepth(unittest.TestCase):
+ # A self-referencing linked-list schema: the classic recursion-bomb shape.
+ NODE = avro.schema.parse(json.dumps({"type": "record", "name": "Node", "fields": [{"name": "next", "type": ["null", "Node"]}]}))
+
+ @staticmethod
+ def _linked_list(depth: int) -> bytes:
+ """Encode a Node linked list nested ``depth`` levels deep.
+
+ Each level selects the ``Node`` union branch (index 1); the final level
+ selects ``null`` (index 0) to terminate. ~1 byte per level.
+ """
+ return _encode_long(1) * depth + _encode_long(0)
+
+ def _read(self, data: bytes) -> object:
+ reader = avro.io.DatumReader(self.NODE, self.NODE)
+ return reader.read(avro.io.BinaryDecoder(io.BytesIO(data)))
+
+ def test_deeply_nested_input_rejected_with_bounded_error(self) -> None:
+ # ~100k levels: far beyond the default depth limit and enough to overflow
+ # the stack if it were left unbounded, yet only ~100kB of input. Must fail
+ # with a bounded AvroException rather than a RecursionError / crash.
+ bomb = self._linked_list(100_000)
+ self.assertRaises(avro.errors.AvroException, self._read, bomb)
+
+ def test_moderately_nested_input_within_limit_still_decodes(self) -> None:
+ # Two structural descents (union + record) are counted per list level, so
+ # keep the level count well under half the default limit.
+ result = self._read(self._linked_list(20))
+ self.assertIsInstance(result, dict)
+
+ def test_custom_depth_limit_env_is_honored(self) -> None:
+ import os
+
+ os.environ[avro.io.MAX_DECODE_DEPTH_ENV] = "6"
+ try:
+ # 6 allows only 3 list levels (2 descents each); 10 levels must fail.
+ self.assertRaises(avro.errors.AvroException, self._read, self._linked_list(10))
+ finally:
+ del os.environ[avro.io.MAX_DECODE_DEPTH_ENV]
Review Comment:
This test mutates AVRO_MAX_DECODE_DEPTH but always deletes it in the finally block. If the variable was already set in the environment (e.g., in a developer shell or CI), this will clobber that configuration for subsequent tests. Preserve and restore the prior value instead of unconditionally deleting it.
##########
lang/py/avro/io.py:
##########
@@ -918,12 +986,15 @@ def read_data(self, writers_schema: avro.schema.Schema, readers_schema: avro.sch
if isinstance(writers_schema, avro.schema.EnumSchema) and isinstance(readers_schema, avro.schema.EnumSchema):
return self.read_enum(writers_schema, readers_schema, decoder)
if isinstance(writers_schema, avro.schema.ArraySchema) and isinstance(readers_schema, avro.schema.ArraySchema):
- return self.read_array(writers_schema, readers_schema, decoder)
+ with self._nested_read():
+ return self.read_array(writers_schema, readers_schema, decoder)
if isinstance(writers_schema, avro.schema.MapSchema) and isinstance(readers_schema, avro.schema.MapSchema):
- return self.read_map(writers_schema, readers_schema, decoder)
+ with self._nested_read():
+ return self.read_map(writers_schema, readers_schema, decoder)
if isinstance(writers_schema, avro.schema.RecordSchema) and isinstance(readers_schema, avro.schema.RecordSchema):
# .type in ["record", "error", "request"]:
- return self.read_record(writers_schema, readers_schema, decoder)
+ with self._nested_read():
+ return self.read_record(writers_schema, readers_schema, decoder)
Review Comment:
The new depth budget is enforced only via read_data() wrappers. However, schema resolution can call skip_data() when the writer has fields absent in the reader (read_record uses skip_data for unknown fields), and skip_data/skip_union/skip_record recurse structurally without any depth check. A hostile payload could still trigger RecursionError/stack exhaustion through the skip path. Consider applying the same depth bound to structural skip operations as well (union/record/array/map).
--
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: issues-unsubscribe@avro.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org