Auto-adapt reflective arguments in case of vararg array type mismatch

Issue: SPR-13328

Author: jhoeller

spring-aop/src/main/java/org/springframework/aop/ProxyMethodInvocation.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2015 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.
@@ -57,14 +57,14 @@ public interface ProxyMethodInvocation extends MethodInvocation {
 	 * @return an invocable clone of this invocation.
 	 * {@code proceed()} can be called once per clone.
 	 */
-	MethodInvocation invocableClone(Object[] arguments);
+	MethodInvocation invocableClone(Object... arguments);
 
 	/**
 	 * Set the arguments to be used on subsequent invocations in the any advice
 	 * in this chain.
 	 * @param arguments the argument array
 	 */
-	void setArguments(Object[] arguments);
+	void setArguments(Object... arguments);
 
 	/**
 	 * Add the specified user attribute with the given value to this invocation.

spring-aop/src/main/java/org/springframework/aop/framework/AopProxyUtils.java

@@ -16,6 +16,8 @@
 
 package org.springframework.aop.framework;
 
+import java.lang.reflect.Array;
+import java.lang.reflect.Method;
 import java.lang.reflect.Proxy;
 import java.util.Arrays;
 
@@ -25,6 +27,7 @@
 import org.springframework.aop.support.AopUtils;
 import org.springframework.aop.target.SingletonTargetSource;
 import org.springframework.util.Assert;
+import org.springframework.util.ObjectUtils;
 
 /**
  * Utility methods for AOP proxy factories.
@@ -161,4 +164,38 @@ public static boolean equalsAdvisors(AdvisedSupport a, AdvisedSupport b) {
 		return Arrays.equals(a.getAdvisors(), b.getAdvisors());
 	}
 
+
+	/**
+	 * Adapt the given arguments to the target signature in the given method,
+	 * if necessary: in particular, if a given vararg argument array does not
+	 * match the array type of the declared vararg parameter in the method.
+	 * @param method the target method
+	 * @param arguments the given arguments
+	 * @return a cloned argument array, or the original if no adaptation is needed
+	 * @since 4.2.3
+	 */
+	static Object[] adaptArgumentsIfNecessary(Method method, Object... arguments) {
+		if (method.isVarArgs() && !ObjectUtils.isEmpty(arguments)) {
+			Class<?>[] paramTypes = method.getParameterTypes();
+			if (paramTypes.length == arguments.length) {
+				int varargIndex = paramTypes.length - 1;
+				Class<?> varargType = paramTypes[varargIndex];
+				if (varargType.isArray()) {
+					Object varargArray = arguments[varargIndex];
+					if (varargArray instanceof Object[] && !varargType.isInstance(varargArray)) {
+						Object[] newArguments = new Object[arguments.length];
+						System.arraycopy(arguments, 0, newArguments, 0, varargIndex);
+						Class<?> targetElementType = varargType.getComponentType();
+						int varargLength = Array.getLength(varargArray);
+						Object newVarargArray = Array.newInstance(targetElementType, varargLength);
+						System.arraycopy(varargArray, 0, newVarargArray, 0, varargLength);
+						newArguments[varargIndex] = newVarargArray;
+						return newArguments;
+					}
+				}
+			}
+		}
+		return arguments;
+	}
+
 }

spring-aop/src/main/java/org/springframework/aop/framework/CglibAopProxy.java

