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


##########
lang/py/avro/io.py:
##########
@@ -795,26 +850,33 @@ def read_array(self, writers_schema: 
avro.schema.ArraySchema, readers_schema: av
         The actual count in this case
         is the absolute value of the count written.
         """
-        read_items = []
+        read_items: List[object] = []
         block_count = decoder.read_long()
         while block_count != 0:
             if block_count < 0:
                 block_count = -block_count
                 decoder.skip_long()
+            check_max_collection_items(len(read_items), block_count, 
self.max_collection_items)
             for i in range(block_count):
                 read_items.append(self.read_data(writers_schema.items, 
readers_schema.items, decoder))
             block_count = decoder.read_long()
         return read_items
 
     def skip_array(self, writers_schema: avro.schema.ArraySchema, decoder: 
BinaryDecoder) -> None:
         block_count = decoder.read_long()
+        items_skipped = 0
         while block_count != 0:
             if block_count < 0:
                 block_size = decoder.read_long()
                 decoder.skip(block_size)

Review Comment:
   Good catch — fixed in c0ba2d8e94. `skip_array` now validates `block_size >= 
0` before calling `decoder.skip()` and raises `InvalidAvroBinaryEncoding` 
otherwise, so a negative block size can no longer seek the decoder backwards. 
Added a test.



##########
lang/py/avro/io.py:
##########
@@ -834,26 +896,37 @@ def read_map(self, writers_schema: avro.schema.MapSchema, 
readers_schema: avro.s
         """
         read_items = {}
         block_count = decoder.read_long()
+        # Track the number of pairs decoded rather than len(read_items): 
repeated
+        # keys collapse in the dict and would otherwise let the cumulative 
check
+        # be bypassed by a stream that keeps rewriting the same key.
+        items_read = 0
         while block_count != 0:
             if block_count < 0:
                 block_count = -block_count
                 decoder.skip_long()
+            check_max_collection_items(items_read, block_count, 
self.max_collection_items)
             for i in range(block_count):
                 key = decoder.read_utf8()
                 read_items[key] = self.read_data(writers_schema.values, 
readers_schema.values, decoder)
+            items_read += block_count
             block_count = decoder.read_long()
         return read_items
 
     def skip_map(self, writers_schema: avro.schema.MapSchema, decoder: 
BinaryDecoder) -> None:
         block_count = decoder.read_long()
+        items_skipped = 0
         while block_count != 0:
             if block_count < 0:
                 block_size = decoder.read_long()
                 decoder.skip(block_size)

Review Comment:
   Fixed in c0ba2d8e94 — same guard added to `skip_map`, with a test covering 
the negative block size case.



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