AVRO-4327: [Java] Bound decode recursion depth

[iemejia]

What is the purpose of the change

Java 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 call stack with a
StackOverflowError before any allocation limit is reached.

This adds a configurable maximum decode nesting depth, enforced by counting
structural descents into records, arrays, maps and unions and rejecting input
that nests deeper than the limit with a bounded SystemLimitException instead
of a StackOverflowError. The default is 100 (matching Protocol Buffers) and
is configurable via the org.apache.avro.limits.decode.maxDepth system
property. The depth is tracked in the existing per-thread decode scope, so a
reader reused concurrently cannot corrupt another thread's counter and no
reader method signatures change. Both reader paths are guarded: the classic
GenericDatumReader (and its Specific/Reflect subclasses) via
readWithoutConversion, and the FastReaderBuilder record/map/union/array
readers.

Verifying this change

This change added tests and can be verified as follows:

• Added TestDecodeRecursionDepth: a ~100k-deep recursive linked-list payload
  is rejected with a bounded SystemLimitException (not a StackOverflowError)
  on both the classic and fast reader paths, while a moderately nested value
  within the limit still decodes.
• Added unit tests in TestSystemLimitException for the depth counter, the
  custom-limit property, and the outer-scope reset.
• Verified across the module's three Surefire runs (default, custom-coders,
  without-fast-reader).

Documentation

• Does this pull request introduce a new feature? (no — DoS hardening)
• If yes, how is the feature documented? (JavaDocs — the new limit is documented
  on SystemLimitException alongside the existing collection/decompress limits)

[Copilot]

Pull request overview

Adds a configurable maximum decode nesting depth in the Java Avro runtime to harden recursive-schema decoding against stack exhaustion, turning potential StackOverflowError crashes into bounded SystemLimitException failures.

Changes:

• Introduces org.apache.avro.limits.decode.maxDepth with a default of 100 and per-thread tracking in SystemLimitException.
• Enforces depth accounting in both classic decoding (GenericDatumReader.readWithoutConversion) and fast-reader paths (FastReaderBuilder union/array/record/map readers).
• Adds regression and unit tests covering default behavior, custom limit, and outer-scope reset semantics.

Reviewed changes

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

Show a summary per file

File	Description
lang/java/avro/src/main/java/org/apache/avro/SystemLimitException.java	Adds decode-depth limit property/default, per-thread counter, and increment/decrement APIs; resets depth at outermost datum scope.
lang/java/avro/src/main/java/org/apache/avro/generic/GenericDatumReader.java	Wraps structural type decoding with depth increment/decrement and factors structural dispatch into a helper.
lang/java/avro/src/main/java/org/apache/avro/io/FastReaderBuilder.java	Adds depth guarding around fast-reader union/array/record/map descents.
lang/java/avro/src/test/java/org/apache/avro/TestSystemLimitException.java	Adds unit tests for decode-depth counter, custom limit, and outer-scope reset; updates property reset.
lang/java/avro/src/test/java/org/apache/avro/generic/TestDecodeRecursionDepth.java	Adds regression test for deep recursive payload rejection on both classic and fast reader paths.

---

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

[Copilot]

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

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

Suppressed comments (4)

lang/java/avro/src/main/java/org/apache/avro/io/FastReaderBuilder.java:657

• RecordReader.read(...) now guards recursion depth, but it still doesn’t open a collection-allocation/decode scope. When the fast reader is used standalone with a top-level record, per-datum zero-byte allocation accounting won’t be cumulative across multiple nested arrays/maps inside the record (each array opens its own outermost scope and resets the running total), and stale decode depth won’t be reset at the datum boundary. Wrapping the record read in beginCollectionAllocationScope()/endCollectionAllocationScope() (before incrementing depth) makes standalone usage consistent with GenericDatumReader.read(...).

    public Object read(Object reuse, Decoder decoder) throws IOException {
      // Bound decode nesting depth: a recursive schema fed deeply nested data
      // would otherwise overflow the stack via this recursive descent.
      SystemLimitException.incrementDecodeDepth();
      try {

lang/java/avro/src/main/java/org/apache/avro/io/FastReaderBuilder.java:683

• MapReader.read(...) increments decode depth but (unlike the array reader and GenericDatumReader.read(...)) doesn’t open a collection-allocation/decode scope. If FastReaderBuilder is used standalone with a top-level map, stale depth won’t be reset at the datum boundary and zero-byte allocation accounting won’t be cumulative across nested collections in the map’s values. Consider scoping the read with beginCollectionAllocationScope()/endCollectionAllocationScope() and doing so before the depth increment (so the outer-scope reset can’t wipe the increment).

    @Override
    public Object read(Object reuse, Decoder decoder) throws IOException {
      SystemLimitException.incrementDecodeDepth();
      try {
        long l = decoder.readMapStart();

lang/java/avro/src/main/java/org/apache/avro/io/FastReaderBuilder.java:425

• createUnionReader(...) increments the decode-depth counter but never opens a collection-allocation/decode scope. When FastReaderBuilder is used standalone (without GenericDatumReader.read(...)), this means (a) stale per-thread depth is not reset at the datum boundary, and (b) zero-byte allocation accounting won’t be cumulative across the whole datum unless some nested array happens to open the scope first. Opening the scope before incrementing depth (mirroring the array reader’s ordering) makes standalone union-top-level reads consistent and prevents the outer-scope reset from wiping the union increment.

This issue also appears in the following locations of the same file:

• line 653
• line 679

    return reusingReader((reuse, decoder) -> {
      SystemLimitException.incrementDecodeDepth();
      try {
        final int selection = decoder.readIndex();
        if (selection < 0 || selection >= unionReaders.length) {

lang/java/avro/src/main/java/org/apache/avro/SystemLimitException.java:548

• resetLimits() assigns maxDecodeDepth using getLimitFromProperty(...), which uses Integer.parseUnsignedInt. Values in the unsigned range [2^31, 2^32-1] parse successfully but wrap to a negative int, making maxDecodeDepth negative and causing all structural decodes to fail (decodeDepth >= maxDecodeDepth is immediately true). Add an explicit negative check (and fallback) for this new property so oversized values don’t silently brick decoding.

    // zero-byte allocation cap consistent with the other collection limits even
    // when it is configured (or derived from a very large heap) above that.
    maxCollectionAllocation = Math.min(maxCollectionAllocation, MAX_ARRAY_VM_LIMIT);
    maxDecodeDepth = getLimitFromProperty(MAX_DECODE_DEPTH_PROPERTY, DEFAULT_MAX_DECODE_DEPTH);
  }