From e8cc6d24096e07e517ca3afdf7e6b6ee6d46b997 Mon Sep 17 00:00:00 2001 From: panlingxiao <784580872@qq.com> Date: Tue, 24 Jul 2018 21:47:31 +0800 Subject: [PATCH 1/7] check afterThrowing signature --- .../aop/MethodSignatureException.java | 29 +++++++++++++++++++ .../MethodBeforeAdviceInterceptor.java | 3 +- .../adapter/ThrowsAdviceInterceptor.java | 13 +++++++++ 3 files changed, 44 insertions(+), 1 deletion(-) create mode 100644 spring-aop/src/main/java/org/springframework/aop/MethodSignatureException.java diff --git a/spring-aop/src/main/java/org/springframework/aop/MethodSignatureException.java b/spring-aop/src/main/java/org/springframework/aop/MethodSignatureException.java new file mode 100644 index 000000000000..e8646f3a9727 --- /dev/null +++ b/spring-aop/src/main/java/org/springframework/aop/MethodSignatureException.java @@ -0,0 +1,29 @@ +package org.springframework.aop; + +/** + * A MethodSignatureException is thrown when afterThrowing that owns 4 params in ThrowsAdvice is invalid + * + * @author panlingxiao + * @see ThrowsAdvice + */ +public class MethodSignatureException extends RuntimeException { + + /** + * Create an MethodSignatureException with a specific message. + * + * @param message the message + */ + public MethodSignatureException(String message) { + super(message); + } + + /** + * Create an MethodSignatureException with a specific message and cause. + * + * @param message the message + * @param cause the cause + */ + public MethodSignatureException(String message, Exception cause) { + super(message, cause); + } +} diff --git a/spring-aop/src/main/java/org/springframework/aop/framework/adapter/MethodBeforeAdviceInterceptor.java b/spring-aop/src/main/java/org/springframework/aop/framework/adapter/MethodBeforeAdviceInterceptor.java index 9ede67ef9818..daa1a4f6d4ff 100644 --- a/spring-aop/src/main/java/org/springframework/aop/framework/adapter/MethodBeforeAdviceInterceptor.java +++ b/spring-aop/src/main/java/org/springframework/aop/framework/adapter/MethodBeforeAdviceInterceptor.java @@ -21,6 +21,7 @@ import org.aopalliance.intercept.MethodInterceptor; import org.aopalliance.intercept.MethodInvocation; +import org.springframework.aop.BeforeAdvice; import org.springframework.aop.MethodBeforeAdvice; import org.springframework.util.Assert; @@ -32,7 +33,7 @@ * @author Rod Johnson */ @SuppressWarnings("serial") -public class MethodBeforeAdviceInterceptor implements MethodInterceptor, Serializable { +public class MethodBeforeAdviceInterceptor implements MethodInterceptor, BeforeAdvice, Serializable { private MethodBeforeAdvice advice; diff --git a/spring-aop/src/main/java/org/springframework/aop/framework/adapter/ThrowsAdviceInterceptor.java b/spring-aop/src/main/java/org/springframework/aop/framework/adapter/ThrowsAdviceInterceptor.java index 5c0f7ee6c21d..d8d35a5e5c55 100644 --- a/spring-aop/src/main/java/org/springframework/aop/framework/adapter/ThrowsAdviceInterceptor.java +++ b/spring-aop/src/main/java/org/springframework/aop/framework/adapter/ThrowsAdviceInterceptor.java @@ -27,6 +27,7 @@ import org.apache.commons.logging.LogFactory; import org.springframework.aop.AfterAdvice; +import org.springframework.aop.MethodSignatureException; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -81,6 +82,7 @@ public ThrowsAdviceInterceptor(Object throwsAdvice) { (method.getParameterCount() == 1 || method.getParameterCount() == 4) && Throwable.class.isAssignableFrom(method.getParameterTypes()[method.getParameterCount() - 1]) ) { + checkMethodSignature(method); // Have an exception handler this.exceptionHandlerMap.put(method.getParameterTypes()[method.getParameterCount() - 1], method); if (logger.isDebugEnabled()) { @@ -151,4 +153,15 @@ private void invokeHandlerMethod(MethodInvocation mi, Throwable ex, Method metho } } + private void checkMethodSignature(Method method) { + if (method.getParameterCount() == 1) { + return; + } + Class[] parameterTypes = method.getParameterTypes(); + if (!parameterTypes[0].isAssignableFrom(Method.class) || !parameterTypes[1].isAssignableFrom(Object[].class)) { + throw new MethodSignatureException("Method signature is illegal,the signature of method that owns 4 params " + + "must be (Method method, Object[] args, Object target, Exception e)"); + } + } + } From b2ff87b663dff72d04059fff62a6e1a063f1e081 Mon Sep 17 00:00:00 2001 From: panlingxiao <784580872@qq.com> Date: Wed, 25 Jul 2018 23:42:00 +0800 Subject: [PATCH 2/7] add comment --- .../aop/framework/adapter/ThrowsAdviceInterceptor.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/spring-aop/src/main/java/org/springframework/aop/framework/adapter/ThrowsAdviceInterceptor.java b/spring-aop/src/main/java/org/springframework/aop/framework/adapter/ThrowsAdviceInterceptor.java index d8d35a5e5c55..f23925efeb54 100644 --- a/spring-aop/src/main/java/org/springframework/aop/framework/adapter/ThrowsAdviceInterceptor.java +++ b/spring-aop/src/main/java/org/springframework/aop/framework/adapter/ThrowsAdviceInterceptor.java @@ -158,9 +158,10 @@ private void checkMethodSignature(Method method) { return; } Class[] parameterTypes = method.getParameterTypes(); + // check param order if (!parameterTypes[0].isAssignableFrom(Method.class) || !parameterTypes[1].isAssignableFrom(Object[].class)) { throw new MethodSignatureException("Method signature is illegal,the signature of method that owns 4 params " - + "must be (Method method, Object[] args, Object target, Exception e)"); + + "must be (Method method, Object[] args, Object target, ThrowableSubclass e)"); } } From 6608cc515f689e8c4b540d80354ab6e8a47eff52 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 25 Jul 2018 19:03:13 +0200 Subject: [PATCH 3/7] ListBasedXMLEventReader uses defensive modifiable copy of given List --- .../util/xml/ListBasedXMLEventReader.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/util/xml/ListBasedXMLEventReader.java b/spring-core/src/main/java/org/springframework/util/xml/ListBasedXMLEventReader.java index 6395ea1e99aa..ed79471b3d48 100644 --- a/spring-core/src/main/java/org/springframework/util/xml/ListBasedXMLEventReader.java +++ b/spring-core/src/main/java/org/springframework/util/xml/ListBasedXMLEventReader.java @@ -16,7 +16,7 @@ package org.springframework.util.xml; -import java.util.Collections; +import java.util.ArrayList; import java.util.List; import java.util.NoSuchElementException; import javax.xml.stream.events.XMLEvent; @@ -25,7 +25,8 @@ import org.springframework.util.Assert; /** - * Implementation of {@code XMLEventReader} based on a list of {@link XMLEvent XMLEvents}. + * Implementation of {@code XMLEventReader} based on a {@link List} + * of {@link XMLEvent} elements. * * @author Arjen Poutsma * @since 5.0 @@ -38,8 +39,8 @@ class ListBasedXMLEventReader extends AbstractXMLEventReader { public ListBasedXMLEventReader(List events) { - Assert.notNull(events, "'events' must not be null"); - this.events = Collections.unmodifiableList(events); + Assert.notNull(events, "XMLEvent List must not be null"); + this.events = new ArrayList<>(events); } From 9454fa4b5c6a3bf7ad206a71b2c69e5d00a75a65 Mon Sep 17 00:00:00 2001 From: Juergen Hoeller Date: Wed, 25 Jul 2018 20:12:14 +0200 Subject: [PATCH 4/7] MethodBeforeAdviceInterceptor implements BeforeAdvice marker interface Includes related polishing in the advice interceptor implementations. Issue: SPR-17088 --- .../AfterReturningAdviceInterceptor.java | 5 +- .../MethodBeforeAdviceInterceptor.java | 7 +- .../adapter/ThrowsAdviceInterceptor.java | 75 +++++++++---------- 3 files changed, 43 insertions(+), 44 deletions(-) diff --git a/spring-aop/src/main/java/org/springframework/aop/framework/adapter/AfterReturningAdviceInterceptor.java b/spring-aop/src/main/java/org/springframework/aop/framework/adapter/AfterReturningAdviceInterceptor.java index 34fd15378e4f..82e5856250b6 100644 --- a/spring-aop/src/main/java/org/springframework/aop/framework/adapter/AfterReturningAdviceInterceptor.java +++ b/spring-aop/src/main/java/org/springframework/aop/framework/adapter/AfterReturningAdviceInterceptor.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2017 the original author or authors. + * Copyright 2002-2018 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. @@ -31,6 +31,8 @@ * to use this class directly. * * @author Rod Johnson + * @see MethodBeforeAdviceInterceptor + * @see ThrowsAdviceInterceptor */ @SuppressWarnings("serial") public class AfterReturningAdviceInterceptor implements MethodInterceptor, AfterAdvice, Serializable { @@ -47,6 +49,7 @@ public AfterReturningAdviceInterceptor(AfterReturningAdvice advice) { this.advice = advice; } + @Override public Object invoke(MethodInvocation mi) throws Throwable { Object retVal = mi.proceed(); diff --git a/spring-aop/src/main/java/org/springframework/aop/framework/adapter/MethodBeforeAdviceInterceptor.java b/spring-aop/src/main/java/org/springframework/aop/framework/adapter/MethodBeforeAdviceInterceptor.java index daa1a4f6d4ff..a58e27cdc366 100644 --- a/spring-aop/src/main/java/org/springframework/aop/framework/adapter/MethodBeforeAdviceInterceptor.java +++ b/spring-aop/src/main/java/org/springframework/aop/framework/adapter/MethodBeforeAdviceInterceptor.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2012 the original author or authors. + * Copyright 2002-2018 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. @@ -31,11 +31,13 @@ * to use this class directly. * * @author Rod Johnson + * @see AfterReturningAdviceInterceptor + * @see ThrowsAdviceInterceptor */ @SuppressWarnings("serial") public class MethodBeforeAdviceInterceptor implements MethodInterceptor, BeforeAdvice, Serializable { - private MethodBeforeAdvice advice; + private final MethodBeforeAdvice advice; /** @@ -47,6 +49,7 @@ public MethodBeforeAdviceInterceptor(MethodBeforeAdvice advice) { this.advice = advice; } + @Override public Object invoke(MethodInvocation mi) throws Throwable { this.advice.before(mi.getMethod(), mi.getArguments(), mi.getThis()); diff --git a/spring-aop/src/main/java/org/springframework/aop/framework/adapter/ThrowsAdviceInterceptor.java b/spring-aop/src/main/java/org/springframework/aop/framework/adapter/ThrowsAdviceInterceptor.java index f23925efeb54..2e3a791d0066 100644 --- a/spring-aop/src/main/java/org/springframework/aop/framework/adapter/ThrowsAdviceInterceptor.java +++ b/spring-aop/src/main/java/org/springframework/aop/framework/adapter/ThrowsAdviceInterceptor.java @@ -27,7 +27,6 @@ import org.apache.commons.logging.LogFactory; import org.springframework.aop.AfterAdvice; -import org.springframework.aop.MethodSignatureException; import org.springframework.lang.Nullable; import org.springframework.util.Assert; @@ -52,6 +51,8 @@ * * @author Rod Johnson * @author Juergen Hoeller + * @see MethodBeforeAdviceInterceptor + * @see AfterReturningAdviceInterceptor */ public class ThrowsAdviceInterceptor implements MethodInterceptor, AfterAdvice { @@ -68,9 +69,8 @@ public class ThrowsAdviceInterceptor implements MethodInterceptor, AfterAdvice { /** * Create a new ThrowsAdviceInterceptor for the given ThrowsAdvice. - * @param throwsAdvice the advice object that defines the exception - * handler methods (usually a {@link org.springframework.aop.ThrowsAdvice} - * implementation) + * @param throwsAdvice the advice object that defines the exception handler methods + * (usually a {@link org.springframework.aop.ThrowsAdvice} implementation) */ public ThrowsAdviceInterceptor(Object throwsAdvice) { Assert.notNull(throwsAdvice, "Advice must not be null"); @@ -79,14 +79,14 @@ public ThrowsAdviceInterceptor(Object throwsAdvice) { Method[] methods = throwsAdvice.getClass().getMethods(); for (Method method : methods) { if (method.getName().equals(AFTER_THROWING) && - (method.getParameterCount() == 1 || method.getParameterCount() == 4) && - Throwable.class.isAssignableFrom(method.getParameterTypes()[method.getParameterCount() - 1]) - ) { - checkMethodSignature(method); - // Have an exception handler - this.exceptionHandlerMap.put(method.getParameterTypes()[method.getParameterCount() - 1], method); - if (logger.isDebugEnabled()) { - logger.debug("Found exception handler method: " + method); + (method.getParameterCount() == 1 || method.getParameterCount() == 4)) { + Class throwableParam = method.getParameterTypes()[method.getParameterCount() - 1]; + if (Throwable.class.isAssignableFrom(throwableParam)) { + // An exception handler to register... + this.exceptionHandlerMap.put(throwableParam, method); + if (logger.isDebugEnabled()) { + logger.debug("Found exception handler method on throws advice: " + method); + } } } } @@ -97,14 +97,33 @@ public ThrowsAdviceInterceptor(Object throwsAdvice) { } } + + /** + * Return the number of handler methods in this advice. + */ public int getHandlerMethodCount() { return this.exceptionHandlerMap.size(); } + + @Override + public Object invoke(MethodInvocation mi) throws Throwable { + try { + return mi.proceed(); + } + catch (Throwable ex) { + Method handlerMethod = getExceptionHandler(ex); + if (handlerMethod != null) { + invokeHandlerMethod(mi, ex, handlerMethod); + } + throw ex; + } + } + /** - * Determine the exception handle method. Can return null if not found. + * Determine the exception handle method for the given exception. * @param exception the exception thrown - * @return a handler for the given exception type + * @return a handler for the given exception type, or {@code null} if none found */ @Nullable private Method getExceptionHandler(Throwable exception) { @@ -123,24 +142,10 @@ private Method getExceptionHandler(Throwable exception) { return handler; } - @Override - public Object invoke(MethodInvocation mi) throws Throwable { - try { - return mi.proceed(); - } - catch (Throwable ex) { - Method handlerMethod = getExceptionHandler(ex); - if (handlerMethod != null) { - invokeHandlerMethod(mi, ex, handlerMethod); - } - throw ex; - } - } - private void invokeHandlerMethod(MethodInvocation mi, Throwable ex, Method method) throws Throwable { Object[] handlerArgs; if (method.getParameterCount() == 1) { - handlerArgs = new Object[] { ex }; + handlerArgs = new Object[] {ex}; } else { handlerArgs = new Object[] {mi.getMethod(), mi.getArguments(), mi.getThis(), ex}; @@ -153,16 +158,4 @@ private void invokeHandlerMethod(MethodInvocation mi, Throwable ex, Method metho } } - private void checkMethodSignature(Method method) { - if (method.getParameterCount() == 1) { - return; - } - Class[] parameterTypes = method.getParameterTypes(); - // check param order - if (!parameterTypes[0].isAssignableFrom(Method.class) || !parameterTypes[1].isAssignableFrom(Object[].class)) { - throw new MethodSignatureException("Method signature is illegal,the signature of method that owns 4 params " - + "must be (Method method, Object[] args, Object target, ThrowableSubclass e)"); - } - } - } From 05deb38994ff6b739d2c9b2a3b3cc8eae9025977 Mon Sep 17 00:00:00 2001 From: panlingxiao <784580872@qq.com> Date: Wed, 25 Jul 2018 23:42:00 +0800 Subject: [PATCH 5/7] add comment --- .../framework/adapter/ThrowsAdviceInterceptor.java | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/spring-aop/src/main/java/org/springframework/aop/framework/adapter/ThrowsAdviceInterceptor.java b/spring-aop/src/main/java/org/springframework/aop/framework/adapter/ThrowsAdviceInterceptor.java index 2e3a791d0066..9b4d99ac04ed 100644 --- a/spring-aop/src/main/java/org/springframework/aop/framework/adapter/ThrowsAdviceInterceptor.java +++ b/spring-aop/src/main/java/org/springframework/aop/framework/adapter/ThrowsAdviceInterceptor.java @@ -158,4 +158,16 @@ private void invokeHandlerMethod(MethodInvocation mi, Throwable ex, Method metho } } + private void checkMethodSignature(Method method) { + if (method.getParameterCount() == 1) { + return; + } + Class[] parameterTypes = method.getParameterTypes(); + // check param order + if (!parameterTypes[0].isAssignableFrom(Method.class) || !parameterTypes[1].isAssignableFrom(Object[].class)) { + throw new MethodSignatureException("Method signature is illegal,the signature of method that owns 4 params " + + "must be (Method method, Object[] args, Object target, ThrowableSubclass e)"); + } + } + } From bfa381a5af693ae11ed80117bf0c28415824d5cc Mon Sep 17 00:00:00 2001 From: panlingxiao <784580872@qq.com> Date: Thu, 26 Jul 2018 02:53:31 +0800 Subject: [PATCH 6/7] validating reflective signatures --- .../aop/framework/adapter/ThrowsAdviceInterceptor.java | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/spring-aop/src/main/java/org/springframework/aop/framework/adapter/ThrowsAdviceInterceptor.java b/spring-aop/src/main/java/org/springframework/aop/framework/adapter/ThrowsAdviceInterceptor.java index 9b4d99ac04ed..9e8784d51549 100644 --- a/spring-aop/src/main/java/org/springframework/aop/framework/adapter/ThrowsAdviceInterceptor.java +++ b/spring-aop/src/main/java/org/springframework/aop/framework/adapter/ThrowsAdviceInterceptor.java @@ -27,6 +27,7 @@ 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; @@ -82,6 +83,7 @@ public ThrowsAdviceInterceptor(Object throwsAdvice) { (method.getParameterCount() == 1 || method.getParameterCount() == 4)) { Class throwableParam = method.getParameterTypes()[method.getParameterCount() - 1]; if (Throwable.class.isAssignableFrom(throwableParam)) { + checkMethodSignature(method); // An exception handler to register... this.exceptionHandlerMap.put(throwableParam, method); if (logger.isDebugEnabled()) { @@ -165,7 +167,7 @@ private void checkMethodSignature(Method method) { Class[] parameterTypes = method.getParameterTypes(); // check param order if (!parameterTypes[0].isAssignableFrom(Method.class) || !parameterTypes[1].isAssignableFrom(Object[].class)) { - throw new MethodSignatureException("Method signature is illegal,the signature of method that owns 4 params " + throw new AopConfigException("Method signature is illegal,the signature of method that owns 4 params " + "must be (Method method, Object[] args, Object target, ThrowableSubclass e)"); } } From be32e39d82320342b349c137c56cfe0a78144de2 Mon Sep 17 00:00:00 2001 From: panlingxiao <784580872@qq.com> Date: Thu, 26 Jul 2018 02:58:18 +0800 Subject: [PATCH 7/7] remove uncommon exception --- .../aop/MethodSignatureException.java | 29 ------------------- 1 file changed, 29 deletions(-) delete mode 100644 spring-aop/src/main/java/org/springframework/aop/MethodSignatureException.java diff --git a/spring-aop/src/main/java/org/springframework/aop/MethodSignatureException.java b/spring-aop/src/main/java/org/springframework/aop/MethodSignatureException.java deleted file mode 100644 index e8646f3a9727..000000000000 --- a/spring-aop/src/main/java/org/springframework/aop/MethodSignatureException.java +++ /dev/null @@ -1,29 +0,0 @@ -package org.springframework.aop; - -/** - * A MethodSignatureException is thrown when afterThrowing that owns 4 params in ThrowsAdvice is invalid - * - * @author panlingxiao - * @see ThrowsAdvice - */ -public class MethodSignatureException extends RuntimeException { - - /** - * Create an MethodSignatureException with a specific message. - * - * @param message the message - */ - public MethodSignatureException(String message) { - super(message); - } - - /** - * Create an MethodSignatureException with a specific message and cause. - * - * @param message the message - * @param cause the cause - */ - public MethodSignatureException(String message, Exception cause) { - super(message, cause); - } -}