@@ -304,13 +304,13 @@ private Callback[] getCallbacks(Class<?> rootClass) throws Exception {
 		Callback targetDispatcher = isStatic ?
 				new StaticDispatcher(this.advised.getTargetSource().getTarget()) : new SerializableNoOp();
 
-		Callback[] mainCallbacks = new Callback[]{
-			aopInterceptor, // for normal advice
-			targetInterceptor, // invoke target without considering advice, if optimized
-			new SerializableNoOp(), // no override for methods mapped to this
-			targetDispatcher, this.advisedDispatcher,
-			new EqualsInterceptor(this.advised),
-			new HashCodeInterceptor(this.advised)
+		Callback[] mainCallbacks = new Callback[] {
+				aopInterceptor,  // for normal advice
+				targetInterceptor,  // invoke target without considering advice, if optimized
+				new SerializableNoOp(),  // no override for methods mapped to this
+				targetDispatcher, this.advisedDispatcher,
+				new EqualsInterceptor(this.advised),
+				new HashCodeInterceptor(this.advised)
 		};
 
 		Callback[] callbacks;
@@ -646,7 +646,8 @@ public Object intercept(Object proxy, Method method, Object[] args, MethodProxy
 					// Note that the final invoker must be an InvokerInterceptor, so we know
 					// it does nothing but a reflective operation on the target, and no hot
 					// swapping or fancy proxying.
-					retVal = methodProxy.invoke(target, args);
+					Object[] argsToUse = AopProxyUtils.adaptArgumentsIfNecessary(method, args);
+					retVal = methodProxy.invoke(target, argsToUse);
 				}
 				else {
 					// We need to create a method invocation...

spring-aop/src/main/java/org/springframework/aop/framework/JdkDynamicAopProxy.java

@@ -198,7 +198,8 @@ public Object invoke(Object proxy, Method method, Object[] args) throws Throwabl
 				// We can skip creating a MethodInvocation: just invoke the target directly
 				// Note that the final invoker must be an InvokerInterceptor so we know it does
 				// nothing but a reflective operation on the target, and no hot swapping or fancy proxying.
-				retVal = AopUtils.invokeJoinpointUsingReflection(target, method, args);
+				Object[] argsToUse = AopProxyUtils.adaptArgumentsIfNecessary(method, args);
+				retVal = AopUtils.invokeJoinpointUsingReflection(target, method, argsToUse);
 			}
 			else {
 				// We need to create a method invocation...

spring-aop/src/main/java/org/springframework/aop/framework/ReflectiveMethodInvocation.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2012 the original author or authors.
+ * Copyright 2002-2015 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.
@@ -109,7 +109,7 @@ protected ReflectiveMethodInvocation(
 		this.target = target;
 		this.targetClass = targetClass;
 		this.method = BridgeMethodResolver.findBridgedMethod(method);
-		this.arguments = arguments;
+		this.arguments = AopProxyUtils.adaptArgumentsIfNecessary(method, arguments);
 		this.interceptorsAndDynamicMethodMatchers = interceptorsAndDynamicMethodMatchers;
 	}
 
@@ -145,7 +145,7 @@ public final Object[] getArguments() {
 	}
 
 	@Override
-	public void setArguments(Object[] arguments) {
+	public void setArguments(Object... arguments) {
 		this.arguments = arguments;
 	}
 
@@ -219,7 +219,7 @@ public MethodInvocation invocableClone() {
 	 * @see java.lang.Object#clone()
 	 */
 	@Override
-	public MethodInvocation invocableClone(Object[] arguments) {
+	public MethodInvocation invocableClone(Object... arguments) {
 		// Force initialization of the user attributes Map,
 		// for having a shared Map reference in the clone.
 		if (this.userAttributes == null) {

spring-aop/src/test/java/org/springframework/aop/aspectj/annotation/AspectProxyFactoryTests.java

@@ -19,18 +19,15 @@
 import java.util.Arrays;
 
 import org.apache.commons.logging.LogFactory;
-
 import org.aspectj.lang.ProceedingJoinPoint;
 import org.aspectj.lang.annotation.Around;
 import org.aspectj.lang.annotation.Aspect;
-
 import org.junit.Ignore;
 import org.junit.Test;
+import test.aop.PerThisAspect;
 
 import org.springframework.util.SerializationTestUtils;
 
-import test.aop.PerThisAspect;
-
 import static org.junit.Assert.*;
 
 /**
@@ -109,18 +106,27 @@ public void testWithNonSingletonAspectInstance() throws Exception {
 	}
 
 	@Test  // SPR-13328
-	public void testVarargsWithEnumArray() throws Exception {
+	public void testProxiedVarargsWithEnumArray() throws Exception {
 		AspectJProxyFactory proxyFactory = new AspectJProxyFactory(new TestBean());
-		proxyFactory.addAspect(LoggingAspect.class);
-		proxyFactory.setProxyTargetClass(true);
-		TestBean proxy = proxyFactory.getProxy();
-		assertTrue(proxy.doWithVarargs(MyEnum.A, MyEnum.B));
+		proxyFactory.addAspect(LoggingAspectOnVarargs.class);
+		ITestBean proxy = proxyFactory.getProxy();
+		assertTrue(proxy.doWithVarargs(MyEnum.A, MyOtherEnum.C));
+	}
+
+	@Test  // SPR-13328
+	public void testUnproxiedVarargsWithEnumArray() throws Exception {
+		AspectJProxyFactory proxyFactory = new AspectJProxyFactory(new TestBean());
+		proxyFactory.addAspect(LoggingAspectOnSetter.class);
+		ITestBean proxy = proxyFactory.getProxy();
+		assertTrue(proxy.doWithVarargs(MyEnum.A, MyOtherEnum.C));
 	}
 
 
 	public interface ITestBean {
 
 		int getAge();
+
+		<V extends MyInterface> boolean doWithVarargs(V... args);
 	}
 
 
@@ -138,6 +144,7 @@ public void setAge(int age) {
 		}
 
 		@SuppressWarnings("unchecked")
+		@Override
 		public <V extends MyInterface> boolean doWithVarargs(V... args) {
 			return true;
 		}
@@ -154,12 +161,29 @@ public enum MyEnum implements MyInterface {
 	}
 
 
+	public enum MyOtherEnum implements MyInterface {
+
+		C, D;
+	}
+
+
 	@Aspect
-	public static class LoggingAspect {
+	public static class LoggingAspectOnVarargs {
 
 		@Around("execution(* doWithVarargs(*))")
 		public Object doLog(ProceedingJoinPoint pjp) throws Throwable {
-			LogFactory.getLog(LoggingAspect.class).debug(Arrays.asList(pjp.getArgs()));
+			LogFactory.getLog(LoggingAspectOnVarargs.class).debug(Arrays.asList(pjp.getArgs()));
+			return pjp.proceed();
+		}
+	}
+
+
+	@Aspect
+	public static class LoggingAspectOnSetter {
+
+		@Around("execution(* setAge(*))")
+		public Object doLog(ProceedingJoinPoint pjp) throws Throwable {
+			LogFactory.getLog(LoggingAspectOnSetter.class).debug(Arrays.asList(pjp.getArgs()));
 			return pjp.proceed();
 		}
 	}

spring-context/src/test/java/org/springframework/aop/framework/CglibProxyTests.java

@@ -21,7 +21,6 @@
 import org.aopalliance.intercept.MethodInterceptor;
 import org.aopalliance.intercept.MethodInvocation;
 import org.junit.Test;
-
 import test.mixin.LockMixinAdvisor;
 
 import org.springframework.aop.ClassFilter;
@@ -395,7 +394,7 @@ public void testProxyTargetClassInCaseOfNoInterfaces() throws Exception {
 	public void testVarargsWithEnumArray() throws Exception {
 		ProxyFactory proxyFactory = new ProxyFactory(new MyBean());
 		MyBean proxy = (MyBean) proxyFactory.getProxy();
-		assertTrue(proxy.doWithVarargs(MyEnum.A, MyEnum.B));
+		assertTrue(proxy.doWithVarargs(MyEnum.A, MyOtherEnum.C));
 	}
 
 
@@ -432,6 +431,12 @@ public enum MyEnum implements MyInterface {
 	}
 
 
+	public enum MyOtherEnum implements MyInterface {
+
+		C, D;
+	}
+
+
 	public static class ExceptionThrower {
 
 		private boolean catchInvoked;

spring-context/src/test/java/org/springframework/aop/framework/JdkDynamicProxyTests.java

@@ -138,6 +138,13 @@ public void testEqualsAndHashCodeDefined() throws Exception {
 		assertEquals("hashCode()", proxy.hashCode(), named.hashCode());
 	}
 
+	@Test  // SPR-13328
+	public void testVarargsWithEnumArray() throws Exception {
+		ProxyFactory proxyFactory = new ProxyFactory(new VarargTestBean());
+		VarargTestInterface proxy = (VarargTestInterface) proxyFactory.getProxy();
+		assertTrue(proxy.doWithVarargs(MyEnum.A, MyOtherEnum.C));
+	}
+
 
 	public interface Foo {
 
@@ -201,4 +208,36 @@ public int hashCode() {
 		}
 	}
 
+
+	public interface VarargTestInterface {
+
+		<V extends MyInterface> boolean doWithVarargs(V... args);
+	}
+
+
+	public static class VarargTestBean implements VarargTestInterface {
+
+		@SuppressWarnings("unchecked")
+		@Override
+		public <V extends MyInterface> boolean doWithVarargs(V... args) {
+			return true;
+		}
+	}
+
+
+	public interface MyInterface {
+	}
+
+
+	public enum MyEnum implements MyInterface {
+
+		A, B;
+	}
+
+
+	public enum MyOtherEnum implements MyInterface {
+
+		C, D;
+	}
+
 }