iemejia opened a new pull request, #3927: URL: https://github.com/apache/avro/pull/3927
## What changes were proposed in this pull request? Follow-up to the AVRO-4241 / AVRO-4300 collection-allocation guards. Those changes capped the number of *zero-byte*-minimum collection elements (`null`, a zero-length `fixed`, an all-zero-byte record, or a recursive schema whose cycle is broken with a `0` minimum) that a decoder will allocate, since such elements consume no input and so cannot be bounded by the "bytes remaining" check. However, the cap was enforced **per collection**: `readArray`/`readCollection`, the static `skip`, and the fast-reader path each start counting from zero. Because an Avro container file carries its own schema, an attacker can declare a record with many `array<null>` fields, each block individually under the limit but jointly unbounded. A tiny payload therefore still drives a huge aggregate allocation (a record of ~16 `array<null>` fields near the per-array cap exhausts the heap; a handful burns tens of seconds of CPU). This is the Java counterpart of the Python fix in #3926 (AVRO-4296). ### Approach Track the cumulative zero-byte allocation **per decode** on a per-thread scope in `SystemLimitException`: - `beginCollectionAllocationScope()` / `endCollectionAllocationScope()` delimit a datum. Scopes **nest** via a depth counter: a delegated fast reader or a skipped writer field accumulates into the enclosing datum budget instead of resetting it; only the outermost scope resets the running total (and clears it on exit so nothing leaks to a later decode on the same thread). - `GenericDatumReader.read(D, Decoder)` and the static `GenericDatumReader.skip(Schema, Decoder)` open the scope in a `try/finally`. `read` covers the classic and (delegated) fast paths as well as `SpecificDatumReader`/`ReflectDatumReader`, which inherit it; `skip` covers schema-projection skips and `BinaryData` (each top-level `skip` is bounded per invocation). - A new cumulative `checkMaxCollectionAllocation(long items)` accumulates into the active scope. **Outside any scope it falls back to the existing per-collection check**, so no existing caller becomes stricter. The zero-byte call sites in `GenericDatumReader` (read/skip), `FastReaderBuilder`, and `ReflectDatumReader` now use it. Positive-size elements are unchanged: they remain bounded per collection by the bytes-remaining check, which consumes input as the position advances. ## How was this patch tested? - Added `recordOfNullArrayFieldsRejectedCumulativelyAcrossDatum` (a record with two `array<null>` fields of 600 each is rejected on the second field at a 1000-element cap) and `recordOfNullArrayFieldsWithinCumulativeLimitStillDecodes` (two 400-element fields still decode, and the budget resets between datums), both exercised on the fast and classic reader paths. - Existing `TestGenericDatumReader`, `TestReflectDatumReader`, and `TestSystemLimitException` pass, plus `TestBinaryData`, `TestDataFile*`, `TestGenericData`, `TestSpecificData`, and `TestResolvingIO` as regression coverage for the `skip`/compare and datafile paths. -- 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]
