Copilot commented on code in PR #3861:
URL: https://github.com/apache/avro/pull/3861#discussion_r3565162634
##########
lang/py/avro/io.py:
##########
@@ -214,11 +222,30 @@ def read(self, n: int) -> bytes:
"""
if n < 0:
raise avro.errors.InvalidAvroBinaryEncoding(f"Requested {n} bytes
to read, expected positive integer.")
+ if n > self._MAX_UNCHECKED_READ:
+ remaining = self.bytes_remaining()
+ if remaining is not None and n > remaining:
+ raise avro.errors.InvalidAvroBinaryEncoding(f"Requested {n}
bytes to read, but only {remaining} remain.")
read_bytes = self.reader.read(n)
if len(read_bytes) != n:
raise avro.errors.InvalidAvroBinaryEncoding(f"Read
{len(read_bytes)} bytes, expected {n} bytes")
return read_bytes
+ def bytes_remaining(self) -> Optional[int]:
+ """
+ Return the number of bytes still available to read, or ``None`` when
+ that count is not known (a non-seekable reader). Used to reject a
+ declared length or collection block count that exceeds the data
+ actually available before allocating for it.
+ """
+ reader = self.reader
+ if not getattr(reader, "seekable", None) or not reader.seekable():
+ return None
+ pos = reader.tell()
+ end = reader.seek(0, os.SEEK_END)
+ reader.seek(pos)
+ return end - pos
Review Comment:
`bytes_remaining()` relies on `reader.seekable()` existing and on `seek()`
returning the new position. Some seekable file-like objects may not implement
`seekable()`, and some `seek()` implementations return `None`, which would
raise a `TypeError` at `end - pos` and/or skip the protection unnecessarily.
Consider using `tell()/seek()` directly with a try/except fallback and use
`tell()` to get the end position after seeking.
##########
lang/py/avro/io.py:
##########
@@ -801,6 +881,7 @@ def read_array(self, writers_schema:
avro.schema.ArraySchema, readers_schema: av
if block_count < 0:
block_count = -block_count
decoder.skip_long()
+ self._ensure_collection_available(decoder, block_count,
_min_bytes_per_element(writers_schema.items))
for i in range(block_count):
Review Comment:
`_min_bytes_per_element(writers_schema.items)` is recomputed for every array
block. For complex schemas and many blocks this adds avoidable overhead;
compute it once before the loop and reuse it.
##########
lang/py/avro/io.py:
##########
@@ -838,6 +919,8 @@ def read_map(self, writers_schema: avro.schema.MapSchema,
readers_schema: avro.s
if block_count < 0:
block_count = -block_count
decoder.skip_long()
+ # Map keys are strings (>= 1 byte length prefix) plus the value.
+ self._ensure_collection_available(decoder, block_count, 1 +
_min_bytes_per_element(writers_schema.values))
for i in range(block_count):
Review Comment:
`1 + _min_bytes_per_element(writers_schema.values)` is recomputed for every
map block. Computing it once before the loop avoids repeated schema traversal
for complex value types.
--
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]
