Skip to content
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

AopTestUtils.getUltimateTargetObject results in stack overflow for proxy backed by LazyInitTargetSource #29215

Closed
lako12 opened this issue Sep 28, 2022 · 6 comments
Assignees
Labels
in: test Issues in the test module status: declined A suggestion or change that we don't feel we should currently apply

Comments

@lako12
Copy link

lako12 commented Sep 28, 2022

Security configuration class

    @Configuration
    @EnableGlobalMethodSecurity(prePostEnabled = true)
    public class SecurityConf {
    
        @Bean
        public AuthenticationManager authenticationManagerBean(AuthenticationConfiguration authenticationConfiguration) throws Exception {
            return authenticationConfiguration.getAuthenticationManager();
        }
    }

Controller

    @RestController
    public class Controller {
        
        @GetMapping("/test")
        public String test(){
            return "test";
        }
    }

Test class

    @ExtendWith(SpringExtension.class)
    @WebMvcTest(controllers = Controller.class)
    @ComponentScan("com.example.demo")
    public class DemoApplicationTests {
    
        @Test
        public void test() {
        }
    }

I have a simple application, if I try to start the main it works, however when I start my test class I get this error:

java.lang.StackOverflowError
            	at java.base/java.lang.StackTraceElement.of(StackTraceElement.java:526)
            	at java.base/java.lang.Throwable.getOurStackTrace(Throwable.java:828)
            	at java.base/java.lang.Throwable.getStackTrace(Throwable.java:820)
            	at ch.qos.logback.classic.spi.ThrowableProxy.<init>(ThrowableProxy.java:79)
            	at ch.qos.logback.classic.spi.ThrowableProxy.<init>(ThrowableProxy.java:89)
            	at ch.qos.logback.classic.spi.ThrowableProxy.<init>(ThrowableProxy.java:89)
            	at ch.qos.logback.classic.spi.ThrowableProxy.<init>(ThrowableProxy.java:89)
            	at ch.qos.logback.classic.spi.ThrowableProxy.<init>(ThrowableProxy.java:89)
            	at ch.qos.logback.classic.spi.ThrowableProxy.<init>(ThrowableProxy.java:89)
            	at ch.qos.logback.classic.spi.ThrowableProxy.<init>(ThrowableProxy.java:89)
            	at ch.qos.logback.classic.spi.ThrowableProxy.<init>(ThrowableProxy.java:89)
            	at ch.qos.logback.classic.spi.ThrowableProxy.<init>(ThrowableProxy.java:89)
            	at ch.qos.logback.classic.spi.ThrowableProxy.<init>(ThrowableProxy.java:89)
            	at ch.qos.logback.classic.spi.ThrowableProxy.<init>(ThrowableProxy.java:89)
            	at ch.qos.logback.classic.spi.ThrowableProxy.<init>(ThrowableProxy.java:89)
            	at ch.qos.logback.classic.spi.ThrowableProxy.<init>(ThrowableProxy.java:89)
            	at ch.qos.logback.classic.spi.ThrowableProxy.<init>(ThrowableProxy.java:89)
            	at ch.qos.logback.classic.spi.ThrowableProxy.<init>(ThrowableProxy.java:89)
            	at ch.qos.logback.classic.spi.ThrowableProxy.<init>(ThrowableProxy.java:89)
            	at ch.qos.logback.classic.spi.ThrowableProxy.<init>(ThrowableProxy.java:89)
            	at ch.qos.logback.classic.spi.ThrowableProxy.<init>(ThrowableProxy.java:89)
            	at ch.qos.logback.classic.spi.ThrowableProxy.<init>(ThrowableProxy.java:89)
            	at ch.qos.logback.classic.spi.ThrowableProxy.<init>(ThrowableProxy.java:89)
            	at ch.qos.logback.classic.spi.ThrowableProxy.<init>(ThrowableProxy.java:89)
            	at ch.qos.logback.classic.spi.ThrowableProxy.<init>(ThrowableProxy.java:89)
            	at ch.qos.logback.classic.spi.ThrowableProxy.<init>(ThrowableProxy.java:89)
            	at ch.qos.logback.classic.spi.ThrowableProxy.<init>(ThrowableProxy.java:89)
            	at ch.qos.logback.classic.spi.ThrowableProxy.<init>(ThrowableProxy.java:89)
            	at ch.qos.logback.classic.spi.ThrowableProxy.<init>(ThrowableProxy.java:89)
            	at ch.qos.logback.classic.spi.ThrowableProxy.<init>(ThrowableProxy.java:89)
            	at ch.qos.logback.classic.spi.ThrowableProxy.<init>(ThrowableProxy.java:89)
        and it always continues the same way ...

