iemejia commented on code in PR #3865:
URL: https://github.com/apache/avro/pull/3865#discussion_r3567381105
##########
lang/java/avro/src/main/java/org/apache/avro/io/ResolvingDecoder.java:
##########
@@ -260,8 +260,18 @@ public int readEnum() throws IOException {
Symbol.EnumAdjustAction top = (Symbol.EnumAdjustAction) parser.popSymbol();
int n = in.readEnum();
if (top.noAdjustments) {
+ // n is used directly as an index into the reader enum's symbols, so it
+ // must fall within the reader symbol count.
+ if (n < 0 || n >= top.size) {
+ throw new AvroTypeException("Enumeration out of range: max is " +
top.size + " but received " + n);
+ }
Review Comment:
Fixed — reworded to 'must be in [0, ' + top.size + '), but received ...'
(half-open), matching the check. Also applied to ValidatingDecoder for
consistency.
##########
lang/java/avro/src/main/java/org/apache/avro/io/ResolvingDecoder.java:
##########
@@ -260,8 +260,18 @@ public int readEnum() throws IOException {
Symbol.EnumAdjustAction top = (Symbol.EnumAdjustAction) parser.popSymbol();
int n = in.readEnum();
if (top.noAdjustments) {
+ // n is used directly as an index into the reader enum's symbols, so it
+ // must fall within the reader symbol count.
+ if (n < 0 || n >= top.size) {
+ throw new AvroTypeException("Enumeration out of range: max is " +
top.size + " but received " + n);
+ }
return n;
}
+ // Otherwise n indexes the writer-to-reader adjustment table; reject an
index
+ // outside it rather than letting the array access throw.
+ if (n < 0 || n >= top.adjustments.length) {
+ throw new AvroTypeException("Enumeration out of range: max is " +
top.adjustments.length + " but received " + n);
+ }
Review Comment:
Fixed — the adjustments branch now uses the half-open '[0,
top.adjustments.length)' wording.
##########
lang/java/avro/src/main/java/org/apache/avro/io/FastReaderBuilder.java:
##########
@@ -469,39 +474,78 @@ 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).
+ boolean zeroByteElements =
GenericDatumReader.isZeroByteSchema(elementType);
return reusingReader((reuse, decoder) -> {
if (reuse instanceof GenericArray) {
GenericArray<Object> reuseArray = (GenericArray<Object>) reuse;
long l = decoder.readArrayStart();
+ long total = 0;
+ checkArrayBlock(decoder, elementType, zeroByteElements, total, l);
reuseArray.clear();
while (l > 0) {
for (long i = 0; i < l; i++) {
reuseArray.add(elementReader.read(reuseArray.peek(), decoder));
}
+ total += l;
l = decoder.arrayNext();
+ checkArrayBlock(decoder, elementType, zeroByteElements, total, l);
}
return reuseArray;
} else {
long l = decoder.readArrayStart();
+ long total = 0;
+ checkArrayBlock(decoder, elementType, zeroByteElements, total, l);
List<Object> array = (reuse instanceof List) ? (List<Object>) reuse
- : new GenericData.Array<>((int) l, readerSchema);
+ : new
GenericData.Array<>(GenericDatumReader.initialCollectionCapacity(l),
readerSchema);
array.clear();
while (l > 0) {
for (long i = 0; i < l; i++) {
array.add(elementReader.read(null, decoder));
}
+ total += l;
l = decoder.arrayNext();
+ checkArrayBlock(decoder, elementType, zeroByteElements, total, l);
}
return array;
}
});
}
+ /**
+ * Validates an array block count before its elements are allocated, applying
+ * the same guards as the classic {@code GenericDatumReader}: the
+ * bytes-remaining check for elements with a positive minimum size, and the
+ * heap-aware allocation cap for zero-byte elements (which the bytes check
+ * cannot bound).
+ */
+ private static void checkArrayBlock(Decoder decoder, Schema elementType,
boolean zeroByteElements, long total,
+ long count) throws IOException {
+ if (count <= 0) {
+ return;
+ }
+ GenericDatumReader.ensureAvailableCollectionBytes(decoder, count,
elementType);
+ if (zeroByteElements) {
+ SystemLimitException.checkMaxCollectionAllocation(total, count);
+ }
+ }
+
private FieldReader createEnumReader(EnumAdjust action) {
return reusingReader((reuse, decoder) -> {
int index = decoder.readEnum();
+ if (index < 0 || index >= action.values.length) {
+ throw new AvroTypeException(
+ "Enumeration out of range: max is " + action.values.length + " but
received " + index);
+ }
Review Comment:
Fixed — FastReaderBuilder's enum message now uses '[0,
action.values.length)'.
##########
lang/java/avro/src/main/java/org/apache/avro/SystemLimitException.java:
##########
@@ -246,6 +293,42 @@ 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}, a
zero-length
+ * fixed, or a record whose fields all encode to zero bytes) consume no input
+ * bytes, so the number that may be declared is not bounded by the bytes
+ * remaining in the stream. Without a cap, a tiny payload can declare an
+ * enormous block count and drive an unbounded backing-array allocation. This
+ * limit is derived from the maximum heap (see
+ * {@link #MAX_COLLECTION_ALLOCATION_PROPERTY}).
Review Comment:
Reworded the checkMaxCollectionAllocation Javadoc to 'minimum encoded size
is zero' (incl. recursive schemas), matching
isZeroByteSchema/minBytesPerElement == 0.
--
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]
