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

Improve handling of JDK dynamic proxies in DefaultAopProxyFactory. [SPR-12870] #17468

Closed
spring-issuemaster opened this issue Mar 31, 2015 · 7 comments

Comments

Projects
None yet
2 participants
@spring-issuemaster
Copy link
Collaborator

commented Mar 31, 2015

Thomas Darimont opened SPR-12870 and commented

Currently the AOP infrastructure cannot deal with of a JDK proxy target instance when asked to create a new cglib-proxy with proxyTargetClass=true which leads to exceptions such as:

Could not generate CGLIB subclass of class [class com.sun.proxy.$Proxy48]: Common causes of this problem include using a final class or a non-visible class; nested exception is java.lang.IllegalArgumentException: Cannot subclass final class class com.sun.proxy.$Proxy48

One common case where this happens is when Spring Security's
@EnableGlobalMethodSecurity(securedEnabled = true, prePostEnabled = true, proxyTargetClass = true) is used with pre/post security checks on Spring Data Repositories, e.g.:

public interface CustomerRepository extends JpaRepository<Customer, Long> {

	@PreAuthorize("hasRole('ADMIN')")
	List<Customer> findByLastnameLike(String lastname);
}

Affects: 4.1.6

Reference URL: https://github.com/thomasdarimont/spring-data-bugs/blob/master/spring-boot-example-double-proxy/src/main/java/demo/App.java

Referenced from: commits e1395a6, f9c2d1d

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 31, 2015

Thomas Darimont commented

Perhaps this is interesting for Rob Winch as well.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 31, 2015

Juergen Hoeller commented

DefaultAopProxyFactory falls back to a JdkDynamicAopProxy when encountering an existing JDK proxy as target class now.

Unfortunately, this also requires AopProxyUtils.completeProxiedInterfaces to explicitly consider JDK proxy classes now. Since this is a common algorithm used for any kind of proxy, it's probably better to restrict this change to 4.2 and let it go through the release candidate phase there.

Juergen

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 31, 2015

Thomas Darimont commented

Thanks for the quick fix!

I gave the latest 4.2.0.BUILD-SNAPSHOT a spin and now I see another exception.
It seems that the list of proxy interfaces now contains duplicates which lead to an IAE in the JDK ProxyClassFactory.

java.lang.reflect.InvocationTargetException
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(Unknown Source)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(Unknown Source)
        at java.lang.reflect.Method.invoke(Unknown Source)
        at org.springframework.boot.loader.MainMethodRunner.run(MainMethodRunner.java:53)
        at java.lang.Thread.run(Unknown Source)
Caused by: org.springframework.beans.factory.BeanCreationException: Error creating bean with name 'customerRepository': Post-processing of FactoryBean's singleton object failed; nested exception is java.lang.IllegalArgumentException: repeated interface: org.springframework.aop.SpringProxy
        at org.springframework.beans.factory.support.FactoryBeanRegistrySupport.getObjectFromFactoryBean(FactoryBeanRegistrySupport.java:116)
        at org.springframework.beans.factory.support.AbstractBeanFactory.getObjectForBeanInstance(AbstractBeanFactory.java:1546)
        at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:252)
        at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:218)
        at org.springframework.beans.factory.support.DefaultListableBeanFactory.getBean(DefaultListableBeanFactory.java:351)
        at org.springframework.beans.factory.support.DefaultListableBeanFactory.getBean(DefaultListableBeanFactory.java:332)
        at org.springframework.context.support.AbstractApplicationContext.getBean(AbstractApplicationContext.java:995)
        at demo.App.main(App.java:30)
        ... 6 more
Caused by: java.lang.IllegalArgumentException: repeated interface: org.springframework.aop.SpringProxy
        at java.lang.reflect.Proxy$ProxyClassFactory.apply(Unknown Source)
        at java.lang.reflect.Proxy$ProxyClassFactory.apply(Unknown Source)
        at java.lang.reflect.WeakCache$Factory.get(Unknown Source)
        at java.lang.reflect.WeakCache.get(Unknown Source)
        at java.lang.reflect.Proxy.getProxyClass0(Unknown Source)
        at java.lang.reflect.Proxy.newProxyInstance(Unknown Source)
        at org.springframework.aop.framework.JdkDynamicAopProxy.getProxy(JdkDynamicAopProxy.java:121)
        at org.springframework.aop.framework.ProxyFactory.getProxy(ProxyFactory.java:109)
        at org.springframework.aop.framework.autoproxy.AbstractAutoProxyCreator.createProxy(AbstractAutoProxyCreator.java:447)
        at org.springframework.aop.framework.autoproxy.AbstractAutoProxyCreator.wrapIfNecessary(AbstractAutoProxyCreator.java:333)
        at org.springframework.aop.framework.autoproxy.AbstractAutoProxyCreator.postProcessAfterInitialization(AbstractAutoProxyCreator.java:293)
        at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.applyBeanPostProcessorsAfterInitialization(AbstractAutowireCapableBeanFactory.java:422)
        at org.springframework.beans.factory.support.AbstractAutowireCapableBeanFactory.postProcessObjectFromFactoryBean(AbstractAutowireCapableBeanFactory.java:1723)
        at org.springframework.beans.factory.support.FactoryBeanRegistrySupport.getObjectFromFactoryBean(FactoryBeanRegistrySupport.java:113)
        ... 13 more
@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Mar 31, 2015

Juergen Hoeller commented

I see... That's just happening with a Spring proxy as target, I suppose. The unit tests only cover manually created proxies at this point.

To be fixed tomorrow :-)

Juergen

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 1, 2015

Thomas Darimont commented

I think thats a general problem in org.springframework.aop.framework.AopProxyUtils.completeProxiedInterfaces(AdvisedSupport).
It's probably not only SpringProxy but also Advised.

It think we need a guard around adding those marker interfaces that ensures that those interfaces are only added once.

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 1, 2015

Juergen Hoeller commented

There is an existing isInterfaceProxied check before they're getting added... However, the 'special' fallback code path for specified interfaces above isn't registering the interfaces with the Advised instance, so the isInterfaceProxied check fails to find them. We simply need to check the actual proxied interfaces there, one way or the other.

Juergen

@spring-issuemaster

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 2, 2015

Thomas Darimont commented

It seems that the commit fixed the issue.

Thank you :)
Thomas

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.