mirror of
https://github.com/retailcrm/graphql-php.git
synced 2024-11-29 00:25:17 +03:00
Fix infinite loop on invalid queries in OverlappingFields
`OverlappingFieldsCanBeMerged` would infinite loop when passed something like ```graphql fragment A on User { name ...A } ``` It's not `OverlappingFieldsCanBeMerged`'s responsibility to detect that validation error, but we still would ideally avoid infinite looping. This detects that case, and pretends that the infinite spread wasn't there for the purposes of this validation step. Also, by memoizing and checking for self-references this removes duplicate reports. ref: graphql/graphql-js#1111
This commit is contained in:
parent
7b05673d8d
commit
6e358eb26c
@ -49,7 +49,7 @@ class OverlappingFieldsCanBeMerged extends AbstractValidationRule
|
|||||||
* dramatically improve the performance of this validator.
|
* dramatically improve the performance of this validator.
|
||||||
* @var PairSet
|
* @var PairSet
|
||||||
*/
|
*/
|
||||||
private $comparedFragments;
|
private $comparedFragmentPairs;
|
||||||
|
|
||||||
/**
|
/**
|
||||||
* A cache for the "field map" and list of fragment names found in any given
|
* A cache for the "field map" and list of fragment names found in any given
|
||||||
@ -62,7 +62,7 @@ class OverlappingFieldsCanBeMerged extends AbstractValidationRule
|
|||||||
|
|
||||||
public function getVisitor(ValidationContext $context)
|
public function getVisitor(ValidationContext $context)
|
||||||
{
|
{
|
||||||
$this->comparedFragments = new PairSet();
|
$this->comparedFragmentPairs = new PairSet();
|
||||||
$this->cachedFieldsAndFragmentNames = new \SplObjectStorage();
|
$this->cachedFieldsAndFragmentNames = new \SplObjectStorage();
|
||||||
|
|
||||||
return [
|
return [
|
||||||
@ -174,29 +174,34 @@ class OverlappingFieldsCanBeMerged extends AbstractValidationRule
|
|||||||
$fieldMap
|
$fieldMap
|
||||||
);
|
);
|
||||||
|
|
||||||
// (B) Then collect conflicts between these fields and those represented by
|
|
||||||
// each spread fragment name found.
|
|
||||||
$fragmentNamesLength = count($fragmentNames);
|
$fragmentNamesLength = count($fragmentNames);
|
||||||
for ($i = 0; $i < $fragmentNamesLength; $i++) {
|
if ($fragmentNamesLength !== 0) {
|
||||||
$this->collectConflictsBetweenFieldsAndFragment(
|
// (B) Then collect conflicts between these fields and those represented by
|
||||||
$context,
|
// each spread fragment name found.
|
||||||
$conflicts,
|
$comparedFragments = [];
|
||||||
false,
|
for ($i = 0; $i < $fragmentNamesLength; $i++) {
|
||||||
$fieldMap,
|
$this->collectConflictsBetweenFieldsAndFragment(
|
||||||
$fragmentNames[$i]
|
|
||||||
);
|
|
||||||
// (C) Then compare this fragment with all other fragments found in this
|
|
||||||
// selection set to collect conflicts between fragments spread together.
|
|
||||||
// This compares each item in the list of fragment names to every other item
|
|
||||||
// in that same list (except for itself).
|
|
||||||
for ($j = $i + 1; $j < $fragmentNamesLength; $j++) {
|
|
||||||
$this->collectConflictsBetweenFragments(
|
|
||||||
$context,
|
$context,
|
||||||
$conflicts,
|
$conflicts,
|
||||||
|
$comparedFragments,
|
||||||
false,
|
false,
|
||||||
$fragmentNames[$i],
|
$fieldMap,
|
||||||
$fragmentNames[$j]
|
$fragmentNames[$i]
|
||||||
);
|
);
|
||||||
|
// (C) Then compare this fragment with all other fragments found in this
|
||||||
|
// selection set to collect conflicts between fragments spread together.
|
||||||
|
// This compares each item in the list of fragment names to every other item
|
||||||
|
// in that same list (except for itself).
|
||||||
|
for ($j = $i + 1; $j < $fragmentNamesLength; $j++) {
|
||||||
|
$this->collectConflictsBetweenFragments(
|
||||||
|
$context,
|
||||||
|
$conflicts,
|
||||||
|
false,
|
||||||
|
$fragmentNames[$i],
|
||||||
|
$fragmentNames[$j]
|
||||||
|
);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -209,6 +214,7 @@ class OverlappingFieldsCanBeMerged extends AbstractValidationRule
|
|||||||
*
|
*
|
||||||
* @param ValidationContext $context
|
* @param ValidationContext $context
|
||||||
* @param array $conflicts
|
* @param array $conflicts
|
||||||
|
* @param array $comparedFragments
|
||||||
* @param bool $areMutuallyExclusive
|
* @param bool $areMutuallyExclusive
|
||||||
* @param array $fieldMap
|
* @param array $fieldMap
|
||||||
* @param string $fragmentName
|
* @param string $fragmentName
|
||||||
@ -216,10 +222,16 @@ class OverlappingFieldsCanBeMerged extends AbstractValidationRule
|
|||||||
private function collectConflictsBetweenFieldsAndFragment(
|
private function collectConflictsBetweenFieldsAndFragment(
|
||||||
ValidationContext $context,
|
ValidationContext $context,
|
||||||
array &$conflicts,
|
array &$conflicts,
|
||||||
|
array &$comparedFragments,
|
||||||
$areMutuallyExclusive,
|
$areMutuallyExclusive,
|
||||||
array $fieldMap,
|
array $fieldMap,
|
||||||
$fragmentName
|
$fragmentName
|
||||||
) {
|
) {
|
||||||
|
if (isset($comparedFragments[$fragmentName])) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
$comparedFragments[$fragmentName] = true;
|
||||||
|
|
||||||
$fragment = $context->getFragment($fragmentName);
|
$fragment = $context->getFragment($fragmentName);
|
||||||
if (!$fragment) {
|
if (!$fragment) {
|
||||||
return;
|
return;
|
||||||
@ -230,6 +242,10 @@ class OverlappingFieldsCanBeMerged extends AbstractValidationRule
|
|||||||
$fragment
|
$fragment
|
||||||
);
|
);
|
||||||
|
|
||||||
|
if ($fieldMap === $fieldMap2) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
// (D) First collect any conflicts between the provided collection of fields
|
// (D) First collect any conflicts between the provided collection of fields
|
||||||
// and the collection of fields represented by the given fragment.
|
// and the collection of fields represented by the given fragment.
|
||||||
$this->collectConflictsBetween(
|
$this->collectConflictsBetween(
|
||||||
@ -247,6 +263,7 @@ class OverlappingFieldsCanBeMerged extends AbstractValidationRule
|
|||||||
$this->collectConflictsBetweenFieldsAndFragment(
|
$this->collectConflictsBetweenFieldsAndFragment(
|
||||||
$context,
|
$context,
|
||||||
$conflicts,
|
$conflicts,
|
||||||
|
$comparedFragments,
|
||||||
$areMutuallyExclusive,
|
$areMutuallyExclusive,
|
||||||
$fieldMap,
|
$fieldMap,
|
||||||
$fragmentNames2[$i]
|
$fragmentNames2[$i]
|
||||||
@ -271,24 +288,32 @@ class OverlappingFieldsCanBeMerged extends AbstractValidationRule
|
|||||||
$fragmentName1,
|
$fragmentName1,
|
||||||
$fragmentName2
|
$fragmentName2
|
||||||
) {
|
) {
|
||||||
$fragment1 = $context->getFragment($fragmentName1);
|
|
||||||
$fragment2 = $context->getFragment($fragmentName2);
|
|
||||||
if (!$fragment1 || !$fragment2) {
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
|
|
||||||
// No need to compare a fragment to itself.
|
// No need to compare a fragment to itself.
|
||||||
if ($fragment1 === $fragment2) {
|
if ($fragmentName1 === $fragmentName2) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Memoize so two fragments are not compared for conflicts more than once.
|
// Memoize so two fragments are not compared for conflicts more than once.
|
||||||
if (
|
if (
|
||||||
$this->comparedFragments->has($fragmentName1, $fragmentName2, $areMutuallyExclusive)
|
$this->comparedFragmentPairs->has(
|
||||||
|
$fragmentName1,
|
||||||
|
$fragmentName2,
|
||||||
|
$areMutuallyExclusive
|
||||||
|
)
|
||||||
) {
|
) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
$this->comparedFragments->add($fragmentName1, $fragmentName2, $areMutuallyExclusive);
|
$this->comparedFragmentPairs->add(
|
||||||
|
$fragmentName1,
|
||||||
|
$fragmentName2,
|
||||||
|
$areMutuallyExclusive
|
||||||
|
);
|
||||||
|
|
||||||
|
$fragment1 = $context->getFragment($fragmentName1);
|
||||||
|
$fragment2 = $context->getFragment($fragmentName2);
|
||||||
|
if (!$fragment1 || !$fragment2) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
list($fieldMap1, $fragmentNames1) = $this->getReferencedFieldsAndFragmentNames(
|
list($fieldMap1, $fragmentNames1) = $this->getReferencedFieldsAndFragmentNames(
|
||||||
$context,
|
$context,
|
||||||
@ -382,27 +407,35 @@ class OverlappingFieldsCanBeMerged extends AbstractValidationRule
|
|||||||
// (I) Then collect conflicts between the first collection of fields and
|
// (I) Then collect conflicts between the first collection of fields and
|
||||||
// those referenced by each fragment name associated with the second.
|
// those referenced by each fragment name associated with the second.
|
||||||
$fragmentNames2Length = count($fragmentNames2);
|
$fragmentNames2Length = count($fragmentNames2);
|
||||||
for ($j = 0; $j < $fragmentNames2Length; $j++) {
|
if ($fragmentNames2Length !== 0) {
|
||||||
$this->collectConflictsBetweenFieldsAndFragment(
|
$comparedFragments = [];
|
||||||
$context,
|
for ($j = 0; $j < $fragmentNames2Length; $j++) {
|
||||||
$conflicts,
|
$this->collectConflictsBetweenFieldsAndFragment(
|
||||||
$areMutuallyExclusive,
|
$context,
|
||||||
$fieldMap1,
|
$conflicts,
|
||||||
$fragmentNames2[$j]
|
$comparedFragments,
|
||||||
);
|
$areMutuallyExclusive,
|
||||||
|
$fieldMap1,
|
||||||
|
$fragmentNames2[$j]
|
||||||
|
);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// (I) Then collect conflicts between the second collection of fields and
|
// (I) Then collect conflicts between the second collection of fields and
|
||||||
// those referenced by each fragment name associated with the first.
|
// those referenced by each fragment name associated with the first.
|
||||||
$fragmentNames1Length = count($fragmentNames1);
|
$fragmentNames1Length = count($fragmentNames1);
|
||||||
for ($i = 0; $i < $fragmentNames2Length; $i++) {
|
if ($fragmentNames1Length !== 0) {
|
||||||
$this->collectConflictsBetweenFieldsAndFragment(
|
$comparedFragments = [];
|
||||||
$context,
|
for ($i = 0; $i < $fragmentNames2Length; $i++) {
|
||||||
$conflicts,
|
$this->collectConflictsBetweenFieldsAndFragment(
|
||||||
$areMutuallyExclusive,
|
$context,
|
||||||
$fieldMap2,
|
$conflicts,
|
||||||
$fragmentNames1[$i]
|
$comparedFragments,
|
||||||
);
|
$areMutuallyExclusive,
|
||||||
|
$fieldMap2,
|
||||||
|
$fragmentNames1[$i]
|
||||||
|
);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// (J) Also collect conflicts between any fragment names by the first and
|
// (J) Also collect conflicts between any fragment names by the first and
|
||||||
|
@ -454,7 +454,7 @@ class OverlappingFieldsCanBeMergedTest extends TestCase
|
|||||||
deeperField {
|
deeperField {
|
||||||
x: b
|
x: b
|
||||||
}
|
}
|
||||||
},
|
}
|
||||||
deepField {
|
deepField {
|
||||||
deeperField {
|
deeperField {
|
||||||
y
|
y
|
||||||
@ -969,6 +969,64 @@ class OverlappingFieldsCanBeMergedTest extends TestCase
|
|||||||
$this->assertStringEndsWith($hint, $error);
|
$this->assertStringEndsWith($hint, $error);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @it does not infinite loop on recursive fragment
|
||||||
|
*/
|
||||||
|
public function testDoesNotInfiniteLoopOnRecursiveFragment()
|
||||||
|
{
|
||||||
|
$this->expectPassesRule(new OverlappingFieldsCanBeMerged, '
|
||||||
|
fragment fragA on Human { name, relatives { name, ...fragA } }
|
||||||
|
');
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @it does not infinite loop on immediately recursive fragment
|
||||||
|
*/
|
||||||
|
public function testDoesNotInfiniteLoopOnImmeditelyRecursiveFragment()
|
||||||
|
{
|
||||||
|
$this->expectPassesRule(new OverlappingFieldsCanBeMerged, '
|
||||||
|
fragment fragA on Human { name, ...fragA }
|
||||||
|
');
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @it does not infinite loop on transitively recursive fragment
|
||||||
|
*/
|
||||||
|
public function testDoesNotInfiniteLoopOnTransitivelyRecursiveFragment()
|
||||||
|
{
|
||||||
|
$this->expectPassesRule(new OverlappingFieldsCanBeMerged, '
|
||||||
|
fragment fragA on Human { name, ...fragB }
|
||||||
|
fragment fragB on Human { name, ...fragC }
|
||||||
|
fragment fragC on Human { name, ...fragA }
|
||||||
|
');
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* @it find invalid case even with immediately recursive fragment
|
||||||
|
*/
|
||||||
|
public function testFindInvalidCaseEvenWithImmediatelyRecursiveFragment()
|
||||||
|
{
|
||||||
|
$this->expectFailsRule(new OverlappingFieldsCanBeMerged, '
|
||||||
|
fragment sameAliasesWithDifferentFieldTargets on Dob {
|
||||||
|
...sameAliasesWithDifferentFieldTargets
|
||||||
|
fido: name
|
||||||
|
fido: nickname
|
||||||
|
}
|
||||||
|
',
|
||||||
|
[
|
||||||
|
FormattedError::create(
|
||||||
|
OverlappingFieldsCanBeMerged::fieldsConflictMessage(
|
||||||
|
'fido',
|
||||||
|
'name and nickname are different fields'
|
||||||
|
),
|
||||||
|
[
|
||||||
|
new SourceLocation(4, 9),
|
||||||
|
new SourceLocation(5, 9),
|
||||||
|
]
|
||||||
|
)
|
||||||
|
]);
|
||||||
|
}
|
||||||
|
|
||||||
private function getSchema()
|
private function getSchema()
|
||||||
{
|
{
|
||||||
$StringBox = null;
|
$StringBox = null;
|
||||||
|
Loading…
Reference in New Issue
Block a user