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

graphql doesn't support directive with default value and mismatch with gqlparser #40

Closed
zhangtbj opened this issue Jun 19, 2024 · 5 comments · Fixed by #42
Closed

graphql doesn't support directive with default value and mismatch with gqlparser #40

zhangtbj opened this issue Jun 19, 2024 · 5 comments · Fixed by #42

Comments

@zhangtbj
Copy link

zhangtbj commented Jun 19, 2024

Hi,

I am using the nautilus gateway and graphql as our gateway server.

Recently, we received a vulnerability report that:
Screenshot 2024-06-19 at 15 31 43

We have to upgrade the gqlparser version for security reason, but after we upgrade the version, the nautilus gateway cannot startup correctly, it reports a failure during the startup:

while gateway setup: while creating gateway: conflict in argument definitions for directive defer: one is a list the other isn't

After the investigation, I found the reason is gqlparser add a new directive defer in its inner schema after release v2.5.2 (But nautilus graphql is still using the v2.0.1:
https://github.com/vektah/gqlparser/blob/v2.5.2/validator/prelude.graphql#L31

directive @defer(if: Boolean = true, label: String) on FRAGMENT_SPREAD | INLINE_FRAGMENT

After adding this directive @defer and with the default Boolean value true, the graphql introspection cannot match its default value because it doesn't support to set the default value for the argument of directive:
https://github.com/nautilus/graphql/blob/master/introspection.go#L340-L344

So the failure will happen during the multiple schema merge:
https://github.com/nautilus/gateway/blob/master/merge.go#L607-L609

So I prepared a workaround to add default value if it exists, it works fine for me, but I think it is not an official fix:
zhangtbj@c0d6961

I am not sure if it can be fixed by nautilus graphql community officially or do you have any other good idea?

I am not sure who can help here, so cc @JohnStarich for help too.

Thanks a lot!

@zhangtbj zhangtbj changed the title The gqlparser package version is very old and cannot be used with new version together graphql doesn't support directive with default value and mismatch with gqlparser Jun 19, 2024
@JohnStarich
Copy link
Member

Hey @zhangtbj, thank you for the issue. I'll take a look soon.

If you happen to have a minimal test that can reproduce this failure, that would be super helpful to verify the fix.

@zhangtbj
Copy link
Author

zhangtbj commented Jun 20, 2024

Thanks for the quick response @JohnStarich 👍 .

No problem, I think the problem is also related the issue #33

You can write a very simple example like this:

package main

import (
	"fmt"
	"github.com/nautilus/graphql"
)

func main() {
	url := "https://api.monday.com/v2/get_schema"
	// Write an example to call IntrospectRemoteSchemas
	remoteSchema, err := graphql.IntrospectRemoteSchemas(url)
	if err != nil {
		panic(err)
	}

	fmt.Println(len(remoteSchema))
}

Then debug it step by step, you will find:

After loading the remote schema, there are 4 directives, and one of the directives deprecated has the DefaultValue "No longer supported":

Screenshot 2024-06-20 at 08 44 29

But after parsing to the schema of nautilus by the func IntrospectAPI and for loop each directive (line 295):

	// add each directive to the schema
	for _, directive := range remoteSchema.Directives {

You will see no parse for the DefaultValue in the func introspectionConvertArgList, line 335:

	// we need to add each argument to the field
	for _, argument := range args {
		result = append(result, &ast.ArgumentDefinition{
			Name:        argument.Name,
			Description: argument.Description,
			Type:        introspectionUnmarshalTypeRef(&argument.Type),
		})
	}

So the result of the schema will be the empty DefaultValue:

Screenshot 2024-06-20 at 08 48 31

Above is the root cause of the DefaultValue, the result is gateway cannot startup correctly if upgrade gqlparser version

nautilus gateway uses a very old gqlparser version which doesn't have the directive defer, but if upgrade gqlparser to a newer version, it includes a direct defer has if with default value true:
directive @defer(if: Boolean = true, label: String) on FRAGMENT_SPREAD | INLINE_FRAGMENT

Because the above bug, nautilus cannot set a default value for my schema directive, so the two of directive @defer are not match then report the above error and gateway failed to startup:

while gateway setup: while creating gateway: conflict in argument definitions for directive defer: one is a list the other isn't

Hope it is helpful :)

@JohnStarich
Copy link
Member

@zhangtbj (likely) fix is merged and released in v0.0.25!

@zhangtbj
Copy link
Author

Awesome! Thanks for the quick fix 👍

Cool and let me try it soon :)

@zhangtbj
Copy link
Author

zhangtbj commented Jul 1, 2024

BTW, I verified on our env, that the fix works fine and the problem has been solved.

Thanks a lot! :) 👍

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

Successfully merging a pull request may close this issue.

2 participants