Skip to content

Commit

Permalink
Fix StackOverflow on cyclic references involving collections.
Browse files Browse the repository at this point in the history
Fixes #4510
  • Loading branch information
xRodney authored and manusa committed Nov 16, 2022
1 parent 3edb1b2 commit 4ab4dbb
Show file tree
Hide file tree
Showing 6 changed files with 99 additions and 9 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -5,6 +5,7 @@
#### Bugs
* Fix #4159: ensure the token refresh obeys how the Config was created
* Fix #4491: added a more explicit shutdown exception for okhttp
* Fix #4510: Fix StackOverflow on cyclic references involving collections.
* Fix #4534: Java Generator CLI default handling of skipGeneratedAnnotations
* Fix #4535: The shell command string will now have single quotes sanitized
* Fix #4543: (Java Generator) additionalProperties JsonAny setter method generated as setAdditionalProperty
Expand Down
Expand Up @@ -33,7 +33,7 @@ public class ClassDependenciesVisitor extends TypedVisitor<TypeDefBuilder> {
private static final Map<String, Set<String>> traversedClasses = new HashMap<>();
private static final Map<String, Set<String>> crdNameToCrClass = new HashMap<>();
private final Set<String> classesForCR;
private final Set<String> processed = new HashSet<>();
private final Set<ClassRef> 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)
Expand All @@ -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;
}

Expand All @@ -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<A> and List<B> are not the same
if (processed.add(classRef)) {
visit(new TypeDefBuilder(Types.typeDefFrom(classRef)));
}
}
}

Expand All @@ -101,7 +103,7 @@ public static Set<String> getDependentClasses(String crClassName) {
public static Set<String> 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());
}
}
@@ -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<CyclicListSpec, CyclicStatus> implements Namespaced {

}
@@ -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<RefList> ref;
}
@@ -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<RefList> ref;

}
Expand Up @@ -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;
Expand Down Expand Up @@ -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()
Expand Down

0 comments on commit 4ab4dbb

Please sign in to comment.