Skip to content

Commit dc2e1af

Browse files
committed
Align Method Traversal with MergedAnnotations
Closes gh-16751
1 parent 4993fa8 commit dc2e1af

File tree

2 files changed

+115
-7
lines changed

2 files changed

+115
-7
lines changed

Diff for: core/src/main/java/org/springframework/security/core/annotation/UniqueSecurityAnnotationScanner.java

+55-7
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,10 @@
1919
import java.lang.annotation.Annotation;
2020
import java.lang.reflect.AnnotatedElement;
2121
import java.lang.reflect.Method;
22+
import java.lang.reflect.Modifier;
2223
import java.lang.reflect.Parameter;
2324
import java.util.ArrayList;
25+
import java.util.Arrays;
2426
import java.util.Collections;
2527
import java.util.HashSet;
2628
import java.util.List;
@@ -29,6 +31,7 @@
2931
import java.util.concurrent.ConcurrentHashMap;
3032

3133
import org.springframework.core.MethodClassKey;
34+
import org.springframework.core.ResolvableType;
3235
import org.springframework.core.annotation.AnnotationConfigurationException;
3336
import org.springframework.core.annotation.MergedAnnotation;
3437
import org.springframework.core.annotation.MergedAnnotations;
@@ -169,18 +172,15 @@ private List<MergedAnnotation<A>> findClosestMethodAnnotations(Method method, Cl
169172
return Collections.emptyList();
170173
}
171174
classesToSkip.add(targetClass);
172-
try {
173-
Method methodToUse = targetClass.getDeclaredMethod(method.getName(), method.getParameterTypes());
175+
Method methodToUse = findMethod(method, targetClass);
176+
if (methodToUse != null) {
174177
List<MergedAnnotation<A>> annotations = findDirectAnnotations(methodToUse);
175178
if (!annotations.isEmpty()) {
176179
return annotations;
177180
}
178181
}
179-
catch (NoSuchMethodException ex) {
180-
// move on
181-
}
182-
List<MergedAnnotation<A>> annotations = new ArrayList<>();
183-
annotations.addAll(findClosestMethodAnnotations(method, targetClass.getSuperclass(), classesToSkip));
182+
List<MergedAnnotation<A>> annotations = new ArrayList<>(
183+
findClosestMethodAnnotations(method, targetClass.getSuperclass(), classesToSkip));
184184
for (Class<?> inter : targetClass.getInterfaces()) {
185185
annotations.addAll(findClosestMethodAnnotations(method, inter, classesToSkip));
186186
}
@@ -212,4 +212,52 @@ private List<MergedAnnotation<A>> findDirectAnnotations(AnnotatedElement element
212212
.toList();
213213
}
214214

215+
private static Method findMethod(Method method, Class<?> targetClass) {
216+
for (Method candidate : targetClass.getDeclaredMethods()) {
217+
if (candidate == method) {
218+
return candidate;
219+
}
220+
if (isOverride(method, candidate)) {
221+
return candidate;
222+
}
223+
}
224+
return null;
225+
}
226+
227+
private static boolean isOverride(Method rootMethod, Method candidateMethod) {
228+
return (!Modifier.isPrivate(candidateMethod.getModifiers())
229+
&& candidateMethod.getName().equals(rootMethod.getName())
230+
&& hasSameParameterTypes(rootMethod, candidateMethod));
231+
}
232+
233+
private static boolean hasSameParameterTypes(Method rootMethod, Method candidateMethod) {
234+
if (candidateMethod.getParameterCount() != rootMethod.getParameterCount()) {
235+
return false;
236+
}
237+
Class<?>[] rootParameterTypes = rootMethod.getParameterTypes();
238+
Class<?>[] candidateParameterTypes = candidateMethod.getParameterTypes();
239+
if (Arrays.equals(candidateParameterTypes, rootParameterTypes)) {
240+
return true;
241+
}
242+
return hasSameGenericTypeParameters(rootMethod, candidateMethod, rootParameterTypes);
243+
}
244+
245+
private static boolean hasSameGenericTypeParameters(Method rootMethod, Method candidateMethod,
246+
Class<?>[] rootParameterTypes) {
247+
248+
Class<?> sourceDeclaringClass = rootMethod.getDeclaringClass();
249+
Class<?> candidateDeclaringClass = candidateMethod.getDeclaringClass();
250+
if (!candidateDeclaringClass.isAssignableFrom(sourceDeclaringClass)) {
251+
return false;
252+
}
253+
for (int i = 0; i < rootParameterTypes.length; i++) {
254+
Class<?> resolvedParameterType = ResolvableType.forMethodParameter(candidateMethod, i, sourceDeclaringClass)
255+
.resolve();
256+
if (rootParameterTypes[i] != resolvedParameterType) {
257+
return false;
258+
}
259+
}
260+
return true;
261+
}
262+
215263
}

Diff for: core/src/test/java/org/springframework/security/core/annotation/UniqueSecurityAnnotationScannerTests.java

+60
Original file line numberDiff line numberDiff line change
@@ -251,6 +251,30 @@ void scanWhenClassInheritingAbstractClassNoAnnotationsThenNoAnnotation() throws
251251
assertThat(preAuthorize).isNull();
252252
}
253253

254+
// gh-16751
255+
@Test
256+
void scanWhenAnnotationOnParameterizedInterfaceTheLocates() throws Exception {
257+
Method method = MyServiceImpl.class.getDeclaredMethod("get", String.class);
258+
PreAuthorize pre = this.scanner.scan(method, method.getDeclaringClass());
259+
assertThat(pre).isNotNull();
260+
}
261+
262+
// gh-16751
263+
@Test
264+
void scanWhenAnnotationOnParameterizedSuperClassThenLocates() throws Exception {
265+
Method method = MyServiceImpl.class.getDeclaredMethod("getExt", Long.class);
266+
PreAuthorize pre = this.scanner.scan(method, method.getDeclaringClass());
267+
assertThat(pre).isNotNull();
268+
}
269+
270+
// gh-16751
271+
@Test
272+
void scanWhenAnnotationOnParameterizedMethodThenLocates() throws Exception {
273+
Method method = MyServiceImpl.class.getDeclaredMethod("getExtByClass", Class.class, Long.class);
274+
PreAuthorize pre = this.scanner.scan(method, method.getDeclaringClass());
275+
assertThat(pre).isNotNull();
276+
}
277+
254278
@PreAuthorize("one")
255279
private interface AnnotationOnInterface {
256280

@@ -577,4 +601,40 @@ private static class ClassInheritingAbstractClassNoAnnotations extends AbstractC
577601

578602
}
579603

604+
interface MyService<C, U> {
605+
606+
@PreAuthorize("thirty")
607+
C get(U u);
608+
609+
}
610+
611+
abstract static class MyServiceExt<T> implements MyService<Integer, String> {
612+
613+
@PreAuthorize("thirtyone")
614+
abstract T getExt(T t);
615+
616+
@PreAuthorize("thirtytwo")
617+
abstract <S extends Number> S getExtByClass(Class<S> clazz, T t);
618+
619+
}
620+
621+
static class MyServiceImpl extends MyServiceExt<Long> {
622+
623+
@Override
624+
public Integer get(final String s) {
625+
return 0;
626+
}
627+
628+
@Override
629+
Long getExt(Long o) {
630+
return 0L;
631+
}
632+
633+
@Override
634+
<S extends Number> S getExtByClass(Class<S> clazz, Long l) {
635+
return null;
636+
}
637+
638+
}
639+
580640
}

0 commit comments

Comments
 (0)