From eee45c358315574fac80b8e5cbdd625eb37471e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=A9bastien=20Deleuze?= Date: Fri, 4 Apr 2025 21:19:11 +0200 Subject: [PATCH] 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 --- .../web/cors/DefaultCorsProcessor.java | 26 +++++++------------ .../cors/reactive/DefaultCorsProcessor.java | 25 +++++++----------- .../web/cors/DefaultCorsProcessorTests.java | 5 ++-- .../reactive/DefaultCorsProcessorTests.java | 6 +++-- .../method/HandlerMethodMappingTests.java | 8 +++--- ...CrossOriginAnnotationIntegrationTests.java | 16 +++++------- .../GlobalCorsConfigIntegrationTests.java | 9 ++++--- .../handler/HandlerMethodMappingTests.java | 4 ++- 8 files changed, 45 insertions(+), 54 deletions(-) diff --git a/spring-web/src/main/java/org/springframework/web/cors/DefaultCorsProcessor.java b/spring-web/src/main/java/org/springframework/web/cors/DefaultCorsProcessor.java index f6cedf292a9..68e7a2eaeff 100644 --- a/spring-web/src/main/java/org/springframework/web/cors/DefaultCorsProcessor.java +++ b/spring-web/src/main/java/org/springframework/web/cors/DefaultCorsProcessor.java @@ -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; * CORS W3C recommendation. * *

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 { public boolean processRequest(@Nullable CorsConfiguration config, HttpServletRequest request, HttpServletResponse response) throws IOException { + if (config == null) { + return true; + } + Collection 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 { 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)); } /** diff --git a/spring-web/src/main/java/org/springframework/web/cors/reactive/DefaultCorsProcessor.java b/spring-web/src/main/java/org/springframework/web/cors/reactive/DefaultCorsProcessor.java index 87d273769c8..8cdb6c6273c 100644 --- a/spring-web/src/main/java/org/springframework/web/cors/reactive/DefaultCorsProcessor.java +++ b/spring-web/src/main/java/org/springframework/web/cors/reactive/DefaultCorsProcessor.java @@ -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; * as defined by the CORS W3C recommendation. * *

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 { @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 { 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)); } /** diff --git a/spring-web/src/test/java/org/springframework/web/cors/DefaultCorsProcessorTests.java b/spring-web/src/test/java/org/springframework/web/cors/DefaultCorsProcessorTests.java index ad5964f5374..bcb4ae85236 100644 --- a/spring-web/src/test/java/org/springframework/web/cors/DefaultCorsProcessorTests.java +++ b/spring-web/src/test/java/org/springframework/web/cors/DefaultCorsProcessorTests.java @@ -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 { 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 diff --git a/spring-web/src/test/java/org/springframework/web/cors/reactive/DefaultCorsProcessorTests.java b/spring-web/src/test/java/org/springframework/web/cors/reactive/DefaultCorsProcessorTests.java index cdafe515aeb..98817599050 100644 --- a/spring-web/src/test/java/org/springframework/web/cors/reactive/DefaultCorsProcessorTests.java +++ b/spring-web/src/test/java/org/springframework/web/cors/reactive/DefaultCorsProcessorTests.java @@ -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; 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 { 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 diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/HandlerMethodMappingTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/HandlerMethodMappingTests.java index 427787cd9bc..9054b3f89d0 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/HandlerMethodMappingTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/HandlerMethodMappingTests.java @@ -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; 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 { 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 diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/CrossOriginAnnotationIntegrationTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/CrossOriginAnnotationIntegrationTests.java index 2439ff7926f..b09b26d28a3 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/CrossOriginAnnotationIntegrationTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/CrossOriginAnnotationIntegrationTests.java @@ -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; 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 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 entity = performOptions("/no", this.headers, Void.class); + assertThat(entity.getStatusCode()).isEqualTo(HttpStatus.OK); + assertThat(entity.getHeaders().getAccessControlAllowOrigin()).isNull(); + assertThat(entity.getHeaders().getAccessControlAllowMethods()).isEmpty(); + } @ParameterizedHttpServerTest diff --git a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/GlobalCorsConfigIntegrationTests.java b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/GlobalCorsConfigIntegrationTests.java index de3fb9d8b63..002feb2aa1a 100644 --- a/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/GlobalCorsConfigIntegrationTests.java +++ b/spring-webflux/src/test/java/org/springframework/web/reactive/result/method/annotation/GlobalCorsConfigIntegrationTests.java @@ -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 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 entity = performOptions("/welcome", this.headers, String.class); + assertThat(entity.getStatusCode()).isEqualTo(HttpStatus.OK); + assertThat(entity.getHeaders().getAccessControlAllowOrigin()).isNull(); + assertThat(entity.getHeaders().getAccessControlAllowMethods()).isEmpty(); } @ParameterizedHttpServerTest diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/handler/HandlerMethodMappingTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/handler/HandlerMethodMappingTests.java index 9e3b3cf4151..865c209e360 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/handler/HandlerMethodMappingTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/handler/HandlerMethodMappingTests.java @@ -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