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.

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

check for Introspection::TYPE_NAME_FIELD_NAME in QueryPlan::analyzeSelectionSet at wrong place? #1293

Closed
jmglsn opened this issue Feb 1, 2023 · 3 comments · Fixed by #1295
Labels

Comments

@jmglsn
Copy link

jmglsn commented Feb 1, 2023

Hi over there,

I'll try to provide more details soon, maybe also an query where the essential problem is visible.
Right now I just have a huge query which is hard to reduce to the key point.

But maybe it's also just moving an assertion.

In QueryPlan::analyzeSelectionSet is an assertion that aims for $parentType instanceof HasFieldsType just a few lines below is a condition that skips the current iteration if the field name is Introspection::TYPE_NAME_FIELD_NAME.
I ran into an error because the $selectionNode is a Introspection::TYPE_NAME_FIELD_NAME - so its not an HasFieldsType.

Could we move the assertion below the check for Introspection::TYPE_NAME_FIELD_NAME?

Thank you :)

    private function analyzeSelectionSet(SelectionSetNode $selectionSet, Type $parentType, array &$implementors): array
    {
        $fields = [];
        $implementors = [];
        foreach ($selectionSet->selections as $selectionNode) {
            if ($selectionNode instanceof FieldNode) {
                $fieldName = $selectionNode->name->value;

                if ($fieldName === Introspection::TYPE_NAME_FIELD_NAME) {
                    continue;
                }

                // now assert
                assert($parentType instanceof HasFieldsType, 'ensured by query validation');
// ...
@jmglsn
Copy link
Author

jmglsn commented Feb 1, 2023

If I got it correctly the following could be used to describe the problem:

query {
   aQuery {
     aUnionType {
       __typename
     }
   }
}

Within the resolver for aQuery I try to access the query plan by calling $resolveInfo->lookAhead()->queryPlan().
This will cause analyzeQueryPlan with analyzeSelectionSet and a call from analyzeSubFields with an instance of UnionType (passes over $type instanceof AbstractType).
The UnionType instance will than cause an error for $parentType instanceof HasFieldsType.

@spawnia
Copy link
Collaborator

spawnia commented Feb 2, 2023

Thanks for the detailed report, fixed with https://github.com/webonyx/graphql-php/releases/tag/v15.0.2.

@jmglsn
Copy link
Author

jmglsn commented Feb 2, 2023

Thank you for the super fast response! 🥇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants