Improve handling for pre-flight requests

1. Update the HandlerMapping contract to state that CORS checks are expected
to be applied before returning a handler.

2. DispatcherHandler checks explicitly for pre-flight requests or CORS failed
requests and skips handling for both. Technically no change since
AbstractHandlerMapping already returns a NO_OP_HANDLER for those cases.
The purpose however is for the DispatcherHandler to also guarantee more
explicitly that no such handling can take place for such cases.

As one consequence, this makes it possible to invoke the DispatcherHandler from
anywhere in the WebFilter chain in order to "handle" a pre-flight request, and
then skip the rest of the WebFilter chain.

See gh-26257

Author: rstoyanchev

spring-webflux/src/main/java/org/springframework/web/reactive/DispatcherHandler.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2019 the original author or authors.
+ * Copyright 2002-2021 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.
@@ -29,7 +29,10 @@
 import org.springframework.context.ApplicationContextAware;
 import org.springframework.core.annotation.AnnotationAwareOrderComparator;
 import org.springframework.http.HttpStatus;
+import org.springframework.http.server.reactive.ServerHttpRequest;
 import org.springframework.lang.Nullable;
+import org.springframework.util.ObjectUtils;
+import org.springframework.web.cors.reactive.CorsUtils;
 import org.springframework.web.server.ResponseStatusException;
 import org.springframework.web.server.ServerWebExchange;
 import org.springframework.web.server.WebHandler;
