Skip to content

fix: return parsing error for empty selection sets #1220

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

Merged
jensneuse merged 2 commits into from
Jul 9, 2025
Merged

fix: return parsing error for empty selection sets #1220

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
87f29ee
fix: return parsing error for empty selection sets
jensneuse Jul 9, 2025
e00bdfd
chore: update tests
jensneuse Jul 9, 2025
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
20 changes: 20 additions & 0 deletions execution/engine/federation_integration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,26 @@ func TestFederationIntegrationTest(t *testing.T) {
assert.Len(t, resp, 0)
})

t.Run("empty fragment", func(t *testing.T) {
setup := federationtesting.NewFederationSetup(addGateway(false))
t.Cleanup(setup.Close)
gqlClient := NewGraphqlClient(http.DefaultClient)
ctx, cancel := context.WithCancel(context.Background())
t.Cleanup(cancel)
resp := gqlClient.QueryStatusCode(ctx, setup.GatewayServer.URL, testQueryPath("queries/empty_fragment.graphql"), nil, http.StatusInternalServerError, t)
assert.Len(t, resp, 0)
})

t.Run("empty fragment variant", func(t *testing.T) {
setup := federationtesting.NewFederationSetup(addGateway(false))
t.Cleanup(setup.Close)
gqlClient := NewGraphqlClient(http.DefaultClient)
ctx, cancel := context.WithCancel(context.Background())
t.Cleanup(cancel)
resp := gqlClient.QueryStatusCode(ctx, setup.GatewayServer.URL, testQueryPath("queries/empty_fragment_variant.graphql"), nil, http.StatusInternalServerError, t)
assert.Len(t, resp, 0)
})
Comment thread
devsergiy marked this conversation as resolved.