In debug I tried to recover the error, but I only see this java.lang.IllegalStateException: Failed to unwrap proxied object.

The test only works if I remove the authenticationManagerBean, but i need authenticationManagerBean to inject AutthenticationManager in my app.

I know that I could use WebSecurityConfigurerAdapter and then add the bean going to override the method of this class. But as WebSecurityConfigurerAdapter is deprecated, I would not want to use it.

This problem arose from the fact that I have switched from WebSecurityConfigurerAdapter to SecurityFilterChain and am getting this error as I changed the config class.

I created this simple project which replicates the error of my main project. If you try to start the application it works, but the test doesn't. https://github.com/lako12/demo

The project is very simple, there are only 3 classes plus a test class.

@spring-projects-issues spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged or decided on label Sep 28, 2022
@wilkinsona
Copy link
Member

wilkinsona commented Sep 28, 2022

Thanks for the sample. It was very helpful in diagnosing the problem.

I believe that the root cause is a bug in AopTestUtils.getUltimateTargetObject in Spring Framework. When it's called with the authenticationManager bean it recurses until the stack overflows. This behavior can be reproduced with the following minimal test:

package com.example.demo;

import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.annotation.Bean;
import org.springframework.context.annotation.Configuration;
import org.springframework.security.authentication.AuthenticationManager;
import org.springframework.security.config.annotation.authentication.configuration.AuthenticationConfiguration;
import org.springframework.security.config.annotation.method.configuration.EnableGlobalMethodSecurity;
import org.springframework.test.context.junit.jupiter.SpringJUnitConfig;
import org.springframework.test.util.AopTestUtils;

@SpringJUnitConfig
class MinimalTests {

	@Test
	void test(@Autowired AuthenticationManager authenticationManager) {
		AopTestUtils.getUltimateTargetObject(authenticationManager);
	}


	@Configuration
	@EnableGlobalMethodSecurity(prePostEnabled = true)
	static class MinimalConfiguration {

		@Bean
		AuthenticationManager authenticationManagerBean(AuthenticationConfiguration authenticationConfiguration) throws Exception {
			return authenticationConfiguration.getAuthenticationManager();
		}
	}

}

And these dependencies (using Spring Boot 2.7.4 purely for dependency management):

<dependency>
	<groupId>org.springframework.security</groupId>
	<artifactId>spring-security-config</artifactId>
</dependency>
<dependency>
	<groupId>org.springframework</groupId>
	<artifactId>spring-test</artifactId>
	<scope>test</scope>
</dependency>
<dependency>
	<groupId>org.junit.jupiter</groupId>
	<artifactId>junit-jupiter</artifactId>
	<scope>test</scope>
</dependency>

We'll transfer this issue to the Spring Framework team so that they can investigate.

@wilkinsona wilkinsona changed the title I get stackOverflowError in spring security test AopTestUtils.getUltimateTargetObject recurses until the stack overflows when called with Spring Security's authentication manager bean Sep 28, 2022
@bclozel bclozel transferred this issue from spring-projects/spring-boot Sep 28, 2022
@sbrannen sbrannen added the in: test Issues in the test module label Sep 29, 2022
@sbrannen sbrannen self-assigned this Sep 29, 2022
@sbrannen sbrannen added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged or decided on labels Sep 29, 2022
@sbrannen sbrannen added this to the 5.3.24 milestone Sep 29, 2022
@sbrannen sbrannen changed the title AopTestUtils.getUltimateTargetObject recurses until the stack overflows when called with Spring Security's authentication manager bean AopTestUtils.getUltimateTargetObject results in stack overflow for proxy backed by LazyInitTargetSource Sep 30, 2022
@sbrannen
Copy link
Member

@lako12, thanks for bringing this to our attention!

@wilkinsona, thanks for analyzing it and providing the minimal reproducer!

It turns out that AopTestUtils.getUltimateTargetObject is indeed part of the issue, but... only when the bean passed to it is a Spring AOP proxy backed by a LazyInitTargetSource -- which is the case for Spring Security's AuthenticationManager (but only in certain scenarios resulting in AuthenticationConfiguration's lazyBean(AuthenticationManager.class) method being invoked).

