iemejia commented on code in PR #3865:
URL: https://github.com/apache/avro/pull/3865#discussion_r3567324619
##########
lang/java/avro/src/main/java/org/apache/avro/SystemLimitException.java:
##########
@@ -28,13 +28,19 @@
* The following system properties can be set to limit the size of bytes,
* strings and collection types to be allocated:
* <ul>
- * <li><tt>org.apache.avro.limits.byte.maxLength</tt></li> limits the maximum
- * size of <tt>byte</tt> types.</li>
- * <li><tt>org.apache.avro.limits.collectionItems.maxLength</tt></li> limits
the
- * maximum number of <tt>map</tt> and <tt>list</tt> items that can be read at
- * once single sequence.</li>
- * <li><tt>org.apache.avro.limits.string.maxLength</tt></li> limits the maximum
- * size of <tt>string</tt> types.</li>
+ * <li><tt>org.apache.avro.limits.bytes.maxLength</tt> limits the maximum size
+ * of <tt>bytes</tt> types.</li>
+ * <li><tt>org.apache.avro.limits.collectionItems.maxLength</tt> limits the
+ * maximum number of <tt>map</tt> and <tt>list</tt> items that can be read in a
+ * single sequence.</li>
+ * <li><tt>org.apache.avro.limits.string.maxLength</tt> limits the maximum size
+ * of <tt>string</tt> types.</li>
+ * <li><tt>org.apache.avro.limits.collectionItems.maxAllocation</tt> limits the
+ * number of <tt>array</tt> elements whose schema encodes to zero bytes (such
as
+ * <tt>null</tt>, a zero-length <tt>fixed</tt>, or a record whose fields all
+ * encode to zero bytes) that may be allocated at once. Unlike other element
+ * types, these cannot be bounded by the number of bytes remaining in the
Review Comment:
Reworded in d6ddf5367b to 'array elements whose minimum encoded size is zero
(... or a recursive schema whose cycle is conservatively broken with a 0
minimum)', matching isZeroByteSchema/minBytesPerElement == 0.
##########
lang/java/avro/src/main/java/org/apache/avro/SystemLimitException.java:
##########
@@ -83,6 +89,32 @@ public class SystemLimitException extends
AvroRuntimeException {
public static final long MAX_DECOMPRESS_LENGTH =
getLongLimitFromProperty(MAX_DECOMPRESS_LENGTH_PROPERTY,
defaultMaxDecompressLength());
+ /**
+ * System property declaring the maximum number of zero-byte-encoded array
+ * elements (e.g. {@code null}, a zero-length fixed, or a record whose fields
+ * all encode to zero bytes) to allocate at once: {@value}.
+ */
Review Comment:
Updated the MAX_COLLECTION_ALLOCATION_PROPERTY Javadoc in d6ddf5367b to
describe elements 'whose minimum encoded size is zero', including the
recursive-schema case.
##########
lang/java/avro/src/main/java/org/apache/avro/SystemLimitException.java:
##########
@@ -246,6 +291,41 @@ 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}).
+ *
+ * @param existing The number of elements already allocated for the
collection.
+ * @param items The next number of elements to allocate.
+ * @return The cumulative element count if and only if it is within the
limit.
+ * @throws SystemLimitException if the cumulative allocation would exceed the
+ * limit.
+ * @throws AvroRuntimeException if either argument is negative.
+ */
+ public static long checkMaxCollectionAllocation(long existing, long items) {
+ if (existing < 0) {
+ throw new AvroRuntimeException("Malformed data. Length is negative: " +
existing);
+ }
+ if (items < 0) {
+ throw new AvroRuntimeException("Malformed data. Length is negative: " +
items);
+ }
+ long total = existing + items;
+ if (total < existing || total > maxCollectionAllocation) {
+ throw new SystemLimitException("Cannot allocate " + (total < existing ?
"more than Long.MAX_VALUE" : total)
+ + " zero-byte collection elements: exceeds the maximum allowed of "
+ maxCollectionAllocation
+ + " (configure with the system property " +
MAX_COLLECTION_ALLOCATION_PROPERTY + ")");
Review Comment:
Reworded the SystemLimitException message in d6ddf5367b to 'collection
elements whose minimum encoded size is zero' so it matches the predicate
operators will see.
##########
lang/java/avro/src/main/java/org/apache/avro/generic/GenericDatumReader.java:
##########
@@ -747,15 +802,31 @@ public static void skip(Schema schema, Decoder in) throws
IOException {
break;
case ARRAY:
Schema elementType = schema.getElementType();
+ // Bound the cumulative element count: skipping a huge block of zero-byte
+ // elements (e.g. null) would otherwise loop unboundedly (a CPU
+ // exhaustion) even though it reads nothing. Zero-byte elements use the
+ // heap-aware allocation cap; others the structural collection cap.
Review Comment:
Reworded the skipArray comment in d6ddf5367b to 'minimum encoded size is
zero', aligned with isZeroByteSchema.
--
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]
