iemejia opened a new pull request, #3865:
URL: https://github.com/apache/avro/pull/3865
## What is the purpose of the change
Fixes AVRO-4300 (sub-task of AVRO-4277). 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.
Two independent gaps let a tiny payload drive an unbounded allocation 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`), and the
collection-length cap is `Integer.MAX_VALUE - 8` (a JVM array-size ceiling, not
a memory budget). 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.**
That change 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`). So 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.
Only the classic reader was protected.
### 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.
- Maps were already safe on both paths because each entry carries a string
key of at least one byte (`ensureAvailableMapBytes` on the classic path; key
reads consume bytes on the fast path).
## Verifying this change
This change added tests and can be verified as follows:
- Added `SystemLimitException.checkMaxCollectionAllocation` tests
(single/cumulative/negative/overflow and heap-derived default).
- Added a full 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 readers: `array<null>` → `SystemLimitException`; `array<long>`,
`array<int>`, `map<null>`, `map<long>` → `EOFException` (the full 5×2 set of
combinations), plus a cumulative multi-block null case and a positive
within-limit decode.
- Manually verified against a PoC (`array<null>`, block count 200,000,000)
under `-Xmx256m`: rejected with a clean `SystemLimitException` (no allocation)
instead of `OutOfMemoryError`, on both reader paths; legitimate collections
within the limit still decode.
- `mvn -pl avro test` for the `generic` and `io` packages passes (no
regressions); Spotless and Checkstyle are clean.
## Documentation
- Does this pull request introduce a new feature? no
- If yes, how is the feature documented? not applicable (adds the
`org.apache.avro.limits.collectionItems.maxAllocation` system property,
documented in `SystemLimitException` JavaDoc alongside the existing limit
properties)
--
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]
