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


##########
lang/php/lib/Datum/AvroIOBinaryDecoder.php:
##########
@@ -65,9 +73,32 @@ public function readBoolean(): bool
      */
     public function read(int $len): string
     {
+        if ($len > self::MAX_UNCHECKED_READ) {
+            $remaining = $this->bytesRemaining();
+            if ($len > $remaining) {
+                throw new AvroException("Cannot read {$len} bytes, only 
{$remaining} remaining.");
+            }
+        }
+

Review Comment:
   Fixed in 5e997eb: read(int) now rejects a negative length before delegating 
to AvroIO::read().



##########
lang/php/lib/Datum/AvroIODatumReader.php:
##########
@@ -538,4 +541,80 @@ private function readDecimal(string $bytes, int $scale): 
string
 
         return (string) ($scale > 0 ? ($int / (10 ** $scale)) : $int);
     }
+
+    /**
+     * Minimum number of bytes a single value of the given schema can occupy on
+     * the wire. Used to reject an array/map block count that could not be 
backed
+     * by the bytes remaining. A type that can encode to zero bytes (null)
+     * returns 0, which disables the collection check for it (so an array of
+     * nulls is not falsely rejected). Types that cannot be resolved cheaply
+     * default to 1, which is safe because the only zero-byte primitive is 
null.
+     *
+     * @param array<int, bool> $visited
+     */
+    private static function minBytesPerElement(mixed $schema, array $visited = 
[]): int
+    {
+        $type = $schema instanceof AvroSchema ? $schema->type() : $schema;
+        // Named/complex field types may nest a schema object; unwrap one 
level.
+        if ($type instanceof AvroSchema) {
+            return self::minBytesPerElement($type, $visited);
+        }
+        if (!is_string($type)) {
+            return 1;
+        }
+        switch ($type) {
+            case AvroSchema::NULL_TYPE:
+                return 0;
+            case AvroSchema::FLOAT_TYPE:
+                return 4;
+            case AvroSchema::DOUBLE_TYPE:
+                return 8;
+            case AvroSchema::FIXED_SCHEMA:
+                return $schema instanceof AvroFixedSchema ? $schema->size() : 
1;
+            case AvroSchema::RECORD_SCHEMA:
+            case AvroSchema::ERROR_SCHEMA:
+                if (!$schema instanceof AvroRecordSchema) {
+                    return 1;
+                }
+                $id = spl_object_id($schema);
+                if (isset($visited[$id])) {
+                    return 0; // break recursion for self-referencing schemas
+                }
+                $visited[$id] = true;
+                $total = 0;
+                foreach ($schema->fields() as $field) {
+                    $total += self::minBytesPerElement($field, $visited);
+                }
+
+                return $total;
+            default:
+                // boolean, int, long, bytes, string, enum, union, array, map:
+                // all encode to at least one byte.
+                return 1;
+        }
+    }
+
+    /**
+     * Rejects a collection (array or map) block whose declared element count
+     * could not be backed by the bytes actually remaining, before iterating.
+     * Skipped when the per-element minimum is zero (e.g. an array of nulls).
+     *
+     * @throws AvroException if the declared count exceeds the bytes remaining
+     */
+    private static function ensureCollectionAvailable(
+        AvroIOBinaryDecoder $decoder,
+        int $count,
+        int $minBytesPerElement
+    ): void {
+        if ($count <= 0 || $minBytesPerElement <= 0) {
+            return;
+        }
+        $remaining = $decoder->bytesRemaining();
+        if ($count * $minBytesPerElement > $remaining) {

Review Comment:
   Fixed in 5e997eb: ensureCollectionAvailable now compares count against 
intdiv(remaining, minBytes) instead of multiplying, avoiding overflow/float 
coercion for large counts.



##########
lang/php/lib/Datum/AvroIODatumReader.php:
##########
@@ -290,6 +290,7 @@ public function readArray(
                 $blockCount = -$blockCount;
                 $decoder->readLong(); // Read (and ignore) block size
             }
+            self::ensureCollectionAvailable($decoder, $blockCount, 
self::minBytesPerElement($writersSchema->items()));
             for ($i = 0; $i < $blockCount; $i++) {

Review Comment:
   Fixed in 5e997eb: readArray now uses a non-strict comparison (0 != 
blockCount), matching readMap, so a GMP numeric-string block count of '0' 
terminates the loop on 32-bit builds.



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