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

Author: snicoll

spring-core/src/main/java/org/springframework/aot/generator/ProtectedAccess.java

@@ -74,7 +74,7 @@ public String getPrivilegedPackageName(String packageName) throws ProtectedAcces
 			return null;
 		}
 		List<String> 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 String getPrivilegedPackageName(String packageName) throws ProtectedAcces
 	private List<ProtectedElement> getProtectedElements(String packageName) {
 		List<ProtectedElement> 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 @@ private List<ProtectedElement> getProtectedElements(String packageName) {
 	 * @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 void analyze(Member member, Options options) {
 		}
 		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 @@ else if (member instanceof Constructor<?> constructor) {
 		}
 		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,37 +144,41 @@ private void analyzeParameterTypes(Executable executable, Function<Integer,
 
 		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<ResolvableType> seen, ResolvableType target) {
+	@Nullable
+	private Class<?> isProtected(Set<ResolvableType> 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()) {
 			for (ResolvableType generic : nonProxyTarget.getGenerics()) {
 				return isProtected(seen, generic);
 			}
 		}
-		return false;
+		return null;
 	}
 
 	private boolean isProtected(Class<?> type) {
@@ -183,14 +190,10 @@ private boolean isProtected(int modifiers) {
 		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.

spring-core/src/main/java/org/springframework/aot/generator/ProtectedElement.java

@@ -18,7 +18,6 @@
 
 import java.lang.reflect.Member;
 
-import org.springframework.core.ResolvableType;
 import org.springframework.lang.Nullable;
 
 /**
@@ -29,25 +28,27 @@
  */
 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 Member getMember() {
 		return this.target;
 	}
 
-	static ProtectedElement of(ResolvableType type, @Nullable Member member) {
+	static ProtectedElement of(Class<?> type, @Nullable Member member) {
 		return new ProtectedElement(type, member);
 	}
 

spring-core/src/test/java/org/springframework/aot/generator/ProtectedAccessTests.java

@@ -25,6 +25,7 @@
 
 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 @@ void analyzeWithPackagePrivateConstructorParameter() {
 		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 @@ void analyzeTypeWithProtectedGenericArgument() {
 	@Test
 	void analyzeWithRecursiveType() {
 		assertThat(this.protectedAccess.isProtected(ResolvableType.forClassWithGenerics(
-				SelfReference.class, SelfReference.class))).isTrue();
+				SelfReference.class, SelfReference.class))).isEqualTo(SelfReference.class);
 	}
 
 	@Test

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<ProtectedType> types) {
+	}
+
+}