diff --git a/spring-web/src/main/java/org/springframework/web/bind/annotation/CrossOrigin.java b/spring-web/src/main/java/org/springframework/web/bind/annotation/CrossOrigin.java index ac50e8e3881..76d49fb8947 100644 --- a/spring-web/src/main/java/org/springframework/web/bind/annotation/CrossOrigin.java +++ b/spring-web/src/main/java/org/springframework/web/bind/annotation/CrossOrigin.java @@ -36,6 +36,12 @@ import org.springframework.web.cors.CorsConfiguration; * {@link CorsConfiguration} and then default values are applied via * {@link CorsConfiguration#applyPermitDefaultValues()}. * + *

The rules for combining global and local configuration are generally + * additive -- e.g. all global and all local origins. For those attributes + * where only a single value can be accepted such as {@code allowCredentials} + * and {@code maxAge}, the local overrides the global value. + * See {@link CorsConfiguration#combine(CorsConfiguration)} for more details. + * * @author Russell Allen * @author Sebastien Deleuze * @author Sam Brannen diff --git a/spring-web/src/main/java/org/springframework/web/cors/CorsConfiguration.java b/spring-web/src/main/java/org/springframework/web/cors/CorsConfiguration.java index c9f0e485672..6b39369d671 100644 --- a/spring-web/src/main/java/org/springframework/web/cors/CorsConfiguration.java +++ b/spring-web/src/main/java/org/springframework/web/cors/CorsConfiguration.java @@ -22,6 +22,7 @@ import java.util.Collections; import java.util.LinkedHashSet; import java.util.List; import java.util.Set; +import java.util.stream.Collectors; import org.springframework.http.HttpMethod; import org.springframework.lang.Nullable; @@ -51,14 +52,14 @@ public class CorsConfiguration { /** Wildcard representing all origins, methods, or headers. */ public static final String ALL = "*"; - private static final List DEFAULT_METHODS; + private static final List DEFAULT_METHODS = + Collections.unmodifiableList(Arrays.asList(HttpMethod.GET, HttpMethod.HEAD)); - static { - List rawMethods = new ArrayList<>(2); - rawMethods.add(HttpMethod.GET); - rawMethods.add(HttpMethod.HEAD); - DEFAULT_METHODS = Collections.unmodifiableList(rawMethods); - } + private static final List DEFAULT_PERMIT_ALL = + Collections.unmodifiableList(Arrays.asList(ALL)); + + private static final List DEFAULT_PERMIT_METHODS = + Collections.unmodifiableList(Arrays.asList(HttpMethod.GET.name(), HttpMethod.HEAD.name(), HttpMethod.POST.name())); @Nullable @@ -132,6 +133,9 @@ public class CorsConfiguration { if (this.allowedOrigins == null) { this.allowedOrigins = new ArrayList<>(4); } + else if (this.allowedOrigins == DEFAULT_PERMIT_ALL) { + setAllowedOrigins(DEFAULT_PERMIT_ALL); + } this.allowedOrigins.add(origin); } @@ -187,6 +191,9 @@ public class CorsConfiguration { this.allowedMethods = new ArrayList<>(4); this.resolvedMethods = new ArrayList<>(4); } + else if (this.allowedMethods == DEFAULT_PERMIT_METHODS) { + setAllowedMethods(DEFAULT_PERMIT_METHODS); + } this.allowedMethods.add(method); if (ALL.equals(method)) { this.resolvedMethods = null; @@ -228,6 +235,9 @@ public class CorsConfiguration { if (this.allowedHeaders == null) { this.allowedHeaders = new ArrayList<>(4); } + else if (this.allowedHeaders == DEFAULT_PERMIT_ALL) { + setAllowedHeaders(DEFAULT_PERMIT_ALL); + } this.allowedHeaders.add(allowedHeader); } @@ -325,25 +335,41 @@ public class CorsConfiguration { */ public CorsConfiguration applyPermitDefaultValues() { if (this.allowedOrigins == null) { - this.addAllowedOrigin(ALL); + this.allowedOrigins = DEFAULT_PERMIT_ALL; } if (this.allowedMethods == null) { - this.setAllowedMethods(Arrays.asList( - HttpMethod.GET.name(), HttpMethod.HEAD.name(), HttpMethod.POST.name())); + this.allowedMethods = DEFAULT_PERMIT_METHODS; + this.resolvedMethods = DEFAULT_PERMIT_METHODS + .stream().map(HttpMethod::resolve).collect(Collectors.toList()); } if (this.allowedHeaders == null) { - this.addAllowedHeader(ALL); + this.allowedHeaders = DEFAULT_PERMIT_ALL; } if (this.maxAge == null) { - this.setMaxAge(1800L); + this.maxAge = 1800L; } return this; } /** - * Combine the supplied {@code CorsConfiguration} with this one. - *

Properties of this configuration are overridden by any non-null - * properties of the supplied one. + * Combine the non-null properties of the supplied + * {@code CorsConfiguration} with this one. + * + *

When combining single values like {@code allowCredentials} or + * {@code maxAge}, {@code this} properties are overridden by non-null + * {@code other} properties if any. + * + *

Combining lists like {@code allowedOrigins}, {@code allowedMethods}, + * {@code allowedHeaders} or {@code exposedHeaders} is done in an additive + * way. For example, combining {@code ["GET", "POST"]} with + * {@code ["PATCH"]} results in {@code ["GET", "POST", "PATCH"]}, but keep + * in mind that combining {@code ["GET", "POST"]} with {@code ["*"]} + * results in {@code ["*"]}. + * + *

Notice that default permit values set by + * {@link CorsConfiguration#applyPermitDefaultValues()} are overridden by + * any value explicitly defined. + * * @return the combined {@code CorsConfiguration} or {@code this} * configuration if the supplied configuration is {@code null} */ @@ -369,12 +395,21 @@ public class CorsConfiguration { } private List combine(@Nullable List source, @Nullable List other) { - if (other == null || other.contains(ALL)) { + if (other == null) { return (source != null ? source : Collections.emptyList()); } - if (source == null || source.contains(ALL)) { + if (source == null) { return other; } + if (source == DEFAULT_PERMIT_ALL || source == DEFAULT_PERMIT_METHODS) { + return other; + } + if (other == DEFAULT_PERMIT_ALL || other == DEFAULT_PERMIT_METHODS) { + return source; + } + if (source.contains(ALL) || other.contains(ALL)) { + return new ArrayList<>(Collections.singletonList(ALL)); + } Set combined = new LinkedHashSet<>(source); combined.addAll(other); return new ArrayList<>(combined); diff --git a/spring-web/src/test/java/org/springframework/web/cors/CorsConfigurationTests.java b/spring-web/src/test/java/org/springframework/web/cors/CorsConfigurationTests.java index e7147d99c0b..2a000c182b1 100644 --- a/spring-web/src/test/java/org/springframework/web/cors/CorsConfigurationTests.java +++ b/spring-web/src/test/java/org/springframework/web/cors/CorsConfigurationTests.java @@ -108,6 +108,37 @@ public class CorsConfigurationTests { assertTrue(config.getAllowCredentials()); } + @Test // SPR-15772 + public void combineWithDefaultPermitValues() { + CorsConfiguration config = new CorsConfiguration().applyPermitDefaultValues(); + CorsConfiguration other = new CorsConfiguration(); + other.addAllowedOrigin("http://domain.com"); + other.addAllowedHeader("header1"); + other.addAllowedMethod(HttpMethod.PUT.name()); + + CorsConfiguration combinedConfig = config.combine(other); + assertEquals(Arrays.asList("http://domain.com"), combinedConfig.getAllowedOrigins()); + assertEquals(Arrays.asList("header1"), combinedConfig.getAllowedHeaders()); + assertEquals(Arrays.asList(HttpMethod.PUT.name()), combinedConfig.getAllowedMethods()); + + combinedConfig = other.combine(config); + assertEquals(Arrays.asList("http://domain.com"), combinedConfig.getAllowedOrigins()); + assertEquals(Arrays.asList("header1"), combinedConfig.getAllowedHeaders()); + assertEquals(Arrays.asList(HttpMethod.PUT.name()), combinedConfig.getAllowedMethods()); + + combinedConfig = config.combine(new CorsConfiguration()); + assertEquals(Arrays.asList("*"), config.getAllowedOrigins()); + assertEquals(Arrays.asList("*"), config.getAllowedHeaders()); + assertEquals(Arrays.asList(HttpMethod.GET.name(), HttpMethod.HEAD.name(), + HttpMethod.POST.name()), combinedConfig.getAllowedMethods()); + + combinedConfig = new CorsConfiguration().combine(config); + assertEquals(Arrays.asList("*"), config.getAllowedOrigins()); + assertEquals(Arrays.asList("*"), config.getAllowedHeaders()); + assertEquals(Arrays.asList(HttpMethod.GET.name(), HttpMethod.HEAD.name(), + HttpMethod.POST.name()), combinedConfig.getAllowedMethods()); + } + @Test public void combineWithAsteriskWildCard() { CorsConfiguration config = new CorsConfiguration(); @@ -120,15 +151,13 @@ public class CorsConfigurationTests { other.addExposedHeader("header2"); other.addAllowedMethod(HttpMethod.PUT.name()); CorsConfiguration combinedConfig = config.combine(other); - assertEquals(Arrays.asList("http://domain.com"), combinedConfig.getAllowedOrigins()); - assertEquals(Arrays.asList("header1"), combinedConfig.getAllowedHeaders()); - assertEquals(Arrays.asList("header2"), combinedConfig.getExposedHeaders()); - assertEquals(Arrays.asList(HttpMethod.PUT.name()), combinedConfig.getAllowedMethods()); + assertEquals(Arrays.asList("*"), combinedConfig.getAllowedOrigins()); + assertEquals(Arrays.asList("*"), combinedConfig.getAllowedHeaders()); + assertEquals(Arrays.asList("*"), combinedConfig.getAllowedMethods()); combinedConfig = other.combine(config); - assertEquals(Arrays.asList("http://domain.com"), combinedConfig.getAllowedOrigins()); - assertEquals(Arrays.asList("header1"), combinedConfig.getAllowedHeaders()); - assertEquals(Arrays.asList("header2"), combinedConfig.getExposedHeaders()); - assertEquals(Arrays.asList(HttpMethod.PUT.name()), combinedConfig.getAllowedMethods()); + assertEquals(Arrays.asList("*"), combinedConfig.getAllowedOrigins()); + assertEquals(Arrays.asList("*"), combinedConfig.getAllowedHeaders()); + assertEquals(Arrays.asList("*"), combinedConfig.getAllowedMethods()); } @Test // SPR-14792 @@ -250,4 +279,15 @@ public class CorsConfigurationTests { assertNull(config.checkHeaders(Arrays.asList("header1"))); } + @Test // SPR-15772 + public void changePermitDefaultValues() { + CorsConfiguration config = new CorsConfiguration().applyPermitDefaultValues(); + config.addAllowedOrigin("http://domain.com"); + config.addAllowedHeader("header1"); + config.addAllowedMethod("PATCH"); + assertEquals(Arrays.asList("*", "http://domain.com"), config.getAllowedOrigins()); + assertEquals(Arrays.asList("*", "header1"), config.getAllowedHeaders()); + assertEquals(Arrays.asList("GET", "HEAD", "POST", "PATCH"), config.getAllowedMethods()); + } + } diff --git a/src/docs/asciidoc/web/webflux-cors.adoc b/src/docs/asciidoc/web/webflux-cors.adoc index 36a051f5407..ca8553b6ee0 100644 --- a/src/docs/asciidoc/web/webflux-cors.adoc +++ b/src/docs/asciidoc/web/webflux-cors.adoc @@ -55,9 +55,10 @@ class or method-level `@CrossOrigin` annotations (other handlers can implement `CorsConfigurationSource`). The rules for combining global and local configuration are generally additive -- e.g. -all global and all local origins. The only exception are those attributes where only a -single value can be accepted such as `allowCredentials` and `maxAge`, in which case the -local overrides the global value. +all global and all local origins. For those attributes where only a single value can be +accepted such as `allowCredentials` and `maxAge`, the local overrides the global value. See +{api-spring-framework}/web/cors/CorsConfiguration.html#combine-org.springframework.web.cors.CorsConfiguration-[`CorsConfiguration#combine(CorsConfiguration)`] +for more details. [TIP] ==== diff --git a/src/docs/asciidoc/web/webmvc-cors.adoc b/src/docs/asciidoc/web/webmvc-cors.adoc index 2881ff48c76..d00fb4f57f8 100644 --- a/src/docs/asciidoc/web/webmvc-cors.adoc +++ b/src/docs/asciidoc/web/webmvc-cors.adoc @@ -55,9 +55,10 @@ class or method-level `@CrossOrigin` annotations (other handlers can implement `CorsConfigurationSource`). The rules for combining global and local configuration are generally additive -- e.g. -all global and all local origins. The only exception are those attributes where only a -single value can be accepted such as `allowCredentials` and `maxAge`, in which case the -local overrides the global value. +all global and all local origins. For those attributes where only a single value can be +accepted such as `allowCredentials` and `maxAge`, the local overrides the global value. See +{api-spring-framework}/web/cors/CorsConfiguration.html#combine-org.springframework.web.cors.CorsConfiguration-[`CorsConfiguration#combine(CorsConfiguration)`] +for more details. [TIP] ====