From d11b9577d52ff8a04c4ea93032511983f1daf3bb Mon Sep 17 00:00:00 2001 From: Rossen Stoyanchev Date: Fri, 5 Feb 2016 05:32:22 +0100 Subject: [PATCH] Fix NPE in InvocableHandlerMethod Issue: SPR-13917 (cherry picked from commit b1a46cc) --- .../support/InvocableHandlerMethod.java | 65 ++++++++++--------- .../support/InvocableHandlerMethodTests.java | 35 ++++++++-- 2 files changed, 64 insertions(+), 36 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/method/support/InvocableHandlerMethod.java b/spring-web/src/main/java/org/springframework/web/method/support/InvocableHandlerMethod.java index 3fb4822ad0f..4d93ad37055 100644 --- a/spring-web/src/main/java/org/springframework/web/method/support/InvocableHandlerMethod.java +++ b/spring-web/src/main/java/org/springframework/web/method/support/InvocableHandlerMethod.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 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. @@ -32,15 +32,15 @@ import org.springframework.web.context.request.NativeWebRequest; import org.springframework.web.method.HandlerMethod; /** - * Provides a method for invoking the handler method for a given request after resolving its method argument - * values through registered {@link HandlerMethodArgumentResolver}s. + * Provides a method for invoking the handler method for a given request after resolving its + * method argument values through registered {@link HandlerMethodArgumentResolver}s. * - *

Argument resolution often requires a {@link WebDataBinder} for data binding or for type conversion. - * Use the {@link #setDataBinderFactory(WebDataBinderFactory)} property to supply a binder factory to pass to - * argument resolvers. + *

Argument resolution often requires a {@link WebDataBinder} for data binding or for type + * conversion. Use the {@link #setDataBinderFactory(WebDataBinderFactory)} property to supply + * a binder factory to pass to argument resolvers. * - *

Use {@link #setHandlerMethodArgumentResolvers(HandlerMethodArgumentResolverComposite)} to customize - * the list of argument resolvers. + *

Use {@link #setHandlerMethodArgumentResolvers(HandlerMethodArgumentResolverComposite)} + * to customize the list of argument resolvers. * * @author Rossen Stoyanchev * @since 3.1 @@ -55,17 +55,17 @@ public class InvocableHandlerMethod extends HandlerMethod { /** - * Create an instance from the given handler and method. + * Create an instance from a {@code HandlerMethod}. */ - public InvocableHandlerMethod(Object bean, Method method) { - super(bean, method); + public InvocableHandlerMethod(HandlerMethod handlerMethod) { + super(handlerMethod); } /** - * Create an instance from a {@code HandlerMethod}. + * Create an instance from a bean instance and a method. */ - public InvocableHandlerMethod(HandlerMethod handlerMethod) { - super(handlerMethod); + public InvocableHandlerMethod(Object bean, Method method) { + super(bean, method); } /** @@ -75,7 +75,9 @@ public class InvocableHandlerMethod extends HandlerMethod { * @param parameterTypes the method parameter types * @throws NoSuchMethodException when the method cannot be found */ - public InvocableHandlerMethod(Object bean, String methodName, Class... parameterTypes) throws NoSuchMethodException { + public InvocableHandlerMethod(Object bean, String methodName, Class... parameterTypes) + throws NoSuchMethodException { + super(bean, methodName, parameterTypes); } @@ -107,16 +109,18 @@ public class InvocableHandlerMethod extends HandlerMethod { /** - * Invoke the method after resolving its argument values in the context of the given request.

Argument - * values are commonly resolved through {@link HandlerMethodArgumentResolver}s. The {@code provideArgs} - * parameter however may supply argument values to be used directly, i.e. without argument resolution. - * Examples of provided argument values include a {@link WebDataBinder}, a {@link SessionStatus}, or - * a thrown exception instance. Provided argument values are checked before argument resolvers. + * Invoke the method after resolving its argument values in the context of the given request. + *

Argument values are commonly resolved through {@link HandlerMethodArgumentResolver}s. + * The {@code providedArgs} parameter however may supply argument values to be used directly, + * i.e. without argument resolution. Examples of provided argument values include a + * {@link WebDataBinder}, a {@link SessionStatus}, or a thrown exception instance. + * Provided argument values are checked before argument resolvers. * @param request the current request * @param mavContainer the ModelAndViewContainer for this request * @param providedArgs "given" arguments matched by type, not resolved * @return the raw value returned by the invoked method - * @exception Exception raised if no suitable argument resolver can be found, or the method raised an exception + * @exception Exception raised if no suitable argument resolver can be found, + * or if the method raised an exception */ public final Object invokeForRequest(NativeWebRequest request, ModelAndViewContainer mavContainer, Object... providedArgs) throws Exception { @@ -129,7 +133,7 @@ public class InvocableHandlerMethod extends HandlerMethod { sb.append(Arrays.asList(args)); logger.trace(sb.toString()); } - Object returnValue = invoke(args); + Object returnValue = doInvoke(args); if (logger.isTraceEnabled()) { logger.trace("Method [" + getMethod().getName() + "] returned [" + returnValue + "]"); } @@ -159,8 +163,8 @@ public class InvocableHandlerMethod extends HandlerMethod { continue; } catch (Exception ex) { - if (logger.isTraceEnabled()) { - logger.trace(getArgumentResolutionErrorMessage("Error resolving argument", i), ex); + if (logger.isDebugEnabled()) { + logger.debug(getArgumentResolutionErrorMessage("Error resolving argument", i), ex); } throw ex; } @@ -180,7 +184,8 @@ public class InvocableHandlerMethod extends HandlerMethod { } /** - * Adds HandlerMethod details such as the controller type and method signature to the given error message. + * Adds HandlerMethod details such as the controller type and method + * signature to the given error message. * @param message error message to append the HandlerMethod details to */ protected String getDetailedErrorMessage(String message) { @@ -206,17 +211,19 @@ public class InvocableHandlerMethod extends HandlerMethod { return null; } + /** * Invoke the handler method with the given argument values. */ - private Object invoke(Object... args) throws Exception { + protected Object doInvoke(Object... args) throws Exception { ReflectionUtils.makeAccessible(getBridgedMethod()); try { return getBridgedMethod().invoke(getBean(), args); } catch (IllegalArgumentException ex) { assertTargetBean(getBridgedMethod(), getBean(), args); - throw new IllegalStateException(getInvocationErrorMessage(ex.getMessage(), args), ex); + String message = (ex.getMessage() != null ? ex.getMessage() : "Illegal argument"); + throw new IllegalStateException(getInvocationErrorMessage(message, args), ex); } catch (InvocationTargetException ex) { // Unwrap for HandlerExceptionResolvers ... @@ -249,7 +256,7 @@ public class InvocableHandlerMethod extends HandlerMethod { Class targetBeanClass = targetBean.getClass(); if (!methodDeclaringClass.isAssignableFrom(targetBeanClass)) { String msg = "The mapped controller method class '" + methodDeclaringClass.getName() + - "' is not an instance of the actual controller bean instance '" + + "' is not an instance of the actual controller bean class '" + targetBeanClass.getName() + "'. If the controller requires proxying " + "(e.g. due to @Transactional), please use class-based proxying."; throw new IllegalStateException(getInvocationErrorMessage(msg, args)); @@ -259,7 +266,7 @@ public class InvocableHandlerMethod extends HandlerMethod { private String getInvocationErrorMessage(String message, Object[] resolvedArgs) { StringBuilder sb = new StringBuilder(getDetailedErrorMessage(message)); sb.append("Resolved arguments: \n"); - for (int i=0; i < resolvedArgs.length; i++) { + for (int i = 0; i < resolvedArgs.length; i++) { sb.append("[").append(i).append("] "); if (resolvedArgs[i] == null) { sb.append("[null] \n"); diff --git a/spring-web/src/test/java/org/springframework/web/method/support/InvocableHandlerMethodTests.java b/spring-web/src/test/java/org/springframework/web/method/support/InvocableHandlerMethodTests.java index e93dce9c424..b044b257b16 100644 --- a/spring-web/src/test/java/org/springframework/web/method/support/InvocableHandlerMethodTests.java +++ b/spring-web/src/test/java/org/springframework/web/method/support/InvocableHandlerMethodTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2014 the original author or authors. + * Copyright 2002-2016 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. @@ -16,16 +16,11 @@ package org.springframework.web.method.support; -import static org.junit.Assert.assertEquals; -import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertSame; -import static org.junit.Assert.assertTrue; -import static org.junit.Assert.fail; - import java.lang.reflect.Method; import org.junit.Before; import org.junit.Test; + import org.springframework.core.MethodParameter; import org.springframework.http.converter.HttpMessageNotReadableException; import org.springframework.mock.web.test.MockHttpServletRequest; @@ -34,6 +29,9 @@ import org.springframework.web.bind.support.WebDataBinderFactory; import org.springframework.web.context.request.NativeWebRequest; import org.springframework.web.context.request.ServletWebRequest; +import static org.hamcrest.Matchers.*; +import static org.junit.Assert.*; + /** * Test fixture for {@link InvocableHandlerMethod} unit tests. * @@ -198,6 +196,26 @@ public class InvocableHandlerMethodTests { } } + @Test // SPR-13917 + public void invocationErrorMessage() throws Exception { + HandlerMethodArgumentResolverComposite composite = new HandlerMethodArgumentResolverComposite(); + composite.addResolver(new StubArgumentResolver(double.class, null)); + + Method method = Handler.class.getDeclaredMethod("handle", double.class); + Object handler = new Handler(); + InvocableHandlerMethod hm = new InvocableHandlerMethod(handler, method); + hm.setHandlerMethodArgumentResolvers(composite); + + try { + hm.invokeForRequest(this.webRequest, new ModelAndViewContainer()); + fail(); + } + catch (IllegalStateException ex) { + assertThat(ex.getMessage(), containsString("Illegal argument")); + } + } + + private void invokeExceptionRaisingHandler(Throwable expected) throws Exception { Method method = ExceptionRaisingHandler.class.getDeclaredMethod("raiseException"); Object handler = new ExceptionRaisingHandler(expected); @@ -212,6 +230,9 @@ public class InvocableHandlerMethodTests { public String handle(Integer intArg, String stringArg) { return intArg + "-" + stringArg; } + + public void handle(double amount) { + } }