Skip to content

Spring does not reliably detect circular imports in Java configurations [SPR-13852] #18425

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.

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

Closed
spring-projects-issues opened this issue Jan 8, 2016 · 1 comment
Assignees
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: bug A general bug
Milestone

Comments

@spring-projects-issues
Copy link
Collaborator

spring-projects-issues commented Jan 8, 2016

Armand Beuvens opened SPR-13852 and commented

After investigation on some issues on non detected circular imports inside Java configuration classes, it seems that there is a bug inside the contains method of the inner class ImportStack of the class ConfigurationClassParser:

@Override
public boolean contains(Object elem) {
	ConfigurationClass configClass = (ConfigurationClass) elem;
	Comparator<ConfigurationClass> comparator = new Comparator<ConfigurationClass>() {
		@Override
		public int compare(ConfigurationClass first, ConfigurationClass second) {
			return (first.getMetadata().getClassName().equals(second.getMetadata().getClassName()) ? 0 : 1);
		}
	};
	return (Collections.binarySearch(this, configClass, comparator) != -1);
}

The bianrySearch method of Collections is called without sorting the collection before which can lead to undefined results.


Affects: 4.2.3

Issue Links:

Referenced from: commits e14c2de, 77b8f4d

@spring-projects-issues
Copy link
Collaborator Author

Juergen Hoeller commented

That code seems to be rather outdated since ConfigurationClass.equals is based on class name equivalence these days anyway. So I'm simply removing it in favor of the standard contains implementation there.

I'm also changing the base class to java.util.ArrayDeque (JDK 6+ which is alright with us these days) in favor of the generally outdated java.util.Stack.

Juergen

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: core Issues in core modules (aop, beans, core, context, expression) type: bug A general bug
Projects
None yet
Development

No branches or pull requests

2 participants