Skip to content

Commit 09a4729

Browse files
authored
Merge pull request #1028 from utopia-php/fix/filter-values-list
fix(validator): refuse filter values that are not a list
2 parents 165cada + ad1a9cb commit 09a4729

3 files changed

Lines changed: 211 additions & 16 deletions

File tree

‎src/Database/Validator/Query/Filter.php‎

Lines changed: 41 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -326,7 +326,7 @@ protected function isValidAttributeAndValues(string $attribute, array $values, M
326326
continue 2;
327327

328328
case ColumnType::Vector:
329-
if (! is_array($value)) {
329+
if (! is_array($value) || ! \array_is_list($value)) {
330330
$this->message = 'Vector query value must be an array';
331331

332332
return false;
@@ -429,15 +429,42 @@ private function joinedSchema(string $alias, string $column): ?array
429429
*/
430430
protected function isEmpty(array $values): bool
431431
{
432-
if (count($values) === 0) {
432+
if ($values === []) {
433433
return true;
434434
}
435435

436-
if (is_array($values[0]) && count($values[0]) === 0) {
437-
return true;
436+
return \reset($values) === [];
437+
}
438+
439+
/**
440+
* A distance query holds one `[geometry, distance, meters]` tuple: a finite distance and whether it is in meters.
441+
*
442+
* @param array<mixed> $values
443+
*/
444+
private function isValidDistance(array $values): bool
445+
{
446+
$tuple = $values[0] ?? null;
447+
if (\count($values) !== 1 || ! \is_array($tuple) || ! \array_is_list($tuple) || \count($tuple) !== 3) {
448+
$this->message = 'Distance query requires [[geometry, distance]] parameters';
449+
450+
return false;
451+
}
452+
453+
[, $distance, $meters] = $tuple;
454+
455+
if (! \is_numeric($distance) || ! \is_finite((float) $distance)) {
456+
$this->message = 'Distance query requires a finite distance';
457+
458+
return false;
438459
}
439460

440-
return false;
461+
if (! \is_bool($meters)) {
462+
$this->message = 'Distance query requires a boolean meters flag';
463+
464+
return false;
465+
}
466+
467+
return true;
441468
}
442469

443470
/**
@@ -489,6 +516,13 @@ public function isValid(mixed $value): bool
489516
{
490517
$method = $value->getMethod();
491518
$attribute = $value->getAttribute();
519+
520+
if (! \array_is_list($value->getValues())) {
521+
$this->message = \ucfirst($method->value).' query values must be a list';
522+
523+
return false;
524+
}
525+
492526
switch ($method) {
493527
case Method::Equal:
494528
case Method::Contains:
@@ -517,16 +551,7 @@ public function isValid(mixed $value): bool
517551
case Method::DistanceNotEqual:
518552
case Method::DistanceGreaterThan:
519553
case Method::DistanceLessThan:
520-
if (count($value->getValues()) !== 1 || ! is_array($value->getValues()[0]) || count($value->getValues()[0]) !== 3) {
521-
$this->message = 'Distance query requires [[geometry, distance]] parameters';
522-
523-
return false;
524-
}
525-
526-
$distance = $value->getValues()[0][1];
527-
if (! \is_numeric($distance) || ! \is_finite((float) $distance)) {
528-
$this->message = 'Distance query requires a finite distance';
529-
554+
if (! $this->isValidDistance($value->getValues())) {
530555
return false;
531556
}
532557

@@ -593,7 +618,7 @@ public function isValid(mixed $value): bool
593618
}
594619

595620
/** @var array<string, mixed> $attributeSchema */
596-
$attributeSchema = $this->schema[$attributeKey];
621+
$attributeSchema = $this->schema[$attributeKey] ?? [];
597622
/** @var ColumnType|null $vectorAttrType */
598623
$vectorAttrType = $attributeSchema['type'] ?? null;
599624
if ($vectorAttrType !== ColumnType::Vector) {

‎tests/e2e/Adapter/Scopes/DocumentTests.php‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1609,6 +1609,74 @@ public function testGetDocument(): void
16091609
$this->assertEquals('Works', $document->getAttribute('with-dash'));
16101610
}
16111611

1612+
/**
1613+
* A query parsed from JSON takes an object for its values as readily as a list. Filter read such values by
1614+
* position and raised "Undefined array key 0" before refusing them. Every adapter refuses them as an invalid
1615+
* query, for find() and count(), without a warning.
1616+
*/
1617+
public function testFilterValuesThatAreNotAListAreAnInvalidQuery(): void
1618+
{
1619+
$database = $this->getDatabase();
1620+
$adapter = $database->getAdapter();
1621+
$spatial = $adapter->hasFeature(Feature\Spatial::class);
1622+
$vectors = $adapter->supports(Capability::Vectors);
1623+
$collection = 'object_values_'.\substr(\uniqid(), -6);
1624+
$database->createCollection(Collection::create(
1625+
id: $collection,
1626+
attributes: [
1627+
Attribute::string(key: 'name', size: 20),
1628+
Attribute::string(key: 'tags', size: 20, array: true),
1629+
Attribute::integer(key: 'rank'),
1630+
...($spatial ? [Attribute::point(key: 'location')] : []),
1631+
...($vectors ? [Attribute::vector(key: 'embedding', dimensions: 2)] : []),
1632+
],
1633+
permissions: [Permission::create(Role::any()), Permission::read(Role::any())],
1634+
));
1635+
$database->createDocument($collection, new Document([
1636+
'name' => 'a',
1637+
'tags' => ['a'],
1638+
'rank' => 1,
1639+
...($spatial ? ['location' => [1.0, 2.0]] : []),
1640+
...($vectors ? ['embedding' => [1.0, 0.0]] : []),
1641+
'$permissions' => [Permission::read(Role::any())],
1642+
]));
1643+
1644+
$queries = [
1645+
'{"method":"equal","attribute":"name","values":{"value":"a"}}',
1646+
'{"method":"equal","attribute":"name","values":{"1":"a"}}',
1647+
'{"method":"notEqual","attribute":"name","values":{"value":"a"}}',
1648+
'{"method":"contains","attribute":"tags","values":{"tag":"a"}}',
1649+
'{"method":"lessThan","attribute":"rank","values":{"bound":5}}',
1650+
'{"method":"between","attribute":"rank","values":{"from":0,"to":5}}',
1651+
'{"method":"startsWith","attribute":"name","values":{"prefix":"a"}}',
1652+
'{"method":"exists","attribute":"","values":{"attribute":"name"}}',
1653+
];
1654+
if ($spatial) {
1655+
$queries[] = '{"method":"intersects","attribute":"location","values":{"type":"Point","coordinates":[1,2]}}';
1656+
$queries[] = '{"method":"distanceLessThan","attribute":"location","values":{"geometry":[[1,2],10,false]}}';
1657+
$queries[] = '{"method":"distanceLessThan","attribute":"location","values":[{"geometry":[1,2],"distance":10,"meters":false}]}';
1658+
}
1659+
if ($vectors) {
1660+
$queries[] = '{"method":"vectorDot","attribute":"embedding","values":{"vector":[1,0]}}';
1661+
}
1662+
1663+
foreach ($queries as $json) {
1664+
$query = Query::parse($json);
1665+
foreach (['find' => fn () => $database->find($collection, [$query]), 'count' => fn () => $database->count($collection, [$query])] as $read => $call) {
1666+
try {
1667+
$call();
1668+
$this->fail("{$read}() refuses {$json}");
1669+
} catch (QueryException $e) {
1670+
$this->assertStringStartsWith('Invalid query: ', $e->getMessage(), "{$read}() of {$json}");
1671+
}
1672+
}
1673+
}
1674+
1675+
$this->assertCount(1, $database->find($collection, [Query::parse('{"method":"equal","attribute":"name","values":["a"]}')]));
1676+
1677+
$database->deleteCollection($collection);
1678+
}
1679+
16121680
public function testFind(): void
16131681
{
16141682
$this->initMoviesFixture();
Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
<?php
2+
3+
namespace Tests\Unit\Validator\Query;
4+
5+
use PHPUnit\Framework\Attributes\DataProvider;
6+
use PHPUnit\Framework\TestCase;
7+
use Utopia\Database\Document;
8+
use Utopia\Database\Query;
9+
use Utopia\Database\Validator\Query\Filter;
10+
use Utopia\Query\Method;
11+
use Utopia\Query\Schema\ColumnType;
12+
13+
/**
14+
* A query parsed from JSON takes a JSON object for its values as readily as a list. The validator read such values
15+
* by position and raised an undefined key warning; it refuses them as invalid instead.
16+
*/
17+
final class FilterMalformedValuesTest extends TestCase
18+
{
19+
private function validator(bool $supportForAttributes = true): Filter
20+
{
21+
return new Filter(
22+
attributes: [
23+
new Document(['$id' => 'name', 'key' => 'name', 'type' => ColumnType::String->value, 'array' => false]),
24+
new Document(['$id' => 'tags', 'key' => 'tags', 'type' => ColumnType::String->value, 'array' => true]),
25+
new Document(['$id' => 'location', 'key' => 'location', 'type' => ColumnType::Point->value, 'array' => false]),
26+
new Document(['$id' => 'embedding', 'key' => 'embedding', 'type' => ColumnType::Vector->value, 'array' => false, 'size' => 2]),
27+
],
28+
idAttributeType: ColumnType::Integer->value,
29+
supportForAttributes: $supportForAttributes,
30+
);
31+
}
32+
33+
/**
34+
* @return iterable<string, array{string}>
35+
*/
36+
public static function objectValues(): iterable
37+
{
38+
yield 'equal' => ['{"method":"equal","attribute":"name","values":{"value":"a"}}'];
39+
yield 'contains' => ['{"method":"contains","attribute":"tags","values":{"tag":"a"}}'];
40+
yield 'notEqual' => ['{"method":"notEqual","attribute":"name","values":{"value":"a"}}'];
41+
yield 'regex' => ['{"method":"regex","attribute":"name","values":{"pattern":"^a"}}'];
42+
yield 'between' => ['{"method":"between","attribute":"name","values":{"from":"a","to":"b"}}'];
43+
yield 'exists' => ['{"method":"exists","attribute":"","values":{"attribute":"name"}}'];
44+
yield 'intersects' => ['{"method":"intersects","attribute":"location","values":{"type":"Point","coordinates":[1,2]}}'];
45+
yield 'distanceLessThan' => ['{"method":"distanceLessThan","attribute":"location","values":{"geometry":[[1,2],10,false]}}'];
46+
yield 'vectorDot' => ['{"method":"vectorDot","attribute":"embedding","values":{"vector":[1,2]}}'];
47+
}
48+
49+
#[DataProvider('objectValues')]
50+
public function testValuesGivenAsAnObjectAreRefused(string $json): void
51+
{
52+
$validator = $this->validator();
53+
$query = Query::parse($json);
54+
55+
$this->assertFalse($validator->isValid($query));
56+
$this->assertSame(\ucfirst($query->getMethod()->value).' query values must be a list', $validator->getDescription());
57+
}
58+
59+
public function testADistanceTupleGivenAsAnObjectIsRefused(): void
60+
{
61+
$validator = $this->validator();
62+
$query = Query::parse('{"method":"distanceLessThan","attribute":"location","values":[{"geometry":[1,2],"distance":10,"meters":false}]}');
63+
64+
$this->assertFalse($validator->isValid($query));
65+
$this->assertSame('Distance query requires [[geometry, distance]] parameters', $validator->getDescription());
66+
}
67+
68+
public function testADistanceWithAMetersFlagThatIsNotABooleanIsRefused(): void
69+
{
70+
$validator = $this->validator();
71+
72+
$this->assertFalse($validator->isValid(new Query(Method::DistanceLessThan, 'location', [[[1, 2], 10, ['yes']]])));
73+
$this->assertSame('Distance query requires a boolean meters flag', $validator->getDescription());
74+
$this->assertTrue($validator->isValid(Query::distanceLessThan('location', [1, 2], 10, true)));
75+
}
76+
77+
public function testAVectorGivenAsAnObjectIsRefused(): void
78+
{
79+
$validator = $this->validator();
80+
81+
$this->assertFalse($validator->isValid(Query::vectorDot('embedding', ['x' => 1.0, 'y' => 2.0])));
82+
$this->assertSame('Vector query value must be an array', $validator->getDescription());
83+
$this->assertTrue($validator->isValid(Query::vectorDot('embedding', [1.0, 2.0])));
84+
}
85+
86+
public function testAVectorQueryOnAnUndeclaredAttributeIsRefusedWithoutAWarning(): void
87+
{
88+
$validator = $this->validator(supportForAttributes: false);
89+
90+
$this->assertFalse($validator->isValid(Query::vectorDot('undeclared', [1.0, 2.0])));
91+
$this->assertSame('Vector queries can only be used on vector attributes', $validator->getDescription());
92+
}
93+
94+
public function testListValuesAreStillValid(): void
95+
{
96+
$validator = $this->validator();
97+
98+
$this->assertTrue($validator->isValid(Query::equal('name', ['a', 'b'])));
99+
$this->assertTrue($validator->isValid(Query::contains('tags', ['a'])));
100+
$this->assertTrue($validator->isValid(Query::intersects('location', [1, 2])));
101+
}
102+
}

0 commit comments

Comments
 (0)