iemejia commented on code in PR #3865:
URL: https://github.com/apache/avro/pull/3865#discussion_r3566979979
##########
lang/java/avro/src/main/java/org/apache/avro/generic/GenericDatumReader.java:
##########
@@ -296,6 +297,15 @@ protected Object readArray(Object old, Schema expected,
ResolvingDecoder in) thr
long base = 0;
if (l > 0) {
ensureAvailableCollectionBytes(in, l, expectedType);
+ // Elements whose schema encodes to zero bytes (null, or a
self-referencing
+ // record) consume no 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.
Review Comment:
Reworded in dc2250c0ce to describe the 'minimum encoded size is zero'
predicate, explicitly noting recursive schemas whose cycle is broken with a 0
minimum.
##########
lang/java/avro/src/main/java/org/apache/avro/generic/GenericDatumReader.java:
##########
@@ -441,6 +455,20 @@ static int minBytesPerElement(Schema schema) {
return minBytesPerElement(schema, Collections.newSetFromMap(new
IdentityHashMap<>()));
}
+ /**
+ * Whether values of the given schema encode to zero bytes (e.g. {@code
null},
+ * zero-length {@code fixed}, or a record whose fields are all zero-byte).
Such
+ * elements cannot be bounded by the number of bytes remaining in the
stream, so
+ * a collection of them must be bounded by a heap-aware allocation limit
+ * instead.
Review Comment:
Updated in dc2250c0ce: the Javadoc now states this is a conservative
'minimum encoded size is zero' check (`minBytesPerElement == 0`), including
recursive schemas, rather than strictly 'encodes to zero bytes'.
##########
lang/java/avro/src/main/java/org/apache/avro/generic/GenericDatumReader.java:
##########
@@ -441,6 +455,20 @@ static int minBytesPerElement(Schema schema) {
return minBytesPerElement(schema, Collections.newSetFromMap(new
IdentityHashMap<>()));
}
+ /**
+ * Whether values of the given schema encode to zero bytes (e.g. {@code
null},
+ * zero-length {@code fixed}, or a record whose fields are all zero-byte).
Such
+ * elements cannot be bounded by the number of bytes remaining in the
stream, so
+ * a collection of them must be bounded by a heap-aware allocation limit
+ * instead.
+ *
+ * @param schema the element (or map value) schema
+ * @return {@code true} if the schema encodes to zero bytes
Review Comment:
Updated the @return in dc2250c0ce to 'true if the schema's minimum encoded
size is zero'.
##########
lang/java/avro/src/main/java/org/apache/avro/io/FastReaderBuilder.java:
##########
@@ -469,36 +470,70 @@ 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 schema encodes to zero bytes (null, or a record with only
+ // zero-byte fields) consume no 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:
Reworded in dc2250c0ce to match the `isZeroByteSchema`/`minBytesPerElement
== 0` predicate, including the recursive-schema case.
--
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]
