iemejia commented on code in PR #3865:
URL: https://github.com/apache/avro/pull/3865#discussion_r3610587128
##########
lang/java/avro/src/main/java/org/apache/avro/generic/GenericDatumReader.java:
##########
@@ -296,9 +317,20 @@ protected Object readArray(Object old, Schema expected,
ResolvingDecoder in) thr
long base = 0;
if (l > 0) {
ensureAvailableCollectionBytes(in, l, expectedType);
+ // Elements whose minimum encoded size is zero (null, a zero-length
fixed, a
+ // record whose fields are all zero-byte, or a recursive schema where the
+ // cycle is broken with a 0 minimum) consume no guaranteed input, so
+ // ensureAvailableCollectionBytes cannot bound their count from the bytes
+ // remaining. Cap such collections against a heap-aware limit so a tiny
+ // payload cannot declare a huge block count and drive an unbounded
+ // backing-array allocation.
+ boolean zeroByteElements = minBytesPerElement(expectedType) == 0;
Review Comment:
Applied in d01583e784 — using `isZeroByteSchema(expectedType)` here,
matching the predicate helper already used in `skip()`. Thanks for catching the
inconsistency.
##########
lang/java/avro/src/main/java/org/apache/avro/io/BinaryDecoder.java:
##########
@@ -408,8 +408,21 @@ protected void doReadBytes(byte[] bytes, int start, int
length) throws IOExcepti
protected long doReadItemCount() throws IOException {
long result = readLong();
if (result < 0L) {
- // Consume byte-count if present
- readLong();
+ // A negative block count is followed by a block byte-size; consume it.
+ final long bytecount = readLong();
+ if (result == Long.MIN_VALUE) {
Review Comment:
Good point — fixed in d01583e784: moved the `Long.MIN_VALUE` check to the
top of the `if (result < 0L)` block so we reject before consuming the block
byte-size. No `readLong` runs once we've decided the count is invalid.
##########
lang/java/avro/src/main/java/org/apache/avro/io/BinaryDecoder.java:
##########
@@ -436,7 +449,17 @@ protected long doReadItemCount() throws IOException {
private long doSkipItems() throws IOException {
long result = readLong();
while (result < 0L) {
+ if (result == Long.MIN_VALUE) {
+ // Consistent with doReadItemCount: Long.MIN_VALUE is not a valid block
+ // count (it cannot be negated), so reject it rather than treating it
as
+ // a byte-sized block and continuing to skip.
+ readLong();
Review Comment:
Removed the `readLong()` in d01583e784 — no reason to consume the byte-size
once we've determined the block count is invalid. Now consistent with the
reordered `doReadItemCount()`.
##########
lang/java/avro/src/main/java/org/apache/avro/io/FastReaderBuilder.java:
##########
@@ -469,39 +474,82 @@ private Optional<Class<?>> findClass(String clazz) {
@SuppressWarnings("unchecked")
private FieldReader createArrayReader(Schema readerSchema, Container action)
throws IOException {
FieldReader elementReader = getReaderFor(action.elementAction, null);
+ Schema elementType = readerSchema.getElementType();
+
+ // Elements whose minimum encoded size is zero (null, a record with only
+ // zero-byte fields, or a recursive schema whose cycle is broken with a 0
+ // minimum) consume no guaranteed input, so the block count cannot be
bounded
+ // by the bytes remaining in the stream. Cap such collections against a
+ // heap-aware limit so a tiny payload cannot declare a huge block count and
+ // drive an unbounded backing-array allocation (AVRO-4300).
Review Comment:
Trimmed in d01583e784 — removed the verbose comment; `isZeroByteSchema` is
self-explanatory and the rationale is already documented on `checkArrayBlock`.
--
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]
