Polishing contribution

Closes gh-29509

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);
 	}
 

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.
-	 * <p>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.
-	 * <p>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
+		}
+	}
+
 }

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);