From 849d45ba9013640159fe3fcfe8876b74336901f3 Mon Sep 17 00:00:00 2001 From: Dusan Jakub Date: Wed, 19 Oct 2022 20:00:44 +0200 Subject: [PATCH] Fix StackOverflow on cyclic references involving collections. Fixes fabric8io/kubernetes-client#4510 --- CHANGELOG.md | 1 + .../visitor/ClassDependenciesVisitor.java | 20 +++++++------- .../crd/example/cyclic/CyclicList.java | 27 +++++++++++++++++++ .../crd/example/cyclic/CyclicListSpec.java | 22 +++++++++++++++ .../fabric8/crd/example/cyclic/RefList.java | 24 +++++++++++++++++ .../crd/generator/CRDGeneratorTest.java | 14 ++++++++++ 6 files changed, 99 insertions(+), 9 deletions(-) create mode 100644 crd-generator/api/src/test/java/io/fabric8/crd/example/cyclic/CyclicList.java create mode 100644 crd-generator/api/src/test/java/io/fabric8/crd/example/cyclic/CyclicListSpec.java create mode 100644 crd-generator/api/src/test/java/io/fabric8/crd/example/cyclic/RefList.java diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d951050132..9cca09a5c09 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ### 6.3-SNAPSHOT #### Bugs +* Fix #4510: Fix StackOverflow on cyclic references involving collections. #### Improvements diff --git a/crd-generator/api/src/main/java/io/fabric8/crd/generator/visitor/ClassDependenciesVisitor.java b/crd-generator/api/src/main/java/io/fabric8/crd/generator/visitor/ClassDependenciesVisitor.java index 93b733f885e..8d1981dcd23 100644 --- a/crd-generator/api/src/main/java/io/fabric8/crd/generator/visitor/ClassDependenciesVisitor.java +++ b/crd-generator/api/src/main/java/io/fabric8/crd/generator/visitor/ClassDependenciesVisitor.java @@ -33,7 +33,7 @@ public class ClassDependenciesVisitor extends TypedVisitor { private static final Map> traversedClasses = new HashMap<>(); private static final Map> crdNameToCrClass = new HashMap<>(); private final Set classesForCR; - private final Set processed = new HashSet<>(); + private final Set processed = new HashSet<>(); public ClassDependenciesVisitor(String crClassName, String crdName) { // need to record all classes associated with the different versions of the CR (not the CRD spec) @@ -49,7 +49,7 @@ public void visit(TypeDefBuilder builder) { // note that we cannot simply check the traversed class set to know if a class has been processed because classes // are usually added to the traversed set before they're looked at in depth final String className = type.getFullyQualifiedName(); - if (ignore(className) || processed.contains(className)) { + if (ignore(className)) { return; } @@ -74,19 +74,21 @@ public void visit(TypeDefBuilder builder) { // add classes from extends list type.getExtendsList().forEach(this::processTypeRef); - - // add class to the processed classes - processed.add(className); } private boolean ignore(String className) { - return (className.startsWith("java.") && !className.startsWith("java.util.")) || className.startsWith("com.fasterxml.jackson") || className.startsWith("jdk."); + return (className.startsWith("java.") && !className.startsWith("java.util.")) + || className.startsWith("com.fasterxml.jackson") || className.startsWith("jdk."); } private void processTypeRef(TypeRef t) { if (t instanceof ClassRef) { ClassRef classRef = (ClassRef) t; - visit(new TypeDefBuilder(Types.typeDefFrom(classRef))); + // only process the class reference if we haven't already + // note that the references are stored in the set including type arguments, so List and List are not the same + if (processed.add(classRef)) { + visit(new TypeDefBuilder(Types.typeDefFrom(classRef))); + } } } @@ -101,7 +103,7 @@ public static Set getDependentClasses(String crClassName) { public static Set getDependentClassesFromCRDName(String crdName) { // retrieve all dependent classes that might affect any of the CR versions return crdNameToCrClass.get(crdName).stream() - .flatMap(crClassName -> traversedClasses.get(crClassName).stream()) - .collect(Collectors.toSet()); + .flatMap(crClassName -> traversedClasses.get(crClassName).stream()) + .collect(Collectors.toSet()); } } diff --git a/crd-generator/api/src/test/java/io/fabric8/crd/example/cyclic/CyclicList.java b/crd-generator/api/src/test/java/io/fabric8/crd/example/cyclic/CyclicList.java new file mode 100644 index 00000000000..fdc047935e4 --- /dev/null +++ b/crd-generator/api/src/test/java/io/fabric8/crd/example/cyclic/CyclicList.java @@ -0,0 +1,27 @@ +/** + * Copyright (C) 2015 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.fabric8.crd.example.cyclic; + +import io.fabric8.kubernetes.api.model.Namespaced; +import io.fabric8.kubernetes.client.CustomResource; +import io.fabric8.kubernetes.model.annotation.Group; +import io.fabric8.kubernetes.model.annotation.Version; + +@Group("sample.fabric8.io") +@Version("v1alpha1") +public class CyclicList extends CustomResource implements Namespaced { + +} diff --git a/crd-generator/api/src/test/java/io/fabric8/crd/example/cyclic/CyclicListSpec.java b/crd-generator/api/src/test/java/io/fabric8/crd/example/cyclic/CyclicListSpec.java new file mode 100644 index 00000000000..a5f0798b2a0 --- /dev/null +++ b/crd-generator/api/src/test/java/io/fabric8/crd/example/cyclic/CyclicListSpec.java @@ -0,0 +1,22 @@ +/** + * Copyright (C) 2015 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.fabric8.crd.example.cyclic; + +import java.util.List; + +public class CyclicListSpec { + private List ref; +} diff --git a/crd-generator/api/src/test/java/io/fabric8/crd/example/cyclic/RefList.java b/crd-generator/api/src/test/java/io/fabric8/crd/example/cyclic/RefList.java new file mode 100644 index 00000000000..f1bd5c9bc25 --- /dev/null +++ b/crd-generator/api/src/test/java/io/fabric8/crd/example/cyclic/RefList.java @@ -0,0 +1,24 @@ +/** + * Copyright (C) 2015 Red Hat, Inc. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package io.fabric8.crd.example.cyclic; + +import java.util.List; + +public class RefList { + + private List ref; + +} diff --git a/crd-generator/api/src/test/java/io/fabric8/crd/generator/CRDGeneratorTest.java b/crd-generator/api/src/test/java/io/fabric8/crd/generator/CRDGeneratorTest.java index 2e8c98c9fa8..43da0e16171 100644 --- a/crd-generator/api/src/test/java/io/fabric8/crd/generator/CRDGeneratorTest.java +++ b/crd-generator/api/src/test/java/io/fabric8/crd/generator/CRDGeneratorTest.java @@ -19,6 +19,7 @@ import io.fabric8.crd.example.basic.BasicSpec; import io.fabric8.crd.example.basic.BasicStatus; import io.fabric8.crd.example.cyclic.Cyclic; +import io.fabric8.crd.example.cyclic.CyclicList; import io.fabric8.crd.example.inherited.*; import io.fabric8.crd.example.joke.Joke; import io.fabric8.crd.example.joke.JokeRequest; @@ -216,6 +217,19 @@ void generatingACycleShouldFail() { "An IllegalArgument Exception hasn't been thrown when generating a CRD with cyclic references"); } + @Test + void generatingACycleInListShouldFail() { + final CRDGenerator generator = new CRDGenerator() + .customResourceClasses(CyclicList.class) + .forCRDVersions("v1", "v1beta1") + .withOutput(output); + + assertThrows( + IllegalArgumentException.class, + () -> generator.detailedGenerate(), + "An IllegalArgument Exception hasn't been thrown when generating a CRD with cyclic references"); + } + @Test void notGeneratingACycleShouldSucceed() { final CRDGenerator generator = new CRDGenerator()