Skip to content

Commit eee45c3

Browse files
sdeleuzesdeleuze
committed
Refine CORS preflight requests handling with no configuration
This commit makes CORS preflight requests handling more flexible by just skipping setting CORS response headers when no configuration is defined instead of rejecting them. That will have the same effect on user agent side (the preflight request will be considered as not authorized and the actual request not performed) but is more flexible and more efficient. Closes gh-31839
1 parent 76d335a commit eee45c3

File tree

8 files changed

+45
-54
lines changed

8 files changed

+45
-54
lines changed

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

Lines changed: 10 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2024 the original author or authors.
2+
* Copyright 2002-2025 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.
@@ -42,9 +42,9 @@
4242
* <a href="https://www.w3.org/TR/cors/">CORS W3C recommendation</a>.
4343
*
4444
* <p>Note that when the supplied {@link CorsConfiguration} is {@code null}, this
45-
* implementation does not reject simple or actual requests outright but simply
46-
* avoids adding CORS headers to the response. CORS processing is also skipped
47-
* if the response already contains CORS headers.
45+
* implementation does not reject CORS requests outright but simply avoids adding
46+
* CORS headers to the response. CORS processing is also skipped if the response
47+
* already contains CORS headers.
4848
*
4949
* @author Sebastien Deleuze
5050
* @author Rossen Stoyanchev
@@ -72,6 +72,10 @@ public class DefaultCorsProcessor implements CorsProcessor {
7272
public boolean processRequest(@Nullable CorsConfiguration config, HttpServletRequest request,
7373
HttpServletResponse response) throws IOException {
7474

75+
if (config == null) {
76+
return true;
77+
}
78+
7579
Collection<String> varyHeaders = response.getHeaders(HttpHeaders.VARY);
7680
if (!varyHeaders.contains(HttpHeaders.ORIGIN)) {
7781
response.addHeader(HttpHeaders.VARY, HttpHeaders.ORIGIN);
@@ -99,18 +103,8 @@ public boolean processRequest(@Nullable CorsConfiguration config, HttpServletReq
99103
return true;
100104
}
101105

102-
boolean preFlightRequest = CorsUtils.isPreFlightRequest(request);
103-
if (config == null) {
104-
if (preFlightRequest) {
105-
rejectRequest(new ServletServerHttpResponse(response));
106-
return false;
107-
}
108-
else {
109-
return true;
110-
}
111-
}
112-
113-
return handleInternal(new ServletServerHttpRequest(request), new ServletServerHttpResponse(response), config, preFlightRequest);
106+
return handleInternal(new ServletServerHttpRequest(request),
107+
new ServletServerHttpResponse(response), config, CorsUtils.isPreFlightRequest(request));
114108
}
115109

116110
/**

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

Lines changed: 9 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2024 the original author or authors.
2+
* Copyright 2002-2025 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.
@@ -37,9 +37,9 @@
3737
* as defined by the <a href="https://www.w3.org/TR/cors/">CORS W3C recommendation</a>.
3838
*
3939
* <p>Note that when the supplied {@link CorsConfiguration} is {@code null}, this
40-
* implementation does not reject simple or actual requests outright but simply
41-
* avoids adding CORS headers to the response. CORS processing is also skipped
42-
* if the response already contains CORS headers.
40+
* implementation does not reject CORS requests outright but simply avoids adding
41+
* CORS headers to the response. CORS processing is also skipped if the response
42+
* already contains CORS headers.
4343
*
4444
* @author Sebastien Deleuze
4545
* @author Rossen Stoyanchev
@@ -67,6 +67,10 @@ public class DefaultCorsProcessor implements CorsProcessor {
6767

6868
@Override
6969
public boolean process(@Nullable CorsConfiguration config, ServerWebExchange exchange) {
70+
if (config == null) {
71+
return true;
72+
}
73+
7074
ServerHttpRequest request = exchange.getRequest();
7175
ServerHttpResponse response = exchange.getResponse();
7276
HttpHeaders responseHeaders = response.getHeaders();
@@ -99,18 +103,7 @@ public boolean process(@Nullable CorsConfiguration config, ServerWebExchange exc
99103
return true;
100104
}
101105

102-
boolean preFlightRequest = CorsUtils.isPreFlightRequest(request);
103-
if (config == null) {
104-
if (preFlightRequest) {
105-
rejectRequest(response);
106-
return false;
107-
}
108-
else {
109-
return true;
110-
}
111-
}
112-
113-
return handleInternal(exchange, config, preFlightRequest);
106+
return handleInternal(exchange, config, CorsUtils.isPreFlightRequest(request));
114107
}
115108

116109
/**

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

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2024 the original author or authors.
2+
* Copyright 2002-2025 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.
@@ -459,7 +459,8 @@ void preflightRequestWithNullConfig() throws Exception {
459459

460460
this.processor.processRequest(null, this.request, this.response);
461461
assertThat(this.response.containsHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN)).isFalse();
462-
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_FORBIDDEN);
462+
assertThat(this.response.containsHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_METHODS)).isFalse();
463+
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_OK);
463464
}
464465

465466
@Test

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2024 the original author or authors.
2+
* Copyright 2002-2025 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.
@@ -31,6 +31,7 @@
3131
import static org.assertj.core.api.Assertions.assertThat;
3232
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
3333
import static org.springframework.http.HttpHeaders.ACCESS_CONTROL_ALLOW_HEADERS;
34+
import static org.springframework.http.HttpHeaders.ACCESS_CONTROL_ALLOW_METHODS;
3435
import static org.springframework.http.HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN;
3536
import static org.springframework.http.HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS;
3637
import static org.springframework.http.HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD;
@@ -480,7 +481,8 @@ void preflightRequestWithNullConfig() {
480481

481482
ServerHttpResponse response = exchange.getResponse();
482483
assertThat(response.getHeaders().containsHeader(ACCESS_CONTROL_ALLOW_ORIGIN)).isFalse();
483-
assertThat(response.getStatusCode()).isEqualTo(HttpStatus.FORBIDDEN);
484+
assertThat(response.getHeaders().containsHeader(ACCESS_CONTROL_ALLOW_METHODS)).isFalse();
485+
assertThat(response.getStatusCode()).isNull();
484486
}
485487

486488
@Test

‎spring-webflux/src/test/java/org/springframework/web/reactive/result/method/HandlerMethodMappingTests.java‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2024 the original author or authors.
2+
* Copyright 2002-2025 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.
@@ -31,7 +31,6 @@
3131
import org.springframework.core.annotation.AnnotatedElementUtils;
3232
import org.springframework.http.HttpHeaders;
3333
import org.springframework.http.HttpMethod;
34-
import org.springframework.http.HttpStatus;
3534
import org.springframework.http.server.PathContainer;
3635
import org.springframework.stereotype.Controller;
3736
import org.springframework.web.bind.annotation.CrossOrigin;
@@ -124,7 +123,10 @@ public void ambiguousMatchOnPreFlightRequestWithoutCorsConfig() {
124123

125124
this.mapping.getHandler(exchange).block();
126125

127-
assertThat(exchange.getResponse().getStatusCode()).isEqualTo(HttpStatus.FORBIDDEN);
126+
MockServerHttpResponse response = exchange.getResponse();
127+
assertThat(response.getStatusCode()).isNull();
128+
assertThat(response.getHeaders().getAccessControlAllowOrigin()).isNull();
129+
assertThat(response.getHeaders().getAccessControlAllowMethods()).isEmpty();
128130
}
129131

130132
@Test // gh-26490

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

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2021 the original author or authors.
2+
* Copyright 2002-2025 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.
@@ -35,13 +35,11 @@
3535
import org.springframework.web.bind.annotation.RequestMapping;
3636
import org.springframework.web.bind.annotation.RequestMethod;
3737
import org.springframework.web.bind.annotation.RestController;
38-
import org.springframework.web.client.HttpClientErrorException;
3938
import org.springframework.web.client.RestTemplate;
4039
import org.springframework.web.reactive.config.EnableWebFlux;
4140
import org.springframework.web.testfixture.http.server.reactive.bootstrap.HttpServer;
4241

4342
import static org.assertj.core.api.Assertions.assertThat;
44-
import static org.assertj.core.api.Assertions.fail;
4543

4644
/**
4745
* Integration tests with {@code @CrossOrigin} and {@code @RequestMapping}
@@ -105,13 +103,11 @@ void optionsRequestWithAccessControlRequestMethod(HttpServer httpServer) throws
105103
void preflightRequestWithoutAnnotation(HttpServer httpServer) throws Exception {
106104
startServer(httpServer);
107105
this.headers.add(HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, "GET");
108-
try {
109-
performOptions("/no", this.headers, Void.class);
110-
fail("Preflight request without CORS configuration should fail");
111-
}
112-
catch (HttpClientErrorException ex) {
113-
assertThat(ex.getStatusCode()).isEqualTo(HttpStatus.FORBIDDEN);
114-
}
106+
ResponseEntity<Void> entity = performOptions("/no", this.headers, Void.class);
107+
assertThat(entity.getStatusCode()).isEqualTo(HttpStatus.OK);
108+
assertThat(entity.getHeaders().getAccessControlAllowOrigin()).isNull();
109+
assertThat(entity.getHeaders().getAccessControlAllowMethods()).isEmpty();
110+
115111
}
116112

117113
@ParameterizedHttpServerTest

‎spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/GlobalCorsConfigIntegrationTests.java‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2022 the original author or authors.
2+
* Copyright 2002-2025 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.
@@ -135,9 +135,10 @@ void preFlightRequestWithoutCorsEnabled(HttpServer httpServer) throws Exception
135135
startServer(httpServer);
136136

137137
this.headers.add(HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, "GET");
138-
assertThatExceptionOfType(HttpClientErrorException.class).isThrownBy(() ->
139-
performOptions("/welcome", this.headers, String.class))
140-
.satisfies(ex -> assertThat(ex.getStatusCode()).isEqualTo(HttpStatus.FORBIDDEN));
138+
ResponseEntity<String> entity = performOptions("/welcome", this.headers, String.class);
139+
assertThat(entity.getStatusCode()).isEqualTo(HttpStatus.OK);
140+
assertThat(entity.getHeaders().getAccessControlAllowOrigin()).isNull();
141+
assertThat(entity.getHeaders().getAccessControlAllowMethods()).isEmpty();
141142
}
142143

143144
@ParameterizedHttpServerTest

‎spring-webmvc/src/test/java/org/springframework/web/servlet/handler/HandlerMethodMappingTests.java‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,9 @@ public void ambiguousMatchOnPreFlightRequestWithoutCorsConfig() throws Exception
135135
chain.getInterceptorList().get(0).preHandle(request, response, chain.getHandler());
136136
new HttpRequestHandlerAdapter().handle(request, response, chain.getHandler());
137137

138-
assertThat(response.getStatus()).isEqualTo(403);
138+
assertThat(response.getStatus()).isEqualTo(200);
139+
assertThat(response.getHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN)).isNull();
140+
assertThat(response.getHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_METHODS)).isNull();
139141
}
140142

141143
@Test // gh-26490

0 commit comments

Comments
 (0)