From f6089afd0e55d87489f4c9f5ddc09064c1a33a39 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 10 Apr 2024 13:23:24 +0200 Subject: [PATCH] Use ClassLoaderAwareGeneratorStrategy with UndeclaredThrowableStrategy delegate See gh-32469 --- .../aop/framework/CglibAopProxy.java | 18 ++-- .../ProxyExceptionHandlingTests.java | 101 +++++++++--------- .../ClassLoaderAwareGeneratorStrategy.java | 34 ++++-- .../impl/UndeclaredThrowableTransformer.java | 10 +- 4 files changed, 91 insertions(+), 72 deletions(-) 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 dbf6bea1c01b..856d48538718 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,6 +19,7 @@ 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; @@ -36,6 +37,7 @@ import org.springframework.aop.support.AopUtils; import org.springframework.cglib.core.ClassLoaderAwareGeneratorStrategy; import org.springframework.cglib.core.CodeGenerationException; +import org.springframework.cglib.core.GeneratorStrategy; import org.springframework.cglib.core.SpringNamingPolicy; import org.springframework.cglib.proxy.Callback; import org.springframework.cglib.proxy.CallbackFilter; @@ -45,6 +47,7 @@ import org.springframework.cglib.proxy.MethodInterceptor; import org.springframework.cglib.proxy.MethodProxy; import org.springframework.cglib.proxy.NoOp; +import org.springframework.cglib.transform.impl.UndeclaredThrowableStrategy; import org.springframework.core.KotlinDetector; import org.springframework.core.MethodParameter; import org.springframework.core.SmartClassLoader; @@ -92,17 +95,20 @@ class CglibAopProxy implements AopProxy, Serializable { private static final int INVOKE_HASHCODE = 6; + private static final String COROUTINES_FLOW_CLASS_NAME = "kotlinx.coroutines.flow.Flow"; + + private static final boolean coroutinesReactorPresent = ClassUtils.isPresent( + "kotlinx.coroutines.reactor.MonoKt", CglibAopProxy.class.getClassLoader()); + + private static final GeneratorStrategy undeclaredThrowableStrategy = + new UndeclaredThrowableStrategy(UndeclaredThrowableException.class); + /** Logger available to subclasses; static to optimize serialization. */ protected static final Log logger = LogFactory.getLog(CglibAopProxy.class); /** Keeps track of the Classes that we have validated for final methods. */ private static final Map, Boolean> validatedClasses = new WeakHashMap<>(); - private static final String COROUTINES_FLOW_CLASS_NAME = "kotlinx.coroutines.flow.Flow"; - - private static final boolean coroutinesReactorPresent = ClassUtils.isPresent("kotlinx.coroutines.reactor.MonoKt", - CglibAopProxy.class.getClassLoader());; - /** The configuration used to configure this proxy. */ protected final AdvisedSupport advised; @@ -200,7 +206,7 @@ private Object buildProxy(@Nullable ClassLoader classLoader, boolean classOnly) enhancer.setInterfaces(AopProxyUtils.completeProxiedInterfaces(this.advised)); enhancer.setNamingPolicy(SpringNamingPolicy.INSTANCE); enhancer.setAttemptLoad(true); - enhancer.setStrategy(new ClassLoaderAwareGeneratorStrategy(classLoader)); + enhancer.setStrategy(new ClassLoaderAwareGeneratorStrategy(classLoader, undeclaredThrowableStrategy)); Callback[] callbacks = getCallbacks(rootClass); Class[] types = new Class[callbacks.length]; 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 index ca9a609ea6c5..a30b4a502a5d 100644 --- a/spring-aop/src/test/java/org/springframework/aop/framework/ProxyExceptionHandlingTests.java +++ b/spring-aop/src/test/java/org/springframework/aop/framework/ProxyExceptionHandlingTests.java @@ -16,7 +16,6 @@ package org.springframework.aop.framework; -import java.io.Serial; import java.lang.reflect.Proxy; import java.lang.reflect.UndeclaredThrowableException; import java.util.Objects; @@ -36,47 +35,72 @@ import static org.mockito.BDDMockito.doThrow; import static org.mockito.BDDMockito.mock; +/** + * @author Mikaël Francoeur + * @since 6.2 + */ 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(); - } + @BeforeEach + void clear() { + Mockito.clearInvocations(target); } + protected void assertProxyType(Object proxy) { + } + + private void invokeProxy() { + throwableSeenByCaller = catchThrowable(() -> Objects.requireNonNull(proxy).doSomething()); + } + + @SuppressWarnings("SameParameterValue") + private Answer sneakyThrow(Throwable throwable) { + return invocation -> { + throw throwable; + }; + } + + 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); + static class CglibAopProxyTests extends ProxyExceptionHandlingTests { + + @BeforeEach + void setup() { + proxyFactory.setProxyTargetClass(true); + } + + @Override + protected void assertProxyType(Object proxy) { + assertThat(Enhancer.isEnhanced(proxy.getClass())).isTrue(); + } } + @Nested class WhenThereIsOneInterceptor { @@ -86,18 +110,14 @@ class WhenThereIsOneInterceptor { @BeforeEach void beforeEach() { proxyFactory.addAdvice(captureThrowable()); - - proxy = (MyInterface) proxyFactory.getProxy(); - + proxy = (MyInterface) proxyFactory.getProxy(ProxyExceptionHandlingTests.class.getClassLoader()); assertProxyType(proxy); } @Test void targetThrowsUndeclaredCheckedException() throws DeclaredCheckedException { doAnswer(sneakyThrow(undeclaredCheckedException)).when(target).doSomething(); - invokeProxy(); - assertThat(throwableSeenByInterceptor).isSameAs(undeclaredCheckedException); assertThat(throwableSeenByCaller) .isInstanceOf(UndeclaredThrowableException.class) @@ -107,9 +127,7 @@ void targetThrowsUndeclaredCheckedException() throws DeclaredCheckedException { @Test void targetThrowsDeclaredCheckedException() throws DeclaredCheckedException { doThrow(declaredCheckedException).when(target).doSomething(); - invokeProxy(); - assertThat(throwableSeenByInterceptor).isSameAs(declaredCheckedException); assertThat(throwableSeenByCaller).isSameAs(declaredCheckedException); } @@ -117,9 +135,7 @@ void targetThrowsDeclaredCheckedException() throws DeclaredCheckedException { @Test void targetThrowsUncheckedException() throws DeclaredCheckedException { doThrow(uncheckedException).when(target).doSomething(); - invokeProxy(); - assertThat(throwableSeenByInterceptor).isSameAs(uncheckedException); assertThat(throwableSeenByCaller).isSameAs(uncheckedException); } @@ -129,30 +145,28 @@ private MethodInterceptor captureThrowable() { try { return invocation.proceed(); } - catch (Exception e) { - throwableSeenByInterceptor = e; - throw e; + catch (Exception ex) { + throwableSeenByInterceptor = ex; + throw ex; } }; } } + @Nested class WhenThereAreNoInterceptors { @BeforeEach void beforeEach() { - proxy = (MyInterface) proxyFactory.getProxy(); - + proxy = (MyInterface) proxyFactory.getProxy(ProxyExceptionHandlingTests.class.getClassLoader()); assertProxyType(proxy); } @Test void targetThrowsUndeclaredCheckedException() throws DeclaredCheckedException { doAnswer(sneakyThrow(undeclaredCheckedException)).when(target).doSomething(); - invokeProxy(); - assertThat(throwableSeenByCaller) .isInstanceOf(UndeclaredThrowableException.class) .hasCauseReference(undeclaredCheckedException); @@ -161,51 +175,38 @@ void targetThrowsUndeclaredCheckedException() throws DeclaredCheckedException { @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; - }; + protected interface MyInterface { + + void doSomething() throws DeclaredCheckedException; } static class MyClass implements MyInterface { + @Override public void doSomething() throws DeclaredCheckedException { throw declaredCheckedException; } } - protected interface MyInterface { - void doSomething() throws DeclaredCheckedException; - } - + @SuppressWarnings("serial") protected static class UndeclaredCheckedException extends Exception { - @Serial - private static final long serialVersionUID = -4199611059122356651L; } + @SuppressWarnings("serial") 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 2d7689dd991c..74853cf8595f 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * 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. @@ -16,10 +16,6 @@ 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. @@ -29,19 +25,37 @@ * @author Juergen Hoeller * @since 5.2 */ -public class ClassLoaderAwareGeneratorStrategy extends UndeclaredThrowableStrategy { +public class ClassLoaderAwareGeneratorStrategy extends DefaultGeneratorStrategy { private final ClassLoader classLoader; + private final GeneratorStrategy delegate; + + + /** + * Create a default GeneratorStrategy, exposing the given ClassLoader. + * @param classLoader the ClassLoader to expose as current thread context ClassLoader + */ public ClassLoaderAwareGeneratorStrategy(ClassLoader classLoader) { - super(UndeclaredThrowableException.class); this.classLoader = classLoader; + this.delegate = super::generate; } + /** + * Create a decorator for the given GeneratorStrategy delegate, exposing the given ClassLoader. + * @param classLoader the ClassLoader to expose as current thread context ClassLoader + * @since 6.2 + */ + public ClassLoaderAwareGeneratorStrategy(ClassLoader classLoader, GeneratorStrategy delegate) { + this.classLoader = classLoader; + this.delegate = delegate; + } + + @Override public byte[] generate(ClassGenerator cg) throws Exception { if (this.classLoader == null) { - return super.generate(cg); + return this.delegate.generate(cg); } Thread currentThread = Thread.currentThread(); @@ -51,7 +65,7 @@ public byte[] generate(ClassGenerator cg) throws Exception { } catch (Throwable ex) { // Cannot access thread context ClassLoader - falling back... - return super.generate(cg); + return this.delegate.generate(cg); } boolean overrideClassLoader = !this.classLoader.equals(threadContextClassLoader); @@ -59,7 +73,7 @@ public byte[] generate(ClassGenerator cg) throws Exception { currentThread.setContextClassLoader(this.classLoader); } try { - return super.generate(cg); + return this.delegate.generate(cg); } finally { if (overrideClassLoader) { 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 bc227e94a388..e4dca7b4c5d6 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.TypeUtils; import org.springframework.cglib.transform.ClassEmitterTransformer; -@SuppressWarnings({"rawtypes" }) +@SuppressWarnings("rawtypes") public class UndeclaredThrowableTransformer extends ClassEmitterTransformer { private final Type wrapper; @@ -57,18 +57,16 @@ public CodeEmitter begin_method(int access, final Signature sig, final Type[] ex return new CodeEmitter(e) { private final boolean isConstructor = Constants.CONSTRUCTOR_NAME.equals(sig.getName()); private Block handler = begin_block(); - private boolean calToSuperWasSeen; - + private boolean callToSuperSeen; @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)) { + if (isConstructor && !callToSuperSeen && Constants.CONSTRUCTOR_NAME.equals(name)) { // we start the entry in the exception table after the call to super handler = begin_block(); - calToSuperWasSeen = true; + callToSuperSeen = true; } } - @Override public void visitMaxs(int maxStack, int maxLocals) { handler.end();