Copilot commented on code in PR #3865:
URL: https://github.com/apache/avro/pull/3865#discussion_r3567569926
##########
lang/java/avro/src/test/java/org/apache/avro/generic/TestGenericDatumReader.java:
##########
@@ -303,4 +333,335 @@ 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.
+ */
Review Comment:
The Javadoc says this matrix uses a huge count with “no element data”, but
the payload includes at least a `0L` varint (used as an element value for
arrays<long/int> and as a 0-length key for maps). Rewording to “insufficient
element/value data” (or similar) makes the test description accurate.
--
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]
