Copilot commented on code in PR #3860:
URL: https://github.com/apache/avro/pull/3860#discussion_r3566951580
##########
lang/csharp/src/apache/main/Generic/GenericReader.cs:
##########
@@ -501,6 +514,141 @@ protected virtual object ReadMap(object reuse, MapSchema
writerSchema, Schema re
return result;
}
+ /// <summary>
+ /// Minimum number of bytes a single value of the given schema can
occupy
+ /// on the wire. Used to reject an array/map block count that could
not be
+ /// backed by the bytes remaining. A type that encodes to zero bytes
+ /// returns 0 (not only <c>null</c>, but also composites that encode to
+ /// nothing, e.g. a record whose fields are all zero-byte), which
disables
+ /// the bytes-remaining check for it (so an array of such elements is
not
+ /// falsely rejected; they are instead bounded by the zero-byte item
cap).
+ /// A depth limit breaks self-referencing schemas.
+ /// </summary>
+ private static int MinBytesPerElement(Schema schema, int depth = 0)
+ {
+ if (schema == null)
+ {
+ return 0;
+ }
+
+ switch (schema.Tag)
+ {
+ case Schema.Type.Null:
+ return 0;
+ case Schema.Type.Float:
+ return 4;
+ case Schema.Type.Double:
+ return 8;
+ case Schema.Type.Fixed:
+ return ((FixedSchema)schema).Size;
+ case Schema.Type.Record:
+ case Schema.Type.Error:
+ if (depth > 64)
+ {
+ // A cyclic or pathologically deep record. Return 1
(not
+ // 0) so the collection check stays enabled; a valid
+ // recursive value always encodes to >= 1 byte. The
depth
+ // guard is applied only here, so zero-byte leaf types
+ // such as null still return 0 regardless of depth.
+ return 1;
+ }
+
+ // Accumulate in a long and clamp so a deeply nested schema
+ // cannot overflow int into a value <= 0, which would
disable
+ // the collection check.
+ long total = 0;
+ foreach (Field f in (RecordSchema)schema)
+ {
+ total += MinBytesPerElement(f.Schema, depth + 1);
+ if (total >= int.MaxValue)
+ {
+ return int.MaxValue;
+ }
+ }
+
+ return (int)total;
+ default:
+ // boolean, int, long, bytes, string, enum, union, array,
map:
+ // all encode to at least one byte.
+ return 1;
+ }
+ }
+
+ // Collection allocation limits, guarding against a block-count DoS.
Both
+ // default to the same values as the other Avro SDKs and can be
overridden
+ // (to a single value capping both) via the AVRO_MAX_COLLECTION_ITEMS
+ // environment variable.
+ private static readonly long MaxCollectionItems =
ReadCollectionLimit(10_000_000L);
+ // The structural cap is additionally clamped to int.MaxValue: the
callers
+ // cast the (cumulative) block count to int to size .NET collections,
so a
+ // structural limit above int.MaxValue (e.g. from a large env override)
+ // would reintroduce an int-overflow on that cast.
+ private static readonly long MaxCollectionStructural =
+ Math.Min(ReadCollectionLimit(2147483639L), int.MaxValue);
+
Review Comment:
MaxCollectionStructural is clamped to int.MaxValue, but the default reader
allocates object[] via Array.Resize, whose maximum supported length is smaller
than int.MaxValue on some TFMs (e.g., 0x7FFFFFC7 on modern .NET, 0x3FFFFFFF on
older runtimes). As a result, a collection size that passes
EnsureCollectionAvailable can still trigger a runtime exception
(OutOfMemoryException/Overflow) during ResizeArray rather than a deterministic
AvroException. Consider clamping the structural limit to the runtime’s max
array length as well, mirroring BinaryDecoder’s MaxDotNetArrayLength behavior.
--
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]
