Skip to content

Commit

Permalink
Fix infinite recursion bug in nested @configuration
Browse files Browse the repository at this point in the history
Prior to this commit, an infinite recursion would occur if a
@configuration class were nested within its superclass, e.g.

  abstract class Parent {
      @configuration
      static class Child extends Parent { ... }
  }

This is because the processing of the nested class automatically
checks the superclass hierarchy for certain reasons, and each
superclass is in turn checked for nested @configuration classes.

The ConfigurationClassParser implementation now prevents this by
keeping track of known superclasses, i.e. once a superclass has been
processed, it is never again checked for nested classes, etc.

Issue: SPR-8955
  • Loading branch information
cbeams committed Feb 15, 2012
1 parent f3651c9 commit 08e2669
Show file tree
Hide file tree
Showing 3 changed files with 99 additions and 18 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,8 @@ class ConfigurationClassParser {

private final ImportStack importStack = new ImportStack();

private final Set<String> knownSuperclasses = new LinkedHashSet<String>();

private final Set<ConfigurationClass> configurationClasses =
new LinkedHashSet<ConfigurationClass>();

Expand Down Expand Up @@ -138,23 +140,12 @@ protected void processConfigurationClass(ConfigurationClass configClass) throws
}
}

while (metadata != null) {
doProcessConfigurationClass(configClass, metadata);
String superClassName = metadata.getSuperClassName();
if (superClassName != null && !Object.class.getName().equals(superClassName)) {
if (metadata instanceof StandardAnnotationMetadata) {
Class<?> clazz = ((StandardAnnotationMetadata) metadata).getIntrospectedClass();
metadata = new StandardAnnotationMetadata(clazz.getSuperclass(), true);
}
else {
MetadataReader reader = this.metadataReaderFactory.getMetadataReader(superClassName);
metadata = reader.getAnnotationMetadata();
}
}
else {
metadata = null;
}
// recursively process the configuration class and its superclass hierarchy
do {
metadata = doProcessConfigurationClass(configClass, metadata);
}
while (metadata != null);

if (this.configurationClasses.contains(configClass) && configClass.getBeanName() != null) {
// Explicit bean definition found, probably replacing an import.
// Let's remove the old one and go with the new one.
Expand All @@ -164,7 +155,11 @@ protected void processConfigurationClass(ConfigurationClass configClass) throws
this.configurationClasses.add(configClass);
}

protected void doProcessConfigurationClass(ConfigurationClass configClass, AnnotationMetadata metadata) throws IOException {
/**
* @return annotation metadata of superclass, null if none found or previously processed
*/
protected AnnotationMetadata doProcessConfigurationClass(
ConfigurationClass configClass, AnnotationMetadata metadata) throws IOException {

// recursively process any member (nested) classes first
for (String memberClassName : metadata.getMemberClassNames()) {
Expand Down Expand Up @@ -227,8 +222,26 @@ protected void doProcessConfigurationClass(ConfigurationClass configClass, Annot
for (MethodMetadata methodMetadata : beanMethods) {
configClass.addBeanMethod(new BeanMethod(methodMetadata, configClass));
}
}

// process superclass, if any
if (metadata.hasSuperClass()) {
String superclass = metadata.getSuperClassName();
if (this.knownSuperclasses.add(superclass)) {
// superclass found, return its annotation metadata and recurse
if (metadata instanceof StandardAnnotationMetadata) {
Class<?> clazz = ((StandardAnnotationMetadata) metadata).getIntrospectedClass();
return new StandardAnnotationMetadata(clazz.getSuperclass(), true);
}
else {
MetadataReader reader = this.metadataReaderFactory.getMetadataReader(superclass);
return reader.getAnnotationMetadata();
}
}
}

// no superclass, processing is complete
return null;
}

/**
* Return a list of attribute maps for all declarations of the given annotation
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/*
* Copyright 2002-2012 the original author or authors.
*
* 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 org.springframework.context.annotation.configuration.spr8955;

import org.springframework.stereotype.Component;

/**
* @author Chris Beams
* @author Willem Dekker
*/
abstract class Spr8955Parent {

@Component
static class Spr8955Child extends Spr8955Parent {

}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
* Copyright 2002-2012 the original author or authors.
*
* 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 org.springframework.context.annotation.configuration.spr8955;

import org.junit.Test;
import org.springframework.context.annotation.AnnotationConfigApplicationContext;


/**
* @author Chris Beams
* @author Willem Dekker
*/
public class Spr8955Tests {

@Test
public void repro() {
AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext();
ctx.scan("org.springframework.context.annotation.configuration.spr8955");
ctx.refresh();
}

}

0 comments on commit 08e2669

Please sign in to comment.