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 b227b90d19..dbf6bea1c0 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 @@ -19,7 +19,6 @@ package org.springframework.aop.framework; import java.io.Serializable; import java.lang.reflect.Method; import java.lang.reflect.Modifier; -import java.lang.reflect.UndeclaredThrowableException; import java.util.ArrayList; import java.util.Collections; import java.util.List; @@ -54,7 +53,6 @@ import org.springframework.util.Assert; import org.springframework.util.ClassUtils; import org.springframework.util.CollectionUtils; import org.springframework.util.ObjectUtils; -import org.springframework.util.ReflectionUtils; /** * CGLIB-based {@link AopProxy} implementation for the Spring AOP framework. @@ -764,27 +762,7 @@ class CglibAopProxy implements AopProxy, Serializable { @Override @Nullable public Object proceed() throws Throwable { - try { return super.proceed(); - } - catch (RuntimeException ex) { - throw ex; - } - catch (Exception ex) { - if (ReflectionUtils.declaresException(getMethod(), ex.getClass()) || - KotlinDetector.isKotlinType(getMethod().getDeclaringClass())) { - // Propagate original exception if declared on the target method - // (with callers expecting it). Always propagate it for Kotlin code - // since checked exceptions do not have to be explicitly declared there. - throw ex; - } - else { - // Checked exception thrown in the interceptor but not declared on the - // target method signature -> apply an UndeclaredThrowableException, - // aligned with standard JDK dynamic proxy behavior. - throw new UndeclaredThrowableException(ex); - } - } } } diff --git a/spring-aop/src/test/java/org/springframework/aop/framework/ClassWithConstructor.java b/spring-aop/src/test/java/org/springframework/aop/framework/ClassWithConstructor.java deleted file mode 100644 index dfbd090e8d..0000000000 --- a/spring-aop/src/test/java/org/springframework/aop/framework/ClassWithConstructor.java +++ /dev/null @@ -1,28 +0,0 @@ -/* - * Copyright 2002-2018 the original author or authors. - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * https://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.springframework.aop.framework; - -public class ClassWithConstructor { - - public ClassWithConstructor(Object object) { - - } - - public void method() { - - } -} diff --git a/spring-aop/src/test/java/org/springframework/aop/framework/ProxyExceptionHandlingTests.java b/spring-aop/src/test/java/org/springframework/aop/framework/ProxyExceptionHandlingTests.java new file mode 100644 index 0000000000..ca9a609ea6 --- /dev/null +++ b/spring-aop/src/test/java/org/springframework/aop/framework/ProxyExceptionHandlingTests.java @@ -0,0 +1,211 @@ +/* + * Copyright 2002-2024 the original author or authors. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.springframework.aop.framework; + +import java.io.Serial; +import java.lang.reflect.Proxy; +import java.lang.reflect.UndeclaredThrowableException; +import java.util.Objects; + +import org.aopalliance.intercept.MethodInterceptor; +import org.assertj.core.api.WithAssertions; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.mockito.Mockito; +import org.mockito.stubbing.Answer; + +import org.springframework.cglib.proxy.Enhancer; +import org.springframework.lang.Nullable; + +import static org.mockito.BDDMockito.doAnswer; +import static org.mockito.BDDMockito.doThrow; +import static org.mockito.BDDMockito.mock; + +abstract class ProxyExceptionHandlingTests implements WithAssertions { + + private static final RuntimeException uncheckedException = new RuntimeException(); + private static final DeclaredCheckedException declaredCheckedException = new DeclaredCheckedException(); + private static final UndeclaredCheckedException undeclaredCheckedException = new UndeclaredCheckedException(); + + protected final MyClass target = mock(MyClass.class); + protected final ProxyFactory proxyFactory = new ProxyFactory(target); + + @Nullable + protected MyInterface proxy; + @Nullable + private Throwable throwableSeenByCaller; + + static class ObjenesisCglibAopProxyTests extends ProxyExceptionHandlingTests { + @BeforeEach + void beforeEach() { + proxyFactory.setProxyTargetClass(true); + } + + @Override + protected void assertProxyType(Object proxy) { + assertThat(Enhancer.isEnhanced(proxy.getClass())).isTrue(); + } + } + + static class JdkAopProxyTests extends ProxyExceptionHandlingTests { + @Override + protected void assertProxyType(Object proxy) { + assertThat(Proxy.isProxyClass(proxy.getClass())).isTrue(); + } + } + + protected void assertProxyType(Object proxy) { + } + + @BeforeEach + void beforeEach() { + Mockito.clearInvocations(target); + } + + @Nested + class WhenThereIsOneInterceptor { + + @Nullable + private Throwable throwableSeenByInterceptor; + + @BeforeEach + void beforeEach() { + proxyFactory.addAdvice(captureThrowable()); + + proxy = (MyInterface) proxyFactory.getProxy(); + + assertProxyType(proxy); + } + + @Test + void targetThrowsUndeclaredCheckedException() throws DeclaredCheckedException { + doAnswer(sneakyThrow(undeclaredCheckedException)).when(target).doSomething(); + + invokeProxy(); + + assertThat(throwableSeenByInterceptor).isSameAs(undeclaredCheckedException); + assertThat(throwableSeenByCaller) + .isInstanceOf(UndeclaredThrowableException.class) + .hasCauseReference(undeclaredCheckedException); + } + + @Test + void targetThrowsDeclaredCheckedException() throws DeclaredCheckedException { + doThrow(declaredCheckedException).when(target).doSomething(); + + invokeProxy(); + + assertThat(throwableSeenByInterceptor).isSameAs(declaredCheckedException); + assertThat(throwableSeenByCaller).isSameAs(declaredCheckedException); + } + + @Test + void targetThrowsUncheckedException() throws DeclaredCheckedException { + doThrow(uncheckedException).when(target).doSomething(); + + invokeProxy(); + + assertThat(throwableSeenByInterceptor).isSameAs(uncheckedException); + assertThat(throwableSeenByCaller).isSameAs(uncheckedException); + } + + private MethodInterceptor captureThrowable() { + return invocation -> { + try { + return invocation.proceed(); + } + catch (Exception e) { + throwableSeenByInterceptor = e; + throw e; + } + }; + } + } + + @Nested + class WhenThereAreNoInterceptors { + + @BeforeEach + void beforeEach() { + proxy = (MyInterface) proxyFactory.getProxy(); + + assertProxyType(proxy); + } + + @Test + void targetThrowsUndeclaredCheckedException() throws DeclaredCheckedException { + doAnswer(sneakyThrow(undeclaredCheckedException)).when(target).doSomething(); + + invokeProxy(); + + assertThat(throwableSeenByCaller) + .isInstanceOf(UndeclaredThrowableException.class) + .hasCauseReference(undeclaredCheckedException); + } + + @Test + void targetThrowsDeclaredCheckedException() throws DeclaredCheckedException { + doThrow(declaredCheckedException).when(target).doSomething(); + + invokeProxy(); + + assertThat(throwableSeenByCaller).isSameAs(declaredCheckedException); + } + + @Test + void targetThrowsUncheckedException() throws DeclaredCheckedException { + doThrow(uncheckedException).when(target).doSomething(); + + invokeProxy(); + + assertThat(throwableSeenByCaller).isSameAs(uncheckedException); + } + } + + private void invokeProxy() { + throwableSeenByCaller = catchThrowable(() -> Objects.requireNonNull(proxy).doSomething()); + } + + private Answer sneakyThrow(@SuppressWarnings("SameParameterValue") Throwable throwable) { + return invocation -> { + throw throwable; + }; + } + + static class MyClass implements MyInterface { + @Override + public void doSomething() throws DeclaredCheckedException { + throw declaredCheckedException; + } + } + + protected interface MyInterface { + void doSomething() throws DeclaredCheckedException; + } + + protected static class UndeclaredCheckedException extends Exception { + @Serial + private static final long serialVersionUID = -4199611059122356651L; + } + + protected static class DeclaredCheckedException extends Exception { + @Serial + private static final long serialVersionUID = 8851375818059230518L; + } + +} diff --git a/spring-core/src/main/java/org/springframework/cglib/core/ClassLoaderAwareGeneratorStrategy.java b/spring-core/src/main/java/org/springframework/cglib/core/ClassLoaderAwareGeneratorStrategy.java index eb746444fc..2d7689dd99 100644 --- a/spring-core/src/main/java/org/springframework/cglib/core/ClassLoaderAwareGeneratorStrategy.java +++ b/spring-core/src/main/java/org/springframework/cglib/core/ClassLoaderAwareGeneratorStrategy.java @@ -16,6 +16,10 @@ package org.springframework.cglib.core; +import java.lang.reflect.UndeclaredThrowableException; + +import org.springframework.cglib.transform.impl.UndeclaredThrowableStrategy; + /** * CGLIB GeneratorStrategy variant which exposes the application ClassLoader * as current thread context ClassLoader for the time of class generation. @@ -25,11 +29,12 @@ package org.springframework.cglib.core; * @author Juergen Hoeller * @since 5.2 */ -public class ClassLoaderAwareGeneratorStrategy extends DefaultGeneratorStrategy { +public class ClassLoaderAwareGeneratorStrategy extends UndeclaredThrowableStrategy { private final ClassLoader classLoader; public ClassLoaderAwareGeneratorStrategy(ClassLoader classLoader) { + super(UndeclaredThrowableException.class); this.classLoader = classLoader; } diff --git a/spring-core/src/main/java/org/springframework/cglib/transform/impl/UndeclaredThrowableTransformer.java b/spring-core/src/main/java/org/springframework/cglib/transform/impl/UndeclaredThrowableTransformer.java index 708e1f1ce6..bc227e94a3 100644 --- a/spring-core/src/main/java/org/springframework/cglib/transform/impl/UndeclaredThrowableTransformer.java +++ b/spring-core/src/main/java/org/springframework/cglib/transform/impl/UndeclaredThrowableTransformer.java @@ -27,7 +27,7 @@ import org.springframework.cglib.core.Signature; import org.springframework.cglib.core.TypeUtils; import org.springframework.cglib.transform.ClassEmitterTransformer; -@SuppressWarnings({"rawtypes", "unchecked"}) +@SuppressWarnings({"rawtypes" }) public class UndeclaredThrowableTransformer extends ClassEmitterTransformer { private final Type wrapper; @@ -55,10 +55,20 @@ public class UndeclaredThrowableTransformer extends ClassEmitterTransformer { return e; } return new CodeEmitter(e) { - private Block handler; - /* init */ { - handler = begin_block(); - } + private final boolean isConstructor = Constants.CONSTRUCTOR_NAME.equals(sig.getName()); + private Block handler = begin_block(); + private boolean calToSuperWasSeen; + + @Override + public void visitMethodInsn(int opcode, String owner, String name, String descriptor, boolean isInterface) { + super.visitMethodInsn(opcode, owner, name, descriptor, isInterface); + if (isConstructor && !calToSuperWasSeen && Constants.CONSTRUCTOR_NAME.equals(name)) { + // we start the entry in the exception table after the call to super + handler = begin_block(); + calToSuperWasSeen = true; + } + } + @Override public void visitMaxs(int maxStack, int maxLocals) { handler.end();