@@ -155,6 +158,12 @@ private <R> Mono<R> createNotFoundError() {
 	}
 
 	private Mono<HandlerResult> invokeHandler(ServerWebExchange exchange, Object handler) {
+		// No handling for CORS rejected requests and pre-flight requests
+		ServerHttpRequest request = exchange.getRequest();
+		HttpStatus status = exchange.getResponse().getStatusCode();
+		if (ObjectUtils.nullSafeEquals(status, HttpStatus.FORBIDDEN) || CorsUtils.isPreFlightRequest(request)) {
+			return Mono.empty();
+		}
 		if (this.handlerAdapters != null) {
 			for (HandlerAdapter handlerAdapter : this.handlerAdapters) {
 				if (handlerAdapter.supports(handler)) {

spring-webflux/src/main/java/org/springframework/web/reactive/HandlerMapping.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 the original author or authors.
+ * Copyright 2002-2021 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.
@@ -86,6 +86,11 @@ public interface HandlerMapping {
 
 	/**
 	 * Return a handler for this request.
+	 * <p>Before returning a handler, an implementing method should check for
+	 * CORS configuration associated with the handler, apply validation checks
+	 * based on it, and update the response accordingly. For pre-flight requests,
+	 * the same should be done based on the handler matching to the expected
+	 * actual request.
 	 * @param exchange current server exchange
 	 * @return a {@link Mono} that emits one value or none in case the request
 	 * cannot be resolved to a handler

spring-webflux/src/main/java/org/springframework/web/reactive/handler/AbstractHandlerMapping.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2020 the original author or authors.
+ * Copyright 2002-2021 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.
@@ -49,7 +49,7 @@
 public abstract class AbstractHandlerMapping extends ApplicationObjectSupport
 		implements HandlerMapping, Ordered, BeanNameAware {
 
-	private static final WebHandler REQUEST_HANDLED_HANDLER = exchange -> Mono.empty();
+	private static final WebHandler NO_OP_HANDLER = exchange -> Mono.empty();
 
 
 	private final PathPatternParser patternParser;
@@ -184,14 +184,15 @@ public Mono<Object> getHandler(ServerWebExchange exchange) {
 			}
 			ServerHttpRequest request = exchange.getRequest();
 			if (hasCorsConfigurationSource(handler) || CorsUtils.isPreFlightRequest(request)) {
-				CorsConfiguration config = (this.corsConfigurationSource != null ? this.corsConfigurationSource.getCorsConfiguration(exchange) : null);
+				CorsConfiguration config = (this.corsConfigurationSource != null ?
+						this.corsConfigurationSource.getCorsConfiguration(exchange) : null);
 				CorsConfiguration handlerConfig = getCorsConfiguration(handler, exchange);
 				config = (config != null ? config.combine(handlerConfig) : handlerConfig);
 				if (config != null) {
 					config.validateAllowCredentials();
 				}
 				if (!this.corsProcessor.process(config, exchange) || CorsUtils.isPreFlightRequest(request)) {
-					return REQUEST_HANDLED_HANDLER;
+					return NO_OP_HANDLER;
 				}
 			}
 			return handler;

spring-webflux/src/test/java/org/springframework/web/reactive/DispatcherHandlerTests.java

@@ -28,7 +28,10 @@
 import org.springframework.core.Ordered;
 import org.springframework.core.io.buffer.DataBuffer;
 import org.springframework.core.io.buffer.DefaultDataBufferFactory;
+import org.springframework.http.HttpHeaders;
+import org.springframework.web.reactive.result.SimpleHandlerAdapter;
 import org.springframework.web.server.ServerWebExchange;
+import org.springframework.web.server.WebHandler;
 import org.springframework.web.testfixture.http.server.reactive.MockServerHttpRequest;
 import org.springframework.web.testfixture.method.ResolvableMethod;
 import org.springframework.web.testfixture.server.MockServerWebExchange;
@@ -37,6 +40,7 @@
 import static org.mockito.ArgumentMatchers.any;
 import static org.mockito.BDDMockito.given;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verifyNoInteractions;
 import static org.mockito.Mockito.withSettings;
 
 /**
@@ -50,7 +54,7 @@ public class DispatcherHandlerTests {
 
 
 	@Test
-	public void handlerMappingOrder() {
+	void handlerMappingOrder() {
 		HandlerMapping hm1 = mock(HandlerMapping.class, withSettings().extraInterfaces(Ordered.class));
 		HandlerMapping hm2 = mock(HandlerMapping.class, withSettings().extraInterfaces(Ordered.class));
 		given(((Ordered) hm1).getOrder()).willReturn(1);
@@ -65,13 +69,34 @@ public void handlerMappingOrder() {
 		context.registerBean(HandlerResultHandler.class, StringHandlerResultHandler::new);
 		context.refresh();
 
-		DispatcherHandler dispatcherHandler = new DispatcherHandler(context);
-
 		MockServerWebExchange exchange = MockServerWebExchange.from(MockServerHttpRequest.get("/"));
-		dispatcherHandler.handle(exchange).block(Duration.ofSeconds(0));
+		new DispatcherHandler(context).handle(exchange).block(Duration.ofSeconds(0));
+
 		assertThat(exchange.getResponse().getBodyAsString().block(Duration.ofSeconds(5))).isEqualTo("1");
 	}
 
+	@Test
+	void preFlightRequest() {
+		WebHandler webHandler = mock(WebHandler.class);
+		HandlerMapping handlerMapping = mock(HandlerMapping.class);
+		given((handlerMapping).getHandler(any())).willReturn(Mono.just(webHandler));
+
+		StaticApplicationContext context = new StaticApplicationContext();
+		context.registerBean("handlerMapping", HandlerMapping.class, () -> handlerMapping);
+		context.registerBean(HandlerAdapter.class, SimpleHandlerAdapter::new);
+		context.registerBean(HandlerResultHandler.class, StringHandlerResultHandler::new);
+		context.refresh();
+
+		MockServerHttpRequest request = MockServerHttpRequest.options("/")
+				.header(HttpHeaders.ORIGIN, "https://domain.com")
+				.header(HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, "GET")
+				.build();
+
+		MockServerWebExchange exchange = MockServerWebExchange.from(request);
+		new DispatcherHandler(context).handle(exchange).block(Duration.ofSeconds(0));
+
+		verifyNoInteractions(webHandler);
+	}
 
 	@SuppressWarnings("unused")
 	private void handle() {}
@@ -101,7 +126,11 @@ public boolean supports(HandlerResult result) {
 
 		@Override
 		public Mono<Void> handleResult(ServerWebExchange exchange, HandlerResult result) {
-			byte[] bytes = ((String) result.getReturnValue()).getBytes(StandardCharsets.UTF_8);
+			Object returnValue = result.getReturnValue();
+			if (returnValue == null) {
+				return Mono.empty();
+			}
+			byte[] bytes = ((String) returnValue).getBytes(StandardCharsets.UTF_8);
 			DataBuffer dataBuffer = DefaultDataBufferFactory.sharedInstance.wrap(bytes);
 			return exchange.getResponse().writeWith(Mono.just(dataBuffer));
 		}