Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion src/Database/Database.php
Original file line numberDiff line numberDiff line change
Expand Up@@ -9520,6 +9520,9 @@ private function validateSelections(Document $collection, array $queries): array
foreach ($queries as $query) {
if ($query->getMethod() == Query::TYPE_SELECT) {
foreach ($query->getValues() as $value) {
if (!\is_string($value)) {
throw new QueryException('Attribute selection must be a string, got ' . \get_debug_type($value));
}
if (\str_contains($value, '.')) {
$relationshipSelections[] = $value;
continue;
Expand DownExpand Up@@ -10011,7 +10014,7 @@ private function processRelationshipQueries(

$values = $query->getValues();
foreach ($values as $valueIndex => $value) {
if (!\str_contains($value, '.')) {
if (!\is_string($value) || !\str_contains($value, '.')) {
continue;
}

Expand Down
10 changes: 10 additions & 0 deletions src/Database/Validator/Query/Select.php
Original file line numberDiff line numberDiff line change
Expand Up@@ -68,6 +68,16 @@ public function isValid($value): bool
return false;
}

// Before the duplicate check: array_unique() stringifies every array element
// to "Array", so two nested values collapse into one and report a misleading
// duplicate instead of the type error that is actually there.
foreach ($value->getValues() as $attribute) {
if (!\is_string($attribute)) {
$this->message = 'Attribute selection must be a string, got ' . \get_debug_type($attribute);
return false;
}
}

if (\count($value->getValues()) !== \count(\array_unique($value->getValues()))) {
$this->message = 'Duplicate attributes selected';
return false;
Expand Down
120 changes: 120 additions & 0 deletions tests/unit/SelectProjectionTest.php
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,120 @@
<?php

namespace Tests\Unit;

use PHPUnit\Framework\TestCase;
use Utopia\Cache\Adapter\Memory as CacheMemory;
use Utopia\Cache\Cache;
use Utopia\Database\Adapter\Memory as DatabaseMemory;
use Utopia\Database\Database;
use Utopia\Database\Document;
use Utopia\Database\Exception\Query as QueryException;
use Utopia\Database\Helpers\Permission;
use Utopia\Database\Helpers\Role;
use Utopia\Database\Query;

/**
* Drives Database::find(), the entry point the HTTP layer calls, rather than the
* validator alone.
*
* A malformed `select` value reached str_contains() and raised a TypeError. A
* TypeError is an Error, not an Exception, so it escaped the QueryException catch
* in the callers and surfaced as a 500 — where every other query method answers the
* same malformation with a typed refusal that becomes a 400.
*/
class SelectProjectionTest extends TestCase
{
private Database $database;

protected function setUp(): void
{
$this->database = new Database(new DatabaseMemory(), new Cache(new CacheMemory()));
$this->database
->setDatabase('utopiaTests')
->setNamespace('select_' . \uniqid());

$this->database->create();
$this->database->createCollection('widgets');
$this->database->createAttribute('widgets', 'sku', Database::VAR_STRING, 255, false);
$this->database->createDocument('widgets', new Document([
'$id' => 'widget',
'$permissions' => [Permission::read(Role::any())],
'sku' => 'abc',
]));
}

/**
* @param array<mixed> $values
*
* @dataProvider malformedSelections
*/
public function testAMalformedSelectionIsRefusedRatherThanFatal(array $values): void
{
$this->expectException(QueryException::class);
$this->expectExceptionMessage('Attribute selection must be a string, got');

$this->database->find('widgets', [Query::select($values)]);
}

/**
* @return array<string, array{array<mixed>}>
*/
public static function malformedSelections(): array
{
return [
'nested array' => [[['sku']]],
'nested wildcard' => [[['*']]],
'mixed flat and nested' => [['sku', ['x']]],
'two nested values' => [[['a'], ['b']]],
];
}

/**
* The refusal must be a catchable Exception. A TypeError is an Error, so a caller
* catching Exception — as the HTTP layer does — never sees it and returns a 500.
*
* @param array<mixed> $values
*
* @dataProvider malformedSelections
*/
public function testTheRefusalIsCatchableAsAnException(array $values): void
{
$caught = null;

try {
$this->database->find('widgets', [Query::select($values)]);
} catch (\Exception $exception) {
$caught = $exception;
}

$this->assertInstanceOf(
QueryException::class,
$caught,
'a malformed selection must be catchable as an Exception, otherwise it escapes as a 500',
);
}

public function testTheLegitimateFlatFormStillProjects(): void
{
$rows = $this->database->find('widgets', [Query::select(['sku'])]);

$this->assertCount(1, $rows);
$this->assertSame('abc', $rows[0]->getAttribute('sku'));
}

public function testTheWildcardStillProjects(): void
{
$this->assertCount(1, $this->database->find('widgets', [Query::select(['*'])]));
}

/**
* The type check must not swallow the schema check that already worked.
*/
public function testAnUnknownAttributeIsStillRefusedBySchema(): void
{
$this->expectException(QueryException::class);
$this->expectExceptionMessage('Attribute not found in schema: nope');

$this->database->find('widgets', [Query::select(['nope'])]);
}
}
56 changes: 56 additions & 0 deletions tests/unit/Validator/Query/SelectTest.php
Original file line numberDiff line numberDiff line change
Expand Up@@ -49,4 +49,60 @@ public function testValueFailure(): void
$this->assertEquals('Invalid query', $this->validator->getDescription());
$this->assertFalse($this->validator->isValid(Query::select(['name.artist'])));
}

/**
* A non-string selection used to reach str_contains() and raise a TypeError,
* which is an Error rather than an Exception, so it escaped every catch on the
* way out and surfaced as a 500. Select was the only query method that answered
* a malformed value that way.
*
* @param array<mixed> $values
*
* @dataProvider nonStringSelections
*/
public function testANonStringSelectionIsRefusedByType(array $values, string $expected): void
{
$this->assertFalse($this->validator->isValid(Query::select($values)));
$this->assertSame($expected, $this->validator->getDescription());
}

/**
* @return array<string, array{array<mixed>, string}>
*/
public static function nonStringSelections(): array
{
return [
'nested array' => [[['attr']], 'Attribute selection must be a string, got array'],
'nested wildcard' => [[['*']], 'Attribute selection must be a string, got array'],
'mixed flat and nested' => [['attr', ['x']], 'Attribute selection must be a string, got array'],
'assoc array' => [[['a' => 1]], 'Attribute selection must be a string, got array'],
'integer' => [[1], 'Attribute selection must be a string, got int'],
'null' => [[null], 'Attribute selection must be a string, got null'],
];
}

/**
* Two nested values used to collapse to one under array_unique(), which casts
* every array to the string "Array", so the duplicate check tripped first and
* reported a duplicate that was not there. The type check has to run before it.
*
* Parsed from JSON rather than built with Query::select(), because that is the
* path a hand-written HTTP client takes and the only one that can carry a value
* the constructor's array<string> type would reject.
*/
public function testTwoNestedSelectionsReportTheTypeNotAFalseDuplicate(): void
{
$query = Query::parse('{"method":"select","values":[["a"],["b"]]}');

$this->assertFalse($this->validator->isValid($query));
$this->assertSame('Attribute selection must be a string, got array', $this->validator->getDescription());
}

public function testTheLegitimateFlatFormStillPasses(): void
{
$this->assertTrue($this->validator->isValid(Query::select(['attr'])));
$this->assertTrue($this->validator->isValid(Query::select(['*'])));
$this->assertTrue($this->validator->isValid(Query::select(['$id', '$createdAt'])));
$this->assertTrue($this->validator->isValid(Query::select(['artist.name'])));
}
}
Loading