Copilot commented on code in PR #3861:
URL: https://github.com/apache/avro/pull/3861#discussion_r3567431240


##########
lang/py/avro/io.py:
##########
@@ -246,6 +286,11 @@ def read_long(self) -> int:
         n = b & 0x7F
         shift = 7
         while (b & 0x80) != 0:
+            # A 64-bit value needs at most 10 bytes (shifts 0..63); reject an
+            # overlong varint rather than accepting a malformed, arbitrarily
+            # large value.
+            if shift >= 70:
+                raise avro.errors.InvalidAvroBinaryEncoding("Varint is too 
long")
             b = ord(self.read(1))
             n |= (b & 0x7F) << shift
             shift += 7

Review Comment:
   The new overlong-varint guard rejects encodings longer than 10 bytes, but a 
10-byte varint can still represent values wider than 64 bits (the 10th byte 
must only carry 0 or 1 in its low 7 bits for a valid 64-bit zig-zag long). 
Without this, malformed inputs can still decode to out-of-range integers and 
drive extremely large length/count values.



##########
lang/py/avro/io.py:
##########
@@ -599,6 +644,90 @@ def write_timestamp_micros_long(self, datum: 
datetime.datetime) -> None:
 #
 # DatumReader/Writer
 #
+#: Environment variable overriding the maximum number of zero-byte-encoded
+#: collection elements (e.g. an array of nulls) to allocate from a single 
decode.
+MAX_COLLECTION_ITEMS_ENV = "AVRO_MAX_COLLECTION_ITEMS"
+
+#: Default maximum number of zero-byte-encoded collection elements to allocate.
+#: Elements whose schema encodes to zero bytes (``null``, a zero-length 
``fixed``,
+#: or a record with only zero-byte fields) consume no input, so the 
bytes-remaining
+#: check cannot bound their count; without a cap a tiny payload can declare a 
huge
+#: block count and exhaust memory. A legitimate collection of zero-byte 
elements is
+#: small, so this default is generous while still rejecting pathological 
input. It
+#: can be raised (or lowered) with the ``AVRO_MAX_COLLECTION_ITEMS`` 
environment
+#: variable.
+DEFAULT_MAX_COLLECTION_ITEMS = 10_000_000
+
+#: Default structural cap on the number of elements in any array or map (a
+#: collection larger than this is treated as malformed regardless of element
+#: type). Matches the historical ``Integer.MAX_VALUE - 8`` limit. Elements 
with a
+#: positive on-wire size are also bounded by the bytes remaining; this cap is 
an
+#: additional overflow/defense-in-depth guard. ``AVRO_MAX_COLLECTION_ITEMS``, 
when
+#: set, overrides both this and :data:`DEFAULT_MAX_COLLECTION_ITEMS`.
+DEFAULT_MAX_COLLECTION_STRUCTURAL = (1 << 31) - 8

Review Comment:
   `DEFAULT_MAX_COLLECTION_STRUCTURAL` is documented as matching 
`Integer.MAX_VALUE - 8`, but `(1 << 31) - 8` evaluates to `2^31 - 8` 
(2147483640), which is one higher than `2^31 - 1 - 8` (2147483639). This 
off-by-one means the code and documentation/spec reference don’t align.



-- 
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]

Reply via email to