CORS-related refinements

After this change CorsProcessor has a single processRequest method and
it also explicitly deals with a null CorsConfiguration, which for
pre-flight requests results in a rejection while for simple requests
results in no CORS headers added.

The AbstractHandlerMapping now uses a LinkedHashMap to preserve the
order in which global patterns are provided.

Author: rstoyanchev

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

@@ -21,35 +21,31 @@
 import javax.servlet.http.HttpServletResponse;
 
 /**
- * Contract for handling CORS preflight requests and intercepting CORS simple
- * and actual requests.
+ * A strategy that takes a request and a {@link CorsConfiguration} and updates
+ * the response.
+ *
+ * <p>This component is not concerned with how a {@code CorsConfiguration} is
+ * selected but rather takes follow-up actions such as applying CORS validation
+ * checks and either rejecting the response or adding CORS headers to the
+ * response.
  *
  * @author Sebastien Deleuze
+ * @author Rossen Stoyanchev
  * @since 4.2
  * @see <a href="http://www.w3.org/TR/cors/">CORS W3C recommandation</a>
+ * @see org.springframework.web.servlet.handler.AbstractHandlerMapping#setCorsProcessor
  */
 public interface CorsProcessor {
 
 	/**
-	 * Process a preflight CORS request given a {@link CorsConfiguration}.
-	 * If the request is not a valid CORS pre-flight request or if it does not
-	 * comply with the configuration it should be rejected.
-	 * If the request is valid and complies with the configuration, CORS headers
-	 * should be added to the response.
-	 * @return {@code false} if the request is rejected, else {@code true}.
-	 */
-	boolean processPreFlightRequest(CorsConfiguration conf, HttpServletRequest request,
-			HttpServletResponse response) throws IOException;
-
-	/**
-	 * Process a simple or actual CORS request given a {@link CorsConfiguration}.
-	 * If the request is not a valid CORS simple or actual request or if it does
-	 * not comply with the configuration, it should be rejected.
-	 * If the request is valid and comply with the configuration, this method adds the related
-	 * CORS headers to the response.
+	 * Process a request given a {@code CorsConfiguration}.
+	 *
+	 * @param configuration the applicable CORS configuration, possibly {@code null}
+	 * @param request the current request
+	 * @param response the current response
 	 * @return {@code false} if the request is rejected, else {@code true}.
 	 */
-	boolean processActualRequest(CorsConfiguration conf, HttpServletRequest request,
+	boolean processRequest(CorsConfiguration configuration, HttpServletRequest request,
 			HttpServletResponse response) throws IOException;
 
 }

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

@@ -34,17 +34,14 @@ public class CorsUtils {
 	 * Returns {@code true} if the request is a valid CORS one.
 	 */
 	public static boolean isCorsRequest(HttpServletRequest request) {
-		return request.getHeader(HttpHeaders.ORIGIN) != null;
+		return (request.getHeader(HttpHeaders.ORIGIN) != null);
 	}
 
 	/**
 	 * Returns {@code true} if the request is a valid CORS pre-flight one.
 	 */
 	public static boolean isPreFlightRequest(HttpServletRequest request) {
-		if (!isCorsRequest(request)) {
-			return false;
-		}
-		return request.getMethod().equals(HttpMethod.OPTIONS.name());
+		return (isCorsRequest(request) && request.getMethod().equals(HttpMethod.OPTIONS.name()));
 	}
 
 }

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

@@ -39,7 +39,11 @@
 
 /**
  * Default implementation of {@link CorsProcessor}, as defined by the
- * <a href="http://www.w3.org/TR/cors/">CORS W3C recommandation</a>.
+ * <a href="http://www.w3.org/TR/cors/">CORS W3C recommendation</a>.
+ *
+ * <p>Note that when input {@link CorsConfiguration} is {@code null}, this
+ * implementation does not reject simple or actual requests outright but simply
+ * avoid adding CORS headers to the response.
  *
  * @author Sebastien Deleuze
  * @author Rossen Stoyanhcev
@@ -49,48 +53,37 @@ public class DefaultCorsProcessor implements CorsProcessor {
 
 	private static final Charset UTF8_CHARSET = Charset.forName("UTF-8");
 
-
-	protected final Log logger = LogFactory.getLog(getClass());
+	private static final Log logger = LogFactory.getLog(DefaultCorsProcessor.class);
 
 
 	@Override
-	public boolean processPreFlightRequest(CorsConfiguration config, HttpServletRequest request,
+	public boolean processRequest(CorsConfiguration config, HttpServletRequest request,
 			HttpServletResponse response) throws IOException {
 
-		Assert.isTrue(CorsUtils.isPreFlightRequest(request));
-
-		ServerHttpResponse serverResponse = new ServletServerHttpResponse(response);
-		if (responseHasCors(serverResponse)) {
-			return true;
-		}
-
-		ServerHttpRequest serverRequest = new ServletServerHttpRequest(request);
-		if (handleInternal(serverRequest, serverResponse, config, true)) {
-			serverResponse.flush();
+		if (!CorsUtils.isCorsRequest(request)) {
 			return true;
 		}
 
-		return false;
-	}
-
-	@Override
-	public boolean processActualRequest(CorsConfiguration config, HttpServletRequest request,
-			HttpServletResponse response) throws IOException {
-
-		Assert.isTrue(CorsUtils.isCorsRequest(request) && !CorsUtils.isPreFlightRequest(request));
-
 		ServletServerHttpResponse serverResponse = new ServletServerHttpResponse(response);
+		ServletServerHttpRequest serverRequest = new ServletServerHttpRequest(request);
+
 		if (responseHasCors(serverResponse)) {
 			return true;
 		}
 
-		ServletServerHttpRequest serverRequest = new ServletServerHttpRequest(request);
-		if (handleInternal(serverRequest, serverResponse, config, false)) {
-			serverResponse.flush();
-			return true;
+		boolean preFlightRequest = CorsUtils.isPreFlightRequest(request);
+
+		if (config == null) {
+			if (preFlightRequest) {
+				rejectRequest(serverResponse);
+				return false;
+			}
+			else {
+				return true;
+			}
 		}
 
-		return false;
+		return handleInternal(serverRequest, serverResponse, config, preFlightRequest);
 	}
 
 	private boolean responseHasCors(ServerHttpResponse response) {
@@ -107,32 +100,45 @@ private boolean responseHasCors(ServerHttpResponse response) {
 		return hasAllowOrigin;
 	}
 
+	/**
+	 * Invoked when one of the CORS checks failed.
+	 * The default implementation sets the response status to 403 and writes
+	 * "Invalid CORS request" to the response.
+	 */
+	protected void rejectRequest(ServerHttpResponse response) throws IOException {
+		response.setStatusCode(HttpStatus.FORBIDDEN);
+		response.getBody().write("Invalid CORS request".getBytes(UTF8_CHARSET));
+	}
+
+	/**
+	 * Handle the given request.
+	 */
 	protected boolean handleInternal(ServerHttpRequest request, ServerHttpResponse response,
-			CorsConfiguration config, boolean isPreFlight) throws IOException {
+			CorsConfiguration config, boolean preFlightRequest) throws IOException {
 
 		String requestOrigin = request.getHeaders().getOrigin();
 		String allowOrigin = checkOrigin(config, requestOrigin);
 
-		HttpMethod requestMethod = getMethodToUse(request, isPreFlight);
+		HttpMethod requestMethod = getMethodToUse(request, preFlightRequest);
 		List<HttpMethod> allowMethods = checkMethods(config, requestMethod);
 
-		List<String> requestHeaders = getHeadersToUse(request, isPreFlight);
+		List<String> requestHeaders = getHeadersToUse(request, preFlightRequest);
 		List<String> allowHeaders = checkHeaders(config, requestHeaders);
 
-		if (allowOrigin == null || allowMethods == null || (isPreFlight && allowHeaders == null)) {
-			handleInvalidCorsRequest(response);
+		if (allowOrigin == null || allowMethods == null || (preFlightRequest && allowHeaders == null)) {
+			rejectRequest(response);
 			return false;
 		}
 
 		HttpHeaders responseHeaders = response.getHeaders();
 		responseHeaders.setAccessControlAllowOrigin(allowOrigin);
 		responseHeaders.add(HttpHeaders.VARY, HttpHeaders.ORIGIN);
 
-		if (isPreFlight) {
+		if (preFlightRequest) {
 			responseHeaders.setAccessControlAllowMethods(allowMethods);
 		}
 
-		if (isPreFlight && !allowHeaders.isEmpty()) {
+		if (preFlightRequest && !allowHeaders.isEmpty()) {
 			responseHeaders.setAccessControlAllowHeaders(allowHeaders);
 		}
 
@@ -144,10 +150,11 @@ protected boolean handleInternal(ServerHttpRequest request, ServerHttpResponse r
 			responseHeaders.setAccessControlAllowCredentials(true);
 		}
 
-		if (isPreFlight && config.getMaxAge() != null) {
+		if (preFlightRequest && config.getMaxAge() != null) {
 			responseHeaders.setAccessControlMaxAge(config.getMaxAge());
 		}
 
+		response.flush();
 		return true;
 	}
 
@@ -187,14 +194,4 @@ private List<String> getHeadersToUse(ServerHttpRequest request, boolean isPreFli
 		return (isPreFlight ? headers.getAccessControlRequestHeaders() : new ArrayList<String>(headers.keySet()));
 	}
 
-	/**
-	 * Invoked when one of the CORS checks failed.
-	 * The default implementation sets the response status to 403 and writes
-	 * "Invalid CORS request" to the response.
-	 */
-	protected void handleInvalidCorsRequest(ServerHttpResponse response) throws IOException {
-		response.setStatusCode(HttpStatus.FORBIDDEN);
-		response.getBody().write("Invalid CORS request".getBytes(UTF8_CHARSET));
-	}
-
 }