iemejia opened a new pull request, #3926:
URL: https://github.com/apache/avro/pull/3926

   ## What changes were proposed in this pull request?
   
   Follow-up to #3861 (AVRO-4296). That change capped the number of 
*zero-byte*-encoded collection elements (`null`, a zero-length `fixed`, or a 
record whose fields are all zero-byte) 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**: `read_array`/`read_map` 
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 allocation:
   
   - 16 `array<null>` fields of ~10M each → an ~80-byte record that raises 
`MemoryError`
   - 8 `array<null>` fields → an ~40-byte record that burns tens of seconds of 
CPU while allocating ~80M `None` references
   
   This PR makes the zero-byte-element budget **cumulative across a single 
decoded datum** rather than per collection:
   
   - The `DatumReader` tracks `self._zero_byte_items_read`, reset at the start 
of each top-level `read()` — the boundary `DataFileReader` uses per record.
   - `_ensure_collection_available` checks the running total for zero-byte 
elements.
   - Positive-size elements are unchanged: they are naturally bounded per 
collection because decoding consumes input and the bytes-remaining check 
shrinks as the position advances. As a side benefit, nested collections such as 
`array<array<null>>` are now also bounded in aggregate instead of getting a 
fresh budget per inner array.
   
   ## How was this patch tested?
   
   - Added `test_record_of_array_of_null_fields_cumulative_across_datum` (a 
multi-field record rejected once its combined zero-byte count exceeds the cap) 
and `test_record_of_array_of_null_fields_within_datum_limit_reads` (a 
within-limit record still decodes, and the budget resets between datums).
   - Existing `TestDatumReaderCollectionSizeLimit` and `test_datafile` suites 
pass.
   - Manually reproduced the reported amplification at the default 10M limit: 
8- and 16-field records are now rejected at the second field before allocating, 
while a single legitimate 9,999,999-element `array<null>` still reads.
   
   An equivalent fix for the Java SDK is tracked separately.


-- 
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]

Reply via email to