Skip to content

Commit d27b5d0

Browse files
sdeleuzesdeleuze
committed
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
1 parent 8714710 commit d27b5d0

File tree

20 files changed

+278
-123
lines changed

20 files changed

+278
-123
lines changed

‎spring-web/src/main/java/org/springframework/web/cors/CorsUtils.java

Lines changed: 34 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2015 the original author or authors.
2+
* Copyright 2002-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -20,6 +20,10 @@
2020

2121
import org.springframework.http.HttpHeaders;
2222
import org.springframework.http.HttpMethod;
23+
import org.springframework.lang.Nullable;
24+
import org.springframework.util.ObjectUtils;
25+
import org.springframework.web.util.UriComponents;
26+
import org.springframework.web.util.UriComponentsBuilder;
2327

2428
/**
2529
* Utility class for CORS request handling based on the
@@ -31,17 +35,43 @@
3135
public abstract class CorsUtils {
3236

3337
/**
34-
* Returns {@code true} if the request is a valid CORS one.
38+
* Returns {@code true} if the request is a valid CORS one by checking {@code Origin}
39+
* header presence and ensuring that origins are different.
3540
*/
3641
public static boolean isCorsRequest(HttpServletRequest request) {
37-
return (request.getHeader(HttpHeaders.ORIGIN) != null);
42+
String origin = request.getHeader(HttpHeaders.ORIGIN);
43+
if (origin == null) {
44+
return false;
45+
}
46+
UriComponents originUrl = UriComponentsBuilder.fromOriginHeader(origin).build();
47+
String scheme = request.getScheme();
48+
String host = request.getServerName();
49+
int port = request.getServerPort();
50+
return !(ObjectUtils.nullSafeEquals(scheme, originUrl.getScheme()) &&
51+
ObjectUtils.nullSafeEquals(host, originUrl.getHost()) &&
52+
getPort(scheme, port) == getPort(originUrl.getScheme(), originUrl.getPort()));
53+
54+
}
55+
56+
private static int getPort(@Nullable String scheme, int port) {
57+
if (port == -1) {
58+
if ("http".equals(scheme) || "ws".equals(scheme)) {
59+
port = 80;
60+
}
61+
else if ("https".equals(scheme) || "wss".equals(scheme)) {
62+
port = 443;
63+
}
64+
}
65+
return port;
3866
}
3967

