Skip to content

Commit d9cb445

Browse files
committed
Backported tests for package-visible methods with CGLIB proxies
Issue: SPR-11618 (cherry picked from commit 90309ab)
1 parent 8e44521 commit d9cb445

File tree

2 files changed

+85
-40
lines changed

2 files changed

+85
-40
lines changed

spring-aop/src/main/java/org/springframework/aop/framework/CglibAopProxy.java

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -261,7 +261,7 @@ private void doValidateClass(Class<?> proxySuperClass) {
261261
if (!Object.class.equals(method.getDeclaringClass()) && !Modifier.isStatic(method.getModifiers()) &&
262262
Modifier.isFinal(method.getModifiers())) {
263263
logger.warn("Unable to proxy method [" + method + "] because it is final: " +
264-
"All calls to this method via a proxy will be routed directly to the proxy.");
264+
"All calls to this method via a proxy will NOT be routed to the target instance.");
265265
}
266266
}
267267
}
@@ -604,7 +604,7 @@ public Object intercept(Object proxy, Method method, Object[] args, MethodProxy
604604
*/
605605
private static class DynamicAdvisedInterceptor implements MethodInterceptor, Serializable {
606606

607-
private AdvisedSupport advised;
607+
private final AdvisedSupport advised;
608608

609609
public DynamicAdvisedInterceptor(AdvisedSupport advised) {
610610
this.advised = advised;
@@ -622,8 +622,8 @@ public Object intercept(Object proxy, Method method, Object[] args, MethodProxy
622622
oldProxy = AopContext.setCurrentProxy(proxy);
623623
setProxyContext = true;
624624
}
625-
// May be null Get as late as possible to minimize the time we
626-
// "own" the target, in case it comes from a pool.
625+
// May be null. Get as late as possible to minimize the time we
626+
// "own" the target, in case it comes from a pool...
627627
target = getTarget();
628628
if (target != null) {
629629
targetClass = target.getClass();
@@ -689,13 +689,13 @@ private static class CglibMethodInvocation extends ReflectiveMethodInvocation {
689689

690690
private final MethodProxy methodProxy;
691691

692-
private boolean protectedMethod;
692+
private final boolean publicMethod;
693693

694694
public CglibMethodInvocation(Object proxy, Object target, Method method, Object[] arguments,
695695
Class<?> targetClass, List<Object> interceptorsAndDynamicMethodMatchers, MethodProxy methodProxy) {
696696
super(proxy, target, method, arguments, targetClass, interceptorsAndDynamicMethodMatchers);
697697
this.methodProxy = methodProxy;
698-
this.protectedMethod = Modifier.isProtected(method.getModifiers());
698+
this.publicMethod = Modifier.isPublic(method.getModifiers());
699699
}
700700

701701
/**
@@ -704,11 +704,11 @@ public CglibMethodInvocation(Object proxy, Object target, Method method, Object[
704704
*/
705705
@Override
706706
protected Object invokeJoinpoint() throws Throwable {
707-
if (this.protectedMethod) {
708-
return super.invokeJoinpoint();
707+
if (this.publicMethod) {
708+
return this.methodProxy.invoke(this.target, this.arguments);
709709
}
710710
else {
711-
return this.methodProxy.invoke(this.target, this.arguments);
711+
return super.invokeJoinpoint();
712712
}
713713
}
714714
}
@@ -829,8 +829,8 @@ public int accept(Method method) {
829829
// of the target type. If so we know it never needs to have return type
830830
// massage and can use a dispatcher.
831831
// If the proxy is being exposed, then must use the interceptor the
832-
// correct one is already configured. If the target is not static cannot
833-
// use a Dispatcher because the target can not then be released.
832+
// correct one is already configured. If the target is not static, then
833+
// cannot use a dispatcher because the target cannot be released.
834834
if (exposeProxy || !isStatic) {
835835
return INVOKE_TARGET;
836836
}

spring-context/src/test/java/org/springframework/aop/framework/CglibProxyTests.java

Lines changed: 74 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -102,15 +102,55 @@ public void testNoTarget() {
102102
@Test
103103
public void testProtectedMethodInvocation() {
104104
ProtectedMethodTestBean bean = new ProtectedMethodTestBean();
105+
bean.value = "foo";
105106
mockTargetSource.setTarget(bean);
106107

107108
AdvisedSupport as = new AdvisedSupport(new Class<?>[]{});
108109
as.setTargetSource(mockTargetSource);
109110
as.addAdvice(new NopInterceptor());
110111
AopProxy aop = new CglibAopProxy(as);
111112

112-
Object proxy = aop.getProxy();
113+
ProtectedMethodTestBean proxy = (ProtectedMethodTestBean) aop.getProxy();
113114
assertTrue(AopUtils.isCglibProxy(proxy));
115+
assertEquals(proxy.getClass().getClassLoader(), bean.getClass().getClassLoader());
116+
assertEquals("foo", proxy.getString());
117+
}
118+
119+
@Test
120+
public void testPackageMethodInvocation() {
121+
PackageMethodTestBean bean = new PackageMethodTestBean();
122+
bean.value = "foo";
123+
mockTargetSource.setTarget(bean);
124+
125+
AdvisedSupport as = new AdvisedSupport(new Class<?>[]{});
126+
as.setTargetSource(mockTargetSource);
127+
as.addAdvice(new NopInterceptor());
128+
AopProxy aop = new CglibAopProxy(as);
129+
130+
PackageMethodTestBean proxy = (PackageMethodTestBean) aop.getProxy();
131+
assertTrue(AopUtils.isCglibProxy(proxy));
132+
assertEquals(proxy.getClass().getClassLoader(), bean.getClass().getClassLoader());
133+
assertEquals("foo", proxy.getString());
134+
}
135+
136+
@Test
137+
public void testPackageMethodInvocationWithDifferentClassLoader() {
138+
ClassLoader child = new ClassLoader(getClass().getClassLoader()) {
139+
};
140+
141+
PackageMethodTestBean bean = new PackageMethodTestBean();
142+
bean.value = "foo";
143+
mockTargetSource.setTarget(bean);
144+
145+
AdvisedSupport as = new AdvisedSupport(new Class<?>[]{});
146+
as.setTargetSource(mockTargetSource);
147+
as.addAdvice(new NopInterceptor());
148+
AopProxy aop = new CglibAopProxy(as);
149+
150+
PackageMethodTestBean proxy = (PackageMethodTestBean) aop.getProxy(child);
151+
assertTrue(AopUtils.isCglibProxy(proxy));
152+
assertNotEquals(proxy.getClass().getClassLoader(), bean.getClass().getClassLoader());
153+
assertNull(proxy.getString()); // we're stuck in the proxy instance
114154
}
115155

116156
@Test
@@ -410,54 +450,59 @@ public void doTest() throws Exception {
410450
}
411451

412452

413-
public static class HasFinalMethod {
453+
public static class NoArgCtorTestBean {
454+
455+
private boolean called = false;
414456

415-
public final void foo() {
457+
public NoArgCtorTestBean(String x, int y) {
458+
called = true;
459+
}
460+
461+
public boolean wasCalled() {
462+
return called;
463+
}
464+
465+
public void reset() {
466+
called = false;
416467
}
417468
}
418-
}
419469

420470

421-
class CglibTestBean {
471+
public static class ProtectedMethodTestBean {
422472

423-
private String name;
473+
public String value;
424474

425-
public CglibTestBean() {
426-
setName("Some Default");
475+
protected String getString() {
476+
return this.value;
477+
}
427478
}
428479

429-
public void setName(String name) {
430-
this.name = name;
431-
}
432480

433-
public String getName() {
434-
return this.name;
481+
public static class PackageMethodTestBean {
482+
483+
public String value;
484+
485+
String getString() {
486+
return this.value;
487+
}
435488
}
436489
}
437490

438491

439-
class NoArgCtorTestBean {
440-
441-
private boolean called = false;
492+
class CglibTestBean {
442493

443-
public NoArgCtorTestBean(String x, int y) {
444-
called = true;
445-
}
494+
private String name;
446495

447-
public boolean wasCalled() {
448-
return called;
496+
public CglibTestBean() {
497+
setName("Some Default");
449498
}
450499

451-
public void reset() {
452-
called = false;
500+
public void setName(String name) {
501+
this.name = name;
453502
}
454-
}
455-
456503

457-
class ProtectedMethodTestBean {
458-
459-
protected String getString() {
460-
return "foo";
504+
public String getName() {
505+
return this.name;
461506
}
462507
}
463508

0 commit comments

Comments
 (0)