Copilot commented on code in PR #3862:
URL: https://github.com/apache/avro/pull/3862#discussion_r3565204720
##########
lang/ruby/lib/avro/io.rb:
##########
@@ -103,10 +110,40 @@ def read_string
end
def read(len)
- # Read n bytes
+ # Read n bytes. Reject a declared length that exceeds the bytes
+ # actually remaining before allocating for it, to guard against an
+ # out-of-memory attack from a malicious or truncated input. The check
+ # is only applied to larger reads; smaller reads and stream readers
that
+ # cannot report their size fall back to reading directly.
+ if len < 0
+ # A negative length would make IO#read return the rest of the stream,
+ # which bypasses the size check and can allocate without bound.
+ raise AvroError, "Cannot read a negative number of bytes: #{len}"
+ end
+ if len > MAX_UNCHECKED_READ
+ remaining = bytes_remaining
+ if remaining && len > remaining
+ raise AvroError, "Cannot read #{len} bytes, only #{remaining}
remaining"
+ end
+ end
@reader.read(len)
end
+ # Number of bytes still available to read, or nil when the reader cannot
+ # report its size. Used to reject a declared length or collection block
+ # count that exceeds the data actually available before allocating for
it.
+ def bytes_remaining
+ return nil unless @reader.respond_to?(:size) &&
@reader.respond_to?(:tell)
+ size = @reader.size
+ pos = @reader.tell
+ return nil unless size.is_a?(Integer) && pos.is_a?(Integer)
+ size - pos
Review Comment:
`bytes_remaining` can return a negative value when the underlying IO's
reported size is smaller than the current position (e.g., file truncated while
reading). That negative value can surface in error messages ("only -N
remaining") and makes the availability checks harder to reason about. Clamp
negative remaining to 0 (or return nil) so callers treat it as no bytes
available.
##########
lang/ruby/lib/avro/io.rb:
##########
@@ -337,6 +375,8 @@ def read_map(writers_schema, readers_schema, decoder)
block_count = -block_count
_block_size = decoder.read_long
end
+ # Map keys are strings (>= 1 byte length prefix) plus the value.
+ ensure_collection_available(decoder, block_count, 1 +
min_bytes_per_element(writers_schema.values))
Review Comment:
`min_bytes_per_element(writers_schema.values)` is recomputed for every map
block. For maps split into many blocks and/or complex value schemas, this adds
avoidable overhead. Cache the computed minimum once per `read_map` invocation
and reuse it for subsequent blocks.
##########
lang/ruby/lib/avro/io.rb:
##########
@@ -318,6 +355,7 @@ def read_array(writers_schema, readers_schema, decoder)
block_count = -block_count
_block_size = decoder.read_long
end
+ ensure_collection_available(decoder, block_count,
min_bytes_per_element(writers_schema.items))
Review Comment:
`min_bytes_per_element(writers_schema.items)` is recomputed for every array
block. For arrays split into many blocks and/or complex item schemas (records),
this adds avoidable overhead on the hot path. Cache the computed minimum once
per `read_array` invocation and reuse it for subsequent blocks.
--
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]
