Copilot commented on code in PR #3865:
URL: https://github.com/apache/avro/pull/3865#discussion_r3567131387
##########
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:
The maxAllocation property is enforced for schemas where the *minimum
encoded size* is 0 (see GenericDatumReader.isZeroByteSchema), which can include
some recursive schemas where the cycle is conservatively broken with a 0
minimum. This Javadoc currently says the schema “encodes to zero bytes”, which
is not always accurate for that recursive case; rewording will prevent
misleading documentation about when the limit applies.
##########
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:
The exception message says “zero-byte collection elements”, but the check is
triggered when the schema’s *minimum* encoded size is 0 (which can include some
recursive schemas that do not always encode to zero bytes). Consider rewording
the message to match the predicate so operators aren’t misled while diagnosing
failures.
##########
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:
This constant’s Javadoc describes “zero-byte-encoded” elements, but the
guard is applied for schemas whose minimum encoded size is 0 (including some
recursive schemas where the minimum is conservatively treated as 0). Rewording
here keeps the public property documentation consistent with the actual
predicate.
##########
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:
This comment describes these as “zero-byte” elements, but the branch
condition is isZeroByteSchema(elementType) (minBytesPerElement == 0), which can
also be true for some recursive schemas where the minimum is conservatively
treated as 0. Rewording the comment to “minimum encoded size is 0” keeps it
aligned with the actual predicate and nearby documentation.
--
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]
