Spring/spring-framework
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Consolidate CORS/OPTIONS request mapping

Browse Source 
The CORS pre-flight request matching logic for all request conditions
was added (in 4.2) to RequestMappingInfo. However the logic for
default handling of all HTTP OPTIONS requests for 4.3 unintentionally
overrode some of the pre-flight request handling thus causing issues.

This commit moves CORS pre-flight matching logic into each respective
RequestMethodCondition implementations so each has to consider in one
place what happens for pre-flight and for all other requests.

Issue: SPR-13130
This commit is contained in:
Rossen Stoyanchev
2016-02-19 13:47:48 -05:00
parent 26575148a5
commit f32aded27a
8 changed files with 93 additions and 53 deletions
Download Patch File Download Diff File Expand all files Collapse all files

9
spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/ConsumesRequestCondition.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2014 the original author or authors.
* Copyright 2002-2016 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.
@@ -30,6 +30,7 @@ import org.springframework.http.MediaType;
import org.springframework.util.StringUtils;
import org.springframework.web.HttpMediaTypeNotSupportedException;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.cors.CorsUtils;
import org.springframework.web.servlet.mvc.condition.HeadersRequestCondition.HeaderExpression;
/**
@@ -46,6 +47,9 @@ import org.springframework.web.servlet.mvc.condition.HeadersRequestCondition.Hea
*/
public final class ConsumesRequestCondition extends AbstractRequestCondition<ConsumesRequestCondition> {
private final static ConsumesRequestCondition PRE_FLIGHT_MATCH = new ConsumesRequestCondition();
private final List<ConsumeMediaTypeExpression> expressions;
@@ -160,6 +164,9 @@ public final class ConsumesRequestCondition extends AbstractRequestCondition<Con
*/
@Override
public ConsumesRequestCondition getMatchingCondition(HttpServletRequest request) {
if (CorsUtils.isPreFlightRequest(request)) {
return PRE_FLIGHT_MATCH;
}
if (isEmpty()) {
return this;
}

9
spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/HeadersRequestCondition.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2014 the original author or authors.
* Copyright 2002-2016 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.
@@ -23,6 +23,7 @@ import java.util.Set;
import javax.servlet.http.HttpServletRequest;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.cors.CorsUtils;
/**
* A logical conjunction (' && ') request condition that matches a request against
@@ -38,6 +39,9 @@ import org.springframework.web.bind.annotation.RequestMapping;
*/
public final class HeadersRequestCondition extends AbstractRequestCondition<HeadersRequestCondition> {
private final static HeadersRequestCondition PRE_FLIGHT_MATCH = new HeadersRequestCondition();
private final Set<HeaderExpression> expressions;
@@ -105,6 +109,9 @@ public final class HeadersRequestCondition extends AbstractRequestCondition<Head
*/
@Override
public HeadersRequestCondition getMatchingCondition(HttpServletRequest request) {
if (CorsUtils.isPreFlightRequest(request)) {
return PRE_FLIGHT_MATCH;
}
for (HeaderExpression expression : expressions) {
if (!expression.match(request)) {
return null;

9
spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/ProducesRequestCondition.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2014 the original author or authors.
* Copyright 2002-2016 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.
@@ -30,6 +30,7 @@ import org.springframework.web.HttpMediaTypeNotAcceptableException;
import org.springframework.web.accept.ContentNegotiationManager;
import org.springframework.web.bind.annotation.RequestMapping;
import org.springframework.web.context.request.ServletWebRequest;
import org.springframework.web.cors.CorsUtils;
import org.springframework.web.servlet.mvc.condition.HeadersRequestCondition.HeaderExpression;
/**
@@ -45,6 +46,9 @@ import org.springframework.web.servlet.mvc.condition.HeadersRequestCondition.Hea
*/
public final class ProducesRequestCondition extends AbstractRequestCondition<ProducesRequestCondition> {
private final static ProducesRequestCondition PRE_FLIGHT_MATCH = new ProducesRequestCondition();
private final List<ProduceMediaTypeExpression> MEDIA_TYPE_ALL_LIST =
Collections.singletonList(new ProduceMediaTypeExpression("*/*"));
@@ -176,6 +180,9 @@ public final class ProducesRequestCondition extends AbstractRequestCondition<Pro
*/
@Override
public ProducesRequestCondition getMatchingCondition(HttpServletRequest request) {
if (CorsUtils.isPreFlightRequest(request)) {
return PRE_FLIGHT_MATCH;
}
if (isEmpty()) {
return this;
}

35
spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/RequestCondition.java
View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2015 the original author or authors.
* Copyright 2002-2016 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.
@@ -18,27 +18,25 @@ package org.springframework.web.servlet.mvc.condition;
import javax.servlet.http.HttpServletRequest;
import org.springframework.web.bind.annotation.RequestMapping;
/**
* The contract for request conditions in Spring MVC's mapping infrastructure.
* Contract for request mapping conditions.
*
* <p>Request conditions can be combined via {@link #combine(Object)}, matched to
* a request via {@link #getMatchingCondition(HttpServletRequest)}, and compared
* to each other via {@link #compareTo(Object, HttpServletRequest)} to determine
* which matches a request more closely.
* which is a closer match for a given request.
*
* @author Rossen Stoyanchev
* @author Arjen Poutsma
* @since 3.1
* @param <T> the type of objects that this RequestCondition can be combined with and compared to
* @param <T> the type of objects that this RequestCondition can be combined
* with and compared to
*/
public interface RequestCondition<T> {
/**
* Defines the rules for combining this condition (i.e. the current instance)
* with another condition. For example combining type- and method-level
* {@link RequestMapping} conditions.
* Combine this condition with another such as conditions from a
* type-level and method-level {@code @RequestMapping} annotation.
* @param other the condition to combine with.
* @return a request condition instance that is the result of combining
* the two condition instances.
@@ -46,18 +44,21 @@ public interface RequestCondition<T> {
T combine(T other);
/**
* Checks if this condition matches the given request and returns a
* potentially new request condition with content tailored to the
* current request. For example a condition with URL patterns might
* return a new condition that contains matching patterns sorted
* with best matching patterns on top.
* @return a condition instance in case of a match;
* or {@code null} if there is no match
* Check if the condition matches the request returning a potentially new
* instance created for the current request. For example a condition with
* multiple URL patterns may return a new instance only with those patterns
* that match the request.
* <p>For CORS pre-flight requests, conditions should match to the would-be,
* actual request (e.g. URL pattern, query parameters, and the HTTP method
* from the "Access-Control-Request-Method" header). If a condition cannot
* be matched to a pre-flight request it should return an instance with
* empty content thus not causing a failure to match.
* @return a condition instance in case of a match or {@code null} otherwise.
*/
T getMatchingCondition(HttpServletRequest request);
/**
* Compares this condition to another condition in the context of
* Compare this condition to another condition in the context of
* a specific request. This method assumes both instances have
* been obtained via {@link #getMatchingCondition(HttpServletRequest)}
* to ensure they have content relevant to current request only.

40
spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/condition/RequestMethodsRequestCondition.java
View File

@@ -24,7 +24,9 @@ import java.util.List;
import java.util.Set;
import javax.servlet.http.HttpServletRequest;
import org.springframework.http.HttpHeaders;
import org.springframework.web.bind.annotation.RequestMethod;
import org.springframework.web.cors.CorsUtils;
/**
* A logical disjunction (' || ') request condition that matches a request
@@ -101,12 +103,38 @@ public final class RequestMethodsRequestCondition extends AbstractRequestConditi
*/
@Override
public RequestMethodsRequestCondition getMatchingCondition(HttpServletRequest request) {
RequestMethod requestMethod = getRequestMethod(request);
if (this.methods.isEmpty()) {
return (RequestMethod.OPTIONS.equals(requestMethod) ? null : this);
if (CorsUtils.isPreFlightRequest(request)) {
return matchPreFlight(request);
}
if (getMethods().isEmpty()) {
if (RequestMethod.OPTIONS.name().equals(request.getMethod())) {
return null; // No implicit match for OPTIONS (we handle it)
}
return this;
}
return matchRequestMethod(request.getMethod());
}
/**
* On a pre-flight request match to the would-be, actual request.
* Hence empty conditions is a match, otherwise try to match to the HTTP
* method in the "Access-Control-Request-Method" header.
*/
private RequestMethodsRequestCondition matchPreFlight(HttpServletRequest request) {
if (getMethods().isEmpty()) {
return this;
}
String expectedMethod = request.getHeader(HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD);
return matchRequestMethod(expectedMethod);
}
private RequestMethodsRequestCondition matchRequestMethod(String httpMethod) {
RequestMethod requestMethod = getRequestMethod(httpMethod);
if (requestMethod != null) {
for (RequestMethod method : this.methods) {
for (RequestMethod method : getMethods()) {
if (method.equals(requestMethod)) {
return new RequestMethodsRequestCondition(method);
}
@@ -118,9 +146,9 @@ public final class RequestMethodsRequestCondition extends AbstractRequestConditi
return null;
}
private RequestMethod getRequestMethod(HttpServletRequest request) {
private RequestMethod getRequestMethod(String httpMethod) {
try {
return RequestMethod.valueOf(request.getMethod());
return RequestMethod.valueOf(httpMethod);
}
catch (IllegalArgumentException ex) {
return null;

26
spring-webmvc/src/main/java/org/springframework/web/servlet/mvc/method/RequestMappingInfo.java
View File

@@ -215,15 +215,7 @@ public final class RequestMappingInfo implements RequestCondition<RequestMapping
ProducesRequestCondition produces = this.producesCondition.getMatchingCondition(request);
if (methods == null || params == null || headers == null || consumes == null || produces == null) {
if (CorsUtils.isPreFlightRequest(request)) {
methods = getAccessControlRequestMethodCondition(request);
if (methods == null || params == null) {
return null;
}
}
else {
return null;
}
return null;
}
PatternsRequestCondition patterns = this.patternsCondition.getMatchingCondition(request);
@@ -240,22 +232,6 @@ public final class RequestMappingInfo implements RequestCondition<RequestMapping
methods, params, headers, consumes, produces, custom.getCondition());
}
/**
* Return a matching RequestMethodsRequestCondition based on the expected
* HTTP method specified in a CORS pre-flight request.
*/
private RequestMethodsRequestCondition getAccessControlRequestMethodCondition(HttpServletRequest request) {
String expectedMethod = request.getHeader(HttpHeaders.ACCESS_CONTROL_REQUEST_METHOD);
if (StringUtils.hasText(expectedMethod)) {
for (RequestMethod method : getMethodsCondition().getMethods()) {
if (expectedMethod.equalsIgnoreCase(method.name())) {
return new RequestMethodsRequestCondition(method);
}
}
}
return null;
}
/**
* Compares "this" info (i.e. the current instance) with another info in the context of a request.
* <p>Note: It is assumed both instances have been obtained via