Skip to content

Commit b2f8753

Browse files
LeMikaelFLeMikaelF
committed
Fix regression with handling of UndeclaredThrowableException.
1 parent 5754002 commit b2f8753

File tree

5 files changed

+203
-63
lines changed

5 files changed

+203
-63
lines changed

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

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -761,27 +761,7 @@ public CglibMethodInvocation(Object proxy, @Nullable Object target, Method metho
761761
@Override
762762
@Nullable
763763
public Object proceed() throws Throwable {
764-
try {
765764
return super.proceed();
766-
}
767-
catch (RuntimeException ex) {
768-
throw ex;
769-
}
770-
catch (Exception ex) {
771-
if (ReflectionUtils.declaresException(getMethod(), ex.getClass()) ||
772-
KotlinDetector.isKotlinType(getMethod().getDeclaringClass())) {
773-
// Propagate original exception if declared on the target method
774-
// (with callers expecting it). Always propagate it for Kotlin code
775-
// since checked exceptions do not have to be explicitly declared there.
776-
throw ex;
777-
}
778-
else {
779-
// Checked exception thrown in the interceptor but not declared on the
780-
// target method signature -> apply an UndeclaredThrowableException,
781-
// aligned with standard JDK dynamic proxy behavior.
782-
throw new UndeclaredThrowableException(ex);
783-
}
784-
}
785765
}
786766
}
787767

‎spring-aop/src/test/java/org/springframework/aop/framework/ClassWithConstructor.java

Lines changed: 0 additions & 28 deletions
This file was deleted.

‎spring-aop/src/test/java/org/springframework/aop/framework/ProxyExceptionHandlingTests.java

Lines changed: 190 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,190 @@
1+
package org.springframework.aop.framework;
2+
3+
import static java.util.Objects.requireNonNull;
4+
import static org.mockito.Mockito.doAnswer;
5+
import static org.mockito.Mockito.doThrow;
6+
import static org.mockito.Mockito.mock;
7+
8+
import java.lang.reflect.Proxy;
9+
import java.lang.reflect.UndeclaredThrowableException;
10+
import java.util.Collection;
11+
import java.util.Set;
12+
13+
import org.aopalliance.aop.Advice;
14+
import org.aopalliance.intercept.MethodInterceptor;
15+
import org.assertj.core.api.WithAssertions;
16+
import org.junit.jupiter.api.BeforeEach;
17+
import org.junit.jupiter.api.Nested;
18+
import org.junit.jupiter.api.Test;
19+
import org.mockito.Mockito;
20+
import org.mockito.stubbing.Answer;
21+
import org.springframework.cglib.proxy.Enhancer;
22+
import org.springframework.lang.Nullable;
23+
24+
abstract class ProxyExceptionHandlingTests implements WithAssertions {
25+
26+
private static final RuntimeException uncheckedException = new RuntimeException();
27+
private static final DeclaredCheckedException declaredCheckedException = new DeclaredCheckedException();
28+
private static final UndeclaredCheckedException undeclaredCheckedException = new UndeclaredCheckedException();
29+
30+
protected final MyClass target = mock(MyClass.class);
31+
protected final ProxyFactory proxyFactory = new ProxyFactory(target);
32+
33+
@Nullable
34+
protected MyInterface proxy;
35+
@Nullable
36+
private Throwable throwableSeenByCaller;
37+
38+
static class ObjenesisCglibAopProxyTest extends ProxyExceptionHandlingTests {
39+
@BeforeEach
40+
void beforeEach() {
41+
proxyFactory.setProxyTargetClass(true);
42+
}
43+
44+
@Override
45+
protected void assertProxyType(Object proxy) {
46+
assertThat(Enhancer.isEnhanced(proxy.getClass())).isTrue();
47+
}
48+
}
49+
50+
static class JdkAopProxyTest extends ProxyExceptionHandlingTests {
51+
@Override
52+
protected void assertProxyType(Object proxy) {
53+
assertThat(Proxy.isProxyClass(proxy.getClass())).isTrue();
54+
}
55+
}
56+
57+
protected void assertProxyType(Object proxy) {};
58+
59+
@BeforeEach
60+
void beforeEach() {
61+
Mockito.clearInvocations(target);
62+
}
63+
64+
@Nested
65+
class WhenThereIsOneInterceptor {
66+
67+
@Nullable
68+
private Throwable throwableSeenByInterceptor;
69+
70+
@BeforeEach
71+
void beforeEach() {
72+
proxyFactory.addAdvice(captureThrowable());
73+
74+
proxy = (MyInterface) proxyFactory.getProxy();
75+
76+
assertProxyType(proxy);
77+
}
78+
79+
@Test
80+
void targetThrowsUndeclaredCheckedException() throws DeclaredCheckedException {
81+
doAnswer(sneakyThrow(undeclaredCheckedException)).when(target).doSomething();
82+
83+
invokeProxy();
84+
85+
assertThat(throwableSeenByInterceptor).isSameAs(undeclaredCheckedException);
86+
assertThat(throwableSeenByCaller)
87+
.isInstanceOf(UndeclaredThrowableException.class)
88+
.hasCauseReference(undeclaredCheckedException);
89+
}
90+
91+
@Test
92+
void targetThrowsDeclaredCheckedException() throws DeclaredCheckedException {
93+
doThrow(declaredCheckedException).when(target).doSomething();
94+
95+
invokeProxy();
96+
97+
assertThat(throwableSeenByInterceptor).isSameAs(declaredCheckedException);
98+
assertThat(throwableSeenByCaller).isSameAs(declaredCheckedException);
99+
}
100+
101+
@Test
102+
void targetThrowsUncheckedException() throws DeclaredCheckedException {
103+
doThrow(uncheckedException).when(target).doSomething();
104+
105+
invokeProxy();
106+
107+
assertThat(throwableSeenByInterceptor).isSameAs(uncheckedException);
108+
assertThat(throwableSeenByCaller).isSameAs(uncheckedException);
109+
}
110+
111+
private MethodInterceptor captureThrowable() {
112+
return invocation -> {
113+
try {
114+
return invocation.proceed();
115+
} catch (Exception e) {
116+
throwableSeenByInterceptor = e;
117+
throw e;
118+
}
119+
};
120+
}
121+
}
122+
123+
@Nested
124+
class WhenThereAreNoInterceptors {
125+
126+
@BeforeEach
127+
void beforeEach() {
128+
proxy = (MyInterface) proxyFactory.getProxy();
129+
130+
assertProxyType(proxy);
131+
}
132+
133+
@Test
134+
void targetThrowsUndeclaredCheckedException() throws DeclaredCheckedException {
135+
doAnswer(sneakyThrow(undeclaredCheckedException)).when(target).doSomething();
136+
137+
invokeProxy();
138+
139+
assertThat(throwableSeenByCaller)
140+
.isInstanceOf(UndeclaredThrowableException.class)
141+
.hasCauseReference(undeclaredCheckedException);
142+
}
143+
144+
@Test
145+
void targetThrowsDeclaredCheckedException() throws DeclaredCheckedException {
146+
doThrow(declaredCheckedException).when(target).doSomething();
147+
148+
invokeProxy();
149+
150+
assertThat(throwableSeenByCaller).isSameAs(declaredCheckedException);
151+
}
152+
153+
@Test
154+
void targetThrowsUncheckedException() throws DeclaredCheckedException {
155+
doThrow(uncheckedException).when(target).doSomething();
156+
157+
invokeProxy();
158+
159+
assertThat(throwableSeenByCaller).isSameAs(uncheckedException);
160+
}
161+
}
162+
163+
private void invokeProxy() {
164+
throwableSeenByCaller = catchThrowable(() -> requireNonNull(proxy).doSomething());
165+
}
166+
167+
private Answer<?> sneakyThrow(@SuppressWarnings("SameParameterValue") Throwable throwable) {
168+
return invocation -> {
169+
throw throwable;
170+
};
171+
}
172+
173+
static class MyClass implements MyInterface {
174+
@Override
175+
public void doSomething() throws DeclaredCheckedException {
176+
throw declaredCheckedException;
177+
}
178+
}
179+
180+
protected interface MyInterface {
181+
void doSomething() throws DeclaredCheckedException;
182+
}
183+
184+
protected static class UndeclaredCheckedException extends Exception {
185+
}
186+
187+
protected static class DeclaredCheckedException extends Exception {
188+
}
189+
190+
}

