iemejia commented on code in PR #3864:
URL: https://github.com/apache/avro/pull/3864#discussion_r3567415893


##########
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:
   Fixed — skip_block now applies the cumulative cap to the absolute declared 
count in the negative (byte-sized) block branch too, so a huge count with a 
small/zero block size (e.g. zero-byte elements) can't bypass the limit.



##########
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:
   Fixed — decode_array now rejects a negative per-block byte-size.



##########
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:
   Fixed — decode_map now rejects a negative per-block byte-size, consistent 
with decode_array and skip_block.



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