iemejia commented on code in PR #3859:
URL: https://github.com/apache/avro/pull/3859#discussion_r3565199529
##########
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:
Fixed in 711e27b: minBytesPerElement() clamps the fixed size to INT64_MAX
and saturates the record field-minima sum, so an overflow can no longer wrap
negative and weaken the guard.
##########
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:
Fixed in 711e27b: the map's (1 + valueMin) is now saturated (only adds 1
when valueMin < INT64_MAX), so it cannot overflow to a negative minBytes.
--
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]
