Browse Source

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
pull/34719/head
Sébastien Deleuze 9 months ago
parent
commit
eee45c3583
  1. 26
      spring-web/src/main/java/org/springframework/web/cors/DefaultCorsProcessor.java
  2. 25
      spring-web/src/main/java/org/springframework/web/cors/reactive/DefaultCorsProcessor.java
  3. 5
      spring-web/src/test/java/org/springframework/web/cors/DefaultCorsProcessorTests.java
  4. 6
      spring-web/src/test/java/org/springframework/web/cors/reactive/DefaultCorsProcessorTests.java
  5. 8
      spring-webflux/src/test/java/org/springframework/web/reactive/result/method/HandlerMethodMappingTests.java
  6. 16
      spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/CrossOriginAnnotationIntegrationTests.java
  7. 9
      spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/GlobalCorsConfigIntegrationTests.java
  8. 4
      spring-webmvc/src/test/java/org/springframework/web/servlet/handler/HandlerMethodMappingTests.java

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

@ -1,5 +1,5 @@ @@ -1,5 +1,5 @@
/*
* Copyright 2002-2024 the original author or authors.
* Copyright 2002-2025 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.
@ -42,9 +42,9 @@ import org.springframework.util.CollectionUtils; @@ -42,9 +42,9 @@ import org.springframework.util.CollectionUtils;
* <a href="https://www.w3.org/TR/cors/">CORS W3C recommendation</a>.
*
* <p>Note that when the supplied {@link CorsConfiguration} is {@code null}, this
* implementation does not reject simple or actual requests outright but simply
* avoids adding CORS headers to the response. CORS processing is also skipped
* if the response already contains CORS headers.
* implementation does not reject CORS requests outright but simply avoids adding
* CORS headers to the response. CORS processing is also skipped if the response
* already contains CORS headers.
*
* @author Sebastien Deleuze
* @author Rossen Stoyanchev
@ -72,6 +72,10 @@ public class DefaultCorsProcessor implements CorsProcessor { @@ -72,6 +72,10 @@ public class DefaultCorsProcessor implements CorsProcessor {
public boolean processRequest(@Nullable CorsConfiguration config, HttpServletRequest request,
HttpServletResponse response) throws IOException {
if (config == null) {
return true;
}
Collection<String> varyHeaders = response.getHeaders(HttpHeaders.VARY);
if (!varyHeaders.contains(HttpHeaders.ORIGIN)) {
response.addHeader(HttpHeaders.VARY, HttpHeaders.ORIGIN);
@ -99,18 +103,8 @@ public class DefaultCorsProcessor implements CorsProcessor { @@ -99,18 +103,8 @@ public class DefaultCorsProcessor implements CorsProcessor {
return true;
}
boolean preFlightRequest = CorsUtils.isPreFlightRequest(request);
if (config == null) {
if (preFlightRequest) {
rejectRequest(new ServletServerHttpResponse(response));
return false;
}
else {
return true;
}
}
return handleInternal(new ServletServerHttpRequest(request), new ServletServerHttpResponse(response), config, preFlightRequest);
return handleInternal(new ServletServerHttpRequest(request),
new ServletServerHttpResponse(response), config, CorsUtils.isPreFlightRequest(request));
}
/**

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

@ -1,5 +1,5 @@ @@ -1,5 +1,5 @@
/*
* Copyright 2002-2024 the original author or authors.
* Copyright 2002-2025 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.
@ -37,9 +37,9 @@ import org.springframework.web.server.ServerWebExchange; @@ -37,9 +37,9 @@ import org.springframework.web.server.ServerWebExchange;
* as defined by the <a href="https://www.w3.org/TR/cors/">CORS W3C recommendation</a>.
*
* <p>Note that when the supplied {@link CorsConfiguration} is {@code null}, this
* implementation does not reject simple or actual requests outright but simply
* avoids adding CORS headers to the response. CORS processing is also skipped
* if the response already contains CORS headers.
* implementation does not reject CORS requests outright but simply avoids adding
* CORS headers to the response. CORS processing is also skipped if the response
* already contains CORS headers.
*
* @author Sebastien Deleuze
* @author Rossen Stoyanchev
@ -67,6 +67,10 @@ public class DefaultCorsProcessor implements CorsProcessor { @@ -67,6 +67,10 @@ public class DefaultCorsProcessor implements CorsProcessor {
@Override
public boolean process(@Nullable CorsConfiguration config, ServerWebExchange exchange) {
if (config == null) {
return true;
}
ServerHttpRequest request = exchange.getRequest();
ServerHttpResponse response = exchange.getResponse();
HttpHeaders responseHeaders = response.getHeaders();
@ -99,18 +103,7 @@ public class DefaultCorsProcessor implements CorsProcessor { @@ -99,18 +103,7 @@ public class DefaultCorsProcessor implements CorsProcessor {
return true;
}
boolean preFlightRequest = CorsUtils.isPreFlightRequest(request);
if (config == null) {
if (preFlightRequest) {
rejectRequest(response);
return false;
}
else {
return true;
}
}
return handleInternal(exchange, config, preFlightRequest);
return handleInternal(exchange, config, CorsUtils.isPreFlightRequest(request));
}
/**

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

@ -1,5 +1,5 @@ @@ -1,5 +1,5 @@
/*
* Copyright 2002-2024 the original author or authors.
* Copyright 2002-2025 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.
@ -459,7 +459,8 @@ class DefaultCorsProcessorTests { @@ -459,7 +459,8 @@ class DefaultCorsProcessorTests {
this.processor.processRequest(null, this.request, this.response);
assertThat(this.response.containsHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN)).isFalse();
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_FORBIDDEN);
assertThat(this.response.containsHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_METHODS)).isFalse();
assertThat(this.response.getStatus()).isEqualTo(HttpServletResponse.SC_OK);
}
@Test

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

@ -1,5 +1,5 @@ @@ -1,5 +1,5 @@
/*
* Copyright 2002-2024 the original author or authors.
* Copyright 2002-2025 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.
@ -31,6 +31,7 @@ import org.springframework.web.testfixture.server.MockServerWebExchange; @@ -31,6 +31,7 @@ import org.springframework.web.testfixture.server.MockServerWebExchange;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
import static org.springframework.http.HttpHeaders.ACCESS_CONTROL_ALLOW_HEADERS;
import static org.springframework.http.HttpHeaders.ACCESS_CONTROL_ALLOW_METHODS;
import static org.springframework.http.HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN;
import static org.springframework.http.HttpHeaders.ACCESS_CONTROL_REQUEST_HEADERS;
import static org.springframework.http.HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD;
@ -480,7 +481,8 @@ class DefaultCorsProcessorTests { @@ -480,7 +481,8 @@ class DefaultCorsProcessorTests {
ServerHttpResponse response = exchange.getResponse();
assertThat(response.getHeaders().containsHeader(ACCESS_CONTROL_ALLOW_ORIGIN)).isFalse();
assertThat(response.getStatusCode()).isEqualTo(HttpStatus.FORBIDDEN);
assertThat(response.getHeaders().containsHeader(ACCESS_CONTROL_ALLOW_METHODS)).isFalse();
assertThat(response.getStatusCode()).isNull();
}
@Test

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

@ -1,5 +1,5 @@ @@ -1,5 +1,5 @@
/*
* Copyright 2002-2024 the original author or authors.
* Copyright 2002-2025 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.
@ -31,7 +31,6 @@ import reactor.test.StepVerifier; @@ -31,7 +31,6 @@ import reactor.test.StepVerifier;
import org.springframework.core.annotation.AnnotatedElementUtils;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpMethod;
import org.springframework.http.HttpStatus;
import org.springframework.http.server.PathContainer;
import org.springframework.stereotype.Controller;
import org.springframework.web.bind.annotation.CrossOrigin;
@ -124,7 +123,10 @@ class HandlerMethodMappingTests { @@ -124,7 +123,10 @@ class HandlerMethodMappingTests {
this.mapping.getHandler(exchange).block();
assertThat(exchange.getResponse().getStatusCode()).isEqualTo(HttpStatus.FORBIDDEN);
MockServerHttpResponse response = exchange.getResponse();
assertThat(response.getStatusCode()).isNull();
assertThat(response.getHeaders().getAccessControlAllowOrigin()).isNull();
assertThat(response.getHeaders().getAccessControlAllowMethods()).isEmpty();
}
@Test // gh-26490

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

@ -1,5 +1,5 @@ @@ -1,5 +1,5 @@
/*
* Copyright 2002-2021 the original author or authors.
* Copyright 2002-2025 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,13 +35,11 @@ import org.springframework.web.bind.annotation.PostMapping; @@ -35,13 +35,11 @@ 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}
@ -105,13 +103,11 @@ class CrossOriginAnnotationIntegrationTests extends AbstractRequestMappingIntegr @@ -105,13 +103,11 @@ class CrossOriginAnnotationIntegrationTests extends AbstractRequestMappingIntegr
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);
}
ResponseEntity<Void> entity = performOptions("/no", this.headers, Void.class);
assertThat(entity.getStatusCode()).isEqualTo(HttpStatus.OK);
assertThat(entity.getHeaders().getAccessControlAllowOrigin()).isNull();
assertThat(entity.getHeaders().getAccessControlAllowMethods()).isEmpty();
}
@ParameterizedHttpServerTest

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

@ -1,5 +1,5 @@ @@ -1,5 +1,5 @@
/*
* Copyright 2002-2022 the original author or authors.
* Copyright 2002-2025 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.
@ -135,9 +135,10 @@ class GlobalCorsConfigIntegrationTests extends AbstractRequestMappingIntegration @@ -135,9 +135,10 @@ class GlobalCorsConfigIntegrationTests extends AbstractRequestMappingIntegration
startServer(httpServer);
this.headers.add(HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD, "GET");
assertThatExceptionOfType(HttpClientErrorException.class).isThrownBy(() ->
performOptions("/welcome", this.headers, String.class))
.satisfies(ex -> assertThat(ex.getStatusCode()).isEqualTo(HttpStatus.FORBIDDEN));
ResponseEntity<String> entity = performOptions("/welcome", this.headers, String.class);
assertThat(entity.getStatusCode()).isEqualTo(HttpStatus.OK);
assertThat(entity.getHeaders().getAccessControlAllowOrigin()).isNull();
assertThat(entity.getHeaders().getAccessControlAllowMethods()).isEmpty();
}
@ParameterizedHttpServerTest

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

@ -135,7 +135,9 @@ public class HandlerMethodMappingTests { @@ -135,7 +135,9 @@ public class HandlerMethodMappingTests {
chain.getInterceptorList().get(0).preHandle(request, response, chain.getHandler());
new HttpRequestHandlerAdapter().handle(request, response, chain.getHandler());
assertThat(response.getStatus()).isEqualTo(403);
assertThat(response.getStatus()).isEqualTo(200);
assertThat(response.getHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_ORIGIN)).isNull();
assertThat(response.getHeader(HttpHeaders.ACCESS_CONTROL_ALLOW_METHODS)).isNull();
}
@Test // gh-26490

Loading…
Cancel
Save