diff --git a/composer.json b/composer.json index c6c5b39c4..2e1197065 100644 --- a/composer.json +++ b/composer.json @@ -19,7 +19,7 @@ "require-dev": { "doctrine/coding-standard": "^12.0", "phpunit/phpunit": "^10.5.35", - "rector/rector": "^2.3.4", + "rector/rector": "~2.5.9", "squizlabs/php_codesniffer": "^3.7", "vimeo/psalm": "~6.14.2" }, diff --git a/src/Collection.php b/src/Collection.php index b85ff0683..855486491 100644 --- a/src/Collection.php +++ b/src/Collection.php @@ -77,6 +77,7 @@ use function current; use function is_array; use function is_bool; +use function str_contains; use function strlen; /** @@ -174,11 +175,11 @@ class Collection implements Stringable */ public function __construct(private Manager $manager, private string $databaseName, private string $collectionName, array $options = []) { - if (strlen($databaseName) < 1) { + if (strlen($databaseName) < 1 || str_contains($databaseName, '.') || str_contains($databaseName, "\0")) { throw new InvalidArgumentException('$databaseName is invalid: ' . $databaseName); } - if (strlen($collectionName) < 1) { + if (strlen($collectionName) < 1 || str_contains($collectionName, "\0")) { throw new InvalidArgumentException('$collectionName is invalid: ' . $collectionName); } diff --git a/src/Database.php b/src/Database.php index 88cf0ef45..ce588711e 100644 --- a/src/Database.php +++ b/src/Database.php @@ -56,6 +56,7 @@ use function is_array; use function is_bool; +use function str_contains; use function strlen; /** @psalm-no-seal-properties */ @@ -112,7 +113,7 @@ class Database implements Stringable */ public function __construct(private Manager $manager, private string $databaseName, array $options = []) { - if (strlen($databaseName) < 1) { + if (strlen($databaseName) < 1 || str_contains($databaseName, '.') || str_contains($databaseName, "\0")) { throw new InvalidArgumentException('$databaseName is invalid: ' . $databaseName); } diff --git a/src/Operation/BulkWrite.php b/src/Operation/BulkWrite.php index de50dc766..6847245c6 100644 --- a/src/Operation/BulkWrite.php +++ b/src/Operation/BulkWrite.php @@ -36,6 +36,7 @@ use function is_array; use function is_bool; use function key; +use function MongoDB\create_namespace; use function MongoDB\is_document; use function MongoDB\is_first_key_operator; use function MongoDB\is_pipeline; @@ -63,6 +64,8 @@ final class BulkWrite private array $options; + private string $namespace; + /** * Constructs a bulk write operation. * @@ -142,8 +145,10 @@ final class BulkWrite * @param array $options Command options * @throws InvalidArgumentException for parameter/option parsing errors */ - public function __construct(private string $databaseName, private string $collectionName, array $operations, array $options = []) + public function __construct(string $databaseName, string $collectionName, array $operations, array $options = []) { + $this->namespace = create_namespace($databaseName, $collectionName); + if (empty($operations)) { throw new InvalidArgumentException('$operations is empty'); } @@ -232,7 +237,7 @@ public function execute(Server $server): BulkWriteResult } } - $writeResult = $server->executeBulkWrite($this->databaseName . '.' . $this->collectionName, $bulk, $this->createExecuteOptions()); + $writeResult = $server->executeBulkWrite($this->namespace, $bulk, $this->createExecuteOptions()); return new BulkWriteResult($writeResult, $insertedIds); } diff --git a/src/Operation/Delete.php b/src/Operation/Delete.php index 84a803c79..6756764b7 100644 --- a/src/Operation/Delete.php +++ b/src/Operation/Delete.php @@ -27,6 +27,7 @@ use MongoDB\Exception\UnsupportedException; use function is_string; +use function MongoDB\create_namespace; use function MongoDB\is_document; use function MongoDB\is_write_concern_acknowledged; use function MongoDB\server_supports_feature; @@ -44,6 +45,8 @@ final class Delete implements Explainable { private const WIRE_VERSION_FOR_HINT = 9; + private string $namespace; + /** * Constructs a delete command. * @@ -80,8 +83,10 @@ final class Delete implements Explainable * @param array $options Command options * @throws InvalidArgumentException for parameter/option parsing errors */ - public function __construct(private string $databaseName, private string $collectionName, private array|object $filter, private int $limit, private array $options = []) + public function __construct(string $databaseName, private string $collectionName, private array|object $filter, private int $limit, private array $options = []) { + $this->namespace = create_namespace($databaseName, $collectionName); + if (! is_document($filter)) { throw InvalidArgumentException::expectedDocumentType('$filter', $filter); } @@ -140,7 +145,7 @@ public function execute(Server $server): DeleteResult $bulk = new Bulk($this->createBulkWriteOptions()); $bulk->delete($this->filter, $this->createDeleteOptions()); - $writeResult = $server->executeBulkWrite($this->databaseName . '.' . $this->collectionName, $bulk, $this->createExecuteOptions()); + $writeResult = $server->executeBulkWrite($this->namespace, $bulk, $this->createExecuteOptions()); return new DeleteResult($writeResult); } diff --git a/src/Operation/Find.php b/src/Operation/Find.php index 8d84c7bf0..0d1c9f211 100644 --- a/src/Operation/Find.php +++ b/src/Operation/Find.php @@ -34,6 +34,7 @@ use function is_bool; use function is_integer; use function is_string; +use function MongoDB\create_namespace; use function MongoDB\is_document; /** @@ -49,6 +50,8 @@ final class Find implements Explainable public const TAILABLE = 2; public const TAILABLE_AWAIT = 3; + private string $namespace; + /** * Constructs a find command. * @@ -130,8 +133,10 @@ final class Find implements Explainable * @param array $options Command options * @throws InvalidArgumentException for parameter/option parsing errors */ - public function __construct(private string $databaseName, private string $collectionName, private array|object $filter, private array $options = []) + public function __construct(string $databaseName, private string $collectionName, private array|object $filter, private array $options = []) { + $this->namespace = create_namespace($databaseName, $collectionName); + if (! is_document($filter)) { throw InvalidArgumentException::expectedDocumentType('$filter', $filter); } @@ -260,7 +265,7 @@ public function execute(Server $server): CursorInterface throw UnsupportedException::readConcernNotSupportedInTransaction(); } - $cursor = $server->executeQuery($this->databaseName . '.' . $this->collectionName, new Query($this->filter, $this->createQueryOptions()), $this->createExecuteOptions()); + $cursor = $server->executeQuery($this->namespace, new Query($this->filter, $this->createQueryOptions()), $this->createExecuteOptions()); if (isset($this->options['codec'])) { return CodecCursor::fromCursor($cursor, $this->options['codec']); diff --git a/src/Operation/InsertMany.php b/src/Operation/InsertMany.php index 70e149076..5906fe03e 100644 --- a/src/Operation/InsertMany.php +++ b/src/Operation/InsertMany.php @@ -29,6 +29,7 @@ use function array_is_list; use function is_bool; +use function MongoDB\create_namespace; use function MongoDB\is_document; use function sprintf; @@ -45,6 +46,8 @@ final class InsertMany private array $options; + private string $namespace; + /** * Constructs an insert command. * @@ -75,8 +78,10 @@ final class InsertMany * @param array $options Command options * @throws InvalidArgumentException for parameter/option parsing errors */ - public function __construct(private string $databaseName, private string $collectionName, array $documents, array $options = []) + public function __construct(string $databaseName, string $collectionName, array $documents, array $options = []) { + $this->namespace = create_namespace($databaseName, $collectionName); + $options += ['ordered' => true]; if (isset($options['bypassDocumentValidation']) && ! is_bool($options['bypassDocumentValidation'])) { @@ -131,7 +136,7 @@ public function execute(Server $server): InsertManyResult $insertedIds[$i] = $bulk->insert($document); } - $writeResult = $server->executeBulkWrite($this->databaseName . '.' . $this->collectionName, $bulk, $this->createExecuteOptions()); + $writeResult = $server->executeBulkWrite($this->namespace, $bulk, $this->createExecuteOptions()); return new InsertManyResult($writeResult, $insertedIds); } diff --git a/src/Operation/InsertOne.php b/src/Operation/InsertOne.php index dff9f79f6..d48d3129f 100644 --- a/src/Operation/InsertOne.php +++ b/src/Operation/InsertOne.php @@ -28,6 +28,7 @@ use MongoDB\InsertOneResult; use function is_bool; +use function MongoDB\create_namespace; use function MongoDB\is_document; /** @@ -40,6 +41,8 @@ final class InsertOne { private array|object $document; + private string $namespace; + /** * Constructs an insert command. * @@ -65,8 +68,10 @@ final class InsertOne * @param array $options Command options * @throws InvalidArgumentException for parameter/option parsing errors */ - public function __construct(private string $databaseName, private string $collectionName, array|object $document, private array $options = []) + public function __construct(string $databaseName, string $collectionName, array|object $document, private array $options = []) { + $this->namespace = create_namespace($databaseName, $collectionName); + if (isset($this->options['bypassDocumentValidation']) && ! is_bool($this->options['bypassDocumentValidation'])) { throw InvalidArgumentException::invalidType('"bypassDocumentValidation" option', $this->options['bypassDocumentValidation'], 'boolean'); } @@ -111,7 +116,7 @@ public function execute(Server $server): InsertOneResult $insertedId = $bulk->insert($this->document); - $writeResult = $server->executeBulkWrite($this->databaseName . '.' . $this->collectionName, $bulk, $this->createExecuteOptions()); + $writeResult = $server->executeBulkWrite($this->namespace, $bulk, $this->createExecuteOptions()); return new InsertOneResult($writeResult, $insertedId); } diff --git a/src/Operation/RenameCollection.php b/src/Operation/RenameCollection.php index b3848c67e..ba61401b4 100644 --- a/src/Operation/RenameCollection.php +++ b/src/Operation/RenameCollection.php @@ -26,6 +26,7 @@ use MongoDB\Exception\UnsupportedException; use function is_bool; +use function MongoDB\create_namespace; /** * Operation for the renameCollection command. @@ -81,8 +82,8 @@ public function __construct(string $fromDatabaseName, string $fromCollectionName throw InvalidArgumentException::invalidType('"dropTarget" option', $this->options['dropTarget'], 'boolean'); } - $this->fromNamespace = $fromDatabaseName . '.' . $fromCollectionName; - $this->toNamespace = $toDatabaseName . '.' . $toCollectionName; + $this->fromNamespace = create_namespace($fromDatabaseName, $fromCollectionName); + $this->toNamespace = create_namespace($toDatabaseName, $toCollectionName); } /** diff --git a/src/Operation/Update.php b/src/Operation/Update.php index 4f785e7a1..2ed88b600 100644 --- a/src/Operation/Update.php +++ b/src/Operation/Update.php @@ -29,6 +29,7 @@ use function is_array; use function is_bool; use function is_string; +use function MongoDB\create_namespace; use function MongoDB\is_document; use function MongoDB\is_first_key_operator; use function MongoDB\is_pipeline; @@ -46,6 +47,8 @@ final class Update implements Explainable { private array $options; + private string $namespace; + /** * Constructs a update command. * @@ -94,8 +97,10 @@ final class Update implements Explainable * @param array $options Command options * @throws InvalidArgumentException for parameter/option parsing errors */ - public function __construct(private string $databaseName, private string $collectionName, private array|object $filter, private array|object $update, array $options = []) + public function __construct(string $databaseName, private string $collectionName, private array|object $filter, private array|object $update, array $options = []) { + $this->namespace = create_namespace($databaseName, $collectionName); + if (! is_document($filter)) { throw InvalidArgumentException::expectedDocumentType('$filter', $filter); } @@ -180,7 +185,7 @@ public function execute(Server $server): UpdateResult $bulk = new Bulk($this->createBulkWriteOptions()); $bulk->update($this->filter, $this->update, $this->createUpdateOptions()); - $writeResult = $server->executeBulkWrite($this->databaseName . '.' . $this->collectionName, $bulk, $this->createExecuteOptions()); + $writeResult = $server->executeBulkWrite($this->namespace, $bulk, $this->createExecuteOptions()); return new UpdateResult($writeResult); } diff --git a/src/functions.php b/src/functions.php index 2e68987c6..1338c8486 100644 --- a/src/functions.php +++ b/src/functions.php @@ -45,6 +45,7 @@ use function is_array; use function is_object; use function is_string; +use function str_contains; use function str_ends_with; use function substr; @@ -435,6 +436,30 @@ function is_string_array(mixed $input): bool return true; } +/** + * Validates a database and collection name and returns the namespace formed + * by concatenating them. + * + * A "." or NUL byte in the database name, or a NUL byte in the collection + * name, would shift the namespace split performed by the server and cause + * the operation to silently target a different database or collection. + * + * @internal + * @throws InvalidArgumentException if either name is invalid + */ +function create_namespace(string $databaseName, string $collectionName): string +{ + if ($databaseName === '' || str_contains($databaseName, '.') || str_contains($databaseName, "\0")) { + throw new InvalidArgumentException('$databaseName is invalid: ' . $databaseName); + } + + if ($collectionName === '' || str_contains($collectionName, "\0")) { + throw new InvalidArgumentException('$collectionName is invalid: ' . $collectionName); + } + + return $databaseName . '.' . $collectionName; +} + /** * Performs a deep copy of a value. * diff --git a/tests/Collection/CollectionFunctionalTest.php b/tests/Collection/CollectionFunctionalTest.php index e5eaa775b..6766af355 100644 --- a/tests/Collection/CollectionFunctionalTest.php +++ b/tests/Collection/CollectionFunctionalTest.php @@ -35,7 +35,7 @@ */ class CollectionFunctionalTest extends FunctionalTestCase { - #[DataProvider('provideInvalidDatabaseAndCollectionNames')] + #[DataProvider('provideInvalidDatabaseNames')] public function testConstructorDatabaseNameArgument($databaseName, string $expectedExceptionClass): void { $this->expectException($expectedExceptionClass); @@ -43,7 +43,7 @@ public function testConstructorDatabaseNameArgument($databaseName, string $expec new Collection($this->manager, $databaseName, $this->getCollectionName()); } - #[DataProvider('provideInvalidDatabaseAndCollectionNames')] + #[DataProvider('provideInvalidCollectionNames')] public function testConstructorCollectionNameArgument($collectionName, string $expectedExceptionClass): void { $this->expectException($expectedExceptionClass); @@ -51,11 +51,29 @@ public function testConstructorCollectionNameArgument($collectionName, string $e new Collection($this->manager, $this->getDatabaseName(), $collectionName); } - public static function provideInvalidDatabaseAndCollectionNames() + public function testConstructorAllowsDotInCollectionName(): void + { + $collection = new Collection($this->manager, $this->getDatabaseName(), 'foo.bar'); + + $this->assertSame('foo.bar', $collection->getCollectionName()); + } + + public static function provideInvalidDatabaseNames() + { + return [ + [null, TypeError::class], + ['', InvalidArgumentException::class], + ['foo.bar', InvalidArgumentException::class], + ["foo\0bar", InvalidArgumentException::class], + ]; + } + + public static function provideInvalidCollectionNames() { return [ [null, TypeError::class], ['', InvalidArgumentException::class], + ["foo\0bar", InvalidArgumentException::class], ]; } diff --git a/tests/Database/DatabaseFunctionalTest.php b/tests/Database/DatabaseFunctionalTest.php index d7349ec21..45a13fbe5 100644 --- a/tests/Database/DatabaseFunctionalTest.php +++ b/tests/Database/DatabaseFunctionalTest.php @@ -39,6 +39,8 @@ public static function provideInvalidDatabaseNames() return [ [null, TypeError::class], ['', InvalidArgumentException::class], + ['foo.bar', InvalidArgumentException::class], + ["foo\0bar", InvalidArgumentException::class], ]; } diff --git a/tests/Operation/BulkWriteTest.php b/tests/Operation/BulkWriteTest.php index 58df1493f..1062b47f4 100644 --- a/tests/Operation/BulkWriteTest.php +++ b/tests/Operation/BulkWriteTest.php @@ -13,6 +13,15 @@ class BulkWriteTest extends TestCase { + #[DataProvider('provideInvalidDatabaseAndCollectionNames')] + public function testConstructorDatabaseAndCollectionNameChecks(string $databaseName, string $collectionName): void + { + $this->expectException(InvalidArgumentException::class); + new BulkWrite($databaseName, $collectionName, [ + [BulkWrite::INSERT_ONE => [['x' => 1]]], + ]); + } + public function testOperationsMustNotBeEmpty(): void { $this->expectException(InvalidArgumentException::class); diff --git a/tests/Operation/DeleteTest.php b/tests/Operation/DeleteTest.php index e1cd5bd74..3360f9cf4 100644 --- a/tests/Operation/DeleteTest.php +++ b/tests/Operation/DeleteTest.php @@ -16,6 +16,13 @@ class DeleteTest extends TestCase { + #[DataProvider('provideInvalidDatabaseAndCollectionNames')] + public function testConstructorDatabaseAndCollectionNameChecks(string $databaseName, string $collectionName): void + { + $this->expectException(InvalidArgumentException::class); + new Delete($databaseName, $collectionName, ['x' => 1], 1); + } + #[DataProvider('provideInvalidDocumentValues')] public function testConstructorFilterArgumentTypeCheck($filter): void { diff --git a/tests/Operation/FindTest.php b/tests/Operation/FindTest.php index 97cc1d388..51f590d13 100644 --- a/tests/Operation/FindTest.php +++ b/tests/Operation/FindTest.php @@ -12,6 +12,13 @@ class FindTest extends TestCase { + #[DataProvider('provideInvalidDatabaseAndCollectionNames')] + public function testConstructorDatabaseAndCollectionNameChecks(string $databaseName, string $collectionName): void + { + $this->expectException(InvalidArgumentException::class); + new Find($databaseName, $collectionName, ['x' => 1]); + } + #[DataProvider('provideInvalidDocumentValues')] public function testConstructorFilterArgumentTypeCheck($filter): void { diff --git a/tests/Operation/InsertManyTest.php b/tests/Operation/InsertManyTest.php index 8ee11ff5d..e37a93f8c 100644 --- a/tests/Operation/InsertManyTest.php +++ b/tests/Operation/InsertManyTest.php @@ -10,6 +10,13 @@ class InsertManyTest extends TestCase { + #[DataProvider('provideInvalidDatabaseAndCollectionNames')] + public function testConstructorDatabaseAndCollectionNameChecks(string $databaseName, string $collectionName): void + { + $this->expectException(InvalidArgumentException::class); + new InsertMany($databaseName, $collectionName, [['x' => 1]]); + } + public function testConstructorDocumentsMustNotBeEmpty(): void { $this->expectException(InvalidArgumentException::class); diff --git a/tests/Operation/InsertOneTest.php b/tests/Operation/InsertOneTest.php index 1f641ea88..6dc821827 100644 --- a/tests/Operation/InsertOneTest.php +++ b/tests/Operation/InsertOneTest.php @@ -12,6 +12,13 @@ class InsertOneTest extends TestCase { + #[DataProvider('provideInvalidDatabaseAndCollectionNames')] + public function testConstructorDatabaseAndCollectionNameChecks(string $databaseName, string $collectionName): void + { + $this->expectException(InvalidArgumentException::class); + new InsertOne($databaseName, $collectionName, ['x' => 1]); + } + #[DataProvider('provideInvalidDocumentValues')] public function testConstructorDocumentArgumentTypeCheck($document): void { diff --git a/tests/Operation/RenameCollectionTest.php b/tests/Operation/RenameCollectionTest.php index e0a438329..211574ad2 100644 --- a/tests/Operation/RenameCollectionTest.php +++ b/tests/Operation/RenameCollectionTest.php @@ -29,4 +29,23 @@ public static function provideInvalidConstructorOptions() 'writeConcern' => self::getInvalidWriteConcernValues(), ]); } + + #[DataProvider('provideInvalidRenameDatabaseAndCollectionNames')] + public function testConstructorDatabaseAndCollectionNameChecks(string $fromDatabaseName, string $fromCollectionName, string $toDatabaseName, string $toCollectionName): void + { + $this->expectException(InvalidArgumentException::class); + new RenameCollection($fromDatabaseName, $fromCollectionName, $toDatabaseName, $toCollectionName); + } + + public static function provideInvalidRenameDatabaseAndCollectionNames(): array + { + return [ + 'dot in fromDatabaseName' => ['foo.bar', 'coll', 'db', 'coll'], + 'NUL byte in fromDatabaseName' => ["foo\0bar", 'coll', 'db', 'coll'], + 'NUL byte in fromCollectionName' => ['db', "foo\0bar", 'db', 'coll'], + 'dot in toDatabaseName' => ['db', 'coll', 'foo.bar', 'coll'], + 'NUL byte in toDatabaseName' => ['db', 'coll', "foo\0bar", 'coll'], + 'NUL byte in toCollectionName' => ['db', 'coll', 'db', "foo\0bar"], + ]; + } } diff --git a/tests/Operation/UpdateTest.php b/tests/Operation/UpdateTest.php index 9617c77e8..a36aba299 100644 --- a/tests/Operation/UpdateTest.php +++ b/tests/Operation/UpdateTest.php @@ -11,6 +11,13 @@ class UpdateTest extends TestCase { + #[DataProvider('provideInvalidDatabaseAndCollectionNames')] + public function testConstructorDatabaseAndCollectionNameChecks(string $databaseName, string $collectionName): void + { + $this->expectException(InvalidArgumentException::class); + new Update($databaseName, $collectionName, ['x' => 1], ['$set' => ['x' => 1]]); + } + #[DataProvider('provideInvalidDocumentValues')] public function testConstructorFilterArgumentTypeCheck($filter): void { diff --git a/tests/SpecTests/Crud/Prose17_DatabaseAndCollectionNameValidationTest.php b/tests/SpecTests/Crud/Prose17_DatabaseAndCollectionNameValidationTest.php new file mode 100644 index 000000000..2337a497c --- /dev/null +++ b/tests/SpecTests/Crud/Prose17_DatabaseAndCollectionNameValidationTest.php @@ -0,0 +1,65 @@ +expectException(InvalidArgumentException::class); + $client->getDatabase('foo.bar')->getCollection('coll')->insertOne([]); + } + + public function testDotInDatabaseNameViaGetCollection(): void + { + $client = self::createTestClient(); + + $this->expectException(InvalidArgumentException::class); + $client->getCollection('foo.bar', 'coll')->insertOne([]); + } + + public function testNulByteInDatabaseName(): void + { + $client = self::createTestClient(); + + $this->expectException(InvalidArgumentException::class); + $client->getDatabase("foo\0bar")->getCollection('coll')->insertOne([]); + } + + public function testNulByteInCollectionName(): void + { + $client = self::createTestClient(); + + $this->expectException(InvalidArgumentException::class); + $client->getDatabase('db')->getCollection("foo\0bar")->insertOne([]); + } + + public function testNulByteInBulkWriteDatabaseName(): void + { + $client = self::createTestClient(); + + $this->expectException(InvalidArgumentException::class); + ClientBulkWrite::createWithCollection($client->getCollection("foo\0bar", 'coll')) + ->insertOne([]); + } + + public function testNulByteInBulkWriteCollectionName(): void + { + $client = self::createTestClient(); + + $this->expectException(InvalidArgumentException::class); + ClientBulkWrite::createWithCollection($client->getCollection('db', "foo\0bar")) + ->insertOne([]); + } +} diff --git a/tests/TestCase.php b/tests/TestCase.php index 8508deb87..f8dbccdf7 100644 --- a/tests/TestCase.php +++ b/tests/TestCase.php @@ -159,6 +159,15 @@ final public static function provideInvalidStringValues(): array return self::wrapValuesForDataProvider(self::getInvalidStringValues()); } + final public static function provideInvalidDatabaseAndCollectionNames(): array + { + return [ + 'dot in databaseName' => ['foo.bar', 'coll'], + 'NUL byte in databaseName' => ["foo\0bar", 'coll'], + 'NUL byte in collectionName' => ['db', "foo\0bar"], + ]; + } + protected function assertDeprecated(callable $execution): mixed { return $this->assertError(E_USER_DEPRECATED | E_DEPRECATED, $execution);