From e873715737fda8fc284bc5c18f0e8c260416c3ba Mon Sep 17 00:00:00 2001 From: Stephane Nicoll Date: Tue, 8 Mar 2022 15:08:59 +0100 Subject: [PATCH] Fix detection of protected generic parameter This commit fixes the algorithm used to analyze a generic parameter. If a type in the generic signature is protected, the type is return rather than the full signature. This makes sure that the appropriate package is used. Previously, it would have incorrectly used the type of the raw class. Using a generic type for such a use case is wrong, and ProtectedElement has been updated to expose a `Class` rather than a `ResolvableType`. See gh-28030 --- .../aot/generator/ProtectedAccess.java | 49 ++++++++++--------- .../aot/generator/ProtectedElement.java | 15 +++--- .../aot/generator/ProtectedAccessTests.java | 9 +++- .../visibility/ProtectedGenericParameter.java | 26 ++++++++++ 4 files changed, 68 insertions(+), 31 deletions(-) create mode 100644 spring-core/src/testFixtures/java/org/springframework/core/testfixture/aot/generator/visibility/ProtectedGenericParameter.java diff --git a/spring-core/src/main/java/org/springframework/aot/generator/ProtectedAccess.java b/spring-core/src/main/java/org/springframework/aot/generator/ProtectedAccess.java index f0fc6c84a2d..884f0a1a279 100644 --- a/spring-core/src/main/java/org/springframework/aot/generator/ProtectedAccess.java +++ b/spring-core/src/main/java/org/springframework/aot/generator/ProtectedAccess.java @@ -74,7 +74,7 @@ public class ProtectedAccess { return null; } List packageNames = protectedElements.stream() - .map(element -> element.getType().toClass().getPackageName()) + .map(element -> element.getType().getPackageName()) .distinct().toList(); if (packageNames.size() == 1) { return packageNames.get(0); @@ -86,7 +86,7 @@ public class ProtectedAccess { private List getProtectedElements(String packageName) { List matches = new ArrayList<>(); for (ProtectedElement element : this.elements) { - if (!element.getType().toClass().getPackage().getName().equals(packageName)) { + if (!element.getType().getPackage().getName().equals(packageName)) { matches.add(element); } } @@ -99,8 +99,9 @@ public class ProtectedAccess { * @param type the type to analyze */ public void analyze(ResolvableType type) { - if (isProtected(type)) { - registerProtectedType(type, null); + Class protectedType = isProtected(type); + if (protectedType != null) { + registerProtectedType(protectedType, null); } } @@ -119,8 +120,9 @@ public class ProtectedAccess { } if (member instanceof Field field) { ResolvableType fieldType = ResolvableType.forField(field); - if (isProtected(fieldType) && options.assignReturnType.apply(field)) { - registerProtectedType(fieldType, field); + Class protectedType = isProtected(fieldType); + if (protectedType != null && options.assignReturnType.apply(field)) { + registerProtectedType(protectedType, field); } } else if (member instanceof Constructor constructor) { @@ -129,8 +131,9 @@ public class ProtectedAccess { } else if (member instanceof Method method) { ResolvableType returnType = ResolvableType.forMethodReturnType(method); - if (isProtected(returnType) && options.assignReturnType.apply(method)) { - registerProtectedType(returnType, method); + Class protectedType = isProtected(returnType); + if (protectedType != null && options.assignReturnType.apply(method)) { + registerProtectedType(protectedType, method); } analyzeParameterTypes(method, i -> ResolvableType.forMethodParameter(method, i)); } @@ -141,29 +144,33 @@ public class ProtectedAccess { for (int i = 0; i < executable.getParameters().length; i++) { ResolvableType parameterType = parameterTypeFactory.apply(i); - if (isProtected(parameterType)) { - registerProtectedType(parameterType, executable); + Class protectedType = isProtected(parameterType); + if (protectedType != null) { + registerProtectedType(protectedType, executable); } } } - boolean isProtected(ResolvableType resolvableType) { + @Nullable + Class isProtected(ResolvableType resolvableType) { return isProtected(new HashSet<>(), resolvableType); } - private boolean isProtected(Set seen, ResolvableType target) { + @Nullable + private Class isProtected(Set seen, ResolvableType target) { if (seen.contains(target)) { - return false; + return null; } seen.add(target); ResolvableType nonProxyTarget = target.as(ClassUtils.getUserClass(target.toClass())); - if (isProtected(nonProxyTarget.toClass())) { - return true; + Class rawClass = nonProxyTarget.toClass(); + if (isProtected(rawClass)) { + return rawClass; } - Class declaringClass = nonProxyTarget.toClass().getDeclaringClass(); + Class declaringClass = rawClass.getDeclaringClass(); if (declaringClass != null) { if (isProtected(declaringClass)) { - return true; + return declaringClass; } } if (nonProxyTarget.hasGenerics()) { @@ -171,7 +178,7 @@ public class ProtectedAccess { return isProtected(seen, generic); } } - return false; + return null; } private boolean isProtected(Class type) { @@ -183,14 +190,10 @@ public class ProtectedAccess { return !Modifier.isPublic(modifiers); } - private void registerProtectedType(ResolvableType type, @Nullable Member member) { + private void registerProtectedType(Class type, @Nullable Member member) { this.elements.add(ProtectedElement.of(type, member)); } - private void registerProtectedType(Class type, Member member) { - registerProtectedType(ResolvableType.forClass(type), member); - } - /** * Options to use to analyze if invoking a {@link Member} requires * privileged access. diff --git a/spring-core/src/main/java/org/springframework/aot/generator/ProtectedElement.java b/spring-core/src/main/java/org/springframework/aot/generator/ProtectedElement.java index eb2615ed7bc..c7b5d6c2e5b 100644 --- a/spring-core/src/main/java/org/springframework/aot/generator/ProtectedElement.java +++ b/spring-core/src/main/java/org/springframework/aot/generator/ProtectedElement.java @@ -18,7 +18,6 @@ package org.springframework.aot.generator; import java.lang.reflect.Member; -import org.springframework.core.ResolvableType; import org.springframework.lang.Nullable; /** @@ -29,25 +28,27 @@ import org.springframework.lang.Nullable; */ public final class ProtectedElement { - private final ResolvableType type; + private final Class type; @Nullable private final Member target; - private ProtectedElement(ResolvableType type, @Nullable Member member) { + private ProtectedElement(Class type, @Nullable Member member) { this.type = type; this.target = member; } /** - * Return the {@link ResolvableType type} that is non-public. For a plain + * Return the {@link Class type} that is non-public. For a plain * protected {@link Member member} access, the type of the declaring class * is used. Otherwise, the type in the member signature, such as a parameter - * type for an executable, or the return type of a field is used. + * type for an executable, or the return type of a field is used. If the + * type is generic, the type that is protected in the generic signature is + * used. * @return the type that is not public */ - public ResolvableType getType() { + public Class getType() { return this.type; } @@ -60,7 +61,7 @@ public final class ProtectedElement { return this.target; } - static ProtectedElement of(ResolvableType type, @Nullable Member member) { + static ProtectedElement of(Class type, @Nullable Member member) { return new ProtectedElement(type, member); } diff --git a/spring-core/src/test/java/org/springframework/aot/generator/ProtectedAccessTests.java b/spring-core/src/test/java/org/springframework/aot/generator/ProtectedAccessTests.java index 9c1cc19897a..9e32b620304 100644 --- a/spring-core/src/test/java/org/springframework/aot/generator/ProtectedAccessTests.java +++ b/spring-core/src/test/java/org/springframework/aot/generator/ProtectedAccessTests.java @@ -25,6 +25,7 @@ import org.junit.jupiter.api.Test; import org.springframework.aot.generator.ProtectedAccess.Options; import org.springframework.core.ResolvableType; +import org.springframework.core.testfixture.aot.generator.visibility.ProtectedGenericParameter; import org.springframework.core.testfixture.aot.generator.visibility.ProtectedParameter; import org.springframework.core.testfixture.aot.generator.visibility.PublicFactoryBean; import org.springframework.util.ReflectionUtils; @@ -82,6 +83,12 @@ class ProtectedAccessTests { assertPrivilegedAccess(ProtectedParameter.class); } + @Test + void analyzeWithPackagePrivateConstructorGenericParameter() { + this.protectedAccess.analyze(ProtectedGenericParameter.class.getConstructors()[0], DEFAULT_OPTIONS); + assertPrivilegedAccess(ProtectedParameter.class); + } + @Test void analyzeWithPackagePrivateMethod() { this.protectedAccess.analyze(method(PublicClass.class, "getProtectedMethod"), DEFAULT_OPTIONS); @@ -164,7 +171,7 @@ class ProtectedAccessTests { @Test void analyzeWithRecursiveType() { assertThat(this.protectedAccess.isProtected(ResolvableType.forClassWithGenerics( - SelfReference.class, SelfReference.class))).isTrue(); + SelfReference.class, SelfReference.class))).isEqualTo(SelfReference.class); } @Test diff --git a/spring-core/src/testFixtures/java/org/springframework/core/testfixture/aot/generator/visibility/ProtectedGenericParameter.java b/spring-core/src/testFixtures/java/org/springframework/core/testfixture/aot/generator/visibility/ProtectedGenericParameter.java new file mode 100644 index 00000000000..35ff2fb86af --- /dev/null +++ b/spring-core/src/testFixtures/java/org/springframework/core/testfixture/aot/generator/visibility/ProtectedGenericParameter.java @@ -0,0 +1,26 @@ +/* + * 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. + * 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.core.testfixture.aot.generator.visibility; + +import java.util.List; + +public class ProtectedGenericParameter { + + public ProtectedGenericParameter(List types) { + } + +}