Copilot commented on code in PR #3859:
URL: https://github.com/apache/avro/pull/3859#discussion_r3565162121
##########
lang/c++/impl/Generic.cc:
##########
@@ -25,6 +25,74 @@ using std::ostringstream;
using std::string;
using std::vector;
+// Minimum number of bytes a single value of the given schema node 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 can encode to zero bytes (null) returns 0,
+// which disables the collection check for it (so an array of nulls is not
+// falsely rejected). A depth limit breaks self-referencing (symbolic) schemas.
+static int64_t minBytesPerElement(const NodePtr &node, int depth) {
+ if (!node) {
+ return 0;
+ }
+ if (depth > 64) {
+ // A cyclic or pathologically deep schema. Return 1 (not 0) so the
+ // collection check stays enabled rather than being silently bypassed;
+ // a valid recursive value always encodes to at least 1 byte.
+ return 1;
+ }
+ switch (node->type()) {
+ case AVRO_NULL:
+ return 0;
+ case AVRO_FLOAT:
+ return 4;
+ case AVRO_DOUBLE:
+ return 8;
+ case AVRO_FIXED:
+ return static_cast<int64_t>(node->fixedSize());
+ case AVRO_RECORD: {
+ int64_t total = 0;
+ for (size_t i = 0; i < node->leaves(); ++i) {
+ total += minBytesPerElement(node->leafAt(i), depth + 1);
+ }
+ return total;
+ }
+ default:
+ // boolean, int, long, bytes, string, enum, union, array, map and
+ // symbolic references all encode to at least one byte.
+ return 1;
+ }
Review Comment:
minBytesPerElement() can overflow int64_t for AVRO_FIXED (fixedSize() >
INT64_MAX) or for AVRO_RECORD when summing child minima. If the overflow wraps
negative/small, ensureCollectionAvailable() may treat minBytes<=0 (or a
too-small value) and skip/underenforce the available-bytes guard, weakening the
OOM hardening this PR adds. Consider saturating these calculations to INT64_MAX.
##########
lang/c++/impl/Generic.cc:
##########
@@ -119,7 +194,11 @@ void GenericReader::read(GenericDatum &datum, Decoder &d,
bool isResolving) {
const NodePtr &nn = v.schema()->leafAt(1);
r.resize(0);
size_t start = 0;
+ // Map keys are strings (>= 1 byte length prefix) plus the value.
+ int64_t minBytes = isResolving ? 0 : (1 + minBytesPerElement(nn,
0));
Review Comment:
In map decoding, minBytes is computed as (1 + minBytesPerElement(...)). If
minBytesPerElement ever returns a very large value (e.g., from a large
fixed/record), this addition can overflow int64_t and turn minBytes negative,
which would disable ensureCollectionAvailable() (minBytes <= 0) and bypass the
intended pre-allocation guard.
--
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]
