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
This commit is contained in:
Rossen Stoyanchev
2021-01-29 22:10:03 +00:00
parent 0ff50d6d9e
commit 95752ef1c9
4 changed files with 55 additions and 11 deletions

View File

@@ -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.ApplicationContext;
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 @@ public class DispatcherHandler implements WebHandler, ApplicationContextAware {
}
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)) {

View File

@@ -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

View File

@@ -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 @@ import org.springframework.web.util.pattern.PathPatternParser;
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 abstract class AbstractHandlerMapping extends ApplicationObjectSupport
}
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;

View File

@@ -28,7 +28,10 @@ import org.springframework.core.MethodParameter;
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.assertj.core.api.Assertions.assertThat;
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 class DispatcherHandlerTests {
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 class DispatcherHandlerTests {
@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));
}