Improve CORS handling

This commit improves CORS support by:
 - Using CORS processing only for CORS-enabled endpoints
 - Skipping CORS processing for same-origin requests
 - Adding Vary headers for non-CORS requests

It introduces an AbstractHandlerMapping#hasCorsConfigurationSource
method in order to be able to check CORS endpoints efficiently.

Closes gh-22273
Closes gh-22496
This commit is contained in:
Sebastien Deleuze
2019-04-01 14:36:38 +02:00
parent 8714710170
commit d27b5d0ab6
20 changed files with 278 additions and 123 deletions

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2015 the original author or authors.
* Copyright 2002-2019 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.
@@ -62,11 +62,6 @@ public class CorsUtilsTests {
request.setMethod(HttpMethod.OPTIONS.name());
request.addHeader(HttpHeaders.ORIGIN, "https://domain.com");
assertFalse(CorsUtils.isPreFlightRequest(request));
request = new MockHttpServletRequest();
request.setMethod(HttpMethod.OPTIONS.name());
request.addHeader(HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, "GET");
assertFalse(CorsUtils.isPreFlightRequest(request));
}
}

View File

@@ -51,13 +51,35 @@ public class DefaultCorsProcessorTests {
public void setup() {
this.request = new MockHttpServletRequest();
this.request.setRequestURI("/test.html");
this.request.setRemoteHost("domain1.com");
this.request.setServerName("domain1.com");
this.conf = new CorsConfiguration();
this.response = new MockHttpServletResponse();
this.response.setStatus(HttpServletResponse.SC_OK);
this.processor = new DefaultCorsProcessor();
}
@Test
public void requestWithoutOriginHeader() throws Exception {
this.request.setMethod(HttpMethod.GET.name());
this.processor.processRequest(this.conf, this.request, this.response);
assertFalse(this.response.containsHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN));
assertThat(this.response.getHeaders(HttpHeaders.VARY), contains(HttpHeaders.ORIGIN,
HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS));
assertEquals(HttpServletResponse.SC_OK, this.response.getStatus());
}
@Test
public void sameOriginRequest() throws Exception {
this.request.setMethod(HttpMethod.GET.name());
this.request.addHeader(HttpHeaders.ORIGIN, "http://domain1.com");
this.processor.processRequest(this.conf, this.request, this.response);
assertFalse(this.response.containsHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN));
assertThat(this.response.getHeaders(HttpHeaders.VARY), contains(HttpHeaders.ORIGIN,
HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS));
assertEquals(HttpServletResponse.SC_OK, this.response.getStatus());
}
@Test
public void actualRequestWithOriginHeader() throws Exception {

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2018 the original author or authors.
* Copyright 2002-2019 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.
@@ -39,7 +39,7 @@ public class CorsUtilsTests {
@Test
public void isCorsRequest() {
ServerHttpRequest request = get("/").header(HttpHeaders.ORIGIN, "https://domain.com").build();
ServerHttpRequest request = get("http://domain.com/").header(HttpHeaders.ORIGIN, "https://domain.com").build();
assertTrue(CorsUtils.isCorsRequest(request));
}
@@ -65,9 +65,6 @@ public class CorsUtilsTests {
request = options("/").header(HttpHeaders.ORIGIN, "https://domain.com").build();
assertFalse(CorsUtils.isPreFlightRequest(request));
request = options("/").header(HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, "GET").build();
assertFalse(CorsUtils.isPreFlightRequest(request));
}
@Test // SPR-16262

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2017 the original author or authors.
* Copyright 2002-2019 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.
@@ -63,6 +63,46 @@ public class CorsWebFilterTests {
filter = new CorsWebFilter(r -> config);
}
@Test
public void nonCorsRequest() {
WebFilterChain filterChain = (filterExchange) -> {
try {
HttpHeaders headers = filterExchange.getResponse().getHeaders();
assertNull(headers.getFirst(ACCESS_CONTROL_ALLOW_ORIGIN));
assertNull(headers.getFirst(ACCESS_CONTROL_EXPOSE_HEADERS));
} catch (AssertionError ex) {
return Mono.error(ex);
}
return Mono.empty();
};
MockServerWebExchange exchange = MockServerWebExchange.from(
MockServerHttpRequest
.get("https://domain1.com/test.html")
.header(HOST, "domain1.com"));
this.filter.filter(exchange, filterChain).block();
}
@Test
public void sameOriginRequest() {
WebFilterChain filterChain = (filterExchange) -> {
try {
HttpHeaders headers = filterExchange.getResponse().getHeaders();
assertNull(headers.getFirst(ACCESS_CONTROL_ALLOW_ORIGIN));
assertNull(headers.getFirst(ACCESS_CONTROL_EXPOSE_HEADERS));
} catch (AssertionError ex) {
return Mono.error(ex);
}
return Mono.empty();
};
MockServerWebExchange exchange = MockServerWebExchange.from(
MockServerHttpRequest
.get("https://domain1.com/test.html")
.header(ORIGIN, "https://domain1.com"));
this.filter.filter(exchange, filterChain).block();
}
@Test
public void validActualRequest() {
WebFilterChain filterChain = (filterExchange) -> {
@@ -82,7 +122,7 @@ public class CorsWebFilterTests {
.header(HOST, "domain1.com")
.header(ORIGIN, "https://domain2.com")
.header("header2", "foo"));
this.filter.filter(exchange, filterChain);
this.filter.filter(exchange, filterChain).block();
}
@Test
@@ -96,8 +136,7 @@ public class CorsWebFilterTests {
WebFilterChain filterChain = (filterExchange) -> Mono.error(
new AssertionError("Invalid requests must not be forwarded to the filter chain"));
filter.filter(exchange, filterChain);
filter.filter(exchange, filterChain).block();
assertNull(exchange.getResponse().getHeaders().getFirst(ACCESS_CONTROL_ALLOW_ORIGIN));
}
@@ -115,7 +154,7 @@ public class CorsWebFilterTests {
WebFilterChain filterChain = (filterExchange) -> Mono.error(
new AssertionError("Preflight requests must not be forwarded to the filter chain"));
filter.filter(exchange, filterChain);
filter.filter(exchange, filterChain).block();
HttpHeaders headers = exchange.getResponse().getHeaders();
assertEquals("https://domain2.com", headers.getFirst(ACCESS_CONTROL_ALLOW_ORIGIN));
@@ -138,7 +177,7 @@ public class CorsWebFilterTests {
WebFilterChain filterChain = (filterExchange) -> Mono.error(
new AssertionError("Preflight requests must not be forwarded to the filter chain"));
filter.filter(exchange, filterChain);
filter.filter(exchange, filterChain).block();
assertNull(exchange.getResponse().getHeaders().getFirst(ACCESS_CONTROL_ALLOW_ORIGIN));
}

View File

@@ -60,6 +60,37 @@ public class DefaultCorsProcessorTests {
}
@Test
public void requestWithoutOriginHeader() throws Exception {
MockServerHttpRequest request = MockServerHttpRequest
.method(HttpMethod.GET, "http://domain1.com/test.html")
.build();
ServerWebExchange exchange = MockServerWebExchange.from(request);
this.processor.process(this.conf, exchange);
ServerHttpResponse response = exchange.getResponse();
assertFalse(response.getHeaders().containsKey(ACCESS_CONTROL_ALLOW_ORIGIN));
assertThat(response.getHeaders().get(VARY), contains(ORIGIN,
ACCESS_CONTROL_REQUEST_METHOD, ACCESS_CONTROL_REQUEST_HEADERS));
assertNull(response.getStatusCode());
}
@Test
public void sameOriginRequest() throws Exception {
MockServerHttpRequest request = MockServerHttpRequest
.method(HttpMethod.GET, "http://domain1.com/test.html")
.header(HttpHeaders.ORIGIN, "http://domain1.com")
.build();
ServerWebExchange exchange = MockServerWebExchange.from(request);
this.processor.process(this.conf, exchange);
ServerHttpResponse response = exchange.getResponse();
assertFalse(response.getHeaders().containsKey(ACCESS_CONTROL_ALLOW_ORIGIN));
assertThat(response.getHeaders().get(VARY), contains(ORIGIN,
ACCESS_CONTROL_REQUEST_METHOD, ACCESS_CONTROL_REQUEST_HEADERS));
assertNull(response.getStatusCode());
}
@Test
public void actualRequestWithOriginHeader() throws Exception {
ServerWebExchange exchange = actualRequest();

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2015 the original author or authors.
* Copyright 2002-2019 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.
@@ -52,6 +52,36 @@ public class CorsFilterTests {
filter = new CorsFilter(r -> config);
}
@Test
public void nonCorsRequest() throws ServletException, IOException {
MockHttpServletRequest request = new MockHttpServletRequest(HttpMethod.GET.name(), "/test.html");
MockHttpServletResponse response = new MockHttpServletResponse();
FilterChain filterChain = (filterRequest, filterResponse) -> {
assertNull(response.getHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN));
assertNull(response.getHeader(HttpHeaders.ACCESS_CONTROL_EXPOSE_HEADERS));
};
filter.doFilter(request, response, filterChain);
}
@Test
public void sameOriginRequest() throws ServletException, IOException {
MockHttpServletRequest request = new MockHttpServletRequest(HttpMethod.GET.name(), "https://domain1.com/test.html");
request.addHeader(HttpHeaders.ORIGIN, "https://domain1.com");
request.setScheme("https");
request.setServerName("domain1.com");
request.setServerPort(443);
MockHttpServletResponse response = new MockHttpServletResponse();
FilterChain filterChain = (filterRequest, filterResponse) -> {
assertNull(response.getHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN));
assertNull(response.getHeader(HttpHeaders.ACCESS_CONTROL_EXPOSE_HEADERS));
};
filter.doFilter(request, response, filterChain);
}
@Test
public void validActualRequest() throws ServletException, IOException {