From 15da5efc865e4fa5946f6fde6e2c2bdee70f1c5e Mon Sep 17 00:00:00 2001 From: Sebastien Deleuze Date: Thu, 4 Jun 2015 17:17:39 +0200 Subject: [PATCH] Fix combining class and method level @CrossOrigin attributes Issue: SPR-13097 --- .../web/bind/annotation/CrossOrigin.java | 41 ++++++++++++------- .../RequestMappingHandlerMapping.java | 16 +++++--- .../method/annotation/CrossOriginTests.java | 34 ++++++++++++++- 3 files changed, 69 insertions(+), 22 deletions(-) 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 194f4be5243..4b2ad6dbe84 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 @@ -39,43 +39,54 @@ import org.springframework.core.annotation.AliasFor; @Documented public @interface CrossOrigin { + String[] DEFAULT_ORIGIN = { "*" }; + + String[] DEFAULT_ALLOWED_HEADERS = { "*" }; + + boolean DEFAULT_ALLOW_CREDENTIALS = true; + + long DEFAULT_MAX_AGE = 1800; + + /** * Alias for {@link #origin}. */ @AliasFor(attribute = "origin") - String[] value() default { "*" }; + String[] value() default {}; /** * List of allowed origins. *

These values are placed in the {@code Access-Control-Allow-Origin} * header of both the pre-flight response and the actual response. - *

Defaults to {@code "*"} which means that all origins are allowed. + * {@code "*"} means that all origins are allowed. + *

If undefined, all origins are allowed. * @see #value */ @AliasFor(attribute = "value") - String[] origin() default { "*" }; + String[] origin() default {}; /** * List of request headers that can be used during the actual request. *

This property controls the value of the pre-flight response's * {@code Access-Control-Allow-Headers} header. - *

Defaults to {@code "*"} which means that all headers requested - * by the client are allowed. + * {@code "*"} means that all headers requested by the client are allowed. + *

If undefined, all requested headers are allowed. */ - String[] allowedHeaders() default { "*" }; + String[] allowedHeaders() default {}; /** * List of response headers that the user-agent will allow the client to access. *

This property controls the value of actual response's * {@code Access-Control-Expose-Headers} header. - *

Defaults to an empty array. + *

If undefined, an empty exposed header list is used. */ String[] exposedHeaders() default {}; /** * List of supported HTTP request methods. *

Methods specified here override those specified via {@code RequestMapping}. - *

Defaults to an empty array. + *

If undefined, methods defined by {@link RequestMapping} annotation + * are used. */ RequestMethod[] method() default {}; @@ -83,22 +94,22 @@ public @interface CrossOrigin { * Whether the browser should include any cookies associated with the * domain of the request being annotated. *

Set to {@code "false"} if such cookies should not included. - *

An empty string ({@code ""}) means undefined. - *

Defaults to {@code "true"} which means that the pre-flight - * response will include the header + * An empty string ({@code ""}) means undefined. + * {@code "true"} means that the pre-flight response will include the header * {@code Access-Control-Allow-Credentials=true}. + *

If undefined, credentials are allowed. */ - String allowCredentials() default "true"; + String allowCredentials() default ""; /** * The maximum age (in seconds) of the cache duration for pre-flight responses. *

This property controls the value of the {@code Access-Control-Max-Age} * header in the pre-flight response. - *

A negative value means undefined. *

Setting this to a reasonable value can reduce the number of pre-flight * request/response interactions required by the browser. - *

Defaults to {@code 1800} seconds (i.e., 30 minutes). + * A negative value means undefined. + *

If undefined, max age is set to {@code 1800} seconds (i.e., 30 minutes). */ - long maxAge() default 1800; + long maxAge() default -1; } diff --git a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMapping.java b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMapping.java index 2149558a2bf..fbfd99df40b 100644 --- a/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMapping.java +++ b/spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/annotation/RequestMappingHandlerMapping.java @@ -18,6 +18,7 @@ package org.springframework.web.servlet.mvc.method.annotation; import java.lang.reflect.AnnotatedElement; import java.lang.reflect.Method; +import java.util.Arrays; import java.util.List; import org.springframework.context.EmbeddedValueResolverAware; @@ -301,17 +302,22 @@ public class RequestMappingHandlerMapping extends RequestMappingInfoHandlerMappi updateCorsConfig(config, typeAnnotation); updateCorsConfig(config, methodAnnotation); + if (CollectionUtils.isEmpty(config.getAllowedOrigins())) { + config.setAllowedOrigins(Arrays.asList(CrossOrigin.DEFAULT_ORIGIN)); + } if (CollectionUtils.isEmpty(config.getAllowedMethods())) { for (RequestMethod allowedMethod : mappingInfo.getMethodsCondition().getMethods()) { config.addAllowedMethod(allowedMethod.name()); } } if (CollectionUtils.isEmpty(config.getAllowedHeaders())) { - for (NameValueExpression headerExpression : mappingInfo.getHeadersCondition().getExpressions()) { - if (!headerExpression.isNegated()) { - config.addAllowedHeader(headerExpression.getName()); - } - } + config.setAllowedHeaders(Arrays.asList(CrossOrigin.DEFAULT_ALLOWED_HEADERS)); + } + if (config.getAllowCredentials() == null) { + config.setAllowCredentials(CrossOrigin.DEFAULT_ALLOW_CREDENTIALS); + } + if (config.getMaxAge() == null) { + config.setMaxAge(CrossOrigin.DEFAULT_MAX_AGE); } return config; } diff --git a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/CrossOriginTests.java b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/CrossOriginTests.java index 145b84611d2..4f58b36cf8f 100644 --- a/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/CrossOriginTests.java +++ b/spring-webmvc/src/test/java/org/springframework/web/servlet/mvc/method/annotation/CrossOriginTests.java @@ -136,7 +136,8 @@ public class CrossOriginTests { public void customOriginDefinedViaValueAttribute() throws Exception { this.handlerMapping.registerHandler(new MethodLevelController()); this.request.setRequestURI("/customOrigin"); - CorsConfiguration config = getCorsConfiguration(this.handlerMapping.getHandler(request), false); + HandlerExecutionChain chain = this.handlerMapping.getHandler(request); + CorsConfiguration config = getCorsConfiguration(chain, false); assertNotNull(config); assertEquals(Arrays.asList("http://example.com"), config.getAllowedOrigins()); assertTrue(config.getAllowCredentials()); @@ -153,12 +154,30 @@ public class CrossOriginTests { @Test public void classLevel() throws Exception { this.handlerMapping.registerHandler(new ClassLevelController()); + this.request.setRequestURI("/foo"); HandlerExecutionChain chain = this.handlerMapping.getHandler(request); CorsConfiguration config = getCorsConfiguration(chain, false); assertNotNull(config); assertArrayEquals(new String[]{"GET"}, config.getAllowedMethods().toArray()); assertArrayEquals(new String[]{"*"}, config.getAllowedOrigins().toArray()); + assertFalse(config.getAllowCredentials()); + + this.request.setRequestURI("/bar"); + chain = this.handlerMapping.getHandler(request); + config = getCorsConfiguration(chain, false); + assertNotNull(config); + assertArrayEquals(new String[]{"GET"}, config.getAllowedMethods().toArray()); + assertArrayEquals(new String[]{"*"}, config.getAllowedOrigins().toArray()); + assertFalse(config.getAllowCredentials()); + + this.request.setRequestURI("/baz"); + chain = this.handlerMapping.getHandler(request); + config = getCorsConfiguration(chain, false); + assertNotNull(config); + assertArrayEquals(new String[]{"GET"}, config.getAllowedMethods().toArray()); + assertArrayEquals(new String[]{"*"}, config.getAllowedOrigins().toArray()); + assertTrue(config.getAllowCredentials()); } @Test @@ -307,12 +326,23 @@ public class CrossOriginTests { } @Controller - @CrossOrigin + @CrossOrigin(allowCredentials = "false") private static class ClassLevelController { @RequestMapping(path = "/foo", method = RequestMethod.GET) public void foo() { } + + @CrossOrigin + @RequestMapping(path = "/bar", method = RequestMethod.GET) + public void bar() { + } + + @CrossOrigin(allowCredentials = "true") + @RequestMapping(path = "/baz", method = RequestMethod.GET) + public void baz() { + } + } private static class TestRequestMappingInfoHandlerMapping extends RequestMappingHandlerMapping {