diff --git a/spring-aop/src/main/java/org/springframework/aop/framework/CglibAopProxy.java b/spring-aop/src/main/java/org/springframework/aop/framework/CglibAopProxy.java index 90e7080fd9c..f60b5d3711c 100644 --- a/spring-aop/src/main/java/org/springframework/aop/framework/CglibAopProxy.java +++ b/spring-aop/src/main/java/org/springframework/aop/framework/CglibAopProxy.java @@ -172,7 +172,7 @@ class CglibAopProxy implements AopProxy, Serializable { } // Validate the class, writing log messages as necessary. - validateClassIfNecessary(proxySuperClass); + validateClassIfNecessary(proxySuperClass, classLoader); // Configure CGLIB Enhancer... Enhancer enhancer = createEnhancer(); @@ -239,11 +239,11 @@ class CglibAopProxy implements AopProxy, Serializable { * Checks to see whether the supplied {@code Class} has already been validated and * validates it if not. */ - private void validateClassIfNecessary(Class proxySuperClass) { - if (logger.isWarnEnabled()) { + private void validateClassIfNecessary(Class proxySuperClass, ClassLoader proxyClassLoader) { + if (logger.isInfoEnabled()) { synchronized (validatedClasses) { if (!validatedClasses.containsKey(proxySuperClass)) { - doValidateClass(proxySuperClass); + doValidateClass(proxySuperClass, proxyClassLoader); validatedClasses.put(proxySuperClass, Boolean.TRUE); } } @@ -251,19 +251,28 @@ class CglibAopProxy implements AopProxy, Serializable { } /** - * Checks for final methods on the {@code Class} and writes warnings to the log - * for each one found. + * Checks for final methods on the given {@code Class}, as well as package-visible + * methods across ClassLoaders, and writes warnings to the log for each one found. */ - private void doValidateClass(Class proxySuperClass) { - if (logger.isWarnEnabled()) { - Method[] methods = proxySuperClass.getMethods(); + private void doValidateClass(Class proxySuperClass, ClassLoader proxyClassLoader) { + if (!Object.class.equals(proxySuperClass)) { + Method[] methods = proxySuperClass.getDeclaredMethods(); for (Method method : methods) { - if (!Object.class.equals(method.getDeclaringClass()) && !Modifier.isStatic(method.getModifiers()) && - Modifier.isFinal(method.getModifiers())) { - logger.warn("Unable to proxy method [" + method + "] because it is final: " + - "All calls to this method via a proxy will be routed directly to the proxy."); + int mod = method.getModifiers(); + if (!Modifier.isStatic(mod)) { + if (Modifier.isFinal(mod)) { + logger.info("Unable to proxy method [" + method + "] because it is final: " + + "All calls to this method via a proxy will NOT be routed to the target instance."); + } + else if (!Modifier.isPublic(mod) && !Modifier.isProtected(mod) && !Modifier.isPrivate(mod) && + proxyClassLoader != null && proxySuperClass.getClassLoader() != proxyClassLoader) { + logger.info("Unable to proxy method [" + method + "] because it is package-visible " + + "across different ClassLoaders: All calls to this method via a proxy will " + + "NOT be routed to the target instance."); + } } } + doValidateClass(proxySuperClass.getSuperclass(), proxyClassLoader); } } @@ -604,7 +613,7 @@ class CglibAopProxy implements AopProxy, Serializable { */ private static class DynamicAdvisedInterceptor implements MethodInterceptor, Serializable { - private AdvisedSupport advised; + private final AdvisedSupport advised; public DynamicAdvisedInterceptor(AdvisedSupport advised) { this.advised = advised; @@ -622,8 +631,8 @@ class CglibAopProxy implements AopProxy, Serializable { oldProxy = AopContext.setCurrentProxy(proxy); setProxyContext = true; } - // May be null Get as late as possible to minimize the time we - // "own" the target, in case it comes from a pool. + // May be null. Get as late as possible to minimize the time we + // "own" the target, in case it comes from a pool... target = getTarget(); if (target != null) { targetClass = target.getClass(); @@ -689,13 +698,13 @@ class CglibAopProxy implements AopProxy, Serializable { private final MethodProxy methodProxy; - private boolean protectedMethod; + private final boolean publicMethod; public CglibMethodInvocation(Object proxy, Object target, Method method, Object[] arguments, Class targetClass, List interceptorsAndDynamicMethodMatchers, MethodProxy methodProxy) { super(proxy, target, method, arguments, targetClass, interceptorsAndDynamicMethodMatchers); this.methodProxy = methodProxy; - this.protectedMethod = Modifier.isProtected(method.getModifiers()); + this.publicMethod = Modifier.isPublic(method.getModifiers()); } /** @@ -704,11 +713,11 @@ class CglibAopProxy implements AopProxy, Serializable { */ @Override protected Object invokeJoinpoint() throws Throwable { - if (this.protectedMethod) { - return super.invokeJoinpoint(); + if (this.publicMethod) { + return this.methodProxy.invoke(this.target, this.arguments); } else { - return this.methodProxy.invoke(this.target, this.arguments); + return super.invokeJoinpoint(); } } } @@ -829,8 +838,8 @@ class CglibAopProxy implements AopProxy, Serializable { // of the target type. If so we know it never needs to have return type // massage and can use a dispatcher. // If the proxy is being exposed, then must use the interceptor the - // correct one is already configured. If the target is not static cannot - // use a Dispatcher because the target can not then be released. + // correct one is already configured. If the target is not static, then + // cannot use a dispatcher because the target cannot be released. if (exposeProxy || !isStatic) { return INVOKE_TARGET; } diff --git a/spring-context/src/test/java/org/springframework/aop/framework/CglibProxyTests.java b/spring-context/src/test/java/org/springframework/aop/framework/CglibProxyTests.java index 8ef8305d9e0..af22633fbd4 100644 --- a/spring-context/src/test/java/org/springframework/aop/framework/CglibProxyTests.java +++ b/spring-context/src/test/java/org/springframework/aop/framework/CglibProxyTests.java @@ -102,6 +102,7 @@ public final class CglibProxyTests extends AbstractAopProxyTests implements Seri @Test public void testProtectedMethodInvocation() { ProtectedMethodTestBean bean = new ProtectedMethodTestBean(); + bean.value = "foo"; mockTargetSource.setTarget(bean); AdvisedSupport as = new AdvisedSupport(new Class[]{}); @@ -109,8 +110,47 @@ public final class CglibProxyTests extends AbstractAopProxyTests implements Seri as.addAdvice(new NopInterceptor()); AopProxy aop = new CglibAopProxy(as); - Object proxy = aop.getProxy(); + ProtectedMethodTestBean proxy = (ProtectedMethodTestBean) aop.getProxy(); assertTrue(AopUtils.isCglibProxy(proxy)); + assertEquals(proxy.getClass().getClassLoader(), bean.getClass().getClassLoader()); + assertEquals("foo", proxy.getString()); + } + + @Test + public void testPackageMethodInvocation() { + PackageMethodTestBean bean = new PackageMethodTestBean(); + bean.value = "foo"; + mockTargetSource.setTarget(bean); + + AdvisedSupport as = new AdvisedSupport(new Class[]{}); + as.setTargetSource(mockTargetSource); + as.addAdvice(new NopInterceptor()); + AopProxy aop = new CglibAopProxy(as); + + PackageMethodTestBean proxy = (PackageMethodTestBean) aop.getProxy(); + assertTrue(AopUtils.isCglibProxy(proxy)); + assertEquals(proxy.getClass().getClassLoader(), bean.getClass().getClassLoader()); + assertEquals("foo", proxy.getString()); + } + + @Test + public void testPackageMethodInvocationWithDifferentClassLoader() { + ClassLoader child = new ClassLoader(getClass().getClassLoader()) { + }; + + PackageMethodTestBean bean = new PackageMethodTestBean(); + bean.value = "foo"; + mockTargetSource.setTarget(bean); + + AdvisedSupport as = new AdvisedSupport(new Class[]{}); + as.setTargetSource(mockTargetSource); + as.addAdvice(new NopInterceptor()); + AopProxy aop = new CglibAopProxy(as); + + PackageMethodTestBean proxy = (PackageMethodTestBean) aop.getProxy(child); + assertTrue(AopUtils.isCglibProxy(proxy)); + assertNotEquals(proxy.getClass().getClassLoader(), bean.getClass().getClassLoader()); + assertNull(proxy.getString()); // we're stuck in the proxy instance } @Test @@ -410,54 +450,59 @@ public final class CglibProxyTests extends AbstractAopProxyTests implements Seri } - public static class HasFinalMethod { + public static class NoArgCtorTestBean { + + private boolean called = false; - public final void foo() { + public NoArgCtorTestBean(String x, int y) { + called = true; + } + + public boolean wasCalled() { + return called; + } + + public void reset() { + called = false; } } -} -class CglibTestBean { + public static class ProtectedMethodTestBean { - private String name; + public String value; - public CglibTestBean() { - setName("Some Default"); + protected String getString() { + return this.value; + } } - public void setName(String name) { - this.name = name; - } - public String getName() { - return this.name; + public static class PackageMethodTestBean { + + public String value; + + String getString() { + return this.value; + } } } -class NoArgCtorTestBean { - - private boolean called = false; +class CglibTestBean { - public NoArgCtorTestBean(String x, int y) { - called = true; - } + private String name; - public boolean wasCalled() { - return called; + public CglibTestBean() { + setName("Some Default"); } - public void reset() { - called = false; + public void setName(String name) { + this.name = name; } -} - -class ProtectedMethodTestBean { - - protected String getString() { - return "foo"; + public String getName() { + return this.name; } }