4068
/**
4169
* Returns {@code true} if the request is a valid CORS pre-flight one.
70+
* To be used in combination with {@link #isCorsRequest(HttpServletRequest)} since
71+
* regular CORS checks are not invoked here for performance reasons.
4272
*/
4373
public static boolean isPreFlightRequest(HttpServletRequest request) {
44-
return (isCorsRequest(request) && HttpMethod.OPTIONS.matches(request.getMethod()) &&
74+
return (HttpMethod.OPTIONS.matches(request.getMethod()) &&
4575
request.getHeader(HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD) != null);
4676
}
4777

‎spring-web/src/main/java/org/springframework/web/cors/DefaultCorsProcessor.java

Lines changed: 10 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2018 the original author or authors.
2+
* Copyright 2002-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -19,7 +19,6 @@
1919
import java.io.IOException;
2020
import java.nio.charset.StandardCharsets;
2121
import java.util.ArrayList;
22-
import java.util.Arrays;
2322
import java.util.List;
2423
import javax.servlet.http.HttpServletRequest;
2524
import javax.servlet.http.HttpServletResponse;
@@ -36,7 +35,6 @@
3635
import org.springframework.http.server.ServletServerHttpResponse;
3736
import org.springframework.lang.Nullable;
3837
import org.springframework.util.CollectionUtils;
39-
import org.springframework.web.util.WebUtils;
4038

4139
/**
4240
* The default implementation of {@link CorsProcessor}, as defined by the
@@ -45,8 +43,7 @@
4543
* <p>Note that when input {@link CorsConfiguration} is {@code null}, this
4644
* implementation does not reject simple or actual requests outright but simply
4745
* avoid adding CORS headers to the response. CORS processing is also skipped
48-
* if the response already contains CORS headers, or if the request is detected
49-
* as a same-origin one.
46+
* if the response already contains CORS headers.
5047
*
5148
* @author Sebastien Deleuze
5249
* @author Rossen Stoyanchev
@@ -62,44 +59,31 @@ public class DefaultCorsProcessor implements CorsProcessor {
6259
public boolean processRequest(@Nullable CorsConfiguration config, HttpServletRequest request,
6360
HttpServletResponse response) throws IOException {
6461

62+
response.addHeader(HttpHeaders.VARY, HttpHeaders.ORIGIN);
63+
response.addHeader(HttpHeaders.VARY, HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD);
64+
response.addHeader(HttpHeaders.VARY, HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS);
65+
6566
if (!CorsUtils.isCorsRequest(request)) {
6667
return true;
6768
}
6869

69-
ServletServerHttpResponse serverResponse = new ServletServerHttpResponse(response);
70-
if (responseHasCors(serverResponse)) {
70+
if (response.getHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN) != null) {
7171
logger.trace("Skip: response already contains \"Access-Control-Allow-Origin\"");
7272
return true;
7373
}
7474

75-
ServletServerHttpRequest serverRequest = new ServletServerHttpRequest(request);
76-
if (WebUtils.isSameOrigin(serverRequest)) {
77-
logger.trace("Skip: request is from same origin");
78-
return true;
79-
}
80-
8175
boolean preFlightRequest = CorsUtils.isPreFlightRequest(request);
8276
if (config == null) {
8377
if (preFlightRequest) {
84-
rejectRequest(serverResponse);
78+
rejectRequest(new ServletServerHttpResponse(response));
8579
return false;
8680
}
8781
else {
8882
return true;
8983
}
9084
}
9185

92-
return handleInternal(serverRequest, serverResponse, config, preFlightRequest);
93-
}
94-
95-
private boolean responseHasCors(ServerHttpResponse response) {
96-
try {
97-
return (response.getHeaders().getAccessControlAllowOrigin() != null);
98-
}
99-
catch (NullPointerException npe) {
100-
// SPR-11919 and https://issues.jboss.org/browse/WFLY-3474
101-
return false;
102-
}
86+
return handleInternal(new ServletServerHttpRequest(request), new ServletServerHttpResponse(response), config, preFlightRequest);
10387
}
10488

10589
/**
@@ -110,6 +94,7 @@ private boolean responseHasCors(ServerHttpResponse response) {
11094
protected void rejectRequest(ServerHttpResponse response) throws IOException {
11195
response.setStatusCode(HttpStatus.FORBIDDEN);
11296
response.getBody().write("Invalid CORS request".getBytes(StandardCharsets.UTF_8));
97+
response.flush();
11398
}
11499

115100
/**
@@ -122,9 +107,6 @@ protected boolean handleInternal(ServerHttpRequest request, ServerHttpResponse r
122107
String allowOrigin = checkOrigin(config, requestOrigin);
123108
HttpHeaders responseHeaders = response.getHeaders();
124109

125-
responseHeaders.addAll(HttpHeaders.VARY, Arrays.asList(HttpHeaders.ORIGIN,
126-
HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS));
127-
128110
if (allowOrigin == null) {
129111
logger.debug("Reject: '" + requestOrigin + "' origin is not allowed");
130112
rejectRequest(response);

‎spring-web/src/main/java/org/springframework/web/cors/reactive/CorsUtils.java

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2018 the original author or authors.
2+
* Copyright 2002-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -36,18 +36,21 @@
3636
public abstract class CorsUtils {
3737

3838
/**
39-
* Returns {@code true} if the request is a valid CORS one.
39+
* Returns {@code true} if the request is a valid CORS one by checking {@code Origin}
40+
* header presence and ensuring that origins are different via {@link #isSameOrigin}.
4041
*/
42+
@SuppressWarnings("deprecation")
4143
public static boolean isCorsRequest(ServerHttpRequest request) {
42-
return (request.getHeaders().get(HttpHeaders.ORIGIN) != null);
44+
return request.getHeaders().containsKey(HttpHeaders.ORIGIN) && !isSameOrigin(request);
4345
}
4446

4547
/**
4648
* Returns {@code true} if the request is a valid CORS pre-flight one.
49+
* To be used in combination with {@link #isCorsRequest(ServerHttpRequest)} since
50+
* regular CORS checks are not invoked here for performance reasons.
4751
*/
4852
public static boolean isPreFlightRequest(ServerHttpRequest request) {
49-
return (request.getMethod() == HttpMethod.OPTIONS && isCorsRequest(request) &&
50-
request.getHeaders().get(HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD) != null);
53+
return (request.getMethod() == HttpMethod.OPTIONS && request.getHeaders().containsKey(HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD));
5154
}
5255

5356
/**
@@ -61,7 +64,9 @@ public static boolean isPreFlightRequest(ServerHttpRequest request) {
6164
*
6265
* @return {@code true} if the request is a same-origin one, {@code false} in case
6366
* of a cross-origin request
67+
* @deprecated as of 5.2, same-origin checks are performed directly by {@link #isCorsRequest}
6468
*/
69+
@Deprecated
6570
public static boolean isSameOrigin(ServerHttpRequest request) {
6671
String origin = request.getHeaders().getOrigin();
6772
if (origin == null) {

‎spring-web/src/main/java/org/springframework/web/cors/reactive/CorsWebFilter.java

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2018 the original author or authors.
2+
* Copyright 2002-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -75,14 +75,10 @@ public CorsWebFilter(CorsConfigurationSource configSource, CorsProcessor process
7575
@Override
7676
public Mono<Void> filter(ServerWebExchange exchange, WebFilterChain chain) {
7777
ServerHttpRequest request = exchange.getRequest();
78-
if (CorsUtils.isCorsRequest(request)) {
79-
CorsConfiguration corsConfiguration = this.configSource.getCorsConfiguration(exchange);
80-
if (corsConfiguration != null) {
81-
boolean isValid = this.processor.process(corsConfiguration, exchange);
82-
if (!isValid || CorsUtils.isPreFlightRequest(request)) {
83-
return Mono.empty();
84-
}
85-
}
78+
CorsConfiguration corsConfiguration = this.configSource.getCorsConfiguration(exchange);
79+
boolean isValid = this.processor.process(corsConfiguration, exchange);
80+
if (!isValid || CorsUtils.isPreFlightRequest(request)) {
81+
return Mono.empty();
8682
}
8783
return chain.filter(exchange);
8884
}

‎spring-web/src/main/java/org/springframework/web/cors/reactive/DefaultCorsProcessor.java

Lines changed: 7 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2018 the original author or authors.
2+
* Copyright 2002-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -40,8 +40,7 @@
4040
* <p>Note that when input {@link CorsConfiguration} is {@code null}, this
4141
* implementation does not reject simple or actual requests outright but simply
4242
* avoid adding CORS headers to the response. CORS processing is also skipped
43-
* if the response already contains CORS headers, or if the request is detected
44-
* as a same-origin one.
43+
* if the response already contains CORS headers.
4544
*
4645
* @author Sebastien Deleuze
4746
* @author Rossen Stoyanchev
@@ -51,27 +50,26 @@ public class DefaultCorsProcessor implements CorsProcessor {
5150

5251
private static final Log logger = LogFactory.getLog(DefaultCorsProcessor.class);
5352

53+
private static final List<String> VARY_HEADERS = Arrays.asList(
54+
HttpHeaders.ORIGIN, HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS);
55+
5456

5557
@Override
5658
public boolean process(@Nullable CorsConfiguration config, ServerWebExchange exchange) {
5759

5860
ServerHttpRequest request = exchange.getRequest();
5961
ServerHttpResponse response = exchange.getResponse();
62+
response.getHeaders().addAll(HttpHeaders.VARY, VARY_HEADERS);
6063

6164
if (!CorsUtils.isCorsRequest(request)) {
6265
return true;
6366
}
6467

65-
if (responseHasCors(response)) {
68+
if (response.getHeaders().getFirst(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN) != null) {
6669
logger.trace("Skip: response already contains \"Access-Control-Allow-Origin\"");
6770
return true;
6871
}
6972

70-
if (CorsUtils.isSameOrigin(request)) {
71-
logger.trace("Skip: request is from same origin");
72-
return true;
73-
}
74-
7573
boolean preFlightRequest = CorsUtils.isPreFlightRequest(request);
7674
if (config == null) {
7775
if (preFlightRequest) {
@@ -86,10 +84,6 @@ public boolean process(@Nullable CorsConfiguration config, ServerWebExchange exc
8684
return handleInternal(exchange, config, preFlightRequest);
8785
}
8886

89-
private boolean responseHasCors(ServerHttpResponse response) {
90-
return response.getHeaders().getFirst(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN) != null;
91-
}
92-
9387
/**
9488
* Invoked when one of the CORS checks failed.
9589
*/
@@ -107,9 +101,6 @@ protected boolean handleInternal(ServerWebExchange exchange,
107101
ServerHttpResponse response = exchange.getResponse();
108102
HttpHeaders responseHeaders = response.getHeaders();
109103

110-
response.getHeaders().addAll(HttpHeaders.VARY, Arrays.asList(HttpHeaders.ORIGIN,
111-
HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS));
112-
113104
String requestOrigin = request.getHeaders().getOrigin();
114105
String allowOrigin = checkOrigin(config, requestOrigin);
115106
if (allowOrigin == null) {

‎spring-web/src/main/java/org/springframework/web/filter/CorsFilter.java

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2016 the original author or authors.
2+
* Copyright 2002-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -83,16 +83,11 @@ public void setCorsProcessor(CorsProcessor processor) {
8383
protected void doFilterInternal(HttpServletRequest request, HttpServletResponse response,
8484
FilterChain filterChain) throws ServletException, IOException {
8585

86-
if (CorsUtils.isCorsRequest(request)) {
87-
CorsConfiguration corsConfiguration = this.configSource.getCorsConfiguration(request);
88-
if (corsConfiguration != null) {
89-
boolean isValid = this.processor.processRequest(corsConfiguration, request, response);
90-
if (!isValid || CorsUtils.isPreFlightRequest(request)) {
91-
return;
92-
}
93-
}
86+
CorsConfiguration corsConfiguration = this.configSource.getCorsConfiguration(request);
87+
boolean isValid = this.processor.processRequest(corsConfiguration, request, response);
88+
if (!isValid || CorsUtils.isPreFlightRequest(request)) {
89+
return;
9490
}
95-
9691
filterChain.doFilter(request, response);
9792
}
9893

‎spring-web/src/test/java/org/springframework/web/cors/CorsUtilsTests.java

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2015 the original author or authors.
2+
* Copyright 2002-2019 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -62,11 +62,6 @@ public void isNotPreFlightRequest() {
6262
request.setMethod(HttpMethod.OPTIONS.name());
6363
request.addHeader(HttpHeaders.ORIGIN, "https://domain.com");
6464
assertFalse(CorsUtils.isPreFlightRequest(request));
65-
66-
request = new MockHttpServletRequest();
67-
request.setMethod(HttpMethod.OPTIONS.name());
68-
request.addHeader(HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, "GET");
69-
assertFalse(CorsUtils.isPreFlightRequest(request));
7065
}
7166

7267
}

‎spring-web/src/test/java/org/springframework/web/cors/DefaultCorsProcessorTests.java

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,13 +51,35 @@ public class DefaultCorsProcessorTests {
5151
public void setup() {
5252
this.request = new MockHttpServletRequest();
5353
this.request.setRequestURI("/test.html");
54-
this.request.setRemoteHost("domain1.com");
54+
this.request.setServerName("domain1.com");
5555
this.conf = new CorsConfiguration();
5656
this.response = new MockHttpServletResponse();
5757
this.response.setStatus(HttpServletResponse.SC_OK);
5858
this.processor = new DefaultCorsProcessor();
5959
}
6060

61+
@Test
62+
public void requestWithoutOriginHeader() throws Exception {
63+
this.request.setMethod(HttpMethod.GET.name());
64+
65+
this.processor.processRequest(this.conf, this.request, this.response);
66+
assertFalse(this.response.containsHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN));
67+
assertThat(this.response.getHeaders(HttpHeaders.VARY), contains(HttpHeaders.ORIGIN,
68+
HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS));
69+
assertEquals(HttpServletResponse.SC_OK, this.response.getStatus());
70+
}
71+
72+
@Test
73+
public void sameOriginRequest() throws Exception {
74+
this.request.setMethod(HttpMethod.GET.name());
75+
this.request.addHeader(HttpHeaders.ORIGIN, "http://domain1.com");
76+
77+
this.processor.processRequest(this.conf, this.request, this.response);
78+
assertFalse(this.response.containsHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN));
79+
assertThat(this.response.getHeaders(HttpHeaders.VARY), contains(HttpHeaders.ORIGIN,
80+
HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS));
81+
assertEquals(HttpServletResponse.SC_OK, this.response.getStatus());
82+
}
6183

6284
@Test
6385
public void actualRequestWithOriginHeader() throws Exception {

0 commit comments

Comments
 (0)