t.Run("Union response type with interface fragments", func(t *testing.T) {
setup := federationtesting.NewFederationSetup(addGateway(false))
t.Cleanup(setup.Close)
Expand Down
5 changes: 5 additions & 0 deletions execution/federationtesting/testdata/queries/empty_fragment.graphql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
fragment A on Query {}

{
...A
}
9 changes: 9 additions & 0 deletions execution/federationtesting/testdata/queries/empty_fragment_variant.graphql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
fragment A on Query {
... {

}
}

{
...A
}
6 changes: 3 additions & 3 deletions v2/pkg/ast/ast_operation_definition_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,19 +30,19 @@ func TestDocument_OperationNameExists(t *testing.T) {
))

t.Run("found on document with a single operations", run(
"query MyOperation {}",
"query MyOperation {other}",
"MyOperation",
true,
))

t.Run("found on document with multiple operations", run(
"query OtherOperation {other} query AnotherOperation {another} query MyOperation {}",
"query OtherOperation {other} query AnotherOperation {another} query MyOperation {other}",
"MyOperation",
true,
))

t.Run("found on a document with preceding root nodes of not operation type", run(
"fragment F on T {field} query MyOperation {}",
"fragment F on T {field} query MyOperation {another}",
"MyOperation",
true,
))
Expand Down
3 changes: 2 additions & 1 deletion v2/pkg/astparser/parser.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1311,7 +1311,8 @@ func (p *Parser) parseSelectionSet() (int, bool) {
set.RBrace = rbraceToken.TextPosition

if len(set.SelectionRefs) == 0 {
return 0, false
p.errUnexpectedToken(rbraceToken, keyword.IDENT, keyword.SPREAD)
return ast.InvalidRef, false
}

p.document.SelectionSets = append(p.document.SelectionSets, set)
Expand Down
47 changes: 30 additions & 17 deletions v2/pkg/astparser/parser_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -778,27 +778,25 @@ func TestParser_Parse(t *testing.T) {
""""
Object Type BlockString to ignore
"""
extend type Person {
extend type Person { a: Int
}
""""
Interface Type BlockString to ignore
"""
extend interface NamedEntity {
extend interface NamedEntity { a: Int
}
""""
Scalar Type BlockString to ignore
"""
extend scalar JSON {
}
extend scalar JSON
""""
Union Type BlockString to ignore
"""
extend union SearchResult = Photo | Person {
}
extend union SearchResult = Photo | Person
""""
Input Type BlockString to ignore
"""
extend input NamedEntity {
extend input NamedEntity { a: Int
}`, parse, false)
})
t.Run("scalar type definition", func(t *testing.T) {
Expand Down Expand Up @@ -2233,18 +2231,18 @@ func TestParser_Parse(t *testing.T) {
mutation: Mutation
subscription: Subscription
}
scalar Scalar1 {}
scalar Scalar2 {}
type Type1 {}
type Type2 {}
interface Iface1 {}
interface Iface2 {}
input Input1 {}
input Input2 {}
scalar Scalar1
scalar Scalar2
type Type1 { a: Int }
type Type2 { a: Int }
interface Iface1 { a: Int }
interface Iface2 { a: Int }
input Input1 { a: Int }
input Input2 { a: Int }
union Union1
union Union2
enum Enum1 {}
enum Enum2 {}
enum Enum1 { A }
enum Enum2 { B }
directive @directive1 on FIELD
directive @directive2 on FIELD
`, parse, false,
Expand Down Expand Up @@ -2586,6 +2584,21 @@ func TestErrorReport(t *testing.T) {
t.Fatalf("want:\n%s\ngot:\n%s\n", want, report.Error())
}
})
t.Run("empty fragment", func(t *testing.T) {
_, report := ParseGraphqlDocumentString(`
fragment A on Query {}
query { ...A }
`)

if !report.HasErrors() {
t.Fatalf("want err, got nil")
}

want := "external: unexpected token - got: RBRACE want one of: [IDENT SPREAD], locations: [{Line:2 Column:25}], path: []"
if report.Error() != want {
t.Fatalf("want:\n%s\ngot:\n%s\n", want, report.Error())
}
})
}

func TestParseStarwars(t *testing.T) {
Expand Down
25 changes: 5 additions & 20 deletions v2/pkg/astvalidation/operation_validation_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3385,14 +3385,6 @@ type Query {
}`,
DirectivesAreInValidLocations(), Invalid)
})
t.Run("150 variant", func(t *testing.T) {
run(t, `
{
...frag @spread
}
fragment frag on Query {}`,
DirectivesAreInValidLocations(), Valid, withDisableNormalization())
})
t.Run("150 variant", func(t *testing.T) {
run(t, ` {
... {
Expand All @@ -3409,13 +3401,6 @@ type Query {
}`,
DirectivesAreInValidLocations(), Invalid)
})
t.Run("150 variant", func(t *testing.T) {
run(t, ` {
...frag
}
fragment frag on Query @fragmentDefinition {}`,
DirectivesAreInValidLocations(), Valid, withDisableNormalization())
})
t.Run("150 variant", func(t *testing.T) {
run(t, ` query @onQuery {
dog
Expand Down Expand Up @@ -3548,15 +3533,15 @@ type Query {
VariablesAreInputTypes(), Valid)
})
t.Run("157", func(t *testing.T) {
run(t, `query takesCat($cat: Cat) {}`,
run(t, `query takesCat($cat: Cat) { a }`,
VariablesAreInputTypes(), Invalid)
run(t, `query takesDogBang($dog: Dog!) {}`,
run(t, `query takesDogBang($dog: Dog!) { a }`,
VariablesAreInputTypes(), Invalid)
run(t, `query takesListOfPet($pets: [Pet]) {}`,
run(t, `query takesListOfPet($pets: [Pet]) { a }`,
VariablesAreInputTypes(), Invalid)
run(t, `query takesCatOrDog($catOrDog: CatOrDog) {}`,
run(t, `query takesCatOrDog($catOrDog: CatOrDog) { a }`,
VariablesAreInputTypes(), Invalid)
run(t, `query takesCatOrDog($catCommand: CatCommand) {}`,
run(t, `query takesCatOrDog($catCommand: CatCommand) { a }`,
VariablesAreInputTypes(), Valid)
})
})
Expand Down
38 changes: 19 additions & 19 deletions v2/pkg/graphqljsonschema/jsonschema_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func runTest(schema, operation, expectedJsonSchema string, valid []string, inval
func TestJsonSchema(t *testing.T) {
t.Run("object", runTest(
`scalar String input Test { str: String }`,
`query ($input: Test){}`,
`query ($input: Test){ a }`,
`{"type":["object","null"],"properties":{"str":{"type":["string","null"]}},"additionalProperties":false}`,
[]string{
`{"str":"validString"}`,
Expand All @@ -62,7 +62,7 @@ func TestJsonSchema(t *testing.T) {
))
t.Run("string", runTest(
`scalar String input Test { str: String }`,
`query ($input: String){}`,
`query ($input: String){ a }`,
`{"type":["string","null"]}`,
[]string{
`"validString"`,
Expand Down Expand Up @@ -90,7 +90,7 @@ func TestJsonSchema(t *testing.T) {
))
t.Run("id", runTest(
`scalar ID input Test { str: String }`,
`query ($input: ID){}`,
`query ($input: ID){ a }`,
`{"type":["string","integer","null"]}`,
[]string{
`"validString"`,
Expand All @@ -104,7 +104,7 @@ func TestJsonSchema(t *testing.T) {
))
t.Run("array", runTest(
`scalar String`,
`query ($input: [String]){}`,
`query ($input: [String]){ a }`,
`{"type":["array","null"],"items":{"type":["string","null"]}}`,
[]string{
`null`,
Expand All @@ -120,7 +120,7 @@ func TestJsonSchema(t *testing.T) {
))
t.Run("input object array", runTest(
`scalar String input StringInput { str: String }`,
`query ($input: [StringInput]){}`,
`query ($input: [StringInput]){ a }`,
`{"type":["array","null"],"items":{"anyOf": [{"type":["null"]},{"$ref":"#/$defs/StringInput"}]},"$defs":{"StringInput":{"type":["object","null"],"properties":{"str":{"type":["string","null"]}},"additionalProperties":false}}}`,
[]string{
`null`,
Expand All @@ -136,7 +136,7 @@ func TestJsonSchema(t *testing.T) {
))
t.Run("nested input object both as required and not required", runTest(
`scalar String input StringInput { str: String } input Input { requireInput: StringInput! optionalInput: StringInput }`,
`query ($input: Input){}`,
`query ($input: Input){ a }`,
`{"type":["object","null"],"properties":{"optionalInput":{"anyOf":[{"type":["null"]},{"$ref":"#/$defs/StringInput"}]},"requireInput":{"$ref":"#/$defs/StringInputNotNull"}},"required":["requireInput"],"additionalProperties":false,"$defs":{"StringInput":{"type":["object","null"],"properties":{"str":{"type":["string","null"]}},"additionalProperties":false},"StringInputNotNull":{"type":["object"],"properties":{"str":{"type":["string","null"]}},"additionalProperties":false}}}`,
[]string{
`{"optionalInput": {}, "requireInput": {"str":"validString"}}`,
Expand All @@ -149,7 +149,7 @@ func TestJsonSchema(t *testing.T) {
))
t.Run("nested input object both as required and not required - reverse order", runTest(
`scalar String input StringInput { str: String } input Input { optionalInput: StringInput requireInput: StringInput! }`,
`query ($input: Input){}`,
`query ($input: Input){ a }`,
`{"type":["object","null"],"properties":{"optionalInput":{"anyOf":[{"type":["null"]},{"$ref":"#/$defs/StringInput"}]},"requireInput":{"$ref":"#/$defs/StringInputNotNull"}},"required":["requireInput"],"additionalProperties":false,"$defs":{"StringInput":{"type":["object","null"],"properties":{"str":{"type":["string","null"]}},"additionalProperties":false},"StringInputNotNull":{"type":["object"],"properties":{"str":{"type":["string","null"]}},"additionalProperties":false}}}`,
[]string{
`{"optionalInput": {}, "requireInput": {"str":"validString"}}`,
Expand All @@ -162,7 +162,7 @@ func TestJsonSchema(t *testing.T) {
))
t.Run("optional nested input object used twice", runTest(
`scalar String input StringInput { str: String } input Input { optionalInput1: StringInput optionalInput2: StringInput }`,
`query ($input: Input){}`,
`query ($input: Input){ a }`,
`{"type":["object","null"],"properties":{"optionalInput1":{"anyOf":[{"type":["null"]},{"$ref":"#/$defs/StringInput"}]},"optionalInput2":{"anyOf":[{"type":["null"]},{"$ref":"#/$defs/StringInput"}]}},"additionalProperties":false,"$defs":{"StringInput":{"type":["object","null"],"properties":{"str":{"type":["string","null"]}},"additionalProperties":false}}}`,
[]string{
`{"optionalInput1": {}, "optionalInput2": {"str":"validString"}}`,
Expand All @@ -173,7 +173,7 @@ func TestJsonSchema(t *testing.T) {
))
t.Run("required nested input object used twice", runTest(
`scalar String input StringInput { str: String } input Input { requireInput1: StringInput! requireInput2: StringInput! }`,
`query ($input: Input){}`,
`query ($input: Input){ a }`,
`{"type":["object","null"],"properties":{"requireInput1":{"$ref":"#/$defs/StringInputNotNull"},"requireInput2":{"$ref":"#/$defs/StringInputNotNull"}},"required":["requireInput1","requireInput2"],"additionalProperties":false,"$defs":{"StringInputNotNull":{"type":["object"],"properties":{"str":{"type":["string","null"]}},"additionalProperties":false}}}`,
[]string{
`{"requireInput1": {"str":"validString"}, "requireInput2": {"str":"validString"}}`,
Expand All @@ -185,7 +185,7 @@ func TestJsonSchema(t *testing.T) {
))
t.Run("required array", runTest(
`scalar String`,
`query ($input: [String]!){}`,
`query ($input: [String]!){ a }`,
`{"type":["array"],"items":{"type":["string","null"]}}`,
[]string{
`[]`,
Expand All @@ -201,7 +201,7 @@ func TestJsonSchema(t *testing.T) {
))
t.Run("required array element", runTest(
`scalar String`,
`query ($input: [String!]){}`,
`query ($input: [String!]){ a }`,
`{"type":["array","null"],"items":{"type":["string"]}}`,
[]string{
`null`,
Expand All @@ -218,7 +218,7 @@ func TestJsonSchema(t *testing.T) {
))
t.Run("nested object", runTest(
`scalar String scalar Boolean input Test { str: String! nested: Nested } input Nested { boo: Boolean }`,
`query ($input: Test){}`,
`query ($input: Test){ a }`,
`{"type":["object","null"],"properties":{"nested":{"anyOf":[{"type":["null"]},{"$ref":"#/$defs/Nested"}]},"str":{"type":["string"]}},"required":["str"],"additionalProperties":false,"$defs":{"Nested":{"type":["object","null"],"properties":{"boo":{"type":["boolean","null"]}},"additionalProperties":false}}}`,
[]string{
`null`,
Expand All @@ -237,7 +237,7 @@ func TestJsonSchema(t *testing.T) {
))
t.Run("nested object with override", runTest(
`scalar String scalar Boolean input Test { str: String! override: Override } input Override { boo: Boolean }`,
`query ($input: Test){}`,
`query ($input: Test){ a }`,
`{"type":["object","null"],"properties":{"override":{"type":["string","null"]},"str":{"type":["string"]}},"required":["str"],"additionalProperties":false}`,
[]string{
`null`,
Expand All @@ -257,7 +257,7 @@ func TestJsonSchema(t *testing.T) {
))
t.Run("recursive object", runTest(
`scalar String scalar Boolean input Test { str: String! nested: Nested } input Nested { boo: Boolean recursive: Test }`,
`query ($input: Test){}`,
`query ($input: Test){ a }`,
`{"type":["object","null"],"properties":{"nested":{"anyOf":[{"type":["null"]},{"$ref":"#/$defs/Nested"}]},"str":{"type":["string"]}},"required":["str"],"additionalProperties":false,"$defs":{"Nested":{"type":["object","null"],"properties":{"boo":{"type":["boolean","null"]},"recursive":{"anyOf": [{"type":["null"]},{"$ref":"#/$defs/Test"}]}},"additionalProperties":false},"Test":{"type":["object","null"],"properties":{"nested":{"anyOf":[{"type":["null"]},{"$ref":"#/$defs/Nested"}]},"str":{"type":["string"]}},"required":["str"],"additionalProperties":false}}}`,
[]string{
`{"str":"validString"}`,
Expand All @@ -273,7 +273,7 @@ func TestJsonSchema(t *testing.T) {
))
t.Run("recursive object with multiple branches", runTest(
`scalar String scalar Boolean input Root { test: Test another: Another } input Test { str: String! nested: Nested } input Nested { boo: Boolean recursive: Test another: Another } input Another { boo: Boolean }`,
`query ($input: Root){}`,
`query ($input: Root){ a }`,
`{"type":["object","null"],"properties":{"another":{"anyOf":[{"type":["null"]},{"$ref":"#/$defs/Another"}]},"test":{"anyOf":[{"type":["null"]},{"$ref":"#/$defs/Test"}]}},"additionalProperties":false,"$defs":{"Another":{"type":["object","null"],"properties":{"boo":{"type":["boolean","null"]}},"additionalProperties":false},"Nested":{"type":["object","null"],"properties":{"another":{"anyOf":[{"type":["null"]},{"$ref":"#/$defs/Another"}]},"boo":{"type":["boolean","null"]},"recursive":{"anyOf":[{"type":["null"]},{"$ref":"#/$defs/Test"}]}},"additionalProperties":false},"Test":{"type":["object","null"],"properties":{"nested":{"anyOf":[{"type":["null"]},{"$ref":"#/$defs/Nested"}]},"str":{"type":["string"]}},"required":["str"],"additionalProperties":false}}}`,
[]string{
`{"test":{"str":"validString"}}`,
Expand All @@ -287,14 +287,14 @@ func TestJsonSchema(t *testing.T) {
))
t.Run("complex recursive schema", runTest(
complexRecursiveSchema,
`query ($input: db_messagesWhereInput){}`,
`query ($input: db_messagesWhereInput){ a }`,
complexRecursiveSchemaResult,
[]string{},
[]string{},
))
t.Run("one level deep sub path", runTest(
"input Human { name: String! } scalar String",
"query ($human: Human!) { }",
"query ($human: Human!) { a }",
`{"type":["string"]}`,
[]string{
`"John Doe"`,
Expand All @@ -306,7 +306,7 @@ func TestJsonSchema(t *testing.T) {
))
t.Run("multi level deep sub path", runTest(
"input Human { name: String! pet: Animal } scalar String type Animal { name: String! }",
"query ($human: Human!) { }",
"query ($human: Human!) { a }",
`{"type":["string"]}`,
[]string{
`"Doggie"`,
Expand All @@ -319,7 +319,7 @@ func TestJsonSchema(t *testing.T) {
))
t.Run("not defined scalar", runTest(
`input Container { name: MyScalar }`,
`query ($input: Container){}`,
`query ($input: Container){ a }`,
`{"type":["object", "null"], "properties": {"name": {}}, "additionalProperties": false}`,
[]string{},
[]string{},
Expand Down
Loading