iemejia commented on code in PR #3865:
URL: https://github.com/apache/avro/pull/3865#discussion_r3566942250


##########
lang/java/avro/src/main/java/org/apache/avro/SystemLimitException.java:
##########
@@ -28,13 +28,19 @@
  * The following system properties can be set to limit the size of bytes,
  * strings and collection types to be allocated:
  * <ul>
- * <li><tt>org.apache.avro.limits.byte.maxLength</tt></li> limits the maximum
- * size of <tt>byte</tt> types.</li>
- * <li><tt>org.apache.avro.limits.collectionItems.maxLength</tt></li> limits 
the
- * maximum number of <tt>map</tt> and <tt>list</tt> items that can be read at
- * once single sequence.</li>
- * <li><tt>org.apache.avro.limits.string.maxLength</tt></li> limits the maximum
- * size of <tt>string</tt> types.</li>
+ * <li><tt>org.apache.avro.limits.bytes.maxLength</tt> limits the maximum size
+ * of <tt>bytes</tt> types.</li>
+ * <li><tt>org.apache.avro.limits.collectionItems.maxLength</tt> limits the
+ * maximum number of <tt>map</tt> and <tt>list</tt> items that can be read in a
+ * single sequence.</li>
+ * <li><tt>org.apache.avro.limits.string.maxLength</tt> limits the maximum size
+ * of <tt>string</tt> types.</li>
+ * <li><tt>org.apache.avro.limits.collectionItems.maxAllocation</tt> limits the
+ * number of <tt>array</tt> elements whose schema encodes to zero bytes (such 
as
+ * <tt>null</tt> or a self-referencing record) that may be allocated at once.
+ * Unlike other element types, these cannot be bounded by the number of bytes

Review Comment:
   Fixed in 6bf53e35d6: reworded to describe actual zero-byte encodings (null, 
a zero-length fixed, or a record whose fields all encode to zero bytes) rather 
than 'a self-referencing record'.



##########
lang/java/avro/src/main/java/org/apache/avro/SystemLimitException.java:
##########
@@ -246,6 +291,40 @@ public static int checkMaxCollectionLength(long items) {
     return (int) items;
   }
 
