From de74b31dfd1606237268fb97cb0a27604735a8f5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Sat, 11 Jul 2026 13:42:26 +0200 Subject: [PATCH 1/5] AVRO-4281: [php] Bound collection size when decoding arrays and maps The block count of an array or map is read from the input and drives allocation of the resulting collection. A very large or malformed block count (for example from truncated input) could request an unbounded allocation. Validate the block count per block and cumulatively against a configurable maximum (AVRO_MAX_COLLECTION_ITEMS), mirroring the Java SDK's collection item limit, and throw AvroIOCollectionSizeException when exceeded. Assisted-by: GitHub Copilot:claude-opus-4.8 --- .../Datum/AvroIOCollectionSizeException.php | 43 +++++++++++ lang/php/lib/Datum/AvroIODatumReader.php | 53 +++++++++++++ lang/php/lib/autoload.php | 1 + lang/php/test/IODatumReaderTest.php | 74 +++++++++++++++++++ 4 files changed, 171 insertions(+) create mode 100644 lang/php/lib/Datum/AvroIOCollectionSizeException.php diff --git a/lang/php/lib/Datum/AvroIOCollectionSizeException.php b/lang/php/lib/Datum/AvroIOCollectionSizeException.php new file mode 100644 index 00000000000..6aa9b0bce0a --- /dev/null +++ b/lang/php/lib/Datum/AvroIOCollectionSizeException.php @@ -0,0 +1,43 @@ +maxCollectionItems = self::defaultMaxCollectionItems(); + } + + /** + * Set the maximum number of items permitted in a single decoded array or map. + */ + public function setMaxCollectionItems(int $maxCollectionItems): void + { + $this->maxCollectionItems = $maxCollectionItems; + } + + private static function defaultMaxCollectionItems(): int + { + $env = getenv(self::MAX_COLLECTION_ITEMS_ENV); + if (false !== $env && ctype_digit($env)) { + return (int) $env; + } + + return self::DEFAULT_MAX_COLLECTION_ITEMS; + } + + /** + * Guard against unbounded allocation when decoding an array or map. + * + * @throws AvroIOCollectionSizeException if the block count is negative or + * the running total would exceed the + * configured maximum. + */ + private function checkCollectionItems(int $existing, int $blockCount): void + { + if ($blockCount < 0 || $existing + $blockCount > $this->maxCollectionItems) { + throw new AvroIOCollectionSizeException($this->maxCollectionItems); + } } public function setWritersSchema(AvroSchema $schema): void @@ -290,6 +341,7 @@ public function readArray( $blockCount = -$blockCount; $decoder->readLong(); // Read (and ignore) block size } + $this->checkCollectionItems(count($items), $blockCount); for ($i = 0; $i < $blockCount; $i++) { $items[] = $this->readData( $writersSchema->items(), @@ -320,6 +372,7 @@ public function readMap( $decoder->readLong(); } + $this->checkCollectionItems(count($items), $pair_count); for ($i = 0; $i < $pair_count; $i++) { $key = $decoder->readString(); $items[$key] = $this->readData( diff --git a/lang/php/lib/autoload.php b/lang/php/lib/autoload.php index de1a863513b..12c9475e794 100644 --- a/lang/php/lib/autoload.php +++ b/lang/php/lib/autoload.php @@ -33,6 +33,7 @@ include __DIR__.'/Datum/AvroIOBinaryDecoder.php'; include __DIR__.'/Datum/AvroIOBinaryEncoder.php'; +include __DIR__.'/Datum/AvroIOCollectionSizeException.php'; include __DIR__.'/Datum/AvroIODatumReader.php'; include __DIR__.'/Datum/AvroIODatumWriter.php'; include __DIR__.'/Datum/AvroIOSchemaMatchException.php'; diff --git a/lang/php/test/IODatumReaderTest.php b/lang/php/test/IODatumReaderTest.php index abb51507c3e..021eea5b296 100644 --- a/lang/php/test/IODatumReaderTest.php +++ b/lang/php/test/IODatumReaderTest.php @@ -22,6 +22,7 @@ use Apache\Avro\Datum\AvroIOBinaryDecoder; use Apache\Avro\Datum\AvroIOBinaryEncoder; +use Apache\Avro\Datum\AvroIOCollectionSizeException; use Apache\Avro\Datum\AvroIODatumReader; use Apache\Avro\Datum\AvroIODatumWriter; use Apache\Avro\Datum\Type\AvroDuration; @@ -327,4 +328,77 @@ public function test_record_with_logical_types(): void $record ); } + + /** + * Encode the given longs (block counts / sizes / markers) into a decoder. + */ + private function decoderForLongs(int ...$values): AvroIOBinaryDecoder + { + $io = new AvroStringIO(); + $encoder = new AvroIOBinaryEncoder($io); + foreach ($values as $value) { + $encoder->writeLong($value); + } + + return new AvroIOBinaryDecoder(new AvroStringIO($io->string())); + } + + public function test_array_block_count_is_bounded(): void + { + $schema = AvroSchema::parse('{"type":"array","items":"null"}'); + // A single block declaring more items than the configured limit must be + // rejected before allocating. + $decoder = $this->decoderForLongs(11, 0); + $reader = new AvroIODatumReader($schema, $schema); + $reader->setMaxCollectionItems(10); + + $this->expectException(AvroIOCollectionSizeException::class); + $reader->read($decoder); + } + + public function test_map_block_count_is_bounded(): void + { + $schema = AvroSchema::parse('{"type":"map","values":"null"}'); + $decoder = $this->decoderForLongs(11, 0); + $reader = new AvroIODatumReader($schema, $schema); + $reader->setMaxCollectionItems(10); + + $this->expectException(AvroIOCollectionSizeException::class); + $reader->read($decoder); + } + + public function test_array_cumulative_limit_across_blocks(): void + { + $schema = AvroSchema::parse('{"type":"array","items":"null"}'); + // Two blocks of 6 items each exceed a limit of 10 even though neither + // block does on its own. + $decoder = $this->decoderForLongs(6, 6, 0); + $reader = new AvroIODatumReader($schema, $schema); + $reader->setMaxCollectionItems(10); + + $this->expectException(AvroIOCollectionSizeException::class); + $reader->read($decoder); + } + + public function test_negative_block_count_is_bounded(): void + { + $schema = AvroSchema::parse('{"type":"array","items":"null"}'); + // Negative count -11 -> 11 items, followed by a block size that is skipped. + $decoder = $this->decoderForLongs(-11, 0); + $reader = new AvroIODatumReader($schema, $schema); + $reader->setMaxCollectionItems(10); + + $this->expectException(AvroIOCollectionSizeException::class); + $reader->read($decoder); + } + + public function test_array_within_limit_still_reads(): void + { + $schema = AvroSchema::parse('{"type":"array","items":"null"}'); + $decoder = $this->decoderForLongs(3, 0); // 3 null items, then end-of-array + $reader = new AvroIODatumReader($schema, $schema); + $reader->setMaxCollectionItems(10); + + $this->assertEquals([null, null, null], $reader->read($decoder)); + } } From 3bd13378bf8646777065bf9e6b6b020b69e6b80b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Sat, 11 Jul 2026 15:09:06 +0200 Subject: [PATCH 2/5] AVRO-4281: [php] Satisfy php-cs-fixer ordered_class_elements Move the new private helper methods below the public methods to match the ordered_class_elements rule enforced by the lint step. Assisted-by: GitHub Copilot:claude-opus-4.8 --- lang/php/lib/Datum/AvroIODatumReader.php | 48 ++++++++++++------------ lang/php/test/IODatumReaderTest.php | 28 +++++++------- 2 files changed, 38 insertions(+), 38 deletions(-) diff --git a/lang/php/lib/Datum/AvroIODatumReader.php b/lang/php/lib/Datum/AvroIODatumReader.php index 70e4497750e..21ceecdf5f8 100644 --- a/lang/php/lib/Datum/AvroIODatumReader.php +++ b/lang/php/lib/Datum/AvroIODatumReader.php @@ -76,30 +76,6 @@ public function setMaxCollectionItems(int $maxCollectionItems): void $this->maxCollectionItems = $maxCollectionItems; } - private static function defaultMaxCollectionItems(): int - { - $env = getenv(self::MAX_COLLECTION_ITEMS_ENV); - if (false !== $env && ctype_digit($env)) { - return (int) $env; - } - - return self::DEFAULT_MAX_COLLECTION_ITEMS; - } - - /** - * Guard against unbounded allocation when decoding an array or map. - * - * @throws AvroIOCollectionSizeException if the block count is negative or - * the running total would exceed the - * configured maximum. - */ - private function checkCollectionItems(int $existing, int $blockCount): void - { - if ($blockCount < 0 || $existing + $blockCount > $this->maxCollectionItems) { - throw new AvroIOCollectionSizeException($this->maxCollectionItems); - } - } - public function setWritersSchema(AvroSchema $schema): void { $this->writersSchema = $schema; @@ -583,6 +559,30 @@ public function readDefaultValue(AvroSchema $fieldSchema, mixed $defaultValue): } } + private static function defaultMaxCollectionItems(): int + { + $env = getenv(self::MAX_COLLECTION_ITEMS_ENV); + if (false !== $env && ctype_digit($env)) { + return (int) $env; + } + + return self::DEFAULT_MAX_COLLECTION_ITEMS; + } + + /** + * Guard against unbounded allocation when decoding an array or map. + * + * @throws AvroIOCollectionSizeException if the block count is negative or + * the running total would exceed the + * configured maximum. + */ + private function checkCollectionItems(int $existing, int $blockCount): void + { + if ($blockCount < 0 || $existing + $blockCount > $this->maxCollectionItems) { + throw new AvroIOCollectionSizeException($this->maxCollectionItems); + } + } + private function readDecimal(string $bytes, int $scale): string { $mostSignificantBit = ord($bytes[0]) & 0x80; diff --git a/lang/php/test/IODatumReaderTest.php b/lang/php/test/IODatumReaderTest.php index 021eea5b296..fde70283b8c 100644 --- a/lang/php/test/IODatumReaderTest.php +++ b/lang/php/test/IODatumReaderTest.php @@ -329,20 +329,6 @@ public function test_record_with_logical_types(): void ); } - /** - * Encode the given longs (block counts / sizes / markers) into a decoder. - */ - private function decoderForLongs(int ...$values): AvroIOBinaryDecoder - { - $io = new AvroStringIO(); - $encoder = new AvroIOBinaryEncoder($io); - foreach ($values as $value) { - $encoder->writeLong($value); - } - - return new AvroIOBinaryDecoder(new AvroStringIO($io->string())); - } - public function test_array_block_count_is_bounded(): void { $schema = AvroSchema::parse('{"type":"array","items":"null"}'); @@ -401,4 +387,18 @@ public function test_array_within_limit_still_reads(): void $this->assertEquals([null, null, null], $reader->read($decoder)); } + + /** + * Encode the given longs (block counts / sizes / markers) into a decoder. + */ + private function decoderForLongs(int ...$values): AvroIOBinaryDecoder + { + $io = new AvroStringIO(); + $encoder = new AvroIOBinaryEncoder($io); + foreach ($values as $value) { + $encoder->writeLong($value); + } + + return new AvroIOBinaryDecoder(new AvroStringIO($io->string())); + } } From 7bd2e3a9c77da938df7e86a34a9b2747b8db6d65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Sat, 11 Jul 2026 15:26:45 +0200 Subject: [PATCH 3/5] AVRO-4281: [php] Address review: map pair count and overflow-safe check - Track decoded pair count in readMap instead of count($items); repeated keys collapse in the result and could otherwise bypass the cumulative limit. - Use a subtraction-based comparison in checkCollectionItems so it cannot overflow when the limit is configured near PHP_INT_MAX, and document that the block count passed in is already normalized. - Add a test covering repeated keys across blocks. Assisted-by: GitHub Copilot:claude-opus-4.8 --- lang/php/lib/Datum/AvroIODatumReader.php | 20 +++++++++++++++----- lang/php/test/IODatumReaderTest.php | 20 ++++++++++++++++++++ 2 files changed, 35 insertions(+), 5 deletions(-) diff --git a/lang/php/lib/Datum/AvroIODatumReader.php b/lang/php/lib/Datum/AvroIODatumReader.php index 21ceecdf5f8..1ae5ee77884 100644 --- a/lang/php/lib/Datum/AvroIODatumReader.php +++ b/lang/php/lib/Datum/AvroIODatumReader.php @@ -340,6 +340,7 @@ public function readMap( AvroIOBinaryDecoder $decoder ): array { $items = []; + $itemsRead = 0; $pair_count = $decoder->readLong(); while (0 != $pair_count) { if ($pair_count < 0) { @@ -348,7 +349,10 @@ public function readMap( $decoder->readLong(); } - $this->checkCollectionItems(count($items), $pair_count); + // Track the number of decoded pairs rather than count($items): repeated + // keys collapse in the result and would otherwise let the cumulative + // check be bypassed by a stream that keeps rewriting the same key. + $this->checkCollectionItems($itemsRead, $pair_count); for ($i = 0; $i < $pair_count; $i++) { $key = $decoder->readString(); $items[$key] = $this->readData( @@ -357,6 +361,7 @@ public function readMap( $decoder ); } + $itemsRead += $pair_count; $pair_count = $decoder->readLong(); } @@ -572,13 +577,18 @@ private static function defaultMaxCollectionItems(): int /** * Guard against unbounded allocation when decoding an array or map. * - * @throws AvroIOCollectionSizeException if the block count is negative or - * the running total would exceed the - * configured maximum. + * @param int $existing the number of items already decoded for the collection + * @param int $blockCount the number of items in the next block; this is the + * normalized (non-negative) count + * + * @throws AvroIOCollectionSizeException if the block count is negative or the + * running total would exceed the maximum */ private function checkCollectionItems(int $existing, int $blockCount): void { - if ($blockCount < 0 || $existing + $blockCount > $this->maxCollectionItems) { + // Use subtraction rather than $existing + $blockCount so the comparison + // cannot overflow when the limit is configured close to PHP_INT_MAX. + if ($blockCount < 0 || $blockCount > $this->maxCollectionItems - $existing) { throw new AvroIOCollectionSizeException($this->maxCollectionItems); } } diff --git a/lang/php/test/IODatumReaderTest.php b/lang/php/test/IODatumReaderTest.php index fde70283b8c..b3d698f464a 100644 --- a/lang/php/test/IODatumReaderTest.php +++ b/lang/php/test/IODatumReaderTest.php @@ -378,6 +378,26 @@ public function test_negative_block_count_is_bounded(): void $reader->read($decoder); } + public function test_map_cumulative_limit_with_repeated_keys(): void + { + // Repeated keys collapse in the result map; the cumulative check must + // still count every decoded pair so the limit cannot be bypassed. + $schema = AvroSchema::parse('{"type":"map","values":"null"}'); + $io = new AvroStringIO(); + $encoder = new AvroIOBinaryEncoder($io); + $encoder->writeLong(6); // first block: 6 pairs, all the same key + for ($i = 0; $i < 6; $i++) { + $encoder->writeString('a'); // identical key; a null value has no bytes + } + $encoder->writeLong(6); // second block would push the total to 12 > 10 + $decoder = new AvroIOBinaryDecoder(new AvroStringIO($io->string())); + $reader = new AvroIODatumReader($schema, $schema); + $reader->setMaxCollectionItems(10); + + $this->expectException(AvroIOCollectionSizeException::class); + $reader->read($decoder); + } + public function test_array_within_limit_still_reads(): void { $schema = AvroSchema::parse('{"type":"array","items":"null"}'); From 175b42fecd07c02b73cf019bfeade2d5b2dcc5a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Sat, 11 Jul 2026 16:12:51 +0200 Subject: [PATCH 4/5] AVRO-4281: [php] Address review: GMP loop termination and comment - readArray now uses a loose comparison against 0 so the loop terminates when readLong() returns a numeric string (GMP on 32-bit builds), matching readMap. - Correct the default-limit comment: 2147483639 is Integer.MAX_VALUE - 8 (matching the Java SDK), not 2^31 - 8. Assisted-by: GitHub Copilot:claude-opus-4.8 --- lang/php/lib/Datum/AvroIODatumReader.php | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lang/php/lib/Datum/AvroIODatumReader.php b/lang/php/lib/Datum/AvroIODatumReader.php index 1ae5ee77884..0f022d8e468 100644 --- a/lang/php/lib/Datum/AvroIODatumReader.php +++ b/lang/php/lib/Datum/AvroIODatumReader.php @@ -51,7 +51,7 @@ class AvroIODatumReader * throws an {@see AvroIOCollectionSizeException} instead of attempting a * potentially huge allocation. Mirrors the Java SDK's collection item limit. */ - public const DEFAULT_MAX_COLLECTION_ITEMS = 2147483639; // 2^31 - 8 + public const DEFAULT_MAX_COLLECTION_ITEMS = 2147483639; // Integer.MAX_VALUE - 8, matching the Java SDK /** * Name of the environment variable used to override the default maximum @@ -312,7 +312,9 @@ public function readArray( ): array { $items = []; $blockCount = $decoder->readLong(); - while (0 !== $blockCount) { + // Loose comparison: on 32-bit builds with GMP, readLong() may return a + // numeric string, so a strict check against 0 would never terminate. + while (0 != $blockCount) { if ($blockCount < 0) { $blockCount = -$blockCount; $decoder->readLong(); // Read (and ignore) block size From 3062721d036740c80c1ac58fdeb218fa198be62b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Sat, 11 Jul 2026 16:28:32 +0200 Subject: [PATCH 5/5] AVRO-4281: [php] Address review: reject negative max collection items setMaxCollectionItems now throws InvalidArgumentException for a negative limit, which would otherwise make the guard nonsensical (non-empty collections always throw while empty ones succeed). Add a test. Assisted-by: GitHub Copilot:claude-opus-4.8 --- lang/php/lib/Datum/AvroIODatumReader.php | 7 +++++++ lang/php/test/IODatumReaderTest.php | 7 +++++++ 2 files changed, 14 insertions(+) diff --git a/lang/php/lib/Datum/AvroIODatumReader.php b/lang/php/lib/Datum/AvroIODatumReader.php index 0f022d8e468..33b34a001d6 100644 --- a/lang/php/lib/Datum/AvroIODatumReader.php +++ b/lang/php/lib/Datum/AvroIODatumReader.php @@ -70,9 +70,16 @@ public function __construct( /** * Set the maximum number of items permitted in a single decoded array or map. + * + * @throws \InvalidArgumentException if $maxCollectionItems is negative */ public function setMaxCollectionItems(int $maxCollectionItems): void { + if ($maxCollectionItems < 0) { + throw new \InvalidArgumentException( + sprintf('maxCollectionItems must not be negative, got %d.', $maxCollectionItems) + ); + } $this->maxCollectionItems = $maxCollectionItems; } diff --git a/lang/php/test/IODatumReaderTest.php b/lang/php/test/IODatumReaderTest.php index b3d698f464a..32a15edaeb7 100644 --- a/lang/php/test/IODatumReaderTest.php +++ b/lang/php/test/IODatumReaderTest.php @@ -408,6 +408,13 @@ public function test_array_within_limit_still_reads(): void $this->assertEquals([null, null, null], $reader->read($decoder)); } + public function test_set_max_collection_items_rejects_negative(): void + { + $reader = new AvroIODatumReader(); + $this->expectException(\InvalidArgumentException::class); + $reader->setMaxCollectionItems(-1); + } + /** * Encode the given longs (block counts / sizes / markers) into a decoder. */