diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolver.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolver.java index 44769641584..e70540a1318 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolver.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolver.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. @@ -17,6 +17,7 @@ package org.springframework.web.servlet.mvc.method.annotation; import java.lang.reflect.Method; +import java.lang.reflect.Proxy; import java.util.ArrayList; import java.util.Collections; import java.util.LinkedHashMap; @@ -27,6 +28,7 @@ import java.util.concurrent.ConcurrentHashMap; import javax.servlet.http.HttpServletRequest; import javax.servlet.http.HttpServletResponse; +import org.springframework.aop.support.AopUtils; import org.springframework.beans.factory.InitializingBean; import org.springframework.context.ApplicationContext; import org.springframework.context.ApplicationContextAware; @@ -440,13 +442,18 @@ public class ExceptionHandlerExceptionResolver extends AbstractHandlerMethodExce * Spring-managed beans were detected. * @param handlerMethod the method where the exception was raised (may be {@code null}) * @param exception the raised exception - * @return a method to handle the exception, or {@code null} + * @return a method to handle the exception, or {@code null} if none */ @Nullable - protected ServletInvocableHandlerMethod getExceptionHandlerMethod(@Nullable HandlerMethod handlerMethod, Exception exception) { - Class handlerType = (handlerMethod != null ? handlerMethod.getBeanType() : null); + protected ServletInvocableHandlerMethod getExceptionHandlerMethod( + @Nullable HandlerMethod handlerMethod, Exception exception) { + + Class handlerType = null; if (handlerMethod != null) { + // Local exception handler methods on the controller class itself. + // To be invoked through the proxy, even in case of an interface-based proxy. + handlerType = handlerMethod.getBeanType(); ExceptionHandlerMethodResolver resolver = this.exceptionHandlerCache.get(handlerType); if (resolver == null) { resolver = new ExceptionHandlerMethodResolver(handlerType); @@ -456,14 +463,20 @@ public class ExceptionHandlerExceptionResolver extends AbstractHandlerMethodExce if (method != null) { return new ServletInvocableHandlerMethod(handlerMethod.getBean(), method); } + // For advice applicability check below (involving base packages, assignable types + // and annotation presence), use target class instead of interface-based proxy. + if (Proxy.isProxyClass(handlerType)) { + handlerType = AopUtils.getTargetClass(handlerMethod.getBean()); + } } for (Entry entry : this.exceptionHandlerAdviceCache.entrySet()) { - if (entry.getKey().isApplicableToBeanType(handlerType)) { + ControllerAdviceBean advice = entry.getKey(); + if (advice.isApplicableToBeanType(handlerType)) { ExceptionHandlerMethodResolver resolver = entry.getValue(); Method method = resolver.resolveMethod(exception); if (method != null) { - return new ServletInvocableHandlerMethod(entry.getKey().resolveBean(), method); + return new ServletInvocableHandlerMethod(advice.resolveBean(), method); } } } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolverTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolverTests.java index 34355dac162..9e266cefb7e 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolverTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/ExceptionHandlerExceptionResolverTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 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. @@ -25,6 +25,7 @@ import org.junit.Before; import org.junit.BeforeClass; import org.junit.Test; +import org.springframework.aop.framework.ProxyFactory; import org.springframework.beans.FatalBeanException; import org.springframework.context.annotation.AnnotationConfigApplicationContext; import org.springframework.context.annotation.Bean; @@ -38,6 +39,7 @@ import org.springframework.util.ClassUtils; import org.springframework.web.bind.annotation.ExceptionHandler; import org.springframework.web.bind.annotation.ResponseBody; import org.springframework.web.bind.annotation.RestControllerAdvice; +import org.springframework.web.context.support.WebApplicationObjectSupport; import org.springframework.web.method.HandlerMethod; import org.springframework.web.method.annotation.ModelMethodProcessor; import org.springframework.web.method.support.HandlerMethodArgumentResolver; @@ -213,8 +215,8 @@ public class ExceptionHandlerExceptionResolverTests { @Test public void resolveExceptionGlobalHandler() throws Exception { - AnnotationConfigApplicationContext cxt = new AnnotationConfigApplicationContext(MyConfig.class); - this.resolver.setApplicationContext(cxt); + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(MyConfig.class); + this.resolver.setApplicationContext(ctx); this.resolver.afterPropertiesSet(); IllegalAccessException ex = new IllegalAccessException(); @@ -228,8 +230,8 @@ public class ExceptionHandlerExceptionResolverTests { @Test public void resolveExceptionGlobalHandlerOrdered() throws Exception { - AnnotationConfigApplicationContext cxt = new AnnotationConfigApplicationContext(MyConfig.class); - this.resolver.setApplicationContext(cxt); + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(MyConfig.class); + this.resolver.setApplicationContext(ctx); this.resolver.afterPropertiesSet(); IllegalStateException ex = new IllegalStateException(); @@ -243,8 +245,8 @@ public class ExceptionHandlerExceptionResolverTests { @Test // SPR-12605 public void resolveExceptionWithHandlerMethodArg() throws Exception { - AnnotationConfigApplicationContext cxt = new AnnotationConfigApplicationContext(MyConfig.class); - this.resolver.setApplicationContext(cxt); + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(MyConfig.class); + this.resolver.setApplicationContext(ctx); this.resolver.afterPropertiesSet(); ArrayIndexOutOfBoundsException ex = new ArrayIndexOutOfBoundsException(); @@ -258,8 +260,8 @@ public class ExceptionHandlerExceptionResolverTests { @Test public void resolveExceptionWithAssertionError() throws Exception { - AnnotationConfigApplicationContext cxt = new AnnotationConfigApplicationContext(MyConfig.class); - this.resolver.setApplicationContext(cxt); + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(MyConfig.class); + this.resolver.setApplicationContext(ctx); this.resolver.afterPropertiesSet(); AssertionError err = new AssertionError("argh"); @@ -274,8 +276,8 @@ public class ExceptionHandlerExceptionResolverTests { @Test public void resolveExceptionWithAssertionErrorAsRootCause() throws Exception { - AnnotationConfigApplicationContext cxt = new AnnotationConfigApplicationContext(MyConfig.class); - this.resolver.setApplicationContext(cxt); + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(MyConfig.class); + this.resolver.setApplicationContext(ctx); this.resolver.afterPropertiesSet(); AssertionError err = new AssertionError("argh"); @@ -290,8 +292,8 @@ public class ExceptionHandlerExceptionResolverTests { @Test public void resolveExceptionControllerAdviceHandler() throws Exception { - AnnotationConfigApplicationContext cxt = new AnnotationConfigApplicationContext(MyControllerAdviceConfig.class); - this.resolver.setApplicationContext(cxt); + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(MyControllerAdviceConfig.class); + this.resolver.setApplicationContext(ctx); this.resolver.afterPropertiesSet(); IllegalStateException ex = new IllegalStateException(); @@ -305,8 +307,8 @@ public class ExceptionHandlerExceptionResolverTests { @Test public void resolveExceptionControllerAdviceNoHandler() throws Exception { - AnnotationConfigApplicationContext cxt = new AnnotationConfigApplicationContext(MyControllerAdviceConfig.class); - this.resolver.setApplicationContext(cxt); + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(MyControllerAdviceConfig.class); + this.resolver.setApplicationContext(ctx); this.resolver.afterPropertiesSet(); IllegalStateException ex = new IllegalStateException(); @@ -317,6 +319,21 @@ public class ExceptionHandlerExceptionResolverTests { assertEquals("DefaultTestExceptionResolver: IllegalStateException", this.response.getContentAsString()); } + @Test // SPR-16496 + public void resolveExceptionControllerAdviceAgainstProxy() throws Exception { + AnnotationConfigApplicationContext ctx = new AnnotationConfigApplicationContext(MyControllerAdviceConfig.class); + this.resolver.setApplicationContext(ctx); + this.resolver.afterPropertiesSet(); + + IllegalStateException ex = new IllegalStateException(); + HandlerMethod handlerMethod = new HandlerMethod(new ProxyFactory(new ResponseBodyController()).getProxy(), "handle"); + ModelAndView mav = this.resolver.resolveException(this.request, this.response, handlerMethod, ex); + + assertNotNull("Exception was not handled", mav); + assertTrue(mav.isEmpty()); + assertEquals("BasePackageTestExceptionResolver: IllegalStateException", this.response.getContentAsString()); + } + private void assertMethodProcessorCount(int resolverCount, int handlerCount) { assertEquals(resolverCount, this.resolver.getArgumentResolvers().getResolvers().size()); @@ -348,8 +365,18 @@ public class ExceptionHandlerExceptionResolverTests { } + interface ResponseBodyInterface { + + void handle(); + + @ExceptionHandler + @ResponseBody + String handleException(IllegalArgumentException ex); + } + + @Controller - static class ResponseBodyController { + static class ResponseBodyController extends WebApplicationObjectSupport implements ResponseBodyInterface { public void handle() {} @@ -454,7 +481,7 @@ public class ExceptionHandlerExceptionResolverTests { } - @RestControllerAdvice("org.springframework.web.servlet.mvc.method.annotation") + @RestControllerAdvice(assignableTypes = WebApplicationObjectSupport.class) @Order(2) static class BasePackageTestExceptionResolver {