Copilot commented on code in PR #3864:
URL: https://github.com/apache/avro/pull/3864#discussion_r3567381149
##########
lang/perl/lib/Avro/BinaryDecoder.pm:
##########
@@ -215,27 +387,67 @@ sub decode_enum {
}
sub skip_block {
- my $class = shift;
- my ($reader, $block_content) = @_;
- my $block_count = decode_long($class, undef, undef, $reader);
+ # Called as a plain function by skip_array/skip_map as
+ # skip_block($reader, $min_bytes, $coderef); it is not a method, so there
is
+ # no leading class argument. decode_long ignores its (shifted) first
argument
+ # and reads from the last, so pass __PACKAGE__ for it and $reader last.
+ my ($reader, $min_bytes, $block_content) = @_;
+ # Zero-byte elements loop with no per-item input, so bound them tightly;
+ # other elements consume bytes (bounded by EOF) so only the structural cap
+ # applies. Every collection is capped by the structural limit, so for
+ # zero-byte elements use whichever of the two limits is tighter.
+ my $limit = $MAX_COLLECTION_STRUCTURAL;
+ if ($min_bytes <= 0 && $MAX_COLLECTION_ITEMS < $limit) {
+ $limit = $MAX_COLLECTION_ITEMS;
+ }
+ my $block_count = decode_long(__PACKAGE__, undef, undef, $reader);
+ my $skipped = 0;
while ($block_count) {
if ($block_count < 0) {
- $reader->seek($block_count, 0);
- next;
+ # A negative count is followed by a long block size in bytes, which
+ # lets the whole block be skipped without decoding each item. Skip
+ # forward by that many bytes relative to the current position
+ # (SEEK_CUR); whence 0 (SEEK_SET) would seek to an absolute (here
Review Comment:
skip_block enforces collection caps only for positive block counts. When a
block is encoded with a negative count (count + block byte-size form), this
path skips by byte-size but never applies the cumulative $limit, so a huge
declared element count can be silently skipped (especially for zero-byte
elements where block_size may be 0), contradicting the intended skip bounding
behavior.
##########
lang/perl/lib/Avro/BinaryDecoder.pm:
##########
@@ -251,13 +463,15 @@ sub decode_array {
my @array;
my $writer_items = $writer_schema->items;
my $reader_items = $reader_schema->items;
+ my $min_bytes = _min_bytes_per_element($writer_items);
while ($block_count) {
my $block_size;
if ($block_count < 0) {
$block_count = -$block_count;
$block_size = decode_long($class, @_);
## XXX we can skip with $reader_schema?
Review Comment:
decode_array reads the per-block byte-size when the block count is negative,
but does not validate it. A negative block size is malformed (and already
rejected in skip_block); rejecting it here makes decoding stricter and avoids
accepting corrupted inputs.
##########
lang/perl/lib/Avro/BinaryDecoder.pm:
##########
@@ -296,13 +511,23 @@ sub decode_map {
my $block_count = decode_long($class, @_);
my $writer_values = $writer_schema->values;
my $reader_values = $reader_schema->values;
+ # A map key is a non-empty string (decode_map rejects empty keys below), so
+ # its minimum on-wire size is 2 bytes: a 1-byte length prefix plus at least
+ # 1 byte of key data, in addition to the value.
+ my $min_bytes = 2 + _min_bytes_per_element($writer_values);
+ # Track the number of decoded entries separately: scalar(keys %hash)
+ # undercounts when duplicate keys overwrite earlier entries, which would
let
+ # a multi-block map exceed the cumulative caps without being rejected.
+ my $decoded = 0;
while ($block_count) {
my $block_size;
if ($block_count < 0) {
$block_count = -$block_count;
$block_size = decode_long($class, @_);
## XXX we can skip with $reader_schema?
Review Comment:
decode_map reads the per-block byte-size when the block count is negative,
but does not validate it. A negative block size is malformed (and already
rejected in skip_block); rejecting it here makes decoding consistent and avoids
accepting corrupted inputs.
--
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]
