Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix double access to ArrayAccess methods #760

Closed
wants to merge 12 commits into from
Closed

Fix double access to ArrayAccess methods #760

Show file tree
Hide file tree
Changes from 11 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
92cac4b
Fix double access to ArrayAccess methods
brunobg Dec 29, 2020
192a7e6
Fix variable name
brunobg Dec 29, 2020
1c6a514
Add comprehensive test for the default resolver
spawnia Dec 30, 2020
818e89a
Fix codestyle
spawnia Dec 30, 2020
2f51a7a
fix test name
spawnia Dec 30, 2020
cc0f3d6
Merge branch 'test-default-resolver' into patch-1
spawnia Dec 30, 2020
0711bb6
Try/catch for ArrayAccess
brunobg Dec 30, 2020
4a3cf0c
Throwable instead of Exception
brunobg Dec 30, 2020
c10cc1b
code style
brunobg Dec 30, 2020
601bf6a
fix style
brunobg Dec 30, 2020
81b95b3
Added docs
brunobg Jan 19, 2021
62e807b
Removed whitespace
brunobg Jan 19, 2021
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
13 changes: 12 additions & 1 deletion src/Executor/Executor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use GraphQL\Language\AST\DocumentNode;
use GraphQL\Type\Definition\ResolveInfo;
use GraphQL\Type\Schema;
use Throwable;
use function is_array;
use function is_object;

Expand Down Expand Up @@ -173,10 +174,20 @@ public static function defaultFieldResolver($objectValue, $args, $contextValue,
$fieldName = $info->fieldName;
$property = null;

if (is_array($objectValue) || $objectValue instanceof ArrayAccess) {
if (is_array($objectValue)) {
if (isset($objectValue[$fieldName])) {
$property = $objectValue[$fieldName];
}
} elseif ($objectValue instanceof ArrayAccess) {
spawnia marked this conversation as resolved.
Show resolved Hide resolved
brunobg marked this conversation as resolved.
Show resolved Hide resolved
// handles #759: ArrayAccess::offsetExists() can call offsetGet() internally,
// which results in calling offsetGet() twice.
// This avoids the double call and handles Exceptions to have the same
// behavior as isset() in the pure array case.
try {
$property = $objectValue[$fieldName];
} catch (Throwable $e) {
// pass
}
} elseif (is_object($objectValue)) {
if (isset($objectValue->{$fieldName})) {
$property = $objectValue->{$fieldName};
Expand Down
183 changes: 183 additions & 0 deletions tests/Executor/ExecutorTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

namespace GraphQL\Tests\Executor;

use ArrayAccess;
use Exception;
use GraphQL\Deferred;
use GraphQL\Error\Error;
use GraphQL\Error\UserError;
Expand Down Expand Up @@ -1225,4 +1227,185 @@ public function testSerializesToEmptyObjectVsEmptyArray() : void
$result->toArray()
);
}

public function testDefaultResolverGrabsValuesOffOfCommonPhpDataStructures() : void
{
$Array = new ObjectType([
'name' => 'Array',
'fields' => [
'set' => Type::int(),
'unset' => Type::int(),
],
]);

$ArrayAccess = new ObjectType([
'name' => 'ArrayAccess',
'fields' => [
'set' => Type::int(),
'unsetNull' => Type::int(),
'unsetThrow' => Type::int(),
],
]);

$ObjectField = new ObjectType([
'name' => 'ObjectField',
'fields' => [
'set' => Type::int(),
'unset' => Type::int(),
'nonExistent' => Type::int(),
],
]);

$ObjectVirtual = new ObjectType([
'name' => 'ObjectVirtual',
'fields' => [
'set' => Type::int(),
'unsetNull' => Type::int(),
'unsetThrow' => Type::int(),
],
]);

$schema = new Schema([
'query' => new ObjectType([
'name' => 'Query',
'fields' => [
'array' => [
'type' => $Array,
'resolve' => static function () : array {
return ['set' => 1];
},
],
'arrayAccess' => [
'type' => $ArrayAccess,
'resolve' => static function () : ArrayAccess {
return new class implements ArrayAccess {
public function offsetExists($offset)
{
switch ($offset) {
case 'set':
return true;
default:
return false;
}
}

public function offsetGet($offset)
{
switch ($offset) {
case 'set':
return 1;
case 'unsetNull':
return null;
default:
throw new Exception('unsetThrow');
}
}

public function offsetSet($offset, $value)
{
}

public function offsetUnset($offset)
{
}
};
},
],
'objectField' => [
'type' => $ObjectField,
'resolve' => static function () : stdClass {
return new class extends stdClass {
/** @var int|null */
public $set = 1;

/** @var int|null */
public $unset;
};
},
],
'objectVirtual' => [
'type' => $ObjectVirtual,
'resolve' => static function () {
return new class {
public function __isset($name) : bool
{
switch ($name) {
case 'set':
return true;
default:
return false;
}
}

public function __get($name) : ?int
{
switch ($name) {
case 'set':
return 1;
case 'unsetNull':
return null;
default:
throw new Exception('unsetThrow');
}
}
};
},
],
],
]),
]);

$query = Parser::parse('
{
array {
set
unset
}
arrayAccess {
set
unsetNull
unsetThrow
}
objectField {
set
unset
nonExistent
}
objectVirtual {
set
unsetNull
unsetThrow
}
}
');

$result = Executor::execute($schema, $query);

self::assertEquals(
[
'data' => [
'array' => [
'set' => 1,
'unset' => null,
],
'arrayAccess' => [
'set' => 1,
'unsetNull' => null,
'unsetThrow' => null,
],
'objectField' => [
'set' => 1,
'unset' => null,
'nonExistent' => null,
],
'objectVirtual' => [
'set' => 1,
'unsetNull' => null,
'unsetThrow' => null,
],
],
],
$result->toArray()
);
}
}