Spring/spring-framework
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Update CORS support

Browse Source 
This commit updates CORS support in order to check Origin header
in CorsUtils#isPreFlightRequest which does not change how Spring
MVC or WebFlux process CORS request but is more correct in term
of behavior since it is a public API potentially used in another
contexts.

It also removes an unnecessary check in
AbstractHandlerMethodMapping#hasCorsConfigurationSource and processes
every preflight request with PreFlightHandler.

Closes gh-24327
This commit is contained in:
Sébastien Deleuze
2020-01-10 14:51:55 +01:00
parent 8396e6bdd1
commit bc7d010485
10 changed files with 74 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files

8
spring-web/src/main/java/org/springframework/web/cors/CorsUtils.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2020 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.
@@ -66,12 +66,12 @@ public abstract class CorsUtils {
}
/**
* Returns {@code true} if the request is a valid CORS pre-flight one.
* To be used in combination with {@link #isCorsRequest(HttpServletRequest)} since
* regular CORS checks are not invoked here for performance reasons.
* Returns {@code true} if the request is a valid CORS pre-flight one by checking {code OPTIONS} method with
* {@code Origin} and {@code Access-Control-Request-Method} headers presence.
*/
public static boolean isPreFlightRequest(HttpServletRequest request) {
return (HttpMethod.OPTIONS.matches(request.getMethod()) &&
request.getHeader(HttpHeaders.ORIGIN) != null &&
request.getHeader(HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD) != null);
}

