Skip to content

Commit 5615838

Browse files
LeMikaelFhengyunabc
LeMikaelF
and
hengyunabc
authored
Consistent handling of undeclared checked exceptions in CGLIB proxies (#32469)
Co-authored-by: hengyunabc <[email protected]> Co-authored-by: Mikaël Francoeur <[email protected]>
1 parent bed3689 commit 5615838

File tree

5 files changed

+232
-56
lines changed

5 files changed

+232
-56
lines changed

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

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import java.io.Serializable;
2020
import java.lang.reflect.Method;
2121
import java.lang.reflect.Modifier;
22-
import java.lang.reflect.UndeclaredThrowableException;
2322
import java.util.ArrayList;
2423
import java.util.Collections;
2524
import java.util.List;
@@ -54,7 +53,6 @@
5453
import org.springframework.util.ClassUtils;
5554
import org.springframework.util.CollectionUtils;
5655
import org.springframework.util.ObjectUtils;
57-
import org.springframework.util.ReflectionUtils;
5856

5957
/**
6058
* CGLIB-based {@link AopProxy} implementation for the Spring AOP framework.
@@ -764,27 +762,7 @@ public CglibMethodInvocation(Object proxy, @Nullable Object target, Method metho
764762
@Override
765763
@Nullable
766764
public Object proceed() throws Throwable {
767-
try {
768765
return super.proceed();
769-
}
770-
catch (RuntimeException ex) {
771-
throw ex;
772-
}
773-
catch (Exception ex) {
774-
if (ReflectionUtils.declaresException(getMethod(), ex.getClass()) ||
775-
KotlinDetector.isKotlinType(getMethod().getDeclaringClass())) {
776-
// Propagate original exception if declared on the target method
777-
// (with callers expecting it). Always propagate it for Kotlin code
778-
// since checked exceptions do not have to be explicitly declared there.
779-
throw ex;
780-
}
781-
else {
782-
// Checked exception thrown in the interceptor but not declared on the
783-
// target method signature -> apply an UndeclaredThrowableException,
784-
// aligned with standard JDK dynamic proxy behavior.
785-
throw new UndeclaredThrowableException(ex);
786-
}
787-
}
788766
}
789767
}
790768

‎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: 211 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,211 @@
1+
/*
2+
* Copyright 2002-2024 the original author or authors.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* https://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
package org.springframework.aop.framework;
18+
19+
import java.io.Serial;
20+
import java.lang.reflect.Proxy;
21+
import java.lang.reflect.UndeclaredThrowableException;
22+
import java.util.Objects;
23+
24+
import org.aopalliance.intercept.MethodInterceptor;
25+
import org.assertj.core.api.WithAssertions;
26+
import org.junit.jupiter.api.BeforeEach;
27+
import org.junit.jupiter.api.Nested;
28+
import org.junit.jupiter.api.Test;
29+
import org.mockito.Mockito;
30+
import org.mockito.stubbing.Answer;
31+
32+
import org.springframework.cglib.proxy.Enhancer;
33+
import org.springframework.lang.Nullable;
34+
35+
import static org.mockito.BDDMockito.doAnswer;
36+
import static org.mockito.BDDMockito.doThrow;
37+
import static org.mockito.BDDMockito.mock;
38+
39+
abstract class ProxyExceptionHandlingTests implements WithAssertions {
40+
41+
private static final RuntimeException uncheckedException = new RuntimeException();
42+
private static final DeclaredCheckedException declaredCheckedException = new DeclaredCheckedException();
43+
private static final UndeclaredCheckedException undeclaredCheckedException = new UndeclaredCheckedException();
44+
45+
protected final MyClass target = mock(MyClass.class);
46+
protected final ProxyFactory proxyFactory = new ProxyFactory(target);
47+
48+
@Nullable
49+
protected MyInterface proxy;
50+
@Nullable
51+
private Throwable throwableSeenByCaller;
52+
53+
static class ObjenesisCglibAopProxyTests extends ProxyExceptionHandlingTests {
54+
@BeforeEach
55+
void beforeEach() {
56+
proxyFactory.setProxyTargetClass(true);
57+
}
58+
59+
@Override
60+
protected void assertProxyType(Object proxy) {
61+
assertThat(Enhancer.isEnhanced(proxy.getClass())).isTrue();
62+
}
63+
}
64+
65+
static class JdkAopProxyTests extends ProxyExceptionHandlingTests {
66+
@Override
67+
protected void assertProxyType(Object proxy) {
68+
assertThat(Proxy.isProxyClass(proxy.getClass())).isTrue();
69+
}
70+
}
71+
72+
protected void assertProxyType(Object proxy) {
73+
}
74+
75+
@BeforeEach
76+
void beforeEach() {
77+
Mockito.clearInvocations(target);
78+
}
79+
80+
@Nested
81+
class WhenThereIsOneInterceptor {
82+
83+
@Nullable
84+
private Throwable throwableSeenByInterceptor;
85+
86+
@BeforeEach
87+
void beforeEach() {
88+
proxyFactory.addAdvice(captureThrowable());
89+
90+
proxy = (MyInterface) proxyFactory.getProxy();
91+
92+
assertProxyType(proxy);
93+
}
94+
95+
@Test
96+
void targetThrowsUndeclaredCheckedException() throws DeclaredCheckedException {
97+
doAnswer(sneakyThrow(undeclaredCheckedException)).when(target).doSomething();
98+
99+
invokeProxy();
100+
101+
assertThat(throwableSeenByInterceptor).isSameAs(undeclaredCheckedException);
102+
assertThat(throwableSeenByCaller)
103+
.isInstanceOf(UndeclaredThrowableException.class)
104+
.hasCauseReference(undeclaredCheckedException);
105+
}
106+
107+
@Test
108+
void targetThrowsDeclaredCheckedException() throws DeclaredCheckedException {
109+
doThrow(declaredCheckedException).when(target).doSomething();
110+
111+
invokeProxy();
112+
113+
assertThat(throwableSeenByInterceptor).isSameAs(declaredCheckedException);
114+
assertThat(throwableSeenByCaller).isSameAs(declaredCheckedException);
115+
}
116+
117+
@Test
118+
void targetThrowsUncheckedException() throws DeclaredCheckedException {
119+
doThrow(uncheckedException).when(target).doSomething();
120+
121+
invokeProxy();
122+
123+
assertThat(throwableSeenByInterceptor).isSameAs(uncheckedException);
124+
assertThat(throwableSeenByCaller).isSameAs(uncheckedException);
125+
}
126+
127+
private MethodInterceptor captureThrowable() {
128+
return invocation -> {
129+
try {
130+
return invocation.proceed();
131+
}
132+
catch (Exception e) {
133+
throwableSeenByInterceptor = e;
134+
throw e;
135+
}
136+
};
137+
}
138+
}
139+
140+
@Nested
141+
class WhenThereAreNoInterceptors {
142+
143+
@BeforeEach
144+
void beforeEach() {
145+
proxy = (MyInterface) proxyFactory.getProxy();
146+
147+
assertProxyType(proxy);
148+
}
149+
150+
@Test
151+
void targetThrowsUndeclaredCheckedException() throws DeclaredCheckedException {
152+
doAnswer(sneakyThrow(undeclaredCheckedException)).when(target).doSomething();
153+
154+
invokeProxy();
155+
156+
assertThat(throwableSeenByCaller)
157+
.isInstanceOf(UndeclaredThrowableException.class)
158+
.hasCauseReference(undeclaredCheckedException);
159+
}
160+
161+
@Test
162+
void targetThrowsDeclaredCheckedException() throws DeclaredCheckedException {
163+
doThrow(declaredCheckedException).when(target).doSomething();
164+
165+
invokeProxy();
166+
167+
assertThat(throwableSeenByCaller).isSameAs(declaredCheckedException);
168+
}
169+
170+
@Test
171+
void targetThrowsUncheckedException() throws DeclaredCheckedException {
172+
doThrow(uncheckedException).when(target).doSomething();
173+
174+
invokeProxy();
175+
176+
assertThat(throwableSeenByCaller).isSameAs(uncheckedException);
177+
}
178+
}
179+
180+
private void invokeProxy() {
181+
throwableSeenByCaller = catchThrowable(() -> Objects.requireNonNull(proxy).doSomething());
182+
}
183+
184+
private Answer<?> sneakyThrow(@SuppressWarnings("SameParameterValue") Throwable throwable) {
185+
return invocation -> {
186+
throw throwable;
187+
};
188+
}
189+
190+
static class MyClass implements MyInterface {
191+
@Override
192+
public void doSomething() throws DeclaredCheckedException {
193+
throw declaredCheckedException;
194+
}
195+
}
196+
197+
protected interface MyInterface {
198+
void doSomething() throws DeclaredCheckedException;
199+
}
200+
201+
protected static class UndeclaredCheckedException extends Exception {
202+
@Serial
203+
private static final long serialVersionUID = -4199611059122356651L;
204+
}
205+
206+
protected static class DeclaredCheckedException extends Exception {
207+
@Serial
208+
private static final long serialVersionUID = 8851375818059230518L;
209+
}
210+
211+
}

‎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: 15 additions & 5 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,10 +55,20 @@ 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-
/* init */ {
60-
handler = begin_block();
61-
}
58+
private final boolean isConstructor = Constants.CONSTRUCTOR_NAME.equals(sig.getName());
59+
private Block handler = begin_block();
60+
private boolean calToSuperWasSeen;
61+
62+
@Override
63+
public void visitMethodInsn(int opcode, String owner, String name, String descriptor, boolean isInterface) {
64+
super.visitMethodInsn(opcode, owner, name, descriptor, isInterface);
65+
if (isConstructor && !calToSuperWasSeen && Constants.CONSTRUCTOR_NAME.equals(name)) {
66+
// we start the entry in the exception table after the call to super
67+
handler = begin_block();
68+
calToSuperWasSeen = true;
69+
}
70+
}
71+
6272
@Override
6373
public void visitMaxs(int maxStack, int maxLocals) {
6474
handler.end();

0 commit comments

Comments
 (0)