‎spring-core/src/main/java/org/springframework/cglib/core/ClassLoaderAwareGeneratorStrategy.java

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,10 @@
1616

1717
package org.springframework.cglib.core;
1818

19+
import java.lang.reflect.UndeclaredThrowableException;
20+
21+
import org.springframework.cglib.transform.impl.UndeclaredThrowableStrategy;
22+
1923
/**
2024
* CGLIB GeneratorStrategy variant which exposes the application ClassLoader
2125
* as current thread context ClassLoader for the time of class generation.
@@ -25,11 +29,12 @@
2529
* @author Juergen Hoeller
2630
* @since 5.2
2731
*/
28-
public class ClassLoaderAwareGeneratorStrategy extends DefaultGeneratorStrategy {
32+
public class ClassLoaderAwareGeneratorStrategy extends UndeclaredThrowableStrategy {
2933

3034
private final ClassLoader classLoader;
3135

3236
public ClassLoaderAwareGeneratorStrategy(ClassLoader classLoader) {
37+
super(UndeclaredThrowableException.class);
3338
this.classLoader = classLoader;
3439
}
3540

‎spring-core/src/main/java/org/springframework/cglib/transform/impl/UndeclaredThrowableTransformer.java

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
import org.springframework.cglib.core.TypeUtils;
2828
import org.springframework.cglib.transform.ClassEmitterTransformer;
2929

30-
@SuppressWarnings({"rawtypes", "unchecked"})
30+
@SuppressWarnings({"rawtypes" })
3131
public class UndeclaredThrowableTransformer extends ClassEmitterTransformer {
3232

3333
private final Type wrapper;
@@ -55,24 +55,17 @@ public CodeEmitter begin_method(int access, final Signature sig, final Type[] ex
5555
return e;
5656
}
5757
return new CodeEmitter(e) {
58-
private Block handler;
59-
private boolean isConstructor;
60-
private boolean fixedConstructorHandler;
61-
62-
{
63-
if (Constants.CONSTRUCTOR_NAME.equals(sig.getName())) {
64-
isConstructor = true;
65-
}
66-
handler = begin_block();
67-
}
58+
private final boolean isConstructor = Constants.CONSTRUCTOR_NAME.equals(sig.getName());
59+
private Block handler = begin_block();
60+
private boolean calToSuperWasSeen;
6861

6962
@Override
7063
public void visitMethodInsn(int opcode, String owner, String name, String descriptor, boolean isInterface) {
7164
super.visitMethodInsn(opcode, owner, name, descriptor, isInterface);
72-
// fix try catch block start label in constructor. #189
73-
if (isConstructor && !fixedConstructorHandler && Constants.CONSTRUCTOR_NAME.equals(name)) {
65+
if (isConstructor && !calToSuperWasSeen && Constants.CONSTRUCTOR_NAME.equals(name)) {
66+
// we start the entry in the exception table after the call to super
7467
handler = begin_block();
75-
fixedConstructorHandler = true;
68+
calToSuperWasSeen = true;
7669
}
7770
}
7871

0 commit comments

Comments
 (0)