12
spring-web/src/main/java/org/springframework/web/cors/reactive/CorsUtils.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2020 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.
@@ -45,12 +45,14 @@ public abstract class CorsUtils {
}
/**
* Returns {@code true} if the request is a valid CORS pre-flight one.
* To be used in combination with {@link #isCorsRequest(ServerHttpRequest)} since
* regular CORS checks are not invoked here for performance reasons.
* Returns {@code true} if the request is a valid CORS pre-flight one by checking {code OPTIONS} method with
* {@code Origin} and {@code Access-Control-Request-Method} headers presence.
*/
public static boolean isPreFlightRequest(ServerHttpRequest request) {
return (request.getMethod() == HttpMethod.OPTIONS && request.getHeaders().containsKey(HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD));
HttpHeaders headers = request.getHeaders();
return (request.getMethod() == HttpMethod.OPTIONS
&& headers.containsKey(HttpHeaders.ORIGIN)
&& headers.containsKey(HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD));
}
/**

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

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2020 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.
@@ -182,8 +182,8 @@ public abstract class AbstractHandlerMapping extends ApplicationObjectSupport
if (logger.isDebugEnabled()) {
logger.debug(exchange.getLogPrefix() + "Mapped to " + handler);
}
if (hasCorsConfigurationSource(handler)) {
ServerHttpRequest request = exchange.getRequest();
ServerHttpRequest request = exchange.getRequest();
if (hasCorsConfigurationSource(handler) || CorsUtils.isPreFlightRequest(request)) {
CorsConfiguration config = (this.corsConfigurationSource != null ? this.corsConfigurationSource.getCorsConfiguration(exchange) : null);
CorsConfiguration handlerConfig = getCorsConfiguration(handler, exchange);
config = (config != null ? config.combine(handlerConfig) : handlerConfig);

5
spring-webflux/src/main/java/org/springframework/web/reactive/result/method/AbstractHandlerMethodMapping.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2020 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.
@@ -374,8 +374,7 @@ public abstract class AbstractHandlerMethodMapping<T> extends AbstractHandlerMap
@Override
protected boolean hasCorsConfigurationSource(Object handler) {
return super.hasCorsConfigurationSource(handler) ||
(handler instanceof HandlerMethod && this.mappingRegistry.getCorsConfiguration((HandlerMethod) handler) != null) ||
handler.equals(PREFLIGHT_AMBIGUOUS_MATCH);
(handler instanceof HandlerMethod && this.mappingRegistry.getCorsConfiguration((HandlerMethod) handler) != null);
}
@Override

4
spring-webflux/src/test/java/org/springframework/web/reactive/handler/CorsUrlHandlerMappingTests.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2020 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.
@@ -70,7 +70,7 @@ public class CorsUrlHandlerMappingTests {
Object actual = this.handlerMapping.getHandler(exchange).block();
assertThat(actual).isNotNull();
assertThat(actual).isSameAs(this.welcomeController);
assertThat(actual).isNotSameAs(this.welcomeController);
}
@Test

26
spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/CrossOriginAnnotationIntegrationTests.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2020 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.
@@ -35,11 +35,13 @@ import org.springframework.web.bind.annotation.PostMapping;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.bind.annotation.RequestMethod;
import org.springframework.web.bind.annotation.RestController;
import org.springframework.web.client.HttpClientErrorException;
import org.springframework.web.client.RestTemplate;
import org.springframework.web.reactive.config.EnableWebFlux;
import org.springframework.web.testfixture.http.server.reactive.bootstrap.HttpServer;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.fail;
/**
* Integration tests with {@code @CrossOrigin} and {@code @RequestMapping}
@@ -89,6 +91,28 @@ class CrossOriginAnnotationIntegrationTests extends AbstractRequestMappingIntegr
assertThat(entity.getBody()).isEqualTo("no");
}
@ParameterizedHttpServerTest
void optionsRequestWithAccessControlRequestMethod(HttpServer httpServer) throws Exception {
startServer(httpServer);
this.headers.clear();
this.headers.add(HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, "GET");
ResponseEntity<String> entity = performOptions("/no", this.headers, String.class);
assertThat(entity.getBody()).isNull();
}
@ParameterizedHttpServerTest
void preflightRequestWithoutAnnotation(HttpServer httpServer) throws Exception {
startServer(httpServer);
this.headers.add(HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, "GET");
try {
performOptions("/no", this.headers, Void.class);
fail("Preflight request without CORS configuration should fail");
}
catch (HttpClientErrorException ex) {
assertThat(ex.getStatusCode()).isEqualTo(HttpStatus.FORBIDDEN);
}
}
@ParameterizedHttpServerTest
void actualPostRequestWithoutAnnotation(HttpServer httpServer) throws Exception {
startServer(httpServer);

4
spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerMapping.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2020 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.
@@ -414,7 +414,7 @@ public abstract class AbstractHandlerMapping extends WebApplicationObjectSupport
logger.debug("Mapped to " + executionChain.getHandler());
}
if (hasCorsConfigurationSource(handler)) {
if (hasCorsConfigurationSource(handler) || CorsUtils.isPreFlightRequest(request)) {
CorsConfiguration config = (this.corsConfigurationSource != null ? this.corsConfigurationSource.getCorsConfiguration(request) : null);
CorsConfiguration handlerConfig = getCorsConfiguration(handler, request);
config = (config != null ? config.combine(handlerConfig) : handlerConfig);

5
spring-webmvc/src/main/java/org/springframework/web/servlet/handler/AbstractHandlerMethodMapping.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2020 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.
@@ -458,8 +458,7 @@ public abstract class AbstractHandlerMethodMapping<T> extends AbstractHandlerMap
@Override
protected boolean hasCorsConfigurationSource(Object handler) {
return super.hasCorsConfigurationSource(handler) ||
(handler instanceof HandlerMethod && this.mappingRegistry.getCorsConfiguration((HandlerMethod) handler) != null) ||
handler.equals(PREFLIGHT_AMBIGUOUS_MATCH);
(handler instanceof HandlerMethod && this.mappingRegistry.getCorsConfiguration((HandlerMethod) handler) != null);
}
@Override

5
spring-webmvc/src/test/java/org/springframework/web/servlet/handler/CorsAbstractHandlerMappingTests.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2020 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,7 +86,8 @@ class CorsAbstractHandlerMappingTests {
HandlerExecutionChain chain = this.handlerMapping.getHandler(this.request);
assertThat(chain).isNotNull();
assertThat(chain.getHandler()).isInstanceOf(SimpleHandler.class);
assertThat(chain.getHandler()).isNotNull();
assertThat(chain.getHandler().getClass().getSimpleName()).isEqualTo("PreFlightHandler");
}
@Test

25
spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/CrossOriginTests.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2020 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.
@@ -69,6 +69,10 @@ public class CrossOriginTests {
private final MockHttpServletRequest request = new MockHttpServletRequest();
private final String optionsHandler = "org.springframework.web.servlet.mvc.method.RequestMappingInfoHandlerMapping$HttpOptionsHandler#handle()";
private final String corsPreflightHandler = "org.springframework.web.servlet.handler.AbstractHandlerMapping$PreFlightHandler";
@BeforeEach
@SuppressWarnings("resource")
@@ -96,6 +100,25 @@ public class CrossOriginTests {
assertThat(getCorsConfiguration(chain, false)).isNull();
}
@Test
public void noAnnotationWithAccessControlRequestMethod() throws Exception {
this.handlerMapping.registerHandler(new MethodLevelController());
MockHttpServletRequest request = new MockHttpServletRequest("OPTIONS", "/no");
request.addHeader(HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, "GET");
HandlerExecutionChain chain = this.handlerMapping.getHandler(request);
assertThat(chain.getHandler().toString()).isEqualTo(optionsHandler);
}
@Test
public void noAnnotationWithPreflightRequest() throws Exception {
this.handlerMapping.registerHandler(new MethodLevelController());
MockHttpServletRequest request = new MockHttpServletRequest("OPTIONS", "/no");
request.addHeader(HttpHeaders.ORIGIN, "https://domain.com/");
request.addHeader(HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, "GET");
HandlerExecutionChain chain = this.handlerMapping.getHandler(request);
assertThat(chain.getHandler().getClass().getName()).isEqualTo(corsPreflightHandler);
}
@Test // SPR-12931
public void noAnnotationWithOrigin() throws Exception {
this.handlerMapping.registerHandler(new MethodLevelController());