Skip to content

Commit

Permalink
PHPLIB-1137: Reject PackedArrays when expecting documents (#1117)
Browse files Browse the repository at this point in the history 
* Refactor data providers

With this refactoring, data providers become more concise; each data entry also gets a distinct name making for easier identification of failing tests

* PHPLIB-1137: Prohibit PackedArray objects where documents are required

* Refactor validity check for bulk update argument

* Fix invalid data providers

* Ensure unique data set names in createOptionDataProvider

* Move invalid update values data provider to operation tests

* Introduce new exception factory for expected document type
  • Loading branch information
@alcaeus
alcaeus authored Jun 27, 2023
1 parent 0d13981 commit 88731a5
Show file tree
Hide file tree
Showing 69 changed files with 633 additions and 1,222 deletions.
81 changes: 28 additions & 53 deletions psalm-baseline.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -196,10 +196,6 @@
</MixedAssignment>
</file>
<file src="src/Model/ChangeStreamIterator.php">
<DocblockTypeContradiction occurrences="2">
<code>! is_array($document) &amp;&amp; ! is_object($document)</code>
<code>isset($initialResumeToken) &amp;&amp; ! is_array($initialResumeToken) &amp;&amp; ! is_object($initialResumeToken)</code>
</DocblockTypeContradiction>
<MixedArgument occurrences="2">
<code>$reply-&gt;cursor-&gt;nextBatch</code>
<code>$this-&gt;current()</code>
Expand Down Expand Up @@ -276,8 +272,9 @@
</MixedAssignment>
</file>
<file src="src/Model/IndexInput.php">
<MixedArgument occurrences="1">
<MixedArgument occurrences="2">
<code>$fieldName</code>
<code>$index['key']</code>
</MixedArgument>
<MixedAssignment occurrences="2">
<code>$fieldName</code>
Expand Down Expand Up @@ -328,11 +325,13 @@
<code>$args[1]</code>
<code>$args[2]</code>
</MixedArgument>
<MixedArrayAccess occurrences="28">
<MixedArrayAccess occurrences="30">
<code>$args[0]</code>
<code>$args[0]</code>
<code>$args[0]</code>
<code>$args[0]</code>
<code>$args[0]</code>
<code>$args[1]</code>
<code>$args[1]</code>
<code>$args[1]</code>
<code>$args[1]</code>
Expand Down Expand Up @@ -395,9 +394,6 @@
</MixedOperand>
</file>
<file src="src/Operation/Count.php">
<DocblockTypeContradiction occurrences="1">
<code>! is_array($filter) &amp;&amp; ! is_object($filter)</code>
</DocblockTypeContradiction>
<MixedAssignment occurrences="6">
<code>$cmd[$option]</code>
<code>$cmd['hint']</code>
Expand All @@ -410,11 +406,6 @@
<code>isInTransaction</code>
</MixedMethodCall>
</file>
<file src="src/Operation/CountDocuments.php">
<DocblockTypeContradiction occurrences="1">
<code>! is_array($filter) &amp;&amp; ! is_object($filter)</code>
</DocblockTypeContradiction>
</file>
<file src="src/Operation/CreateCollection.php">
<MixedArgument occurrences="2">
<code>$options['pipeline']</code>
Expand All @@ -427,7 +418,8 @@
</MixedAssignment>
</file>
<file src="src/Operation/CreateEncryptedCollection.php">
<MixedArgument occurrences="1">
<MixedArgument occurrences="2">
<code>$options['encryptedFields']</code>
<code>$this-&gt;options['encryptedFields']</code>
</MixedArgument>
</file>
Expand All @@ -446,9 +438,6 @@
</MixedMethodCall>
</file>
<file src="src/Operation/DatabaseCommand.php">
<DocblockTypeContradiction occurrences="1">
<code>! is_array($command) &amp;&amp; ! is_object($command)</code>
</DocblockTypeContradiction>
<MixedArgument occurrences="1">
<code>$this-&gt;options['typeMap']</code>
</MixedArgument>
Expand All @@ -458,9 +447,6 @@
</MixedAssignment>
</file>
<file src="src/Operation/Delete.php">
<DocblockTypeContradiction occurrences="1">
<code>! is_array($filter) &amp;&amp; ! is_object($filter)</code>
</DocblockTypeContradiction>
<MixedArgument occurrences="1">
<code>$this-&gt;options['writeConcern']</code>
</MixedArgument>
Expand All @@ -476,9 +462,6 @@
</MixedMethodCall>
</file>
<file src="src/Operation/Distinct.php">
<DocblockTypeContradiction occurrences="1">
<code>! is_array($filter) &amp;&amp; ! is_object($filter)</code>
</DocblockTypeContradiction>
<MixedArgument occurrences="1">
<code>$this-&gt;options['typeMap']</code>
</MixedArgument>
Expand Down Expand Up @@ -516,6 +499,11 @@
<code>$options['writeConcern']</code>
</MixedAssignment>
</file>
<file src="src/Operation/DropEncryptedCollection.php">
<MixedArgument occurrences="1">
<code>$options['encryptedFields']</code>
</MixedArgument>
</file>
<file src="src/Operation/DropIndexes.php">
<MixedArgument occurrences="1">
<code>$this-&gt;options['typeMap']</code>
Expand All @@ -540,9 +528,6 @@
</MixedAssignment>
</file>
<file src="src/Operation/Find.php">
<DocblockTypeContradiction occurrences="1">
<code>! is_array($filter) &amp;&amp; ! is_object($filter)</code>
</DocblockTypeContradiction>
<MixedArgument occurrences="1">
<code>$this-&gt;options['typeMap']</code>
</MixedArgument>
Expand Down Expand Up @@ -583,32 +568,30 @@
</MixedReturnStatement>
</file>
<file src="src/Operation/FindOneAndDelete.php">
<DocblockTypeContradiction occurrences="1">
<code>! is_array($filter) &amp;&amp; ! is_object($filter)</code>
</DocblockTypeContradiction>
<MixedAssignment occurrences="1">
<code>$options['fields']</code>
</MixedAssignment>
</file>
<file src="src/Operation/FindOneAndReplace.php">
<DocblockTypeContradiction occurrences="2">
<code>! is_array($filter) &amp;&amp; ! is_object($filter)</code>
<code>! is_array($replacement) &amp;&amp; ! is_object($replacement)</code>
</DocblockTypeContradiction>
<MixedAssignment occurrences="1">
<code>$options['fields']</code>
</MixedAssignment>
<MixedOperand occurrences="1">
<code>$options['returnDocument']</code>
</MixedOperand>
</file>
<file src="src/Operation/FindOneAndUpdate.php">
<DocblockTypeContradiction occurrences="2">
<code>! is_array($filter) &amp;&amp; ! is_object($filter)</code>
<DocblockTypeContradiction occurrences="1">
<code>! is_array($update) &amp;&amp; ! is_object($update)</code>
</DocblockTypeContradiction>
<MixedAssignment occurrences="1">
<code>$options['fields']</code>
</MixedAssignment>
<MixedOperand occurrences="1">
<code>$options['returnDocument']</code>
</MixedOperand>
</file>
<file src="src/Operation/InsertMany.php">
<DocblockTypeContradiction occurrences="1">
<code>! is_array($document) &amp;&amp; ! is_object($document)</code>
</DocblockTypeContradiction>
<MixedAssignment occurrences="4">
<code>$insertedIds[$i]</code>
<code>$options[$option]</code>
Expand All @@ -621,9 +604,6 @@
</MixedMethodCall>
</file>
<file src="src/Operation/InsertOne.php">
<DocblockTypeContradiction occurrences="1">
<code>! is_array($document) &amp;&amp; ! is_object($document)</code>
</DocblockTypeContradiction>
<MixedAssignment occurrences="4">
<code>$insertedId</code>
<code>$options[$option]</code>
Expand Down Expand Up @@ -692,14 +672,8 @@
<code>isInTransaction</code>
</MixedMethodCall>
</file>
<file src="src/Operation/ReplaceOne.php">
<DocblockTypeContradiction occurrences="1">
<code>! is_array($replacement) &amp;&amp; ! is_object($replacement)</code>
</DocblockTypeContradiction>
</file>
<file src="src/Operation/Update.php">
<DocblockTypeContradiction occurrences="2">
<code>! is_array($filter) &amp;&amp; ! is_object($filter)</code>
<DocblockTypeContradiction occurrences="1">
<code>! is_array($update) &amp;&amp; ! is_object($update)</code>
</DocblockTypeContradiction>
<MixedArgument occurrences="1">
Expand Down Expand Up @@ -767,11 +741,11 @@
</MixedAssignment>
</file>
<file src="src/functions.php">
<DocblockTypeContradiction occurrences="2">
<code>! is_array($document) &amp;&amp; ! is_object($document)</code>
<DocblockTypeContradiction occurrences="1">
<code>is_array($document)</code>
</DocblockTypeContradiction>
<MixedArgument occurrences="1">
<MixedArgument occurrences="2">
<code>$stage</code>
<code>$wireVersionForWriteStageOnSecondary</code>
</MixedArgument>
<MixedArgumentTypeCoercion occurrences="1">
Expand All @@ -784,8 +758,9 @@
<code>$typeMap['fieldPaths'][$fieldPath]</code>
<code>$typeMap['fieldPaths'][substr($fieldPath, 0, -2)]</code>
</MixedArrayAssignment>
<MixedAssignment occurrences="5">
<MixedAssignment occurrences="6">
<code>$element[$key]</code>
<code>$stage</code>
<code>$type</code>
<code>$typeMap['fieldPaths'][$fieldPath . '.' . $existingFieldPath]</code>
<code>$typeMap['fieldPaths'][$fieldPath]</code>
Expand Down
7 changes: 3 additions & 4 deletions src/Command/ListCollections.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,9 @@
use MongoDB\Model\CachingIterator;
use MongoDB\Operation\Executable;

use function is_array;
use function is_bool;
use function is_integer;
use function is_object;
use function MongoDB\is_document;

/**
* Wrapper for the listCollections command.
Expand Down Expand Up @@ -79,8 +78,8 @@ public function __construct(string $databaseName, array $options = [])
throw InvalidArgumentException::invalidType('"authorizedCollections" option', $options['authorizedCollections'], 'boolean');
}

if (isset($options['filter']) && ! is_array($options['filter']) && ! is_object($options['filter'])) {
throw InvalidArgumentException::invalidType('"filter" option', $options['filter'], 'array or object');
if (isset($options['filter']) && ! is_document($options['filter'])) {
throw InvalidArgumentException::expectedDocumentType('"filter" option', $options['filter']);
}

if (isset($options['maxTimeMS']) && ! is_integer($options['maxTimeMS'])) {
Expand Down
6 changes: 3 additions & 3 deletions src/Command/ListDatabases.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
use function is_array;
use function is_bool;
use function is_integer;
use function is_object;
use function MongoDB\is_document;

/**
* Wrapper for the ListDatabases command.
Expand Down Expand Up @@ -76,8 +76,8 @@ public function __construct(array $options = [])
throw InvalidArgumentException::invalidType('"authorizedDatabases" option', $options['authorizedDatabases'], 'boolean');
}

if (isset($options['filter']) && ! is_array($options['filter']) && ! is_object($options['filter'])) {
throw InvalidArgumentException::invalidType('"filter" option', $options['filter'], ['array', 'object']);
if (isset($options['filter']) && ! is_document($options['filter'])) {
throw InvalidArgumentException::expectedDocumentType('"filter" option', $options['filter']);
}

if (isset($options['maxTimeMS']) && ! is_integer($options['maxTimeMS'])) {
Expand Down
11 changes: 11 additions & 0 deletions src/Exception/InvalidArgumentException.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,17 @@

class InvalidArgumentException extends DriverInvalidArgumentException implements Exception
{
/**
* Thrown when an argument or option is expected to be a document.
*
* @param string $name Name of the argument or option
* @param mixed $value Actual value (used to derive the type)
*/
public static function expectedDocumentType(string $name, $value): self
{
return new self(sprintf('Expected %s to have type "document" (array or object) but found "%s"', $name, get_debug_type($value)));
}

/**
* Thrown when an argument or option has an invalid type.
*
Expand Down
7 changes: 3 additions & 4 deletions src/GridFS/WritableStream.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,10 @@
use function hash_final;
use function hash_init;
use function hash_update;
use function is_array;
use function is_bool;
use function is_integer;
use function is_object;
use function is_string;
use function MongoDB\is_document;
use function MongoDB\is_string_array;
use function sprintf;
use function strlen;
Expand Down Expand Up @@ -130,8 +129,8 @@ public function __construct(CollectionWrapper $collectionWrapper, string $filena
throw InvalidArgumentException::invalidType('"contentType" option', $options['contentType'], 'string');
}

if (isset($options['metadata']) && ! is_array($options['metadata']) && ! is_object($options['metadata'])) {
throw InvalidArgumentException::invalidType('"metadata" option', $options['metadata'], 'array or object');
if (isset($options['metadata']) && ! is_document($options['metadata'])) {
throw InvalidArgumentException::expectedDocumentType('"metadata" option', $options['metadata']);
}

$this->chunkSize = $options['chunkSizeBytes'];
Expand Down
9 changes: 5 additions & 4 deletions src/Model/ChangeStreamIterator.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@
use function is_object;
use function MongoDB\Driver\Monitoring\addSubscriber;
use function MongoDB\Driver\Monitoring\removeSubscriber;
use function MongoDB\is_document;

/**
* ChangeStreamIterator wraps a change stream's tailable cursor.
Expand Down Expand Up @@ -75,8 +76,8 @@ class ChangeStreamIterator extends IteratorIterator implements CommandSubscriber
*/
public function __construct(Cursor $cursor, int $firstBatchSize, $initialResumeToken, ?object $postBatchResumeToken)
{
if (isset($initialResumeToken) && ! is_array($initialResumeToken) && ! is_object($initialResumeToken)) {
throw InvalidArgumentException::invalidType('$initialResumeToken', $initialResumeToken, 'array or object');
if (isset($initialResumeToken) && ! is_document($initialResumeToken)) {
throw InvalidArgumentException::expectedDocumentType('$initialResumeToken', $initialResumeToken);
}

parent::__construct($cursor);
Expand Down Expand Up @@ -234,8 +235,8 @@ public function valid(): bool
*/
private function extractResumeToken($document)
{
if (! is_array($document) && ! is_object($document)) {
throw InvalidArgumentException::invalidType('$document', $document, 'array or object');
if (! is_document($document)) {
throw InvalidArgumentException::expectedDocumentType('$document', $document);
}

if ($document instanceof Serializable) {
Expand Down
7 changes: 3 additions & 4 deletions src/Model/IndexInput.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,11 @@
use MongoDB\BSON\Serializable;
use MongoDB\Exception\InvalidArgumentException;

use function is_array;
use function is_float;
use function is_int;
use function is_object;
use function is_string;
use function MongoDB\document_to_array;
use function MongoDB\is_document;
use function sprintf;

/**
Expand Down Expand Up @@ -53,8 +52,8 @@ public function __construct(array $index)
throw new InvalidArgumentException('Required "key" document is missing from index specification');
}

if (! is_array($index['key']) && ! is_object($index['key'])) {
throw InvalidArgumentException::invalidType('"key" option', $index['key'], 'array or object');
if (! is_document($index['key'])) {
throw InvalidArgumentException::expectedDocumentType('"key" option', $index['key']);
}

foreach ($index['key'] as $fieldName => $order) {
Expand Down
9 changes: 5 additions & 4 deletions src/Operation/Aggregate.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
use function is_object;
use function is_string;
use function MongoDB\create_field_path_type_map;
use function MongoDB\is_document;
use function MongoDB\is_last_pipeline_operator_write;
use function MongoDB\is_pipeline;

Expand Down Expand Up @@ -155,8 +156,8 @@ public function __construct(string $databaseName, ?string $collectionName, array
throw InvalidArgumentException::invalidType('"bypassDocumentValidation" option', $options['bypassDocumentValidation'], 'boolean');
}

if (isset($options['collation']) && ! is_array($options['collation']) && ! is_object($options['collation'])) {
throw InvalidArgumentException::invalidType('"collation" option', $options['collation'], 'array or object');
if (isset($options['collation']) && ! is_document($options['collation'])) {
throw InvalidArgumentException::expectedDocumentType('"collation" option', $options['collation']);
}

if (isset($options['explain']) && ! is_bool($options['explain'])) {
Expand All @@ -167,8 +168,8 @@ public function __construct(string $databaseName, ?string $collectionName, array
throw InvalidArgumentException::invalidType('"hint" option', $options['hint'], 'string or array or object');
}

if (isset($options['let']) && ! is_array($options['let']) && ! is_object($options['let'])) {
throw InvalidArgumentException::invalidType('"let" option', $options['let'], ['array', 'object']);
if (isset($options['let']) && ! is_document($options['let'])) {
throw InvalidArgumentException::expectedDocumentType('"let" option', $options['let']);
}

if (isset($options['maxAwaitTimeMS']) && ! is_integer($options['maxAwaitTimeMS'])) {
Expand Down
Loading

0 comments on commit 88731a5

Please sign in to comment.