From 5d7a632965aff526cb6bbd0380b445f102819dca Mon Sep 17 00:00:00 2001 From: Sam Brannen Date: Fri, 4 Feb 2022 19:41:46 +0100 Subject: [PATCH] Ensure Spring AOP generates JDK dynamic proxies for lambdas Prior to this commit, if AOP proxy generation was configured with proxyTargetClass=true (which is the default behavior in recent versions of Spring Boot), beans implemented as lambda expressions or method references could not be proxied with CGLIB on Java 16 or higher without specifying `--add-opens java.base/java.lang=ALL-UNNAMED`. This commit addresses this shortcoming by ensuring that beans implemented as lambda expressions or method references are always proxied using a JDK dynamic proxy even if proxyTargetClass=true. Closes gh-27971 --- .../aop/framework/AopProxyUtils.java | 19 +++++- .../aop/framework/DefaultAopProxyFactory.java | 5 +- .../aop/framework/AopProxyUtilsTests.java | 61 ++++++++++++++++++- .../AspectJAutoProxyCreatorTests.java | 50 +++++++++++++++ src/checkstyle/checkstyle.xml | 2 +- 5 files changed, 131 insertions(+), 6 deletions(-) diff --git a/spring-aop/src/main/java/org/springframework/aop/framework/AopProxyUtils.java b/spring-aop/src/main/java/org/springframework/aop/framework/AopProxyUtils.java index 12ce1ce86cd..b7c3de885c0 100644 --- a/spring-aop/src/main/java/org/springframework/aop/framework/AopProxyUtils.java +++ b/spring-aop/src/main/java/org/springframework/aop/framework/AopProxyUtils.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2022 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. @@ -44,6 +44,7 @@ import org.springframework.util.ReflectionUtils; * * @author Rod Johnson * @author Juergen Hoeller + * @author Sam Brannen * @see org.springframework.aop.support.AopUtils */ public abstract class AopProxyUtils { @@ -133,7 +134,7 @@ public abstract class AopProxyUtils { if (targetClass.isInterface()) { advised.setInterfaces(targetClass); } - else if (Proxy.isProxyClass(targetClass)) { + else if (Proxy.isProxyClass(targetClass) || isLambda(targetClass)) { advised.setInterfaces(targetClass.getInterfaces()); } specifiedInterfaces = advised.getProxiedInterfaces(); @@ -244,4 +245,18 @@ public abstract class AopProxyUtils { return arguments; } + /** + * Determine if the supplied {@link Class} is a JVM-generated implementation + * class for a lambda expression or method reference. + *

This method makes a best-effort attempt at determining this, based on + * checks that work on modern, main stream JVMs. + * @param clazz the class to check + * @return {@code true} if the class is a lambda implementation class + * @since 5.2.16 + */ + static boolean isLambda(Class clazz) { + return (clazz.isSynthetic() && (clazz.getSuperclass() == Object.class) && + (clazz.getInterfaces().length > 0) && clazz.getName().contains("$$Lambda")); + } + } diff --git a/spring-aop/src/main/java/org/springframework/aop/framework/DefaultAopProxyFactory.java b/spring-aop/src/main/java/org/springframework/aop/framework/DefaultAopProxyFactory.java index 8692371d3a8..5f1acad9a9a 100644 --- a/spring-aop/src/main/java/org/springframework/aop/framework/DefaultAopProxyFactory.java +++ b/spring-aop/src/main/java/org/springframework/aop/framework/DefaultAopProxyFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2021 the original author or authors. + * Copyright 2002-2022 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. @@ -40,6 +40,7 @@ import org.springframework.core.NativeDetector; * @author Rod Johnson * @author Juergen Hoeller * @author Sebastien Deleuze + * @author Sam Brannen * @since 12.03.2004 * @see AdvisedSupport#setOptimize * @see AdvisedSupport#setProxyTargetClass @@ -59,7 +60,7 @@ public class DefaultAopProxyFactory implements AopProxyFactory, Serializable { throw new AopConfigException("TargetSource cannot determine target class: " + "Either an interface or a target is required for proxy creation."); } - if (targetClass.isInterface() || Proxy.isProxyClass(targetClass)) { + if (targetClass.isInterface() || Proxy.isProxyClass(targetClass) || AopProxyUtils.isLambda(targetClass)) { return new JdkDynamicAopProxy(config); } return new ObjenesisCglibAopProxy(config); diff --git a/spring-aop/src/test/java/org/springframework/aop/framework/AopProxyUtilsTests.java b/spring-aop/src/test/java/org/springframework/aop/framework/AopProxyUtilsTests.java index c8475794f33..3dbf550a121 100644 --- a/spring-aop/src/test/java/org/springframework/aop/framework/AopProxyUtilsTests.java +++ b/spring-aop/src/test/java/org/springframework/aop/framework/AopProxyUtilsTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2022 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. @@ -19,6 +19,7 @@ package org.springframework.aop.framework; import java.lang.reflect.Proxy; import java.util.Arrays; import java.util.List; +import java.util.function.Supplier; import org.junit.jupiter.api.Test; @@ -32,6 +33,7 @@ import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException /** * @author Rod Johnson * @author Chris Beams + * @author Sam Brannen */ public class AopProxyUtilsTests { @@ -132,4 +134,61 @@ public class AopProxyUtilsTests { AopProxyUtils.proxiedUserInterfaces(proxy)); } + @Test + void isLambda() { + assertIsLambda(AopProxyUtilsTests.staticLambdaExpression); + assertIsLambda(AopProxyUtilsTests::staticStringFactory); + + assertIsLambda(this.instanceLambdaExpression); + assertIsLambda(this::instanceStringFactory); + } + + @Test + void isNotLambda() { + assertIsNotLambda(new EnigmaSupplier()); + + assertIsNotLambda(new Supplier() { + @Override + public String get() { + return "anonymous inner class"; + } + }); + + assertIsNotLambda(new Fake$$LambdaSupplier()); + } + + private static void assertIsLambda(Supplier supplier) { + assertThat(AopProxyUtils.isLambda(supplier.getClass())).isTrue(); + } + + private static void assertIsNotLambda(Supplier supplier) { + assertThat(AopProxyUtils.isLambda(supplier.getClass())).isFalse(); + } + + private static final Supplier staticLambdaExpression = () -> "static lambda expression"; + + private final Supplier instanceLambdaExpression = () -> "instance lambda expressions"; + + private static String staticStringFactory() { + return "static string factory"; + } + + private String instanceStringFactory() { + return "instance string factory"; + } + + private static class EnigmaSupplier implements Supplier { + @Override + public String get() { + return "enigma"; + } + } + + private static class Fake$$LambdaSupplier implements Supplier { + @Override + public String get() { + return "fake lambda"; + } + } + } diff --git a/spring-context/src/test/java/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests.java b/spring-context/src/test/java/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests.java index 91e58722fea..7c017cfa1aa 100644 --- a/spring-context/src/test/java/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests.java +++ b/spring-context/src/test/java/org/springframework/aop/aspectj/autoproxy/AspectJAutoProxyCreatorTests.java @@ -19,6 +19,7 @@ package org.springframework.aop.aspectj.autoproxy; import java.lang.annotation.Retention; import java.lang.annotation.RetentionPolicy; import java.lang.reflect.Method; +import java.util.function.Supplier; import org.aspectj.lang.JoinPoint; import org.aspectj.lang.ProceedingJoinPoint; @@ -27,6 +28,8 @@ import org.aspectj.lang.annotation.Aspect; import org.aspectj.lang.annotation.Before; import org.aspectj.lang.annotation.Pointcut; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.ValueSource; import org.springframework.aop.MethodBeforeAdvice; import org.springframework.aop.aspectj.annotation.AnnotationAwareAspectJAutoProxyCreator; @@ -42,6 +45,11 @@ import org.springframework.beans.factory.support.RootBeanDefinition; import org.springframework.beans.testfixture.beans.ITestBean; import org.springframework.beans.testfixture.beans.TestBean; import org.springframework.context.ApplicationContext; +import org.springframework.context.ConfigurableApplicationContext; +import org.springframework.context.annotation.AnnotationConfigApplicationContext; +import org.springframework.context.annotation.Bean; +import org.springframework.context.annotation.Configuration; +import org.springframework.context.annotation.EnableAspectJAutoProxy; import org.springframework.context.support.ClassPathXmlApplicationContext; import org.springframework.context.support.GenericApplicationContext; import org.springframework.core.NestedRuntimeException; @@ -292,6 +300,16 @@ public class AspectJAutoProxyCreatorTests { assertThat(tb.getAge()).isEqualTo(68); } + @ParameterizedTest(name = "[{index}] {0}") + @ValueSource(classes = {ProxyTargetClassFalseConfig.class, ProxyTargetClassTrueConfig.class}) + void lambdaIsAlwaysProxiedWithJdkProxy(Class configClass) { + try (ConfigurableApplicationContext context = new AnnotationConfigApplicationContext(configClass)) { + Supplier supplier = context.getBean(Supplier.class); + assertThat(AopUtils.isAopProxy(supplier)).as("AOP proxy").isTrue(); + assertThat(AopUtils.isJdkDynamicProxy(supplier)).as("JDK Dynamic proxy").isTrue(); + assertThat(supplier.get()).asString().isEqualTo("advised: lambda"); + } + } /** * Returns a new {@link ClassPathXmlApplicationContext} for the file ending in fileSuffix. @@ -566,3 +584,35 @@ class TestBeanAdvisor extends StaticMethodMatcherPointcutAdvisor { } } + +abstract class AbstractProxyTargetClassConfig { + + @Bean + Supplier stringSupplier() { + return () -> "lambda"; + } + + @Bean + SupplierAdvice supplierAdvice() { + return new SupplierAdvice(); + } + + @Aspect + static class SupplierAdvice { + + @Around("execution(public * org.springframework.aop.aspectj.autoproxy..*.*(..))") + Object aroundSupplier(ProceedingJoinPoint joinPoint) throws Throwable { + return "advised: " + joinPoint.proceed(); + } + } +} + +@Configuration(proxyBeanMethods = false) +@EnableAspectJAutoProxy(proxyTargetClass = false) +class ProxyTargetClassFalseConfig extends AbstractProxyTargetClassConfig { +} + +@Configuration(proxyBeanMethods = false) +@EnableAspectJAutoProxy(proxyTargetClass = true) +class ProxyTargetClassTrueConfig extends AbstractProxyTargetClassConfig { +} diff --git a/src/checkstyle/checkstyle.xml b/src/checkstyle/checkstyle.xml index 5a9318ef3cb..f94de141f9a 100644 --- a/src/checkstyle/checkstyle.xml +++ b/src/checkstyle/checkstyle.xml @@ -51,7 +51,7 @@ - +