Copilot commented on code in PR #3860:
URL: https://github.com/apache/avro/pull/3860#discussion_r3567462453
##########
lang/csharp/src/apache/main/Generic/GenericReader.cs:
##########
@@ -404,11 +404,52 @@ 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
Review Comment:
The new collection hardening here
(MinBytesPerElement/EnsureCollectionAvailable + bounded preallocation/growth)
only applies to DefaultReader/GenericReader. The main datum-reader path used by
GenericDatumReader<T>/SpecificDatumReader<T> (via PreresolvingDatumReader<T>)
still reads array/map blocks with `(int)decoder.ReadArrayStart()` /
`(int)decoder.ReadMapStart()` and then preallocates/loops based on that count
without any RemainingBytes/MaxCollection* checks (see
PreresolvingDatumReader.cs:374-399 and its skip lambdas at 487-505). That
leaves the original allocation/CPU DoS vector (especially for zero-byte
elements like `null`) when callers use those readers, and it also means the new
tests in BinaryCodecTests (which use GenericReader) won’t cover the common
GenericDatumReader/SpecificDatumReader code path.
--
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]
