Copilot commented on code in PR #3865:
URL: https://github.com/apache/avro/pull/3865#discussion_r3568131040
##########
lang/java/avro/src/test/java/org/apache/avro/generic/TestGenericDatumReader.java:
##########
@@ -216,6 +222,30 @@ void arrayOfIntsRejectsHugeCount() throws Exception {
assertThrows(EOFException.class, () -> reader.read(null, decoder));
}
+ /**
+ * On a stream source the decoder cannot know how many bytes remain, so the
+ * bytes-available guard is skipped and a large declared array count reaches
the
+ * allocation path directly. The initial backing-array capacity must be
clamped
+ * so a hostile count cannot drive a huge up-front allocation; a truncated
+ * stream therefore fails with {@link EOFException} (once the declared
elements
+ * cannot be read) rather than attempting to preallocate hundreds of
millions of
+ * slots.
+ */
+ @Test
+ void arrayHugeCountOnStreamClampsPreallocation() throws Exception {
+ Schema schema = Schema.createArray(Schema.create(Schema.Type.LONG));
+ GenericDatumReader<Object> reader = new GenericDatumReader<>(schema);
+
+ // A huge (non-zero-byte) block count followed by no element data.
Wrapping in
+ // a BufferedInputStream keeps the source from being a
ByteArrayInputStream, so
+ // it cannot report its remaining byte count (remainingBytes() == -1) and
the
+ // bytes-available guard is disabled -- exercising the preallocation clamp.
+ byte[] data = encodeVarints(200_000_000L);
+ InputStream stream = new BufferedInputStream(new
ByteArrayInputStream(data));
+ BinaryDecoder decoder = DecoderFactory.get().binaryDecoder(stream, null);
+ assertThrows(EOFException.class, () -> reader.read(null, decoder));
+ }
Review Comment:
`arrayHugeCountOnStreamClampsPreallocation` can miss regressions on
large-heap test runs: if a future change reintroduces `new Object[(int) count]`
preallocation, allocating ~200M slots (~1.6GiB) may succeed on a big CI JVM and
the test would still pass with an `EOFException`. Make the test deterministic
by asserting the decoder is on the unknown-remaining-bytes path and by
overriding `GenericDatumReader.newArray` to assert the initial allocation size
is clamped (via `GenericDatumReader.initialCollectionCapacity(...)`).
--
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]