MergedAnnotations finds duplicate annotations on method in multi-level interface hierarchy

[jzheaux]

Related to spring-projects/spring-security#13625 (GitHub reproducer)

In a hierarchy like this:

interface Hello {
    @PreAuthorize("...")
    void sayHello();
}

interface SayHello extends Hello {}

class HelloImpl implements SayHello {
    public void sayHello() {}
}

a call to MergedAnnoatations like this:

MergedAnnotations mergedAnnotations = MergedAnnotations.from(HelloImpl.class.getMethod("sayHello"),
	SearchStrategy.TYPE_HIERARCHY);

will return multiple instances of MergedAnnotation for PreAuthorize.class.

It's expected that such an arrangement would only produce one MergedAnnotation instance since there is only one in the hierarchy.

Thanks to @philwebb for helping me find a workaround. Since PreAuthorize is not a repeatable annotation, Spring Security can ignore subsequent MergedAnnotation instances from the same MergedAnnotation#getSource.

[sbrannen]

Related workaround in Spring Security: spring-projects/spring-security@be11812

[sbrannen]

Thanks for reporting the issue. 👍

I've been able to confirm it with the following standalone test class.

package demo;

import java.lang.annotation.ElementType;
import java.lang.annotation.Retention;
import java.lang.annotation.RetentionPolicy;
import java.lang.annotation.Target;
import java.lang.reflect.Method;

import org.junit.jupiter.api.Test;

import org.springframework.core.annotation.MergedAnnotation;
import org.springframework.core.annotation.MergedAnnotations;

import static org.assertj.core.api.Assertions.assertThat;
import static org.springframework.core.annotation.MergedAnnotations.search;
import static org.springframework.core.annotation.MergedAnnotations.SearchStrategy.TYPE_HIERARCHY;

class ReproTests {

	@Test
	void test() throws Exception {
		Method method = HelloImpl.class.getMethod("sayHello");
		Stream<PreAuthorize> stream = search(TYPE_HIERARCHY)
				.from(method)
				.stream(PreAuthorize.class)
				.map(MergedAnnotation::synthesize);

		assertThat(stream).hasSize(1);
	}


	interface Hello {

		@PreAuthorize("demo")
		void sayHello();
	}

	interface SayHello extends Hello {
	}

	static class HelloImpl implements SayHello {

		@Override
		public void sayHello() {
		}
	}

	@Target(ElementType.METHOD)
	@Retention(RetentionPolicy.RUNTIME)
	@interface PreAuthorize {
		String value();
	}

}

The above fails with:

java.lang.AssertionError: 
Expected size: 1 but was: 2 in:
[@demo.ReproTests$PreAuthorize("demo"), @demo.ReproTests$PreAuthorize("demo")]
	at demo.ReproTests.test(ReproTests.java:42)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

Furthermore, I verified that the test passes if HelloImpl implements Hello directly.

[sbrannen]

This has been fixed in 75da9c3 for inclusion in the upcoming Spring Framework 6.1.2 release.

Because this bug has existed since 5.2 when the MergedAnnotations API was introduced, the fix has also been backported to 6.0.15 and 5.3.32.