[
https://issues.apache.org/jira/browse/AVRO-4300?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18097415#comment-18097415
]
ASF subversion and git services commented on AVRO-4300:
-------------------------------------------------------
Commit 61270d45bf53d2b05f88d8b02c503e868e23fcaf in avro's branch
refs/heads/main from Ismaël Mejía
[ https://gitbox.apache.org/repos/asf?p=avro.git;h=61270d45bf ]
AVRO-4300: [java] Bound array/map allocation and skipping for zero-byte
elements and on the fast reader path (#3865)
* AVRO-4300: [java] Bound zero-byte array element allocation
An array whose element schema encodes to zero bytes (null, a zero-length
fixed, or a record with only zero-byte fields) consumes no input per element,
so the number of elements a block declares cannot be bounded by the bytes
remaining in the stream. ensureAvailableCollectionBytes therefore skips the
check for such elements, and the collection-length cap is Integer.MAX_VALUE-8
(a VM array-size ceiling, not a memory budget). A tiny payload declaring a huge
block count of such elements (e.g. {"type":"array","items":"null"} with a
count of 200,000,000) drives an unbounded backing-array allocation and exhausts
the heap. This affects both the classic GenericDatumReader.readArray path and
the default fast-reader path (FastReaderBuilder), which had no collection guard
at all.
Add SystemLimitException.checkMaxCollectionAllocation, a heap-aware cumulative
cap (default: maxMemory()/4/8 elements, overridable via the
org.apache.avro.limits.collectionItems.maxAllocation system property, mirroring
the existing decompression limit). Enforce it before allocating in both reader
paths, keyed on GenericDatumReader.isZeroByteSchema so only the unbounded
zero-byte case is affected; all other element types remain bounded by
ensureAvailableCollectionBytes and are unchanged. Maps are already bounded
because each entry carries a string key of at least one byte.
Assisted-by: GitHub Copilot:claude-opus-4.8
* AVRO-4300: [java] Apply the available-bytes check on the fast reader path
The fast reader (FastReaderBuilder, the default decode path) never received the
AVRO-4241 bytes-remaining guard: that change only touched the classic
GenericDatumReader. As a result an array of non-zero-byte elements with a huge
declared block count and no data (e.g. array<long>/array<int> with a count of
200,000,000) still pre-allocated new GenericData.Array<>((int) count) on the
default path and exhausted the heap.
Expose GenericDatumReader.ensureAvailableCollectionBytes and apply it, together
with the zero-byte allocation cap, before allocating each array block in
FastReaderBuilder, so the fast and classic readers enforce identical guards.
Maps were already safe on both paths (each entry carries a >=1-byte key).
Verified with a matrix of array<null|long|int> and map<null|long> at a huge
count under -Xmx256m: every combination is now rejected (SystemLimitException
for zero-byte elements, EOFException otherwise) on both reader paths instead of
OOM.
Assisted-by: GitHub Copilot:claude-opus-4.8
* AVRO-4300: [java] Cover all collection/reader combinations in tests
Add a matrix test asserting every collection kind is rejected (never OOM) with
a huge block count and no data, on both the fast (default) and classic reader:
array<null> via the heap-aware allocation cap (SystemLimitException) and
array<long>, array<int>, map<null>, map<long> via the bytes-remaining check
(EOFException) -- the full 5x2 set of combinations. Keep a cumulative
multi-block null test and a positive within-limit decode test on both readers.
Assisted-by: GitHub Copilot:claude-opus-4.8
* AVRO-4300: [java] Test negative block count is bounded on both readers
The C and Python collection-limit tests cover a negative block count (abs(count)
zero-byte elements preceded by a block byte-size); the Java tests did not. The
decoder normalizes the negative count to a positive one, which must still be
bounded by the heap-aware allocation cap. Add a test asserting this on both the
fast and classic reader paths.
Assisted-by: GitHub Copilot:claude-opus-4.8
* AVRO-4300: [java] Test Long.MIN_VALUE block count is handled safely
Mirror the C SDK's INT64_MIN edge case. Long.MIN_VALUE as a block count is the
pathological overflow: negating it overflows back to a negative value. Java's
decoder normalizes this to an empty collection (no allocation) rather than
rejecting it as the C SDK does, which is equally non-exploitable. Add a test
asserting the safe, allocation-free result on both the fast and classic reader
paths.
Assisted-by: GitHub Copilot:claude-opus-4.8
* AVRO-4300: [java] Bound the array/map skip path
The skip path was unbounded: BinaryDecoder.skipArray()/skipMap() returned
doSkipItems() without calling checkMaxCollectionLength, and
GenericDatumReader.skip() looped over the returned count. Skipping a huge block
of zero-byte elements (e.g. a writer array<null> field absent from the reader
schema, skipped during projection) could therefore loop unboundedly -- a CPU
exhaustion even though skipping reads and allocates nothing.
Two complementary bounds:
- BinaryDecoder.skipArray()/skipMap() now apply the structural collection cap
(checkMaxCollectionLength), mirroring readArrayStart()/readMapStart() and
covering the resolving-decoder projection skip path on both reader types.
- GenericDatumReader.skip() additionally bounds the cumulative count, using the
heap-aware allocation cap for zero-byte element arrays and the structural cap
otherwise, matching the read path and the other language SDKs.
Assisted-by: GitHub Copilot:claude-opus-4.8
* AVRO-4300: [java] Fix limit Javadoc markup; clamp allocation cap to VM limit
Addresses review feedback:
- Fix the malformed <li> markup in the limit-properties list (each item closed
</li> prematurely after the <tt> tag) and correct the bytes property name
(org.apache.avro.limits.bytes.maxLength) so the Javadoc renders correctly.
- Clamp maxCollectionAllocation to MAX_ARRAY_VM_LIMIT when refreshing limits,
so
a configured (or large-heap-derived) zero-byte allocation cap stays
consistent
with the other collection caps and cannot exceed the VM array ceiling.
Assisted-by: GitHub Copilot:claude-opus-4.8
* AVRO-4300: [java] Fix limit Javadoc grammar; make heap-cap test deterministic
Addresses review feedback:
- Javadoc: "read at once single sequence" -> "read in a single sequence".
- testCheckMaxCollectionAllocation asserts with MAX_ARRAY_VM_LIMIT + 1 instead
of Integer.MAX_VALUE - 8. Since the default allocation cap is derived from
the
heap and then clamped to MAX_ARRAY_VM_LIMIT, a value equal to that limit may
not exceed the computed default on a very large heap; +1 guarantees it does,
keeping the test deterministic across environments.
Assisted-by: GitHub Copilot:claude-opus-4.8
* AVRO-4300: [java] Reword zero-byte Javadoc; tighten skip test assertions
Addresses review feedback:
- Javadoc: "a self-referencing record" is not inherently zero-byte (the code
only computes a 0-byte minimum for some recursive schemas to break
recursion).
Reword the three occurrences to describe actual zero-byte encodings: null, a
zero-length fixed, or a record whose fields all encode to zero bytes.
- skipMapRejectsHugeCount now asserts UnsupportedOperationException (a count of
Integer.MAX_VALUE deterministically hits the VM structural-limit path), and
resolvingSkipOfHugeNullArrayFieldIsBounded asserts SystemLimitException, so
the
tests pin the intended exception rather than a generic RuntimeException.
Assisted-by: GitHub Copilot:claude-opus-4.8
* AVRO-4300: [java] Apply spotless formatting to reworded Javadoc
The zero-byte Javadoc rewording did not match the Eclipse formatter's
line-wrapping, failing the spotless check. Reflowed via `mvn spotless:apply`.
Assisted-by: GitHub Copilot:claude-opus-4.8
* AVRO-4300: [java] Clarify zero-byte comments to match the actual predicate
Addresses review feedback: the "encodes to zero bytes" wording in
GenericDatumReader (the array cap comment and isZeroByteSchema Javadoc) and
FastReaderBuilder is imprecise. The guard is minBytesPerElement(schema) == 0,
which is also true for recursive schemas whose cycle is broken by returning a 0
minimum. Reword to describe the "minimum encoded size is zero" predicate so the
docs match the actual condition under which the heap-aware cap applies.
Assisted-by: GitHub Copilot:claude-opus-4.8
* AVRO-4300: [java] Reject Long.MIN_VALUE block count as malformed
Addresses review feedback: doReadItemCount() negates a negative block count, but
Long.MIN_VALUE negates back to Long.MIN_VALUE (still negative). The single-arg
checkMaxCollectionLength does not reject negatives, so it was truncated via
(int) cast to 0, silently ending the collection without consuming the
end-of-array/map marker and desynchronizing decoding of subsequent fields.
Reject Long.MIN_VALUE outright as malformed, matching the C SDK, instead of
normalizing to an empty collection. Update the test to assert the rejection.
Assisted-by: GitHub Copilot:claude-opus-4.8
* AVRO-4300: [java] Clamp collection preallocation from declared block count
The array/map readers passed the declared block count straight to newArray/
newMap as the initial capacity. On a stream source the bytes-available guard is
skipped (BinaryDecoder.remainingBytes() returns -1 for sources other than
ByteArrayInputStream/ByteBufferInputStream), so a large declared count reaches
the allocation path directly and drives a huge up-front allocation (e.g.
new Object[count]) before a single element is read.
Clamp the initial capacity to a modest bound (MAX_COLLECTION_PREALLOC) via
initialCollectionCapacity(); the backing array/map still grows on demand as
elements are decoded, so a truncated or hostile stream now fails with
EOFException after a bounded allocation instead of attempting to preallocate
hundreds of millions of slots. Applies to both the classic and fast readers.
Assisted-by: GitHub Copilot:claude-opus-4.8
* AVRO-4300: [java] Reject out-of-range union and enum indices with
AvroTypeException
The union branch index and enum symbol index were used to index the branch/
symbol tables without an explicit range check on several paths, surfacing a
malformed index as a generic IndexOutOfBoundsException instead of a clear
Avro-specific error:
- Union: Symbol.Alternative.getSymbol (used by both the validating and resolving
decoders, and therefore by GenericDatumReader and the fast reader) indexed
symbols[] directly. Add a [0, size) check throwing AvroTypeException.
- Enum: ResolvingDecoder.readEnum returned the raw index in the no-adjustments
case and indexed the adjustment table otherwise, both without validation
(ValidatingDecoder.readEnum already checked). Add the range checks.
- The fast reader reads from a plain decoder (resolution is pre-baked), so its
union and enum readers need their own [0, length) checks.
Adds tests covering negative and too-large union and enum indices on both the
classic and fast reader paths.
Assisted-by: GitHub Copilot:claude-opus-4.8
* AVRO-4300: [java] Reword remaining "zero-byte" docs to the actual predicate
The maxAllocation limit is applied when a schema's minimum encoded size is zero
(GenericDatumReader.isZeroByteSchema, i.e. minBytesPerElement == 0), which also
covers recursive schemas whose cycle is conservatively broken with a 0 minimum.
Reword the remaining "encodes to zero bytes" wording -- the class Javadoc, the
MAX_COLLECTION_ALLOCATION_PROPERTY Javadoc, the SystemLimitException message,
and
the skipArray comment -- to say "minimum encoded size is zero" so the docs match
the condition under which the cap actually applies.
Assisted-by: GitHub Copilot:claude-opus-4.8
* AVRO-4300: [java] Half-open range in enum messages; finish zero-byte reword
Address review feedback:
- The enum out-of-range messages said "max is <size>", implying <size> is a
valid index. Reword to the half-open "must be in [0, <size>)" to match the
actual check and the union messages, in ResolvingDecoder (both the
no-adjustments and adjustments branches), FastReaderBuilder, and
ValidatingDecoder.
- Reword the remaining "encodes to zero bytes" wording in the
checkMaxCollectionAllocation Javadoc to "minimum encoded size is zero"
(including the recursive-schema case), matching isZeroByteSchema.
Assisted-by: GitHub Copilot:claude-opus-4.8
* AVRO-4300: [java] Enforce structural cap when skipping zero-byte arrays; tidy
Address review feedback:
- skip(ARRAY) bounded zero-byte-element arrays only by the heap-aware allocation
cap, so the configurable structural collection limit
(MAX_COLLECTION_LENGTH_PROPERTY) was not enforced cumulatively and a large
count split into many blocks could drive a long skip loop. Always enforce
checkMaxCollectionLength cumulatively, and additionally
checkMaxCollectionAllocation for zero-byte schemas.
- FastReaderBuilder.checkArrayBlock: skip the ensureAvailableCollectionBytes
call for zero-byte elements (it recomputes minBytesPerElement and no-ops),
applying the allocation cap directly instead.
- Reword the checkMaxCollectionAllocation summary line from "zero-byte-encoded"
to "minimum encoded size is zero".
Assisted-by: GitHub Copilot:claude-opus-4.8
* AVRO-4300: [java] Reword defaultMaxCollectionAllocation Javadoc
Update the last "zero-byte-encoded" wording (the private
defaultMaxCollectionAllocation helper) to "minimum encoded size is zero",
consistent with the public property/Javadoc and the isZeroByteSchema predicate.
Assisted-by: GitHub Copilot:claude-opus-4.8
* AVRO-4300: [java] Reword last zero-byte field doc; sort test imports
- Reword the maxCollectionAllocation field Javadoc from "zero-byte-encoded" to
"minimum encoded size is zero".
- Sort the java.io imports in TestGenericDatumReader (BufferedInputStream before
the ByteArray* imports) to satisfy Spotless.
Assisted-by: GitHub Copilot:claude-opus-4.8
* AVRO-4300: [java] Reject Long.MIN_VALUE and negative byte-size in doSkipItems
doSkipItems accepted a Long.MIN_VALUE block count (treating it as a byte-sized
block and continuing to skip), inconsistent with doReadItemCount which rejects
it. Reject Long.MIN_VALUE, and also reject a negative block byte-size, so the
skip path fails fast on malformed input like the read path.
Assisted-by: GitHub Copilot:claude-opus-4.8
* AVRO-4300: [java] Reject negative block byte-size in doReadItemCount
doReadItemCount consumed the block byte-size for a negative-count block without
validating it. doSkipItems now rejects a negative byte-size, so the read path
does the same for consistency and to reject malformed encodings.
Assisted-by: GitHub Copilot:claude-opus-4.8
* AVRO-4300: [java] Clarify test Javadoc: trailing 0L varint, not "no data"
The huge-collection matrix payload includes a trailing 0L varint (an element
value for arrays, a 0-length key for maps); reword "no element data" to reflect
that.
Assisted-by: GitHub Copilot:claude-opus-4.8
* AVRO-4300: [java] Reword heap-fraction Javadoc to the actual predicate
Reword the DEFAULT_MAX_COLLECTION_ALLOCATION_HEAP_FRACTION Javadoc from
"zero-byte elements" to "elements whose minimum encoded size is zero".
Assisted-by: GitHub Copilot:claude-opus-4.8
* AVRO-4300: [java] Make array preallocation-clamp test regression-proof
arrayHugeCountOnStreamClampsPreallocation only asserted an EOFException, which a
reintroduced new Object[(int) count] preallocation could still satisfy on a
large-heap JVM (allocating ~200M slots then hitting EOF). Override newArray to
record the largest requested capacity and assert it stays clamped to
initialCollectionCapacity, and assert remainingBytes() == -1 to confirm the
unknown-remaining-bytes precondition.
Assisted-by: GitHub Copilot:claude-opus-4.8
* AVRO-4300: [java] Address review nits: use isZeroByteSchema, tidy block-count
guards
Apply RyanSkraba's review feedback:
- GenericDatumReader.readArray uses the isZeroByteSchema helper instead
of inlining minBytesPerElement == 0, matching skip().
- BinaryDecoder.doReadItemCount rejects Long.MIN_VALUE before consuming
the block byte-size, so no readLong runs once the count is invalid.
- BinaryDecoder.doSkipItems drops the readLong before throwing on an
invalid Long.MIN_VALUE count, consistent with doReadItemCount.
- FastReaderBuilder.createArrayReader drops the redundant comment;
the rationale is documented on checkArrayBlock.
> [java] Bound array/map allocation and skipping when decoding on both the
> classic and fast readers (zero-byte elements + missing fast-reader
> available-bytes check)
> ------------------------------------------------------------------------------------------------------------------------------------------------------------------
>
> Key: AVRO-4300
> URL: https://issues.apache.org/jira/browse/AVRO-4300
> Project: Apache Avro
> Issue Type: Sub-task
> Components: java
> Affects Versions: 1.11.5, 1.12.1
> Reporter: Ismaël Mejía
> Assignee: Ismaël Mejía
> Priority: Major
> Labels: pull-request-available
> Fix For: 1.12.2
>
> Time Spent: 6h
> Remaining Estimate: 0h
>
> When GenericDatumReader decodes an array it reads the block item count from
> the stream and pre-allocates the backing store (newArray -> new
> Object[count]) before decoding any element. Several independent gaps let a
> tiny payload drive an unbounded allocation or an unbounded skip loop and
> exhaust the heap:
> 1. Zero-byte elements (classic and fast reader).
> Elements whose schema encodes to zero bytes (null, a zero-length fixed, or a
> record with only zero-byte fields) consume no input, so
> ensureAvailableCollectionBytes (AVRO-4241) skips the check for them
> (minBytesPerElement == 0). The collection-length cap is Integer.MAX_VALUE-8,
> a JVM array-size ceiling rather than a memory budget. So an array such as
> {"type":"array","items":"null"} declaring a block count of 200,000,000 is a
> ~6 byte payload that allocates a 200M-slot array (~1.6 GB).
> 2. The fast reader never had the AVRO-4241 available-bytes guard at all.
> AVRO-4241 only modified the classic GenericDatumReader (and BinaryDecoder /
> Decoder / ValidatingDecoder); it never touched FastReaderBuilder, which is
> the default decode path (avro.io.fastread defaults to true). As a result, on
> the default reader even a non-zero-byte array such as array<long> or
> array<int> with a huge block count and no data pre-allocated new
> GenericData.Array<>((int) count) and exhausted the heap.
> 3. The skip path was unbounded.
> BinaryDecoder.skipArray()/skipMap() returned doSkipItems() without applying
> the collection cap, and GenericDatumReader.skip() looped over that count, so
> skipping a huge block of zero-byte elements (e.g. a writer array<null> field
> absent from the reader schema, skipped during projection) could loop
> unboundedly even though skipping allocates nothing.
> Fix (applied identically on the classic and fast reader paths):
> - Add SystemLimitException.checkMaxCollectionAllocation, a heap-aware
> cumulative cap for zero-byte elements (default maxMemory()/4/8 elements,
> overridable via the org.apache.avro.limits.collectionItems.maxAllocation
> system property, mirroring the existing decompression limit).
> - Expose GenericDatumReader.ensureAvailableCollectionBytes and apply it,
> together with the zero-byte cap, before allocating each array block in
> FastReaderBuilder, so the fast and classic readers enforce identical guards.
> - Bound the skip path: BinaryDecoder.skipArray()/skipMap() now apply the
> structural collection cap (covering the resolving-decoder projection skip on
> both reader types), and GenericDatumReader.skip() additionally bounds the
> cumulative count using the heap-aware cap for zero-byte elements and the
> structural cap otherwise.
> - Maps were already safe against pre-allocation on both paths because each
> entry carries a string key of at least one byte.
> Verification: a full matrix of array<null|long|int> and map<null|long> at
> count 200,000,000 under -Xmx256m is rejected on both the fast and classic
> readers (SystemLimitException for zero-byte elements, EOFException otherwise)
> instead of OutOfMemoryError; the skip path is bounded on both readers
> including under schema resolution; legitimate collections within the limit
> still decode. This is part of AVRO-4292.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
