Unit test for empty Access-Control-Request-Headers (Chrome 52)

Includes optimized method/header resolution in CorsConfiguration.

Issue: SPR-14617
(cherry picked from commit d047174)

Author: jhoeller

spring-web/src/main/java/org/springframework/http/HttpHeaders.java

@@ -546,8 +546,8 @@ public List<String> getAccessControlRequestHeaders() {
 	/**
 	 * Set the (new) value of the {@code Access-Control-Request-Method} request header.
 	 */
-	public void setAccessControlRequestMethod(HttpMethod requestedMethod) {
-		set(ACCESS_CONTROL_REQUEST_METHOD, requestedMethod.name());
+	public void setAccessControlRequestMethod(HttpMethod requestMethod) {
+		set(ACCESS_CONTROL_REQUEST_METHOD, requestMethod.name());
 	}
 
 	/**

spring-web/src/main/java/org/springframework/web/cors/CorsConfiguration.java

@@ -5,7 +5,7 @@
  * you may not use this file except in compliance with the License.
  * You may obtain a copy of the License at
  *
- * http://www.apache.org/licenses/LICENSE-2.0
+ *      http://www.apache.org/licenses/LICENSE-2.0
  *
  * Unless required by applicable law or agreed to in writing, software
  * distributed under the License is distributed on an "AS IS" BASIS,
@@ -21,6 +21,7 @@
 import java.util.List;
 
 import org.springframework.http.HttpMethod;
+import org.springframework.util.CollectionUtils;
 import org.springframework.util.ObjectUtils;
 import org.springframework.util.StringUtils;
 
@@ -30,6 +31,7 @@
  *
  * @author Sebastien Deleuze
  * @author Rossen Stoyanchev
+ * @author Juergen Hoeller
  * @author Sam Brannen
  * @since 4.2
  * @see <a href="http://www.w3.org/TR/cors/">CORS W3C recommendation</a>
@@ -41,10 +43,22 @@ public class CorsConfiguration {
 	 */
 	public static final String ALL = "*";
 
+	private static final List<HttpMethod> DEFAULT_METHODS;
+
+	static {
+		List<HttpMethod> rawMethods = new ArrayList<HttpMethod>(2);
+		rawMethods.add(HttpMethod.GET);
+		rawMethods.add(HttpMethod.HEAD);
+		DEFAULT_METHODS = Collections.unmodifiableList(rawMethods);
+	}
+
+
 	private List<String> allowedOrigins;
 
 	private List<String> allowedMethods;
 
+	private List<HttpMethod> resolvedMethods = DEFAULT_METHODS;
+
 	private List<String> allowedHeaders;
 
 	private List<String> exposedHeaders;
@@ -67,6 +81,7 @@ public CorsConfiguration() {
 	public CorsConfiguration(CorsConfiguration other) {
 		this.allowedOrigins = other.allowedOrigins;
 		this.allowedMethods = other.allowedMethods;
+		this.resolvedMethods = other.resolvedMethods;
 		this.allowedHeaders = other.allowedHeaders;
 		this.exposedHeaders = other.exposedHeaders;
 		this.allowCredentials = other.allowCredentials;
@@ -86,10 +101,10 @@ public CorsConfiguration combine(CorsConfiguration other) {
 			return this;
 		}
 		CorsConfiguration config = new CorsConfiguration(this);
-		config.setAllowedOrigins(combine(this.getAllowedOrigins(), other.getAllowedOrigins()));
-		config.setAllowedMethods(combine(this.getAllowedMethods(), other.getAllowedMethods()));
-		config.setAllowedHeaders(combine(this.getAllowedHeaders(), other.getAllowedHeaders()));
-		config.setExposedHeaders(combine(this.getExposedHeaders(), other.getExposedHeaders()));
+		config.setAllowedOrigins(combine(getAllowedOrigins(), other.getAllowedOrigins()));
+		config.setAllowedMethods(combine(getAllowedMethods(), other.getAllowedMethods()));
+		config.setAllowedHeaders(combine(getAllowedHeaders(), other.getAllowedHeaders()));
+		config.setExposedHeaders(combine(getExposedHeaders(), other.getExposedHeaders()));
 		Boolean allowCredentials = other.getAllowCredentials();
 		if (allowCredentials != null) {
 			config.setAllowCredentials(allowCredentials);
@@ -137,7 +152,7 @@ public List<String> getAllowedOrigins() {
 	 */
 	public void addAllowedOrigin(String origin) {
 		if (this.allowedOrigins == null) {
-			this.allowedOrigins = new ArrayList<String>();
+			this.allowedOrigins = new ArrayList<String>(4);
 		}
 		this.allowedOrigins.add(origin);
 	}
@@ -146,16 +161,29 @@ public void addAllowedOrigin(String origin) {
 	 * Set the HTTP methods to allow, e.g. {@code "GET"}, {@code "POST"},
 	 * {@code "PUT"}, etc.
 	 * <p>The special value {@code "*"} allows all methods.
-	 * <p>If not set, only {@code "GET"} is allowed.
+	 * <p>If not set, only {@code "GET"} and {@code "HEAD"} are allowed.
 	 * <p>By default this is not set.
 	 */
 	public void setAllowedMethods(List<String> allowedMethods) {
 		this.allowedMethods = (allowedMethods != null ? new ArrayList<String>(allowedMethods) : null);
+		if (!CollectionUtils.isEmpty(allowedMethods)) {
+			this.resolvedMethods = new ArrayList<HttpMethod>(allowedMethods.size());
+			for (String method : allowedMethods) {
+				if (ALL.equals(method)) {
+					this.resolvedMethods = null;
+					break;
+				}
+				this.resolvedMethods.add(HttpMethod.resolve(method));
+			}
+		}
+		else {
+			this.resolvedMethods = DEFAULT_METHODS;
+		}
 	}
 
 	/**
 	 * Return the allowed HTTP methods, possibly {@code null} in which case
-	 * only {@code "GET"} is allowed.
+	 * only {@code "GET"} and {@code "HEAD"} allowed.
 	 * @see #addAllowedMethod(HttpMethod)
 	 * @see #addAllowedMethod(String)
 	 * @see #setAllowedMethods(List)
@@ -179,9 +207,16 @@ public void addAllowedMethod(HttpMethod method) {
 	public void addAllowedMethod(String method) {
 		if (StringUtils.hasText(method)) {
 			if (this.allowedMethods == null) {
-				this.allowedMethods = new ArrayList<String>();
+				this.allowedMethods = new ArrayList<String>(4);
+				this.resolvedMethods = new ArrayList<HttpMethod>(4);
 			}
 			this.allowedMethods.add(method);
+			if (ALL.equals(method)) {
+				this.resolvedMethods = null;
+			}
+			else if (this.resolvedMethods != null) {
+				this.resolvedMethods.add(HttpMethod.resolve(method));
+			}
 		}
 	}
 
@@ -213,7 +248,7 @@ public List<String> getAllowedHeaders() {
 	 */
 	public void addAllowedHeader(String allowedHeader) {
 		if (this.allowedHeaders == null) {
-			this.allowedHeaders = new ArrayList<String>();
+			this.allowedHeaders = new ArrayList<String>(4);
 		}
 		this.allowedHeaders.add(allowedHeader);
 	}
@@ -230,7 +265,7 @@ public void setExposedHeaders(List<String> exposedHeaders) {
 		if (exposedHeaders != null && exposedHeaders.contains(ALL)) {
 			throw new IllegalArgumentException("'*' is not a valid exposed header value");
 		}
-		this.exposedHeaders = (exposedHeaders == null ? null : new ArrayList<String>(exposedHeaders));
+		this.exposedHeaders = (exposedHeaders != null ? new ArrayList<String>(exposedHeaders) : null);
 	}
 
 	/**
@@ -333,27 +368,10 @@ public List<HttpMethod> checkHttpMethod(HttpMethod requestMethod) {
 		if (requestMethod == null) {
 			return null;
 		}
-		List<String> allowedMethods =
-				(this.allowedMethods != null ? this.allowedMethods : new ArrayList<String>());
-		if (allowedMethods.contains(ALL)) {
+		if (this.resolvedMethods == null) {
 			return Collections.singletonList(requestMethod);
 		}
-		if (allowedMethods.isEmpty()) {
-			allowedMethods.add(HttpMethod.GET.name());
-			allowedMethods.add(HttpMethod.HEAD.name());
-		}
-		List<HttpMethod> result = new ArrayList<HttpMethod>(allowedMethods.size());
-		boolean allowed = false;
-		for (String method : allowedMethods) {
-			if (requestMethod.matches(method)) {
-				allowed = true;
-			}
-			HttpMethod resolved = HttpMethod.resolve(method);
-			if (resolved != null) {
-				result.add(resolved);
-			}
-		}
-		return (allowed ? result : null);
+		return (this.resolvedMethods.contains(requestMethod) ? this.resolvedMethods : null);
 	}
 
 	/**
@@ -376,14 +394,19 @@ public List<String> checkHeaders(List<String> requestHeaders) {
 		}
 
 		boolean allowAnyHeader = this.allowedHeaders.contains(ALL);
-		List<String> result = new ArrayList<String>();
+		List<String> result = new ArrayList<String>(requestHeaders.size());
 		for (String requestHeader : requestHeaders) {
 			if (StringUtils.hasText(requestHeader)) {
 				requestHeader = requestHeader.trim();
-				for (String allowedHeader : this.allowedHeaders) {
-					if (allowAnyHeader || requestHeader.equalsIgnoreCase(allowedHeader)) {
-						result.add(requestHeader);
-						break;
+				if (allowAnyHeader) {
+					result.add(requestHeader);
+				}
+				else {
+					for (String allowedHeader : this.allowedHeaders) {
+						if (requestHeader.equalsIgnoreCase(allowedHeader)) {
+							result.add(requestHeader);
+							break;
+						}
 					}
 				}
 			}