+  /**
+   * Check to ensure that allocating storage for the specified number of
+   * zero-byte-encoded array elements remains within the heap-aware limit.
+   * <p>
+   * Elements whose schema encodes to zero bytes (e.g. {@code null} or a
+   * self-referencing record) consume no input bytes, so the number that may be
+   * declared is not bounded by the bytes remaining in the stream. Without a 
cap,

Review Comment:
   Fixed in 6bf53e35d6: same rewording applied to this occurrence.



##########
lang/java/avro/src/test/java/org/apache/avro/generic/TestGenericDatumReader.java:
##########
@@ -303,4 +305,297 @@ void mapOfRecordsRejectsHugeCountUsingFullRecordSize() 
throws Exception {
     BinaryDecoder decoder = DecoderFactory.get().binaryDecoder(data, null);
     assertThrows(EOFException.class, () -> reader.read(null, decoder));
   }
+
+  // --- Zero-byte element collection allocation limit (AVRO-4300) ---
+
+  /**
+   * An array of {@code null} elements encodes each element as 0 bytes, so the
+   * bytes-remaining check cannot bound the block count. A huge count must be
+   * rejected by the heap-aware allocation limit rather than driving an 
unbounded
+   * backing-array allocation.
+   */
+  @Test
+  void arrayOfNullsRejectsCountAboveAllocationLimit() throws Exception {
+    
System.setProperty(SystemLimitException.MAX_COLLECTION_ALLOCATION_PROPERTY, 
"1000");
+    org.apache.avro.TestSystemLimitException.resetLimits();
+    try {
+      Schema schema = Schema.createArray(Schema.create(Schema.Type.NULL));
+      GenericDatumReader<Object> reader = new GenericDatumReader<>(schema);
+
+      // A single block declaring 200,000 null elements: only ~4 payload bytes,
+      // but would allocate a 200,000-slot backing array. Exceeds the limit of
+      // 1000, so it must be rejected before allocating.
+      byte[] data = encodeVarints(200_000L, 0L);
+      BinaryDecoder decoder = DecoderFactory.get().binaryDecoder(data, null);
+      assertThrows(SystemLimitException.class, () -> reader.read(null, 
decoder));
+
+      // Cumulative growth across multiple blocks is also rejected: two blocks 
of
+      // 600 nulls each (1200 > 1000) must throw on the second block.
+      byte[] cumulative = encodeVarints(600L, 600L, 0L);
+      BinaryDecoder decoder2 = DecoderFactory.get().binaryDecoder(cumulative, 
null);
+      assertThrows(SystemLimitException.class, () -> reader.read(null, 
decoder2));
+    } finally {
+      
System.clearProperty(SystemLimitException.MAX_COLLECTION_ALLOCATION_PROPERTY);
+      org.apache.avro.TestSystemLimitException.resetLimits();
+    }
+  }
+
+  /**
+   * A legitimate array of {@code null} elements within the allocation limit 
still
+   * decodes correctly, so the guard does not reject valid data.
+   */
+  @Test
+  void arrayOfNullsWithinAllocationLimitStillDecodes() throws Exception {
+    
System.setProperty(SystemLimitException.MAX_COLLECTION_ALLOCATION_PROPERTY, 
"1000");
+    org.apache.avro.TestSystemLimitException.resetLimits();
+    try {
+      Schema schema = Schema.createArray(Schema.create(Schema.Type.NULL));
+      GenericDatumReader<Object> reader = new GenericDatumReader<>(schema);
+
+      byte[] data = encodeVarints(1000L, 0L);
+      BinaryDecoder decoder = DecoderFactory.get().binaryDecoder(data, null);
+      GenericData.Array<?> result = (GenericData.Array<?>) reader.read(null, 
decoder);
+      assertEquals(1000, result.size());
+    } finally {
+      
System.clearProperty(SystemLimitException.MAX_COLLECTION_ALLOCATION_PROPERTY);
+      org.apache.avro.TestSystemLimitException.resetLimits();
+    }
+  }
+
+  /**
+   * An array whose element is a record with only {@code null} fields also 
encodes
+   * to 0 bytes per element and must be bounded by the allocation limit.
+   */
+  @Test
+  void arrayOfAllNullRecordsRejectsCountAboveAllocationLimit() throws 
Exception {
+    
System.setProperty(SystemLimitException.MAX_COLLECTION_ALLOCATION_PROPERTY, 
"1000");
+    org.apache.avro.TestSystemLimitException.resetLimits();
+    try {
+      Schema recWithNull = Schema.createRecord("AllNull", null, "test", false);
+      recWithNull.setFields(Collections.singletonList(new Schema.Field("n", 
Schema.create(Schema.Type.NULL))));
+      Schema schema = Schema.createArray(recWithNull);
+      GenericDatumReader<Object> reader = new GenericDatumReader<>(schema);
+
+      byte[] data = encodeVarints(200_000L, 0L);
+      BinaryDecoder decoder = DecoderFactory.get().binaryDecoder(data, null);
+      assertThrows(SystemLimitException.class, () -> reader.read(null, 
decoder));
+    } finally {
+      
System.clearProperty(SystemLimitException.MAX_COLLECTION_ALLOCATION_PROPERTY);
+      org.apache.avro.TestSystemLimitException.resetLimits();
+    }
+  }
+
+  private static GenericDatumReader<Object> arrayReader(Schema elementType, 
boolean fastReader) {
+    return readerFor(Schema.createArray(elementType), fastReader);
+  }
+
+  private static GenericDatumReader<Object> readerFor(Schema schema, boolean 
fastReader) {
+    GenericData data = new GenericData();
+    data.setFastReaderEnabled(fastReader);
+    return new GenericDatumReader<>(schema, schema, data);
+  }
+
+  /**
+   * Full matrix: every collection kind must be rejected (never OOM) with a 
huge
+   * declared block count and no element data, on both the fast (default) and 
the
+   * classic reader. Zero-byte-element arrays are bounded by the heap-aware
+   * allocation cap (SystemLimitException); every other kind is bounded by the
+   * bytes-remaining check (EOFException). Maps always carry a >=1-byte key so
+   * they fall in the latter group regardless of the value type.
+   */
+  @Test
+  void hugeCollectionsRejectedOnBothReaderPaths() throws Exception {
+    // element/value type -> expected exception when the count is huge and no 
data
+    Schema nullType = Schema.create(Schema.Type.NULL);
+    Schema longType = Schema.create(Schema.Type.LONG);
+    Schema intType = Schema.create(Schema.Type.INT);
+
+    // (schema, expected exception). array<null> is the only zero-byte case 
here.
+    Object[][] cases = { { Schema.createArray(nullType), 
SystemLimitException.class },
+        { Schema.createArray(longType), EOFException.class }, { 
Schema.createArray(intType), EOFException.class },
+        { Schema.createMap(nullType), EOFException.class }, { 
Schema.createMap(longType), EOFException.class }, };
+
+    // Small allocation limit so the zero-byte array<null> case is rejected
+    // deterministically regardless of the test JVM heap size.
+    
System.setProperty(SystemLimitException.MAX_COLLECTION_ALLOCATION_PROPERTY, 
"1000");
+    org.apache.avro.TestSystemLimitException.resetLimits();
+    try {
+      for (Object[] c : cases) {
+        Schema schema = (Schema) c[0];
+        @SuppressWarnings("unchecked")
+        Class<? extends Throwable> expected = (Class<? extends Throwable>) 
c[1];
+        for (boolean fast : new boolean[] { true, false }) {
+          GenericDatumReader<Object> reader = readerFor(schema, fast);
+          byte[] data = encodeVarints(200_000_000L, 0L);
+          BinaryDecoder decoder = DecoderFactory.get().binaryDecoder(data, 
null);
+          assertThrows(expected, () -> reader.read(null, decoder), () -> 
schema + " fast=" + fast);
+        }
+      }
+    } finally {
+      
System.clearProperty(SystemLimitException.MAX_COLLECTION_ALLOCATION_PROPERTY);
+      org.apache.avro.TestSystemLimitException.resetLimits();
+    }
+  }
+
+  /**
+   * The fast reader (the default decode path) must apply the same guards as 
the
+   * classic reader. A non-zero-byte element array with a huge count and no 
data
+   * must be rejected by the bytes-remaining check rather than pre-allocating a
+   * huge backing array. Also verifies the cumulative allocation cap across
+   * multiple blocks for the zero-byte case, which the single-block matrix does
+   * not exercise.
+   */
+  @Test
+  void fastAndClassicReaderRejectCumulativeNullBlocks() throws Exception {
+    
System.setProperty(SystemLimitException.MAX_COLLECTION_ALLOCATION_PROPERTY, 
"1000");
+    org.apache.avro.TestSystemLimitException.resetLimits();
+    try {
+      for (boolean fast : new boolean[] { true, false }) {
+        GenericDatumReader<Object> reader = 
arrayReader(Schema.create(Schema.Type.NULL), fast);
+        // Two blocks of 600 nulls each (1200 > 1000) must throw on the second.
+        byte[] data = encodeVarints(600L, 600L, 0L);
+        BinaryDecoder decoder = DecoderFactory.get().binaryDecoder(data, null);
+        assertThrows(SystemLimitException.class, () -> reader.read(null, 
decoder), "fastReader=" + fast);
+      }
+    } finally {
+      
System.clearProperty(SystemLimitException.MAX_COLLECTION_ALLOCATION_PROPERTY);
+      org.apache.avro.TestSystemLimitException.resetLimits();
+    }
+  }
+
+  /**
+   * A negative block count encodes {@code abs(count)} zero-byte elements 
preceded
+   * by a block byte-size; the decoder normalizes it to a positive count, which
+   * must still be bounded by the allocation cap on both reader paths (matching
+   * the negative-block-count coverage of the C and Python SDKs).
+   */
+  @Test
+  void fastAndClassicReaderRejectNegativeNullBlockCount() throws Exception {
+    
System.setProperty(SystemLimitException.MAX_COLLECTION_ALLOCATION_PROPERTY, 
"1000");
+    org.apache.avro.TestSystemLimitException.resetLimits();
+    try {
+      for (boolean fast : new boolean[] { true, false }) {
+        GenericDatumReader<Object> reader = 
arrayReader(Schema.create(Schema.Type.NULL), fast);
+        // -200000 items (zigzag negative), followed by a block byte-size of 0,
+        // then the end-of-array terminator. Normalized to 200000 > 1000.
+        byte[] data = encodeVarints(-200_000L, 0L, 0L);
+        BinaryDecoder decoder = DecoderFactory.get().binaryDecoder(data, null);
+        assertThrows(SystemLimitException.class, () -> reader.read(null, 
decoder), "fastReader=" + fast);
+      }
+    } finally {
+      
System.clearProperty(SystemLimitException.MAX_COLLECTION_ALLOCATION_PROPERTY);
+      org.apache.avro.TestSystemLimitException.resetLimits();
+    }
+  }
+
+  /**
+   * {@code Long.MIN_VALUE} as a block count is the pathological overflow case:
+   * negating it overflows back to a negative value. It must be handled safely
+   * without allocating -- the decoder normalizes it to an empty collection 
rather
+   * than a huge one -- on both reader paths. (The C SDK rejects it outright; 
this
+   * normalization is equally non-exploitable.)
+   */
+  @Test
+  void fastAndClassicReaderHandleMinValueBlockCountSafely() throws Exception {
+    for (boolean fast : new boolean[] { true, false }) {
+      GenericDatumReader<Object> reader = 
arrayReader(Schema.create(Schema.Type.NULL), fast);
+      // Long.MIN_VALUE items (zigzag), a block byte-size of 0, then the
+      // end-of-array terminator. Negating Long.MIN_VALUE overflows, so this 
must
+      // not drive an allocation.
+      byte[] data = encodeVarints(Long.MIN_VALUE, 0L, 0L);
+      BinaryDecoder decoder = DecoderFactory.get().binaryDecoder(data, null);
+      Collection<?> result = (Collection<?>) reader.read(null, decoder);
+      assertEquals(0, result.size(), "fastReader=" + fast);
+    }
+  }
+
+  /**
+   * A legitimate array of nulls within the limit still decodes on both reader
+   * paths.
+   */
+  @Test
+  void fastAndClassicReaderDecodeSmallNullArray() throws Exception {
+    for (boolean fast : new boolean[] { true, false }) {
+      GenericDatumReader<Object> reader = 
arrayReader(Schema.create(Schema.Type.NULL), fast);
+      byte[] data = encodeVarints(1000L, 0L);
+      BinaryDecoder decoder = DecoderFactory.get().binaryDecoder(data, null);
+      Collection<?> result = (Collection<?>) reader.read(null, decoder);
+      assertEquals(1000, result.size(), "fastReader=" + fast);
+    }
+  }
+
+  /**
+   * Skipping a huge zero-byte array (e.g. during schema projection) is 
bounded by
+   * the heap-aware allocation cap, so it cannot loop unboundedly even though 
it
+   * reads nothing.
+   */
+  @Test
+  void skipArrayOfNullRejectsHugeCount() throws Exception {
+    
System.setProperty(SystemLimitException.MAX_COLLECTION_ALLOCATION_PROPERTY, 
"1000");
+    org.apache.avro.TestSystemLimitException.resetLimits();
+    try {
+      Schema schema = Schema.createArray(Schema.create(Schema.Type.NULL));
+      byte[] data = encodeVarints(200_000L, 0L);
+      BinaryDecoder decoder = DecoderFactory.get().binaryDecoder(data, null);
+      assertThrows(SystemLimitException.class, () -> 
GenericDatumReader.skip(schema, decoder));
+    } finally {
+      
System.clearProperty(SystemLimitException.MAX_COLLECTION_ALLOCATION_PROPERTY);
+      org.apache.avro.TestSystemLimitException.resetLimits();
+    }
+  }
+
+  /**
+   * A legitimate small zero-byte array is skipped without error.
+   */
+  @Test
+  void skipSmallNullArraySucceeds() throws Exception {
+    Schema schema = Schema.createArray(Schema.create(Schema.Type.NULL));
+    // 1000 nulls then the end marker, followed by a trailing long we can read 
to
+    // confirm the skip advanced correctly.
+    byte[] data = encodeVarints(1000L, 0L, 42L);
+    BinaryDecoder decoder = DecoderFactory.get().binaryDecoder(data, null);
+    GenericDatumReader.skip(schema, decoder);
+    assertEquals(42L, decoder.readLong());
+  }
+
+  /**
+   * Skipping a huge map is bounded by the structural collection cap.
+   */
+  @Test
+  void skipMapRejectsHugeCount() throws Exception {
+    Schema schema = Schema.createMap(Schema.create(Schema.Type.NULL));
+    // A single block declaring more than Integer.MAX_VALUE - 8 entries.
+    byte[] data = encodeVarints((long) Integer.MAX_VALUE, 0L);
+    BinaryDecoder decoder = DecoderFactory.get().binaryDecoder(data, null);
+    assertThrows(RuntimeException.class, () -> GenericDatumReader.skip(schema, 
decoder));

Review Comment:
   Fixed in 6bf53e35d6: skipMapRejectsHugeCount now asserts 
UnsupportedOperationException (Integer.MAX_VALUE deterministically hits the VM 
structural-limit path).



##########
lang/java/avro/src/test/java/org/apache/avro/generic/TestGenericDatumReader.java:
##########
@@ -303,4 +305,297 @@ void mapOfRecordsRejectsHugeCountUsingFullRecordSize() 
throws Exception {
     BinaryDecoder decoder = DecoderFactory.get().binaryDecoder(data, null);
     assertThrows(EOFException.class, () -> reader.read(null, decoder));
   }
+
+  // --- Zero-byte element collection allocation limit (AVRO-4300) ---
+
+  /**
+   * An array of {@code null} elements encodes each element as 0 bytes, so the
+   * bytes-remaining check cannot bound the block count. A huge count must be
+   * rejected by the heap-aware allocation limit rather than driving an 
unbounded
+   * backing-array allocation.
+   */
+  @Test
+  void arrayOfNullsRejectsCountAboveAllocationLimit() throws Exception {
+    
System.setProperty(SystemLimitException.MAX_COLLECTION_ALLOCATION_PROPERTY, 
"1000");
+    org.apache.avro.TestSystemLimitException.resetLimits();
+    try {
+      Schema schema = Schema.createArray(Schema.create(Schema.Type.NULL));
+      GenericDatumReader<Object> reader = new GenericDatumReader<>(schema);
+
+      // A single block declaring 200,000 null elements: only ~4 payload bytes,
+      // but would allocate a 200,000-slot backing array. Exceeds the limit of
+      // 1000, so it must be rejected before allocating.
+      byte[] data = encodeVarints(200_000L, 0L);
+      BinaryDecoder decoder = DecoderFactory.get().binaryDecoder(data, null);
+      assertThrows(SystemLimitException.class, () -> reader.read(null, 
decoder));
+
+      // Cumulative growth across multiple blocks is also rejected: two blocks 
of
+      // 600 nulls each (1200 > 1000) must throw on the second block.
+      byte[] cumulative = encodeVarints(600L, 600L, 0L);
+      BinaryDecoder decoder2 = DecoderFactory.get().binaryDecoder(cumulative, 
null);
+      assertThrows(SystemLimitException.class, () -> reader.read(null, 
decoder2));
+    } finally {
+      
System.clearProperty(SystemLimitException.MAX_COLLECTION_ALLOCATION_PROPERTY);
+      org.apache.avro.TestSystemLimitException.resetLimits();
+    }
+  }
+
+  /**
+   * A legitimate array of {@code null} elements within the allocation limit 
still
+   * decodes correctly, so the guard does not reject valid data.
+   */
+  @Test
+  void arrayOfNullsWithinAllocationLimitStillDecodes() throws Exception {
+    
System.setProperty(SystemLimitException.MAX_COLLECTION_ALLOCATION_PROPERTY, 
"1000");
+    org.apache.avro.TestSystemLimitException.resetLimits();
+    try {
+      Schema schema = Schema.createArray(Schema.create(Schema.Type.NULL));
+      GenericDatumReader<Object> reader = new GenericDatumReader<>(schema);
+
+      byte[] data = encodeVarints(1000L, 0L);
+      BinaryDecoder decoder = DecoderFactory.get().binaryDecoder(data, null);
+      GenericData.Array<?> result = (GenericData.Array<?>) reader.read(null, 
decoder);
+      assertEquals(1000, result.size());
+    } finally {
+      
System.clearProperty(SystemLimitException.MAX_COLLECTION_ALLOCATION_PROPERTY);
+      org.apache.avro.TestSystemLimitException.resetLimits();
+    }
+  }
+
+  /**
+   * An array whose element is a record with only {@code null} fields also 
encodes
+   * to 0 bytes per element and must be bounded by the allocation limit.
+   */
+  @Test
+  void arrayOfAllNullRecordsRejectsCountAboveAllocationLimit() throws 
Exception {
+    
System.setProperty(SystemLimitException.MAX_COLLECTION_ALLOCATION_PROPERTY, 
"1000");
+    org.apache.avro.TestSystemLimitException.resetLimits();
+    try {
+      Schema recWithNull = Schema.createRecord("AllNull", null, "test", false);
+      recWithNull.setFields(Collections.singletonList(new Schema.Field("n", 
Schema.create(Schema.Type.NULL))));
+      Schema schema = Schema.createArray(recWithNull);
+      GenericDatumReader<Object> reader = new GenericDatumReader<>(schema);
+
+      byte[] data = encodeVarints(200_000L, 0L);
+      BinaryDecoder decoder = DecoderFactory.get().binaryDecoder(data, null);
+      assertThrows(SystemLimitException.class, () -> reader.read(null, 
decoder));
+    } finally {
+      
System.clearProperty(SystemLimitException.MAX_COLLECTION_ALLOCATION_PROPERTY);
+      org.apache.avro.TestSystemLimitException.resetLimits();
+    }
+  }
+
+  private static GenericDatumReader<Object> arrayReader(Schema elementType, 
boolean fastReader) {
+    return readerFor(Schema.createArray(elementType), fastReader);
+  }
+
+  private static GenericDatumReader<Object> readerFor(Schema schema, boolean 
fastReader) {
+    GenericData data = new GenericData();
+    data.setFastReaderEnabled(fastReader);
+    return new GenericDatumReader<>(schema, schema, data);
+  }
+
+  /**
+   * Full matrix: every collection kind must be rejected (never OOM) with a 
huge
+   * declared block count and no element data, on both the fast (default) and 
the
+   * classic reader. Zero-byte-element arrays are bounded by the heap-aware
+   * allocation cap (SystemLimitException); every other kind is bounded by the
+   * bytes-remaining check (EOFException). Maps always carry a >=1-byte key so
+   * they fall in the latter group regardless of the value type.
+   */
+  @Test
+  void hugeCollectionsRejectedOnBothReaderPaths() throws Exception {
+    // element/value type -> expected exception when the count is huge and no 
data
+    Schema nullType = Schema.create(Schema.Type.NULL);
+    Schema longType = Schema.create(Schema.Type.LONG);
+    Schema intType = Schema.create(Schema.Type.INT);
+
+    // (schema, expected exception). array<null> is the only zero-byte case 
here.
+    Object[][] cases = { { Schema.createArray(nullType), 
SystemLimitException.class },
+        { Schema.createArray(longType), EOFException.class }, { 
Schema.createArray(intType), EOFException.class },
+        { Schema.createMap(nullType), EOFException.class }, { 
Schema.createMap(longType), EOFException.class }, };
+
+    // Small allocation limit so the zero-byte array<null> case is rejected
+    // deterministically regardless of the test JVM heap size.
+    
System.setProperty(SystemLimitException.MAX_COLLECTION_ALLOCATION_PROPERTY, 
"1000");
+    org.apache.avro.TestSystemLimitException.resetLimits();
+    try {
+      for (Object[] c : cases) {
+        Schema schema = (Schema) c[0];
+        @SuppressWarnings("unchecked")
+        Class<? extends Throwable> expected = (Class<? extends Throwable>) 
c[1];
+        for (boolean fast : new boolean[] { true, false }) {
+          GenericDatumReader<Object> reader = readerFor(schema, fast);
+          byte[] data = encodeVarints(200_000_000L, 0L);
+          BinaryDecoder decoder = DecoderFactory.get().binaryDecoder(data, 
null);
+          assertThrows(expected, () -> reader.read(null, decoder), () -> 
schema + " fast=" + fast);
+        }
+      }
+    } finally {
+      
System.clearProperty(SystemLimitException.MAX_COLLECTION_ALLOCATION_PROPERTY);
+      org.apache.avro.TestSystemLimitException.resetLimits();
+    }
+  }
+
+  /**
+   * The fast reader (the default decode path) must apply the same guards as 
the
+   * classic reader. A non-zero-byte element array with a huge count and no 
data
+   * must be rejected by the bytes-remaining check rather than pre-allocating a
+   * huge backing array. Also verifies the cumulative allocation cap across
+   * multiple blocks for the zero-byte case, which the single-block matrix does
+   * not exercise.
+   */
+  @Test
+  void fastAndClassicReaderRejectCumulativeNullBlocks() throws Exception {
+    
System.setProperty(SystemLimitException.MAX_COLLECTION_ALLOCATION_PROPERTY, 
"1000");
+    org.apache.avro.TestSystemLimitException.resetLimits();
+    try {
+      for (boolean fast : new boolean[] { true, false }) {
+        GenericDatumReader<Object> reader = 
arrayReader(Schema.create(Schema.Type.NULL), fast);
+        // Two blocks of 600 nulls each (1200 > 1000) must throw on the second.
+        byte[] data = encodeVarints(600L, 600L, 0L);
+        BinaryDecoder decoder = DecoderFactory.get().binaryDecoder(data, null);
+        assertThrows(SystemLimitException.class, () -> reader.read(null, 
decoder), "fastReader=" + fast);
+      }
+    } finally {
+      
System.clearProperty(SystemLimitException.MAX_COLLECTION_ALLOCATION_PROPERTY);
+      org.apache.avro.TestSystemLimitException.resetLimits();
+    }
+  }
+
+  /**
+   * A negative block count encodes {@code abs(count)} zero-byte elements 
preceded
+   * by a block byte-size; the decoder normalizes it to a positive count, which
+   * must still be bounded by the allocation cap on both reader paths (matching
+   * the negative-block-count coverage of the C and Python SDKs).
+   */
+  @Test
+  void fastAndClassicReaderRejectNegativeNullBlockCount() throws Exception {
+    
System.setProperty(SystemLimitException.MAX_COLLECTION_ALLOCATION_PROPERTY, 
"1000");
+    org.apache.avro.TestSystemLimitException.resetLimits();
+    try {
+      for (boolean fast : new boolean[] { true, false }) {
+        GenericDatumReader<Object> reader = 
arrayReader(Schema.create(Schema.Type.NULL), fast);
+        // -200000 items (zigzag negative), followed by a block byte-size of 0,
+        // then the end-of-array terminator. Normalized to 200000 > 1000.
+        byte[] data = encodeVarints(-200_000L, 0L, 0L);
+        BinaryDecoder decoder = DecoderFactory.get().binaryDecoder(data, null);
+        assertThrows(SystemLimitException.class, () -> reader.read(null, 
decoder), "fastReader=" + fast);
+      }
+    } finally {
+      
System.clearProperty(SystemLimitException.MAX_COLLECTION_ALLOCATION_PROPERTY);
+      org.apache.avro.TestSystemLimitException.resetLimits();
+    }
+  }
+
+  /**
+   * {@code Long.MIN_VALUE} as a block count is the pathological overflow case:
+   * negating it overflows back to a negative value. It must be handled safely
+   * without allocating -- the decoder normalizes it to an empty collection 
rather
+   * than a huge one -- on both reader paths. (The C SDK rejects it outright; 
this
+   * normalization is equally non-exploitable.)
+   */
+  @Test
+  void fastAndClassicReaderHandleMinValueBlockCountSafely() throws Exception {
+    for (boolean fast : new boolean[] { true, false }) {
+      GenericDatumReader<Object> reader = 
arrayReader(Schema.create(Schema.Type.NULL), fast);
+      // Long.MIN_VALUE items (zigzag), a block byte-size of 0, then the
+      // end-of-array terminator. Negating Long.MIN_VALUE overflows, so this 
must
+      // not drive an allocation.
+      byte[] data = encodeVarints(Long.MIN_VALUE, 0L, 0L);
+      BinaryDecoder decoder = DecoderFactory.get().binaryDecoder(data, null);
+      Collection<?> result = (Collection<?>) reader.read(null, decoder);
+      assertEquals(0, result.size(), "fastReader=" + fast);
+    }
+  }
+
+  /**
+   * A legitimate array of nulls within the limit still decodes on both reader
+   * paths.
+   */
+  @Test
+  void fastAndClassicReaderDecodeSmallNullArray() throws Exception {
+    for (boolean fast : new boolean[] { true, false }) {
+      GenericDatumReader<Object> reader = 
arrayReader(Schema.create(Schema.Type.NULL), fast);
+      byte[] data = encodeVarints(1000L, 0L);
+      BinaryDecoder decoder = DecoderFactory.get().binaryDecoder(data, null);
+      Collection<?> result = (Collection<?>) reader.read(null, decoder);
+      assertEquals(1000, result.size(), "fastReader=" + fast);
+    }
+  }
+
+  /**
+   * Skipping a huge zero-byte array (e.g. during schema projection) is 
bounded by
+   * the heap-aware allocation cap, so it cannot loop unboundedly even though 
it
+   * reads nothing.
+   */
+  @Test
+  void skipArrayOfNullRejectsHugeCount() throws Exception {
+    
System.setProperty(SystemLimitException.MAX_COLLECTION_ALLOCATION_PROPERTY, 
"1000");
+    org.apache.avro.TestSystemLimitException.resetLimits();
+    try {
+      Schema schema = Schema.createArray(Schema.create(Schema.Type.NULL));
+      byte[] data = encodeVarints(200_000L, 0L);
+      BinaryDecoder decoder = DecoderFactory.get().binaryDecoder(data, null);
+      assertThrows(SystemLimitException.class, () -> 
GenericDatumReader.skip(schema, decoder));
+    } finally {
+      
System.clearProperty(SystemLimitException.MAX_COLLECTION_ALLOCATION_PROPERTY);
+      org.apache.avro.TestSystemLimitException.resetLimits();
+    }
+  }
+
+  /**
+   * A legitimate small zero-byte array is skipped without error.
+   */
+  @Test
+  void skipSmallNullArraySucceeds() throws Exception {
+    Schema schema = Schema.createArray(Schema.create(Schema.Type.NULL));
+    // 1000 nulls then the end marker, followed by a trailing long we can read 
to
+    // confirm the skip advanced correctly.
+    byte[] data = encodeVarints(1000L, 0L, 42L);
+    BinaryDecoder decoder = DecoderFactory.get().binaryDecoder(data, null);
+    GenericDatumReader.skip(schema, decoder);
+    assertEquals(42L, decoder.readLong());
+  }
+
+  /**
+   * Skipping a huge map is bounded by the structural collection cap.
+   */
+  @Test
+  void skipMapRejectsHugeCount() throws Exception {
+    Schema schema = Schema.createMap(Schema.create(Schema.Type.NULL));
+    // A single block declaring more than Integer.MAX_VALUE - 8 entries.
+    byte[] data = encodeVarints((long) Integer.MAX_VALUE, 0L);
+    BinaryDecoder decoder = DecoderFactory.get().binaryDecoder(data, null);
+    assertThrows(RuntimeException.class, () -> GenericDatumReader.skip(schema, 
decoder));
+  }
+
+  /**
+   * A writer array field that the reader schema omits is skipped during
+   * resolution through the decoder's skipArray; a huge count must be bounded 
on
+   * both the fast and classic reader paths.
+   */
+  @Test
+  void resolvingSkipOfHugeNullArrayFieldIsBounded() throws Exception {
+    System.setProperty(SystemLimitException.MAX_COLLECTION_LENGTH_PROPERTY, 
"1000");
+    org.apache.avro.TestSystemLimitException.resetLimits();
+    try {
+      Schema writer = new 
Schema.Parser().parse("{\"type\":\"record\",\"name\":\"R\",\"fields\":["
+          + 
"{\"name\":\"arr\",\"type\":{\"type\":\"array\",\"items\":\"null\"}},"
+          + "{\"name\":\"a\",\"type\":\"long\"}]}");
+      Schema reader = new Schema.Parser()
+          
.parse("{\"type\":\"record\",\"name\":\"R\",\"fields\":[{\"name\":\"a\",\"type\":\"long\"}]}");
+      byte[] data = encodeVarints(2000L, 0L, 42L);
+      for (boolean fast : new boolean[] { true, false }) {
+        GenericData data2 = new GenericData();
+        data2.setFastReaderEnabled(fast);
+        GenericDatumReader<Object> r = new GenericDatumReader<>(writer, 
reader, data2);
+        BinaryDecoder decoder = DecoderFactory.get().binaryDecoder(data, null);
+        assertThrows(RuntimeException.class, () -> r.read(null, decoder), 
"fastReader=" + fast);

Review Comment:
   Fixed in 6bf53e35d6: resolvingSkipOfHugeNullArrayFieldIsBounded now asserts 
SystemLimitException (with MAX_COLLECTION_LENGTH_PROPERTY=1000 and a 2000-entry 
block).



-- 
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]

Reply via email to