If that scenario occurs, getUltimateTargetObject() ends up traversing an infinite object graph along the lines of: proxy -> Advised -> LazyInitTargetSource -> proxy ->Advised -> LazyInitTargetSource -> proxy -> ...

In light of that, my plan is to revise AopTestUtils.getUltimateTargetObject so that it returns the original object if the TargetSource is non-static (i.e., dynamic like LazyInitTargetSource). I've tested that locally; it resolves the issue; and I hope the change doesn't introduce adverse side effects for anyone intentionally invoking AopTestUtils.getUltimateTargetObject for other types of non-static target sources.

@marcusdacoregio
Copy link

marcusdacoregio commented Sep 30, 2022

Hi everyone.

Despite the AOP bug, the Spring Security team does not recommend exposing the AuthenticationManager bean by using AuthenticationConfiguration#getAuthenticationManager. It can lead to weird behaviors like StackOverflowException. Oftentimes, folks just want to use their UserDetailsService and PasswordEncoder to authenticate the user, with that said, you can create the AuthenticationManager like so:

@Bean
AuthenticationManager authenticationManager(UserDetailsService myUserDetailsService, PasswordEncoder encoder) {
    DaoAuthenticationProvider provider = new DaoAuthenticationProvider();
    provider.setUserDetailsService(myUserDetailsService);
    provider.setPasswordEncoder(encoder);
    return new ProviderManager(provider);
}

You can also rely on the AuthenticationFilter in order to create a custom authentication mechanism.

See:

@sbrannen
Copy link
Member

sbrannen commented Oct 7, 2022

Team Decision

After further consideration, we have decided not to make any change to AopTestUtils.getUltimateTargetObject() with regard to a non-static TargetSource check.

AopTestUtils.getUltimateTargetObject() is not designed to be applied to every singleton bean within an ApplicationContext. Rather, the methods in AopTestUtils are designed for end users who wish to unwrap a proxy within the scope of an individual test where they know that the object is a proxied bean, and we would like to retain that focus.

However, it does make sense to highlight the limitations of getUltimateTargetObject() with regard to a non-static TargetSource, and we will add a note to the Javadoc to raise awareness.

Regarding the use of AopTestUtils.getUltimateTargetObject() in SpringBootMockResolver, we suggest introducing a more defensive variant of that code in Spring Boot. For example, something along the lines of the following may work well.

public static <T> T getUltimateTargetObject(Object candidate) {
	Assert.notNull(candidate, "Candidate must not be null");
	try {
		if (AopUtils.isAopProxy(candidate) && candidate instanceof Advised advised) {
			TargetSource targetSource = advised.getTargetSource();
			if (targetSource.isStatic()) {
				Object target = targetSource.getTarget();
				if (target != null) {
					return (T) getUltimateTargetObject(target);
				}
			}
		}
	}
	catch (Throwable ex) {
		throw new IllegalStateException("Failed to unwrap proxied object", ex);
	}
	return (T) candidate;
}

In light of the above, we are closing this issue.

On the Boot side, I've created spring-projects/spring-boot#32632 as a follow up.

On the Framework side, I've created #29276 to address the documentation enhancement.

@Rapter1990
Copy link

Hi everyone.

Despite the AOP bug, the Spring Security team does not recommend exposing the AuthenticationManager bean by using AuthenticationConfiguration#getAuthenticationManager. It can lead to weird behaviors like StackOverflowException. Oftentimes, folks just want to use their UserDetailsService and PasswordEncoder to authenticate the user, with that said, you can create the AuthenticationManager like so:

@Bean
AuthenticationManager authenticationManager(UserDetailsService myUserDetailsService, PasswordEncoder encoder) {
    DaoAuthenticationProvider provider = new DaoAuthenticationProvider();
    provider.setUserDetailsService(myUserDetailsService);
    provider.setPasswordEncoder(encoder);
    return new ProviderManager(provider);
}

You can also rely on the AuthenticationFilter in order to create a custom authentication mechanism.

See:

@marcusdacoregio I tried to implement a spring boot microservice example. I have the same issue in my OrderServiceTest and OrderControllerTest in order service. I asked a question in stackoverflow. Here is the link : https://stackoverflow.com/questions/74633891/spring-boot-microservices-servicetest-and-controllertest-for-junit-throwing-ja
How can I do that?

@marcusdacoregio
Copy link

Hi @Rapter1990, please see this issue and contribute if necessary.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: test Issues in the test module status: declined A suggestion or change that we don't feel we should currently apply
Projects
None yet
Development

No branches or pull requests

6 participants