Reject invalid afterThrowing signature on ThrowsAdvice

Closes gh-1896

spring-aop/src/main/java/org/springframework/aop/framework/adapter/ThrowsAdviceInterceptor.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2020 the original author or authors.
+ * Copyright 2002-2023 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.
@@ -27,6 +27,7 @@ import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
 
 import org.springframework.aop.AfterAdvice;
+import org.springframework.aop.framework.AopConfigException;
 import org.springframework.lang.Nullable;
 import org.springframework.util.Assert;
 
@@ -78,21 +79,44 @@ public class ThrowsAdviceInterceptor implements MethodInterceptor, AfterAdvice {
 
 		Method[] methods = throwsAdvice.getClass().getMethods();
 		for (Method method : methods) {
-			if (method.getName().equals(AFTER_THROWING) &&
+			if (method.getName().equals(AFTER_THROWING)) {
-					(method.getParameterCount() == 1 || method.getParameterCount() == 4)) {
+				Class<?> throwableParam = null;
-				Class<?> throwableParam = method.getParameterTypes()[method.getParameterCount() - 1];
+				if (method.getParameterCount() == 1) {
-				if (Throwable.class.isAssignableFrom(throwableParam)) {
+					// just a Throwable parameter
+					throwableParam = method.getParameterTypes()[0];
+					if (!Throwable.class.isAssignableFrom(throwableParam)) {
+						throw new AopConfigException("Invalid afterThrowing signature: " +
+								"single argument must be a Throwable subclass");
+					}
+				}
+				else if (method.getParameterCount() == 4) {
+					// Method, Object[], target, throwable
+					Class<?>[] paramTypes = method.getParameterTypes();
+					if (!Method.class.equals(paramTypes[0]) || !Object[].class.equals(paramTypes[1]) ||
+							Throwable.class.equals(paramTypes[2]) || !Throwable.class.isAssignableFrom(paramTypes[3])) {
+						throw new AopConfigException("Invalid afterThrowing signature: " +
+								"four arguments must be Method, Object[], target, throwable: " + method);
+					}
+					throwableParam = paramTypes[3];
+				}
+				if (throwableParam == null) {
+					throw new AopConfigException("Unsupported afterThrowing signature: single throwable argument " +
+							"or four arguments Method, Object[], target, throwable expected: " + method);
+				}
 				// An exception handler to register...
-					this.exceptionHandlerMap.put(throwableParam, method);
+				Method existingMethod = this.exceptionHandlerMap.put(throwableParam, method);
+				if (existingMethod != null) {
+					throw new AopConfigException("Only one afterThrowing method per specific Throwable subclass " +
+							"allowed: " + method + " / " + existingMethod);
+				}
 				if (logger.isDebugEnabled()) {
 					logger.debug("Found exception handler method on throws advice: " + method);
 				}
 			}
 		}
-		}
 
 		if (this.exceptionHandlerMap.isEmpty()) {
-			throw new IllegalArgumentException(
+			throw new AopConfigException(
 					"At least one handler method must be found in class [" + throwsAdvice.getClass() + "]");
 		}
 	}

spring-aop/src/test/java/org/springframework/aop/framework/adapter/ThrowsAdviceInterceptorTests.java

@@ -23,25 +23,26 @@ import java.rmi.RemoteException;
 import org.aopalliance.intercept.MethodInvocation;
 import org.junit.jupiter.api.Test;
 
+import org.springframework.aop.framework.AopConfigException;
 import org.springframework.aop.testfixture.advice.MyThrowsHandler;
 
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.assertj.core.api.Assertions.assertThatException;
 import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
-import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
 import static org.mockito.BDDMockito.given;
 import static org.mockito.Mockito.mock;
 
 /**
  * @author Rod Johnson
  * @author Chris Beams
+ * @author Juergen Hoeller
  */
 public class ThrowsAdviceInterceptorTests {
 
 	@Test
 	public void testNoHandlerMethods() {
 		// should require one handler method at least
-		assertThatIllegalArgumentException().isThrownBy(() ->
+		assertThatExceptionOfType(AopConfigException.class).isThrownBy(() ->
 				new ThrowsAdviceInterceptor(new Object()));
 	}
 
@@ -77,9 +78,7 @@ public class ThrowsAdviceInterceptorTests {
 		given(mi.getMethod()).willReturn(Object.class.getMethod("hashCode"));
 		given(mi.getThis()).willReturn(new Object());
 		given(mi.proceed()).willThrow(ex);
-		assertThatExceptionOfType(FileNotFoundException.class).isThrownBy(() ->
+		assertThatExceptionOfType(FileNotFoundException.class).isThrownBy(() -> ti.invoke(mi)).isSameAs(ex);
-				ti.invoke(mi))
-			.isSameAs(ex);
 		assertThat(th.getCalls()).isEqualTo(1);
 		assertThat(th.getCalls("ioException")).isEqualTo(1);
 	}
@@ -92,9 +91,7 @@ public class ThrowsAdviceInterceptorTests {
 		ConnectException ex = new ConnectException("");
 		MethodInvocation mi = mock();
 		given(mi.proceed()).willThrow(ex);
-		assertThatExceptionOfType(ConnectException.class).isThrownBy(() ->
+		assertThatExceptionOfType(ConnectException.class).isThrownBy(() -> ti.invoke(mi)).isSameAs(ex);
-				ti.invoke(mi))
-			.isSameAs(ex);
 		assertThat(th.getCalls()).isEqualTo(1);
 		assertThat(th.getCalls("remoteException")).isEqualTo(1);
 	}
@@ -117,9 +114,7 @@ public class ThrowsAdviceInterceptorTests {
 		ConnectException ex = new ConnectException("");
 		MethodInvocation mi = mock();
 		given(mi.proceed()).willThrow(ex);
-		assertThatExceptionOfType(Throwable.class).isThrownBy(() ->
+		assertThatExceptionOfType(Throwable.class).isThrownBy(() -> ti.invoke(mi)).isSameAs(t);
-				ti.invoke(mi))
-			.isSameAs(t);
 		assertThat(th.getCalls()).isEqualTo(1);
 		assertThat(th.getCalls("remoteException")).isEqualTo(1);
 	}

spring-aop/src/testFixtures/java/org/springframework/aop/testfixture/advice/MyThrowsHandler.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2023 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.
@@ -34,9 +34,4 @@ public class MyThrowsHandler extends MethodCounter implements ThrowsAdvice {
 		count("remoteException");
 	}
 
-	/** Not valid, wrong number of arguments */
-	public void afterThrowing(Method m, Exception ex) throws Throwable {
-		throw new UnsupportedOperationException("Shouldn't be called");
-	}
-
 }