Skip to content

Commit b92964b

Browse files
abnegateabnegate
authored
Merge pull request #1022 from utopia-php/qfix-unknown-column
fix(query): refuse a nested path on an attribute that holds no nested values
2 parents 91d5e8b + 384369e commit b92964b

6 files changed

Lines changed: 136 additions & 1 deletion

File tree

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

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,12 @@ protected function isValidAttribute(string $attribute): bool
8484
return false;
8585
}
8686

87+
if (! $this->isNestable($alias)) {
88+
$this->message = 'Cannot query nested attribute on: '.$alias;
89+
90+
return false;
91+
}
92+
8793
// For relationships, just validate the top level.
8894
// will validate each nested level during the recursive calls.
8995
$attribute = $alias;
@@ -98,6 +104,16 @@ protected function isValidAttribute(string $attribute): bool
98104
return true;
99105
}
100106

107+
private function isNestable(string $base): bool
108+
{
109+
$definition = $this->schema[$base] ?? null;
110+
if (! $this->supportForAttributes || ! \is_array($definition)) {
111+
return true;
112+
}
113+
114+
return \in_array($definition['type'] ?? null, [ColumnType::Relationship, ColumnType::Object], true);
115+
}
116+
101117
private function isValidObjectPath(string $base, string $path): bool
102118
{
103119
/** @var array<string, mixed> $definition */

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
use Utopia\Database\Validator\Query\Joined\Attributes;
1010
use Utopia\Query\Method;
1111
use Utopia\Query\Query as BaseQuery;
12+
use Utopia\Query\Schema\ColumnType;
1213

1314
/**
1415
* Validates select query methods ensuring referenced attributes exist in the schema, are not duplicated and, in an
@@ -30,6 +31,13 @@ class Select extends Base
3031
*/
3132
private array $relationships = [];
3233

34+
/**
35+
* The object attributes of the collection.
36+
*
37+
* @var array<int|string, true>
38+
*/
39+
private array $objects = [];
40+
3341
/**
3442
* Whether the query set holds an aggregate.
3543
*/
@@ -55,6 +63,10 @@ public function __construct(array $attributes = [], protected bool $supportForAt
5563
if ($attribute->relationship !== null) {
5664
$this->relationships[$attribute->key] = true;
5765
}
66+
67+
if ($attribute->type === ColumnType::Object) {
68+
$this->objects[$attribute->key] = true;
69+
}
5870
}
5971
}
6072

@@ -156,6 +168,12 @@ public function isValid(mixed $value): bool
156168
return $this->rejectUngrouped($attribute);
157169
}
158170

171+
if (! $this->isAggregation() && $this->supportForAttributes && isset($this->schema[$alias]) && ! isset($this->relationships[$alias]) && ! isset($this->objects[$alias])) {
172+
$this->message = 'Cannot select nested attribute on: '.$alias;
173+
174+
return false;
175+
}
176+
159177
// For relationships, just validate the top level.
160178
// Will validate each nested level during the recursive calls.
161179
$attribute = $alias;

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

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11214,4 +11214,43 @@ public function testDistinctIsRefusedWhereTheAdapterCannotDeduplicate(): void
1121411214
$database->deleteCollection($collection);
1121511215
}
1121611216
}
11217+
11218+
public function testNestedPathOnAPlainAttributeIsAnInvalidQuery(): void
11219+
{
11220+
/** @var Database $database */
11221+
$database = $this->getDatabase();
11222+
11223+
if (! $database->getAdapter()->supports(Capability::DefinedAttributes)) {
11224+
$this->expectNotToPerformAssertions();
11225+
11226+
return;
11227+
}
11228+
11229+
$collection = 'nestedPathPlain';
11230+
$database->createCollection(Collection::create(id: $collection, permissions: [Permission::read(Role::any()), Permission::create(Role::any())]));
11231+
11232+
try {
11233+
$database->createAttribute($collection, Attribute::string(key: 'name', size: 64, required: true));
11234+
$database->createDocument($collection, new Document(['name' => 'a', '$permissions' => [Permission::read(Role::any())]]));
11235+
11236+
$reads = [
11237+
'filter' => fn (): array => $database->find($collection, [Query::equal('name.x', ['a'])]),
11238+
'count' => fn (): int => $database->count($collection, [Query::equal('name.x', ['a'])]),
11239+
'select' => fn (): array => $database->find($collection, [Query::select(['name.x'])]),
11240+
];
11241+
11242+
foreach ($reads as $shape => $read) {
11243+
try {
11244+
$read();
11245+
$this->fail("A {$shape} on a nested path under a plain attribute must be refused");
11246+
} catch (QueryException $e) {
11247+
$this->assertStringContainsString('nested attribute on: name', $e->getMessage(), $shape);
11248+
}
11249+
}
11250+
11251+
$this->assertCount(1, $database->find($collection, [Query::equal('name', ['a'])]));
11252+
} finally {
11253+
$database->deleteCollection($collection);
11254+
}
11255+
}
1121711256
}

