fix(validator): refuse filter values that are not a list

src/Database/Validator/Query/Filter.php

@@ -326,7 +326,7 @@ protected function isValidAttributeAndValues(string $attribute, array $values, M
                     continue 2;
 
                 case ColumnType::Vector:
-                    if (! is_array($value)) {
+                    if (! is_array($value) || ! \array_is_list($value)) {
                         $this->message = 'Vector query value must be an array';
 
                         return false;
@@ -429,15 +429,42 @@ private function joinedSchema(string $alias, string $column): ?array
      */
     protected function isEmpty(array $values): bool
     {
-        if (count($values) === 0) {
+        if ($values === []) {
             return true;
         }
 
-        if (is_array($values[0]) && count($values[0]) === 0) {
-            return true;
+        return \reset($values) === [];
+    }
+
+    /**
+     * A distance query holds one `[geometry, distance, meters]` tuple: a finite distance and whether it is in meters.
+     *
+     * @param  array<mixed>  $values
+     */
+    private function isValidDistance(array $values): bool
+    {
+        $tuple = $values[0] ?? null;
+        if (\count($values) !== 1 || ! \is_array($tuple) || ! \array_is_list($tuple) || \count($tuple) !== 3) {
+            $this->message = 'Distance query requires [[geometry, distance]] parameters';
+
+            return false;
+        }
+
+        [, $distance, $meters] = $tuple;
+
+        if (! \is_numeric($distance) || ! \is_finite((float) $distance)) {
+            $this->message = 'Distance query requires a finite distance';
+
+            return false;
         }
 
-        return false;
+        if (! \is_bool($meters)) {
+            $this->message = 'Distance query requires a boolean meters flag';
+
+            return false;
+        }
+
+        return true;
     }
 
     /**
@@ -489,6 +516,13 @@ public function isValid(mixed $value): bool
     {
         $method = $value->getMethod();
         $attribute = $value->getAttribute();
+
+        if (! \array_is_list($value->getValues())) {
+            $this->message = \ucfirst($method->value).' query values must be a list';
+
+            return false;
+        }
+
         switch ($method) {
             case Method::Equal:
             case Method::Contains:
@@ -517,16 +551,7 @@ public function isValid(mixed $value): bool
             case Method::DistanceNotEqual:
             case Method::DistanceGreaterThan:
             case Method::DistanceLessThan:
-                if (count($value->getValues()) !== 1 || ! is_array($value->getValues()[0]) || count($value->getValues()[0]) !== 3) {
-                    $this->message = 'Distance query requires [[geometry, distance]] parameters';
-
-                    return false;
-                }
-
-                $distance = $value->getValues()[0][1];
-                if (! \is_numeric($distance) || ! \is_finite((float) $distance)) {
-                    $this->message = 'Distance query requires a finite distance';
-
+                if (! $this->isValidDistance($value->getValues())) {
                     return false;
                 }
 
@@ -593,7 +618,7 @@ public function isValid(mixed $value): bool
                 }
 
                 /** @var array<string, mixed> $attributeSchema */
-                $attributeSchema = $this->schema[$attributeKey];
+                $attributeSchema = $this->schema[$attributeKey] ?? [];
                 /** @var ColumnType|null $vectorAttrType */
                 $vectorAttrType = $attributeSchema['type'] ?? null;
                 if ($vectorAttrType !== ColumnType::Vector) {

tests/e2e/Adapter/Scopes/DocumentTests.php

@@ -1609,6 +1609,74 @@ public function testGetDocument(): void
         $this->assertEquals('Works', $document->getAttribute('with-dash'));
     }
 
+    /**
+     * A query parsed from JSON takes an object for its values as readily as a list. Filter read such values by
+     * position and raised "Undefined array key 0" before refusing them. Every adapter refuses them as an invalid
+     * query, for find() and count(), without a warning.
+     */
+    public function testFilterValuesThatAreNotAListAreAnInvalidQuery(): void
+    {
+        $database = $this->getDatabase();
+        $adapter = $database->getAdapter();
+        $spatial = $adapter->hasFeature(Feature\Spatial::class);
+        $vectors = $adapter->supports(Capability::Vectors);
+        $collection = 'object_values_'.\substr(\uniqid(), -6);
+        $database->createCollection(Collection::create(
+            id: $collection,
+            attributes: [
+                Attribute::string(key: 'name', size: 20),
+                Attribute::string(key: 'tags', size: 20, array: true),
+                Attribute::integer(key: 'rank'),
+                ...($spatial ? [Attribute::point(key: 'location')] : []),
+                ...($vectors ? [Attribute::vector(key: 'embedding', dimensions: 2)] : []),
+            ],
+            permissions: [Permission::create(Role::any()), Permission::read(Role::any())],
+        ));
+        $database->createDocument($collection, new Document([
+            'name' => 'a',
+            'tags' => ['a'],
+            'rank' => 1,
+            ...($spatial ? ['location' => [1.0, 2.0]] : []),
+            ...($vectors ? ['embedding' => [1.0, 0.0]] : []),
+            '$permissions' => [Permission::read(Role::any())],
+        ]));
+
+        $queries = [
+            '{"method":"equal","attribute":"name","values":{"value":"a"}}',
+            '{"method":"equal","attribute":"name","values":{"1":"a"}}',
+            '{"method":"notEqual","attribute":"name","values":{"value":"a"}}',
+            '{"method":"contains","attribute":"tags","values":{"tag":"a"}}',
+            '{"method":"lessThan","attribute":"rank","values":{"bound":5}}',
+            '{"method":"between","attribute":"rank","values":{"from":0,"to":5}}',
+            '{"method":"startsWith","attribute":"name","values":{"prefix":"a"}}',
+            '{"method":"exists","attribute":"","values":{"attribute":"name"}}',
+        ];
+        if ($spatial) {
+            $queries[] = '{"method":"intersects","attribute":"location","values":{"type":"Point","coordinates":[1,2]}}';
+            $queries[] = '{"method":"distanceLessThan","attribute":"location","values":{"geometry":[[1,2],10,false]}}';
+            $queries[] = '{"method":"distanceLessThan","attribute":"location","values":[{"geometry":[1,2],"distance":10,"meters":false}]}';
+        }
+        if ($vectors) {
+            $queries[] = '{"method":"vectorDot","attribute":"embedding","values":{"vector":[1,0]}}';
+        }
+
+        foreach ($queries as $json) {
+            $query = Query::parse($json);
+            foreach (['find' => fn () => $database->find($collection, [$query]), 'count' => fn () => $database->count($collection, [$query])] as $read => $call) {
+                try {
+                    $call();
+                    $this->fail("{$read}() refuses {$json}");
+                } catch (QueryException $e) {
+                    $this->assertStringStartsWith('Invalid query: ', $e->getMessage(), "{$read}() of {$json}");
+                }
+            }
+        }
+
+        $this->assertCount(1, $database->find($collection, [Query::parse('{"method":"equal","attribute":"name","values":["a"]}')]));
+
+        $database->deleteCollection($collection);
+    }
+
     public function testFind(): void
     {
         $this->initMoviesFixture();

tests/unit/Validator/Query/FilterMalformedValuesTest.php

@@ -0,0 +1,102 @@
+<?php
+
+namespace Tests\Unit\Validator\Query;
+
+use PHPUnit\Framework\Attributes\DataProvider;
+use PHPUnit\Framework\TestCase;
+use Utopia\Database\Document;
+use Utopia\Database\Query;
+use Utopia\Database\Validator\Query\Filter;
+use Utopia\Query\Method;
+use Utopia\Query\Schema\ColumnType;
+
+/**
+ * A query parsed from JSON takes a JSON object for its values as readily as a list. The validator read such values
+ * by position and raised an undefined key warning; it refuses them as invalid instead.
+ */
+final class FilterMalformedValuesTest extends TestCase
+{
+    private function validator(bool $supportForAttributes = true): Filter
+    {
+        return new Filter(
+            attributes: [
+                new Document(['$id' => 'name', 'key' => 'name', 'type' => ColumnType::String->value, 'array' => false]),
+                new Document(['$id' => 'tags', 'key' => 'tags', 'type' => ColumnType::String->value, 'array' => true]),
+                new Document(['$id' => 'location', 'key' => 'location', 'type' => ColumnType::Point->value, 'array' => false]),
+                new Document(['$id' => 'embedding', 'key' => 'embedding', 'type' => ColumnType::Vector->value, 'array' => false, 'size' => 2]),
+            ],
+            idAttributeType: ColumnType::Integer->value,
+            supportForAttributes: $supportForAttributes,
+        );
+    }
+
+    /**
+     * @return iterable<string, array{string}>
+     */
+    public static function objectValues(): iterable
+    {
+        yield 'equal' => ['{"method":"equal","attribute":"name","values":{"value":"a"}}'];
+        yield 'contains' => ['{"method":"contains","attribute":"tags","values":{"tag":"a"}}'];
+        yield 'notEqual' => ['{"method":"notEqual","attribute":"name","values":{"value":"a"}}'];
+        yield 'regex' => ['{"method":"regex","attribute":"name","values":{"pattern":"^a"}}'];
+        yield 'between' => ['{"method":"between","attribute":"name","values":{"from":"a","to":"b"}}'];
+        yield 'exists' => ['{"method":"exists","attribute":"","values":{"attribute":"name"}}'];
+        yield 'intersects' => ['{"method":"intersects","attribute":"location","values":{"type":"Point","coordinates":[1,2]}}'];
+        yield 'distanceLessThan' => ['{"method":"distanceLessThan","attribute":"location","values":{"geometry":[[1,2],10,false]}}'];
+        yield 'vectorDot' => ['{"method":"vectorDot","attribute":"embedding","values":{"vector":[1,2]}}'];
+    }
+
+    #[DataProvider('objectValues')]
+    public function testValuesGivenAsAnObjectAreRefused(string $json): void
+    {
+        $validator = $this->validator();
+        $query = Query::parse($json);
+
+        $this->assertFalse($validator->isValid($query));
+        $this->assertSame(\ucfirst($query->getMethod()->value).' query values must be a list', $validator->getDescription());
+    }
+
+    public function testADistanceTupleGivenAsAnObjectIsRefused(): void
+    {
+        $validator = $this->validator();
+        $query = Query::parse('{"method":"distanceLessThan","attribute":"location","values":[{"geometry":[1,2],"distance":10,"meters":false}]}');
+
+        $this->assertFalse($validator->isValid($query));
+        $this->assertSame('Distance query requires [[geometry, distance]] parameters', $validator->getDescription());
+    }
+
+    public function testADistanceWithAMetersFlagThatIsNotABooleanIsRefused(): void
+    {
+        $validator = $this->validator();
+
+        $this->assertFalse($validator->isValid(new Query(Method::DistanceLessThan, 'location', [[[1, 2], 10, ['yes']]])));
+        $this->assertSame('Distance query requires a boolean meters flag', $validator->getDescription());
+        $this->assertTrue($validator->isValid(Query::distanceLessThan('location', [1, 2], 10, true)));
+    }
+
+    public function testAVectorGivenAsAnObjectIsRefused(): void
+    {
+        $validator = $this->validator();
+
+        $this->assertFalse($validator->isValid(Query::vectorDot('embedding', ['x' => 1.0, 'y' => 2.0])));
+        $this->assertSame('Vector query value must be an array', $validator->getDescription());
+        $this->assertTrue($validator->isValid(Query::vectorDot('embedding', [1.0, 2.0])));
+    }
+
+    public function testAVectorQueryOnAnUndeclaredAttributeIsRefusedWithoutAWarning(): void
+    {
+        $validator = $this->validator(supportForAttributes: false);
+
+        $this->assertFalse($validator->isValid(Query::vectorDot('undeclared', [1.0, 2.0])));
+        $this->assertSame('Vector queries can only be used on vector attributes', $validator->getDescription());
+    }
+
+    public function testListValuesAreStillValid(): void
+    {
+        $validator = $this->validator();
+
+        $this->assertTrue($validator->isValid(Query::equal('name', ['a', 'b'])));
+        $this->assertTrue($validator->isValid(Query::contains('tags', ['a'])));
+        $this->assertTrue($validator->isValid(Query::intersects('location', [1, 2])));
+    }
+}