iemejia commented on code in PR #3866:
URL: https://github.com/apache/avro/pull/3866#discussion_r3566777165
##########
lang/js/lib/schemas.js:
##########
@@ -2153,6 +2199,74 @@ function readArraySize(tap) {
return n;
}
+/**
+ * Minimum number of bytes a value of the given type can occupy on the wire.
+ *
+ * Used to bound collection block counts against the bytes actually remaining.
+ * Memoized on the type; recursive records are handled with a `seen` guard
+ * (a directly self-referential required field cannot encode in finite bytes,
+ * and self-references reached through an array/map/union already contribute at
+ * least one byte before recursing, so returning 0 on a cycle is safe).
+ */
+function getMinBytes(type, seen) {
+ if (type._minBytes !== undefined) {
+ return type._minBytes;
+ }
+ var mb;
+ if (type instanceof NullType) {
+ mb = 0;
+ } else if (type instanceof FixedType) {
+ mb = type._size;
+ } else if (type instanceof FloatType) {
+ mb = 4;
+ } else if (type instanceof DoubleType) {
+ mb = 8;
+ } else if (type instanceof LogicalType) {
+ mb = getMinBytes(type._underlyingType, seen);
+ } else if (type instanceof RecordType) {
+ seen = seen || [];
+ if (~seen.indexOf(type)) {
+ return 0; // Cycle; see note above.
+ }
+ seen.push(type);
+ mb = 0;
+ var fields = type._fields;
+ for (var i = 0, l = fields.length; i < l; i++) {
+ mb += getMinBytes(fields[i]._type, seen);
+ }
+ seen.pop();
+ } else {
+ // Booleans, ints, longs, strings, bytes, enums (index), unions (index),
+ // and arrays/maps (empty block terminator) all occupy at least one byte.
+ mb = 1;
+ }
Review Comment:
Fixed in 6ae9c61c35: getMinBytes now handles unions as 1 (branch index) +
the smallest branch minimum, so ['int','long'] is at least 2 bytes and the
available-bytes check isn't underestimated.
##########
lang/js/lib/schemas.js:
##########
@@ -1162,12 +1188,16 @@ MapType.prototype._read = function (tap) {
MapType.prototype._skip = function (tap) {
var values = this._values;
+ var minBytes = 1 + getMinBytes(values); // Each entry carries a >=1-byte key.
+ var total = 0;
var len, n;
while ((n = tap.readLong())) {
if (n < 0) {
len = tap.readLong();
tap.pos += len;
} else {
Review Comment:
Fixed in 6ae9c61c35: the negative (byte-sized) block branch of _skip now
applies the cumulative total and checkCollectionBlock too, so the caps can't be
bypassed with a negative count. Added a regression test.
##########
lang/js/lib/schemas.js:
##########
@@ -1303,14 +1341,19 @@ ArrayType.prototype._read = function (tap) {
};
ArrayType.prototype._skip = function (tap) {
+ var items = this._items;
+ var minBytes = getMinBytes(items);
+ var total = 0;
var len, n;
while ((n = tap.readLong())) {
if (n < 0) {
len = tap.readLong();
tap.pos += len;
} else {
Review Comment:
Fixed in 6ae9c61c35: same fix applied to the other collection's _skip
negative-block branch.
--
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]