‎tests/unit/Validator/DocumentQueriesTest.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ public function test_valid_queries(): void
6767
$this->assertEquals(true, $validator->isValid($queries));
6868

6969
$queries[] = Query::select(['price.relation']);
70-
$this->assertEquals(true, $validator->isValid($queries));
70+
$this->assertFalse($validator->isValid($queries), 'price is a plain attribute, so a path under it names a column that does not exist');
7171
}
7272

7373
/**

‎tests/unit/Validator/Query/FilterTest.php‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,12 @@
33
namespace Tests\Unit\Validator\Query;
44

55
use PHPUnit\Framework\TestCase;
6+
use Utopia\Database\Attribute;
67
use Utopia\Database\Document;
78
use Utopia\Database\Exception;
89
use Utopia\Database\Query;
10+
use Utopia\Database\Relationship;
11+
use Utopia\Database\RelationshipSide;
912
use Utopia\Database\Validator\Query\Filter;
1013
use Utopia\Query\Method;
1114
use Utopia\Query\Schema\ColumnType;
@@ -217,6 +220,45 @@ public function test_not_between(): void
217220
$this->assertEquals('NotBetween queries require exactly two values.', $this->validator->getDescription());
218221
}
219222

223+
public function test_nested_path_on_a_plain_attribute_is_rejected(): void
224+
{
225+
$this->assertFalse($this->validator->isValid(Query::equal('string.x', ['a'])));
226+
$this->assertSame('Cannot query nested attribute on: string', $this->validator->getDescription());
227+
228+
$this->assertFalse($this->validator->isValid(Query::greaterThan('integer.x', 1)));
229+
$this->assertSame('Cannot query nested attribute on: integer', $this->validator->getDescription());
230+
}
231+
232+
public function test_nested_path_on_a_relationship_or_object_is_accepted(): void
233+
{
234+
$validator = new Filter(
235+
attributes: [
236+
Attribute::relationship('artist', Relationship::manyToOne('artists'), RelationshipSide::Parent)->toDocument(),
237+
new Document([
238+
'$id' => 'meta',
239+
'key' => 'meta',
240+
'type' => ColumnType::Object->value,
241+
'array' => false,
242+
]),
243+
],
244+
idAttributeType: ColumnType::Integer->value,
245+
);
246+
247+
$this->assertTrue($validator->isValid(Query::equal('artist.name', ['a'])), $validator->getDescription());
248+
$this->assertTrue($validator->isValid(Query::equal('meta.key', ['a'])), $validator->getDescription());
249+
}
250+
251+
public function test_nested_path_on_a_plain_attribute_is_left_to_a_schemaless_adapter(): void
252+
{
253+
$validator = new Filter(
254+
attributes: [new Document(['$id' => 'string', 'key' => 'string', 'type' => ColumnType::String->value, 'array' => false])],
255+
idAttributeType: ColumnType::Integer->value,
256+
supportForAttributes: false,
257+
);
258+
259+
$this->assertTrue($validator->isValid(Query::equal('string.x', ['a'])), $validator->getDescription());
260+
}
261+
220262
public function test_dotted_join_alias_is_accepted_after_allow_join_aliases(): void
221263
{
222264
$this->validator->allowJoinAliases(['sec']);

‎tests/unit/Validator/Query/SelectTest.php‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,26 @@ public function test_value_failure(): void
5050
$this->assertFalse($this->validator->isValid(Query::select(['name.artist'])));
5151
}
5252

53+
public function testANestedPathOnAPlainAttributeIsRefused(): void
54+
{
55+
$this->assertFalse($this->validator->isValid(Query::select(['attr.x'])));
56+
$this->assertSame('Cannot select nested attribute on: attr', $this->validator->getDescription());
57+
}
58+
59+
public function testANestedPathOnAnObjectAttributeIsAccepted(): void
60+
{
61+
$validator = new Select([
62+
new Document([
63+
'$id' => 'meta',
64+
'key' => 'meta',
65+
'type' => ColumnType::Object->value,
66+
'array' => false,
67+
]),
68+
]);
69+
70+
$this->assertTrue($validator->isValid(Query::select(['meta.key'])), $validator->getDescription());
71+
}
72+
5373
/**
5474
* A non-string selection used to reach str_contains() and raise a TypeError,
5575
* which is an Error rather than an Exception, so it escaped every catch on the

0 commit comments

Comments
 (0)