From 53133d949d6dac586e308d4a844c2d498f729a76 Mon Sep 17 00:00:00 2001 From: rstoyanchev Date: Fri, 8 Sep 2023 08:27:36 +0100 Subject: [PATCH] Polishing contribution Closes gh-29509 --- .../web/servlet/HandlerExecutionChain.java | 2 +- .../handler/AbstractHandlerMapping.java | 70 ++++++++----------- .../handler/HandlerMethodMappingTests.java | 13 ++-- 3 files changed, 35 insertions(+), 50 deletions(-) diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/HandlerExecutionChain.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/HandlerExecutionChain.java index 78bfa587000..f90bf35cc01 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/HandlerExecutionChain.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/HandlerExecutionChain.java @@ -110,7 +110,7 @@ public class HandlerExecutionChain { /** * Add the given interceptors to the end of this chain. */ - public void addInterceptors(@Nullable HandlerInterceptor... interceptors) { + public void addInterceptors(HandlerInterceptor... interceptors) { CollectionUtils.mergeArrayIntoCollection(interceptors, this.interceptorList); } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerMapping.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerMapping.java index 85996db0cb8..ec695ea368d 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerMapping.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerMapping.java @@ -16,7 +16,6 @@ package org.springframework.web.servlet.handler; -import java.io.IOException; import java.util.ArrayList; import java.util.Arrays; import java.util.List; @@ -662,46 +661,25 @@ public abstract class AbstractHandlerMapping extends WebApplicationObjectSupport } /** - * Update the HandlerExecutionChain for CORS-related handling. - *

For pre-flight requests, the default implementation inserts a - * HandlerInterceptor that makes CORS-related checks and adds CORS headers. - * But does not abort the execution chain. - *

For actual requests, the default implementation inserts a - * HandlerInterceptor that makes CORS-related checks and adds CORS headers. + * Update {@link HandlerExecutionChain} for CORS requests, inserting an + * interceptor at the start of the chain to perform CORS checks, and + * also using a no-op handler for preflight requests. * @param request the current request - * @param chain the handler chain - * @param config the applicable CORS configuration (possibly {@code null}) + * @param chain the chain to update + * @param config the CORS configuration applicable to the handler * @since 4.2 */ - protected HandlerExecutionChain getCorsHandlerExecutionChain(HttpServletRequest request, - HandlerExecutionChain chain, @Nullable CorsConfiguration config) { - boolean isPreFlightRequest = CorsUtils.isPreFlightRequest(request); - if (isPreFlightRequest) { - chain = new HandlerExecutionChain(new PreFlightHandler(config), chain.getInterceptors()); - } - chain.addInterceptor(0, new CorsInterceptor(config, isPreFlightRequest)); - return chain; - } - - - private class PreFlightHandler implements HttpRequestHandler, CorsConfigurationSource { - - @Nullable - private final CorsConfiguration config; + protected HandlerExecutionChain getCorsHandlerExecutionChain( + HttpServletRequest request, HandlerExecutionChain chain, @Nullable CorsConfiguration config) { - public PreFlightHandler(@Nullable CorsConfiguration config) { - this.config = config; + if (CorsUtils.isPreFlightRequest(request)) { + PreFlightHandler preFlightHandler = new PreFlightHandler(config); + chain.addInterceptor(0, preFlightHandler); + return new HandlerExecutionChain(preFlightHandler, chain.getInterceptors()); } - - @Override - public void handleRequest(HttpServletRequest request, HttpServletResponse response) throws IOException { - // no-op - } - - @Override - @Nullable - public CorsConfiguration getCorsConfiguration(HttpServletRequest request) { - return this.config; + else { + chain.addInterceptor(0, new CorsInterceptor(config)); + return chain; } } @@ -710,11 +688,9 @@ public abstract class AbstractHandlerMapping extends WebApplicationObjectSupport @Nullable private final CorsConfiguration config; - private final boolean alwaysProceed; - public CorsInterceptor(@Nullable CorsConfiguration config, boolean alwaysProceed) { + public CorsInterceptor(@Nullable CorsConfiguration config) { this.config = config; - this.alwaysProceed = alwaysProceed; } @Override @@ -727,8 +703,7 @@ public abstract class AbstractHandlerMapping extends WebApplicationObjectSupport return true; } - boolean proceed = corsProcessor.processRequest(this.config, request, response); - return this.alwaysProceed || proceed; + return corsProcessor.processRequest(this.config, request, response); } @Override @@ -738,4 +713,17 @@ public abstract class AbstractHandlerMapping extends WebApplicationObjectSupport } } + + private class PreFlightHandler extends CorsInterceptor implements HttpRequestHandler { + + public PreFlightHandler(@Nullable CorsConfiguration config) { + super(config); + } + + @Override + public void handleRequest(HttpServletRequest request, HttpServletResponse response) { + // no-op + } + } + } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/handler/HandlerMethodMappingTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/handler/HandlerMethodMappingTests.java index 36b445ac10d..8cbf99ecf22 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/handler/HandlerMethodMappingTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/handler/HandlerMethodMappingTests.java @@ -39,9 +39,7 @@ import org.springframework.web.bind.annotation.RequestMapping; import org.springframework.web.context.support.StaticWebApplicationContext; import org.springframework.web.cors.CorsConfiguration; import org.springframework.web.method.HandlerMethod; -import org.springframework.web.servlet.ComplexWebApplicationContext; import org.springframework.web.servlet.HandlerExecutionChain; -import org.springframework.web.servlet.HandlerInterceptor; import org.springframework.web.servlet.HandlerMapping; import org.springframework.web.servlet.mvc.HttpRequestHandlerAdapter; import org.springframework.web.testfixture.servlet.MockHttpServletRequest; @@ -133,6 +131,7 @@ public class HandlerMethodMappingTests { assertThat(chain).isNotNull(); assertThat(chain.getInterceptorList()).isNotEmpty(); assertThat(chain.getHandler()).isInstanceOf(HttpRequestHandler.class); + chain.getInterceptorList().get(0).preHandle(request, response, chain.getHandler()); new HttpRequestHandlerAdapter().handle(request, response, chain.getHandler()); @@ -154,6 +153,7 @@ public class HandlerMethodMappingTests { assertThat(chain).isNotNull(); assertThat(chain.getInterceptorList()).isNotEmpty(); assertThat(chain.getHandler()).isInstanceOf(HttpRequestHandler.class); + chain.getInterceptorList().get(0).preHandle(request, response, chain.getHandler()); new HttpRequestHandlerAdapter().handle(request, response, chain.getHandler()); @@ -175,13 +175,10 @@ public class HandlerMethodMappingTests { HandlerExecutionChain chain = this.mapping.getHandler(request); assertThat(chain).isNotNull(); - chain.addInterceptor(new ComplexWebApplicationContext.MyHandlerInterceptor1()); - chain.addInterceptor(new ComplexWebApplicationContext.MyHandlerInterceptor2()); - assertThat(chain.getInterceptorList().size()).isEqualTo(3); assertThat(chain.getHandler()).isInstanceOf(HttpRequestHandler.class); - for (HandlerInterceptor interceptor : chain.getInterceptorList()) { - interceptor.preHandle(request, response, chain.getHandler()); - } + assertThat(chain.getInterceptorList()).isNotEmpty(); + + chain.getInterceptorList().get(0).preHandle(request, response, chain.getHandler()); new HttpRequestHandlerAdapter().handle(request, response, chain.getHandler()); assertThat(response.getStatus()).isEqualTo(200);