Merge pull request #902 from graphql-java-kickstart/bugfix/886

Fail with a clear error when unions contain themselves

Author: oryan-block

src/main/kotlin/graphql/kickstart/tools/SchemaParser.kt

@@ -345,7 +345,11 @@ class SchemaParser internal constructor(
         return builder.build()
     }
 
-    private fun getLeafUnionObjects(definition: UnionTypeDefinition, types: List<GraphQLObjectType>): List<GraphQLObjectType> {
+    private fun getLeafUnionObjects(
+        definition: UnionTypeDefinition,
+        types: List<GraphQLObjectType>,
+        path: List<UnionTypeDefinition> = listOf(definition),
+    ): List<GraphQLObjectType> {
         val name = definition.name
         val leafObjects = mutableListOf<GraphQLObjectType>()
 
@@ -356,7 +360,11 @@ class SchemaParser internal constructor(
             val nestedUnion: UnionTypeDefinition? = unionDefinitions.find { otherDefinition -> typeName == otherDefinition.name }
 
             if (nestedUnion != null) {
-                leafObjects.addAll(getLeafUnionObjects(nestedUnion, types))
+                if (nestedUnion in path) {
+                    val cycle = (path.dropWhile { union -> union != nestedUnion } + nestedUnion).joinToString(" -> ") { union -> union.name }
+                    throw SchemaError("Union '$typeName' contains itself through $cycle${it.sourceLocation.describe()}.")
+                }
+                leafObjects.addAll(getLeafUnionObjects(nestedUnion, types, path + nestedUnion))
             } else {
                 leafObjects.add(types.find { type -> type.name == typeName }
                     ?: throw SchemaError("Expected object type '$typeName' for union type '$name', but found none!"))

src/test/kotlin/graphql/kickstart/tools/SchemaParserTest.kt

@@ -277,6 +277,67 @@ class SchemaParserTest {
         assertEquals(error.message, "Type 'Nested' used by input field 'TestInput.nested' at line 6, column 14 is not defined in the schema.")
     }
 
+    @Test
+    fun `parser should throw descriptive exception when unions list each other`() {
+        val error = assertThrows(SchemaError::class.java) {
+            unionParser(
+                """
+                |union Result = Book | Media
+                |union Media = Movie | Result
+                """.trimMargin())
+        }
+
+        assertEquals(error.message, "Union 'Result' contains itself through Result -> Media -> Result at line 6, column 23.")
+    }
+
+    @Test
+    fun `parser should throw descriptive exception when nested unions list each other`() {
+        val error = assertThrows(SchemaError::class.java) {
+            unionParser(
+                """
+                |union Result = Book | A
+                |union A = B
+                |union B = C
+                |union C = A | Movie
+                """.trimMargin())
+        }
+
+        assertEquals(error.message, "Union 'A' contains itself through A -> B -> C -> A at line 8, column 11.")
+    }
+
+    @Test
+    fun `parser should throw descriptive exception when union lists itself`() {
+        val error = assertThrows(SchemaError::class.java) {
+            unionParser("union Result = Book | Result")
+        }
+
+        assertEquals(error.message, "Union 'Result' contains itself through Result -> Result at line 5, column 23.")
+    }
+
+    private fun unionParser(unions: String) = SchemaParser.newParser()
+        .schemaString(
+            """
+            |type Query {
+            |    results: [Result!]!
+            |}
+            |
+            |$unions
+            |
+            |type Book {
+            |    title: String!
+            |}
+            |
+            |type Movie {
+            |    title: String!
+            |}
+            """.trimMargin())
+        .resolvers(object : GraphQLQueryResolver {
+            fun results(): List<Any> = listOf()
+        })
+        .dictionary(Book::class, Movie::class)
+        .build()
+        .makeExecutableSchema()
+
     @Test
     fun `parser should throw descriptive exception when type was never reached by the scanner`() {
         val error = assertThrows(SchemaError::class.java) {
@@ -1062,6 +1123,10 @@ class SchemaParserTest {
 
     class RootObj
 
+    data class Book(val title: String)
+
+    data class Movie(val title: String)
+
     class ProxiedResolver : GraphQLQueryResolver {
         fun test(): List<String> = listOf()
     }