From c733ae0f22cacfa67398b6291a419867938a7475 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Simon=20Basl=C3=A9?= Date: Wed, 10 May 2023 14:52:52 +0200 Subject: [PATCH] Fix ApplicationListenerMethodAdapter#supportsEventType check This commit fixes the check by avoiding a fallback to eventType's hasUnresolvableGenerics(). This could previously lead to checking a generic event type `A` against a listener which accepts unrelated `B` and return `true` despite the inconsistency. Note that this wouldn't necessarily surface to the user because there is a `catch (ClassCastException e)` down the line, which was primarily put in place to deal with lambda-based listeners but happens to catch an exception thrown due to the bad result of `supportsEventType`. The `supportsEventType` now matches generic `PayloadApplicationEvent` types with a raw counterpart, using the above fallback only in that case and otherwise ultimately returning `false`. Closes gh-30399 --- .../ApplicationListenerMethodAdapter.java | 4 +- ...ApplicationListenerMethodAdapterTests.java | 76 +++++++++++++++++++ 2 files changed, 78 insertions(+), 2 deletions(-) diff --git a/spring-context/src/main/java/org/springframework/context/event/ApplicationListenerMethodAdapter.java b/spring-context/src/main/java/org/springframework/context/event/ApplicationListenerMethodAdapter.java index 0ad22b94d4d..ee8783a0964 100644 --- a/spring-context/src/main/java/org/springframework/context/event/ApplicationListenerMethodAdapter.java +++ b/spring-context/src/main/java/org/springframework/context/event/ApplicationListenerMethodAdapter.java @@ -173,12 +173,12 @@ public class ApplicationListenerMethodAdapter implements GenericApplicationListe } if (PayloadApplicationEvent.class.isAssignableFrom(eventType.toClass())) { ResolvableType payloadType = eventType.as(PayloadApplicationEvent.class).getGeneric(); - if (declaredEventType.isAssignableFrom(payloadType)) { + if (declaredEventType.isAssignableFrom(payloadType) || eventType.hasUnresolvableGenerics()) { return true; } } } - return eventType.hasUnresolvableGenerics(); + return false; } @Override diff --git a/spring-context/src/test/java/org/springframework/context/event/ApplicationListenerMethodAdapterTests.java b/spring-context/src/test/java/org/springframework/context/event/ApplicationListenerMethodAdapterTests.java index 1d6775f9a13..edc69829742 100644 --- a/spring-context/src/test/java/org/springframework/context/event/ApplicationListenerMethodAdapterTests.java +++ b/spring-context/src/test/java/org/springframework/context/event/ApplicationListenerMethodAdapterTests.java @@ -19,6 +19,7 @@ package org.springframework.context.event; import java.io.IOException; import java.lang.reflect.Method; import java.lang.reflect.UndeclaredThrowableException; +import java.util.List; import org.junit.jupiter.api.Test; @@ -325,6 +326,77 @@ public class ApplicationListenerMethodAdapterTests extends AbstractApplicationEv verify(this.context, times(2)).getBean("testBean"); } + // see https://github.com/spring-projects/spring-framework/issues/30399 + @Test + void simplePayloadDoesNotSupportArbitraryGenericEventType() throws Exception { + var method = SampleEvents.class.getDeclaredMethod("handleString", String.class); + var adapter = new ApplicationListenerMethodAdapter(null, ApplicationListenerMethodAdapterTests.class, method); + + assertThat(adapter.supportsEventType(ResolvableType.forClassWithGenerics(EntityWrapper.class, Integer.class))) + .as("handleString(String) with EntityWrapper").isFalse(); + assertThat(adapter.supportsEventType(ResolvableType.forClass(EntityWrapper.class))) + .as("handleString(String) with EntityWrapper").isFalse(); + assertThat(adapter.supportsEventType(ResolvableType.forClass(String.class))) + .as("handleString(String) with String").isTrue(); + } + + // see https://github.com/spring-projects/spring-framework/issues/30399 + @Test + void genericPayloadDoesNotSupportArbitraryGenericEventType() throws Exception { + var method = SampleEvents.class.getDeclaredMethod("handleGenericStringPayload", EntityWrapper.class); + var adapter = new ApplicationListenerMethodAdapter(null, ApplicationListenerMethodAdapterTests.class, method); + + assertThat(adapter.supportsEventType(ResolvableType.forClass(EntityWrapper.class))) + .as("handleGenericStringPayload(EntityWrapper) with EntityWrapper").isFalse(); + assertThat(adapter.supportsEventType(ResolvableType.forClassWithGenerics(EntityWrapper.class, Integer.class))) + .as("handleGenericStringPayload(EntityWrapper) with EntityWrapper").isFalse(); + assertThat(adapter.supportsEventType(ResolvableType.forClassWithGenerics(EntityWrapper.class, String.class))) + .as("handleGenericStringPayload(EntityWrapper) with EntityWrapper").isTrue(); + } + + // see https://github.com/spring-projects/spring-framework/issues/30399 + @Test + void rawGenericPayloadDoesNotSupportArbitraryGenericEventType() throws Exception { + var method = SampleEvents.class.getDeclaredMethod("handleGenericAnyPayload", EntityWrapper.class); + var adapter = new ApplicationListenerMethodAdapter(null, ApplicationListenerMethodAdapterTests.class, method); + + assertThat(adapter.supportsEventType(ResolvableType.forClass(EntityWrapper.class))) + .as("handleGenericAnyPayload(EntityWrapper) with EntityWrapper").isTrue(); + assertThat(adapter.supportsEventType(ResolvableType.forClassWithGenerics(EntityWrapper.class, Integer.class))) + .as("handleGenericAnyPayload(EntityWrapper) with EntityWrapper").isTrue(); + assertThat(adapter.supportsEventType(ResolvableType.forClassWithGenerics(EntityWrapper.class, String.class))) + .as("handleGenericAnyPayload(EntityWrapper) with EntityWrapper").isTrue(); + assertThat(adapter.supportsEventType(ResolvableType.forClass(List.class))) + .as("handleGenericAnyPayload(EntityWrapper) with List").isFalse(); + assertThat(adapter.supportsEventType(ResolvableType.forClassWithGenerics(List.class, String.class))) + .as("handleGenericAnyPayload(EntityWrapper) with List").isFalse(); + } + + @Test + void genericApplicationEventSupportsSpecificType() throws Exception { + var method = SampleEvents.class.getDeclaredMethod("handleGenericString", GenericTestEvent.class); + var adapter = new ApplicationListenerMethodAdapter(null, ApplicationListenerMethodAdapterTests.class, method); + + assertThat(adapter.supportsEventType(ResolvableType.forClass(GenericTestEvent.class))) + .as("handleGenericString(GenericTestEvent) with GenericTestEvent").isFalse(); + assertThat(adapter.supportsEventType(ResolvableType.forClassWithGenerics(GenericTestEvent.class, Integer.class))) + .as("handleGenericString(GenericTestEvent) with GenericTestEvent").isFalse(); + assertThat(adapter.supportsEventType(ResolvableType.forClassWithGenerics(GenericTestEvent.class, String.class))) + .as("handleGenericString(GenericTestEvent) with GenericTestEvent").isTrue(); + } + + @Test + void genericRawApplicationEventSupportsRawTypeAndAnySpecificType() throws Exception { + var method = SampleEvents.class.getDeclaredMethod("handleGenericRaw", GenericTestEvent.class); + var adapter = new ApplicationListenerMethodAdapter(null, ApplicationListenerMethodAdapterTests.class, method); + + assertThat(adapter.supportsEventType(ResolvableType.forClass(GenericTestEvent.class))) + .as("handleGenericRaw(GenericTestEvent) with GenericTestEvent").isTrue(); + assertThat(adapter.supportsEventType(ResolvableType.forClassWithGenerics(GenericTestEvent.class, String.class))) + .as("handleGenericRaw(GenericTestEvent) with GenericTestEvent").isTrue(); + assertThat(adapter.supportsEventType(ResolvableType.forClassWithGenerics(GenericTestEvent.class, Integer.class))) + .as("handleGenericRaw(GenericTestEvent) with GenericTestEvent").isTrue(); + } private void supportsEventType(boolean match, Method method, ResolvableType eventType) { ApplicationListenerMethodAdapter adapter = createTestInstance(method); @@ -373,6 +445,10 @@ public class ApplicationListenerMethodAdapterTests extends AbstractApplicationEv public void handleGenericString(GenericTestEvent event) { } + @EventListener + public void handleGenericRaw(GenericTestEvent event) { + } + @EventListener public void handleString(String payload) { }