Copilot commented on code in PR #3860:
URL: https://github.com/apache/avro/pull/3860#discussion_r3567335620
##########
lang/csharp/src/apache/main/Generic/GenericReader.cs:
##########
@@ -404,11 +404,38 @@ protected virtual object ReadArray(object reuse,
ArraySchema writerSchema, Schem
ArraySchema rs = (ArraySchema)readerSchema;
object result = CreateArray(reuse, rs);
int i = 0;
- for (int n = (int)d.ReadArrayStart(); n != 0; n =
(int)d.ReadArrayNext())
+ long minBytes = MinBytesPerElement(writerSchema.ItemSchema);
+ long total = 0;
+ for (long nl = d.ReadArrayStart(); nl != 0; nl = d.ReadArrayNext())
{
- if (GetArraySize(result) < (i + n)) ResizeArray(ref result, i
+ n);
+ // Reject a block whose element count could not be backed by
the
+ // bytes remaining (or, for zero-byte elements, that exceeds
the
+ // item cap) before allocating for it. Checked on the raw long,
+ // which also avoids the int cast below overflowing.
+ total = EnsureCollectionAvailable(d, total, nl, minBytes);
+ int n = (int)nl;
+ // Preallocate only a bounded amount up front, then grow on
demand
+ // below. On a non-seekable stream EnsureCollectionAvailable
cannot
+ // bound the count, so resizing straight to i+n could allocate
a
+ // huge array before any element is read; a truncated stream
instead
+ // fails within Read() after a bounded growth. Blocks no
larger than
+ // the cap keep the original single-resize fast path.
+ int prealloc = i + Math.Min(n, MaxCollectionPrealloc);
+ if (GetArraySize(result) < prealloc) ResizeArray(ref result,
prealloc);
for (int j = 0; j < n; j++, i++)
{
+ if (GetArraySize(result) <= i)
+ {
+ int current = GetArraySize(result);
+ int grown = current + (current >> 1) + 1;
+ if (grown <= i)
+ {
+ grown = i + 1;
+ }
+
+ ResizeArray(ref result, grown);
Review Comment:
The dynamic growth path in ReadArray can resize the backing array beyond the
enforced collection limits. `grown = current + (current >> 1) + 1` can
overshoot the allowed element count (e.g., zero-byte arrays capped by
MaxCollectionItems) and can even exceed
MaxDotNetArrayLength/MaxCollectionStructural, causing Array.Resize to throw a
runtime exception despite the declared count being within limits. Clamp `grown`
to the applicable cap before resizing so allocations stay within the
deterministic AvroException bounds.
--
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]
