Fix NPE ExpressionBasedPreInvocationAdviceTests
Getting NPE if @PreFilter argument filterType is not provided and method accept more then one argument. Add related exception message. fixes gh-6803
This commit is contained in:
parent
82ce9aa6f8
commit
59acda04cf
|
@ -1,5 +1,5 @@
|
||||||
/*
|
/*
|
||||||
* Copyright 2002-2016 the original author or authors.
|
* Copyright 2002-2019 the original author or authors.
|
||||||
*
|
*
|
||||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||||
* you may not use this file except in compliance with the License.
|
* you may not use this file except in compliance with the License.
|
||||||
|
@ -81,6 +81,9 @@ public class ExpressionBasedPreInvocationAdvice implements
|
||||||
"A PreFilter expression was set but the method argument type"
|
"A PreFilter expression was set but the method argument type"
|
||||||
+ arg.getClass() + " is not filterable");
|
+ arg.getClass() + " is not filterable");
|
||||||
}
|
}
|
||||||
|
} else if (mi.getArguments().length > 1) {
|
||||||
|
throw new IllegalArgumentException(
|
||||||
|
"Unable to determine the method argument for filtering. Specify the filter target.");
|
||||||
}
|
}
|
||||||
|
|
||||||
if (filterTarget.getClass().isArray()) {
|
if (filterTarget.getClass().isArray()) {
|
||||||
|
|
|
@ -0,0 +1,162 @@
|
||||||
|
/*
|
||||||
|
* Copyright 2002-2019 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.
|
||||||
|
* You may obtain a copy of the License at
|
||||||
|
*
|
||||||
|
* https://www.apache.org/licenses/LICENSE-2.0
|
||||||
|
*
|
||||||
|
* Unless required by applicable law or agreed to in writing, software
|
||||||
|
* distributed under the License is distributed on an "AS IS" BASIS,
|
||||||
|
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||||
|
* See the License for the specific language governing permissions and
|
||||||
|
* limitations under the License.
|
||||||
|
*/
|
||||||
|
package org.springframework.security.access.expression.method;
|
||||||
|
|
||||||
|
import org.junit.Before;
|
||||||
|
import org.junit.Test;
|
||||||
|
import org.junit.runner.RunWith;
|
||||||
|
import org.mockito.Mock;
|
||||||
|
import org.mockito.junit.MockitoJUnitRunner;
|
||||||
|
import org.springframework.security.access.intercept.method.MockMethodInvocation;
|
||||||
|
import org.springframework.security.access.prepost.PreInvocationAttribute;
|
||||||
|
import org.springframework.security.core.Authentication;
|
||||||
|
|
||||||
|
import java.util.ArrayList;
|
||||||
|
import java.util.List;
|
||||||
|
|
||||||
|
import static org.assertj.core.api.Assertions.assertThat;
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Tests {@link ExpressionBasedPreInvocationAdvice}
|
||||||
|
*
|
||||||
|
* @author Maksim Vinogradov
|
||||||
|
* @since 5.2
|
||||||
|
*/
|
||||||
|
@RunWith(MockitoJUnitRunner.class)
|
||||||
|
public class ExpressionBasedPreInvocationAdviceTests {
|
||||||
|
|
||||||
|
@Mock
|
||||||
|
private Authentication authentication;
|
||||||
|
|
||||||
|
private ExpressionBasedPreInvocationAdvice expressionBasedPreInvocationAdvice;
|
||||||
|
|
||||||
|
@Before
|
||||||
|
public void setUp() throws Exception {
|
||||||
|
expressionBasedPreInvocationAdvice = new ExpressionBasedPreInvocationAdvice();
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test(expected = IllegalArgumentException.class)
|
||||||
|
public void findFilterTargetNameProvidedButNotMatch() throws Exception {
|
||||||
|
//given
|
||||||
|
PreInvocationAttribute attribute = new PreInvocationExpressionAttribute("true",
|
||||||
|
"filterTargetDoesNotMatch",
|
||||||
|
null);
|
||||||
|
MockMethodInvocation methodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class,
|
||||||
|
"doSomethingCollection",
|
||||||
|
new Class[]{List.class},
|
||||||
|
new Object[]{new ArrayList<>()});
|
||||||
|
//when - then
|
||||||
|
expressionBasedPreInvocationAdvice.before(authentication, methodInvocation, attribute);
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test(expected = IllegalArgumentException.class)
|
||||||
|
public void findFilterTargetNameProvidedArrayUnsupported() throws Exception {
|
||||||
|
//given
|
||||||
|
PreInvocationAttribute attribute = new PreInvocationExpressionAttribute("true",
|
||||||
|
"param", null);
|
||||||
|
MockMethodInvocation methodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class,
|
||||||
|
"doSomethingArray",
|
||||||
|
new Class[]{String[].class},
|
||||||
|
new Object[]{new String[0]});
|
||||||
|
//when - then
|
||||||
|
expressionBasedPreInvocationAdvice.before(authentication, methodInvocation, attribute);
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void findFilterTargetNameProvided() throws Exception {
|
||||||
|
//given
|
||||||
|
PreInvocationAttribute attribute = new PreInvocationExpressionAttribute("true", "param", null);
|
||||||
|
MockMethodInvocation methodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class,
|
||||||
|
"doSomethingCollection",
|
||||||
|
new Class[]{List.class},
|
||||||
|
new Object[]{new ArrayList<>()});
|
||||||
|
|
||||||
|
//when
|
||||||
|
boolean result = expressionBasedPreInvocationAdvice
|
||||||
|
.before(authentication, methodInvocation, attribute);
|
||||||
|
//then
|
||||||
|
assertThat(result).isTrue();
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test(expected = IllegalArgumentException.class)
|
||||||
|
public void findFilterTargetNameNotProvidedArrayUnsupported() throws Exception {
|
||||||
|
//given
|
||||||
|
PreInvocationAttribute attribute = new PreInvocationExpressionAttribute("true", "", null);
|
||||||
|
MockMethodInvocation methodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class,
|
||||||
|
"doSomethingArray",
|
||||||
|
new Class[]{String[].class},
|
||||||
|
new Object[]{new String[0]});
|
||||||
|
//when - then
|
||||||
|
expressionBasedPreInvocationAdvice.before(authentication, methodInvocation, attribute);
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
public void findFilterTargetNameNotProvided() throws Exception {
|
||||||
|
//given
|
||||||
|
PreInvocationAttribute attribute = new PreInvocationExpressionAttribute("true", "", null);
|
||||||
|
MockMethodInvocation methodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class,
|
||||||
|
"doSomethingCollection",
|
||||||
|
new Class[]{List.class},
|
||||||
|
new Object[]{new ArrayList<>()});
|
||||||
|
//when
|
||||||
|
boolean result = expressionBasedPreInvocationAdvice.before(authentication, methodInvocation, attribute);
|
||||||
|
//then
|
||||||
|
assertThat(result).isTrue();
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test(expected = IllegalArgumentException.class)
|
||||||
|
public void findFilterTargetNameNotProvidedTypeNotSupported() throws Exception {
|
||||||
|
//given
|
||||||
|
PreInvocationAttribute attribute = new PreInvocationExpressionAttribute("true", "", null);
|
||||||
|
MockMethodInvocation methodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class,
|
||||||
|
"doSomethingString",
|
||||||
|
new Class[]{String.class},
|
||||||
|
new Object[]{"param"});
|
||||||
|
//when - then
|
||||||
|
expressionBasedPreInvocationAdvice.before(authentication, methodInvocation, attribute);
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test(expected = IllegalArgumentException.class)
|
||||||
|
public void findFilterTargetNameNotProvidedMethodAcceptMoreThenOneArgument() throws Exception {
|
||||||
|
//given
|
||||||
|
PreInvocationAttribute attribute = new PreInvocationExpressionAttribute("true", "", null);
|
||||||
|
MockMethodInvocation methodInvocation = new MockMethodInvocation(new TestClass(), TestClass.class,
|
||||||
|
"doSomethingTwoArgs",
|
||||||
|
new Class[]{String.class, List.class},
|
||||||
|
new Object[]{"param", new ArrayList<>()});
|
||||||
|
//when - then
|
||||||
|
expressionBasedPreInvocationAdvice.before(authentication, methodInvocation, attribute);
|
||||||
|
}
|
||||||
|
|
||||||
|
private class TestClass {
|
||||||
|
|
||||||
|
public Boolean doSomethingCollection(List<?> param) {
|
||||||
|
return Boolean.TRUE;
|
||||||
|
}
|
||||||
|
|
||||||
|
public Boolean doSomethingArray(String[] param) {
|
||||||
|
return Boolean.TRUE;
|
||||||
|
}
|
||||||
|
|
||||||
|
public Boolean doSomethingString(String param) {
|
||||||
|
return Boolean.TRUE;
|
||||||
|
}
|
||||||
|
|
||||||
|
public Boolean doSomethingTwoArgs(String param, List<?> list) {
|
||||||
|
return Boolean.TRUE;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
|
@ -1,5 +1,5 @@
|
||||||
/*
|
/*
|
||||||
* Copyright 2002-2016 the original author or authors.
|
* Copyright 2002-2019 the original author or authors.
|
||||||
*
|
*
|
||||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||||
* you may not use this file except in compliance with the License.
|
* you may not use this file except in compliance with the License.
|
||||||
|
@ -15,15 +15,22 @@
|
||||||
*/
|
*/
|
||||||
package org.springframework.security.access.intercept.method;
|
package org.springframework.security.access.intercept.method;
|
||||||
|
|
||||||
|
import org.aopalliance.intercept.MethodInvocation;
|
||||||
|
|
||||||
import java.lang.reflect.AccessibleObject;
|
import java.lang.reflect.AccessibleObject;
|
||||||
import java.lang.reflect.Method;
|
import java.lang.reflect.Method;
|
||||||
|
|
||||||
import org.aopalliance.intercept.MethodInvocation;
|
|
||||||
|
|
||||||
@SuppressWarnings("unchecked")
|
@SuppressWarnings("unchecked")
|
||||||
public class MockMethodInvocation implements MethodInvocation {
|
public class MockMethodInvocation implements MethodInvocation {
|
||||||
private Method method;
|
private Method method;
|
||||||
private Object targetObject;
|
private Object targetObject;
|
||||||
|
private Object[] arguments = new Object[0];
|
||||||
|
|
||||||
|
public MockMethodInvocation(Object targetObject, Class clazz, String methodName, Class[] parameterTypes,
|
||||||
|
Object[] arguments) throws NoSuchMethodException {
|
||||||
|
this(targetObject, clazz, methodName, parameterTypes);
|
||||||
|
this.arguments = arguments;
|
||||||
|
}
|
||||||
|
|
||||||
public MockMethodInvocation(Object targetObject, Class clazz, String methodName,
|
public MockMethodInvocation(Object targetObject, Class clazz, String methodName,
|
||||||
Class... parameterTypes) throws NoSuchMethodException {
|
Class... parameterTypes) throws NoSuchMethodException {
|
||||||
|
@ -32,7 +39,7 @@ public class MockMethodInvocation implements MethodInvocation {
|
||||||
}
|
}
|
||||||
|
|
||||||
public Object[] getArguments() {
|
public Object[] getArguments() {
|
||||||
return null;
|
return arguments;
|
||||||
}
|
}
|
||||||
|
|
||||||
public Method getMethod() {
|
public Method getMethod() {
|
||||||
|
|
Loading…
Reference in New Issue