You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2018/07/09 14:30:22 UTC
svn commit: r1835427 - in /tomcat/trunk:
java/org/apache/catalina/filters/CorsFilter.java webapps/docs/changelog.xml
Author: markt
Date: Mon Jul 9 14:30:22 2018
New Revision: 1835427
URL: http://svn.apache.org/viewvc?rev=1835427&view=rev
Log:
Ensure that the HTTP Vary header is set correctly when using the CORS filter and improve the cacheability of requests that pass through the OPRS filter.
Modified:
tomcat/trunk/java/org/apache/catalina/filters/CorsFilter.java
tomcat/trunk/webapps/docs/changelog.xml
Modified: tomcat/trunk/java/org/apache/catalina/filters/CorsFilter.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/filters/CorsFilter.java?rev=1835427&r1=1835426&r2=1835427&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/filters/CorsFilter.java (original)
+++ tomcat/trunk/java/org/apache/catalina/filters/CorsFilter.java Mon Jul 9 14:30:22 2018
@@ -148,20 +148,6 @@ public class CorsFilter extends GenericF
HttpServletRequest request = (HttpServletRequest) servletRequest;
HttpServletResponse response = (HttpServletResponse) servletResponse;
- // For any request that passes through this filter, the response (and the
- // associated headers) will depend on the origin.
- ResponseUtil.addVaryFieldName(response, CorsFilter.REQUEST_HEADER_ORIGIN);
-
- if ("OPTIONS".equals(request.getMethod())) {
- // For any OPTIONS request, the response will vary based on the
- // value or absence of the following headers. Hence they need be be
- // included in the Vary header.
- ResponseUtil.addVaryFieldName(response,
- CorsFilter.REQUEST_HEADER_ACCESS_CONTROL_REQUEST_METHOD);
- ResponseUtil.addVaryFieldName(response,
- CorsFilter.REQUEST_HEADER_ACCESS_CONTROL_REQUEST_HEADERS);
- }
-
// Determines the CORS request type.
CorsFilter.CORSRequestType requestType = checkRequestType(request);
@@ -253,8 +239,7 @@ public class CorsFilter extends GenericF
CorsFilter.CORSRequestType.ACTUAL));
}
- final String origin = request
- .getHeader(CorsFilter.REQUEST_HEADER_ORIGIN);
+ final String origin = request.getHeader(CorsFilter.REQUEST_HEADER_ORIGIN);
final String method = request.getMethod();
// Section 6.1.2
@@ -268,41 +253,7 @@ public class CorsFilter extends GenericF
return;
}
- // Section 6.1.3
- // Add a single Access-Control-Allow-Origin header.
- if (anyOriginAllowed) {
- // If any origin is allowed, return header with '*'.
- response.addHeader(
- CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN,
- "*");
- } else {
- // Add a single Access-Control-Allow-Origin header, with the value
- // of the Origin header as value.
- response.addHeader(
- CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN,
- origin);
- }
-
- // Section 6.1.3
- // If the resource supports credentials, add a single
- // Access-Control-Allow-Credentials header with the case-sensitive
- // string "true" as value.
- if (supportsCredentials) {
- response.addHeader(
- CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_CREDENTIALS,
- "true");
- }
-
- // Section 6.1.4
- // If the list of exposed headers is not empty add one or more
- // Access-Control-Expose-Headers headers, with as values the header
- // field names given in the list of exposed headers.
- if ((exposedHeaders != null) && (exposedHeaders.size() > 0)) {
- String exposedHeadersString = join(exposedHeaders, ",");
- response.addHeader(
- CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_EXPOSE_HEADERS,
- exposedHeadersString);
- }
+ addStandardHeaders(request, response);
// Forward the request down the filter chain.
filterChain.doFilter(request, response);
@@ -328,8 +279,7 @@ public class CorsFilter extends GenericF
CORSRequestType.PRE_FLIGHT.name().toLowerCase(Locale.ENGLISH)));
}
- final String origin = request
- .getHeader(CorsFilter.REQUEST_HEADER_ORIGIN);
+ final String origin = request.getHeader(CorsFilter.REQUEST_HEADER_ORIGIN);
// Section 6.2.2
if (!isOriginAllowed(origin)) {
@@ -376,44 +326,7 @@ public class CorsFilter extends GenericF
}
}
- // Section 6.2.7
- if (supportsCredentials) {
- response.addHeader(
- CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN,
- origin);
- response.addHeader(
- CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_CREDENTIALS,
- "true");
- } else {
- if (anyOriginAllowed) {
- response.addHeader(
- CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN,
- "*");
- } else {
- response.addHeader(
- CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN,
- origin);
- }
- }
-
- // Section 6.2.8
- if (preflightMaxAge > 0) {
- response.addHeader(
- CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_MAX_AGE,
- String.valueOf(preflightMaxAge));
- }
-
- // Section 6.2.9
- response.addHeader(
- CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_METHODS,
- accessControlRequestMethod);
-
- // Section 6.2.10
- if ((allowedHttpHeaders != null) && (!allowedHttpHeaders.isEmpty())) {
- response.addHeader(
- CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_HEADERS,
- join(allowedHttpHeaders, ","));
- }
+ addStandardHeaders(request, response);
// Do not forward the request down the filter chain.
}
@@ -433,6 +346,9 @@ public class CorsFilter extends GenericF
private void handleNonCORS(final HttpServletRequest request,
final HttpServletResponse response, final FilterChain filterChain)
throws IOException, ServletException {
+
+ addStandardHeaders(request, response);
+
// Let request pass.
filterChain.doFilter(request, response);
}
@@ -471,6 +387,90 @@ public class CorsFilter extends GenericF
}
}
+
+ /*
+ * Sets a standard set of headers to reduce response variation which in turn
+ * is intended to aid caching.
+ */
+ private void addStandardHeaders(final HttpServletRequest request,
+ final HttpServletResponse response) {
+
+ final String method = request.getMethod();
+ final String origin = request.getHeader(CorsFilter.REQUEST_HEADER_ORIGIN);
+
+ if (!anyOriginAllowed) {
+ // If only specific origins are allowed, the response will vary by
+ // origin
+ ResponseUtil.addVaryFieldName(response, CorsFilter.REQUEST_HEADER_ORIGIN);
+ }
+
+ // CORS requests (SIMPLE, ACTUAL, PRE_FLIGHT) set the following headers
+ // although non-CORS requests do not need to. The headers are always set
+ // as a) they do no harm in the non-CORS case and b) it allows the same
+ // response to be cached for CORS and non-CORS requests.
+
+ // Add a single Access-Control-Allow-Origin header.
+ if (anyOriginAllowed) {
+ // If any origin is allowed, return header with '*'.
+ response.addHeader(CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN, "*");
+ } else {
+ // Add a single Access-Control-Allow-Origin header, with the value
+ // of the Origin header as value.
+ response.addHeader(CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_ORIGIN, origin);
+ }
+
+ // If the resource supports credentials, add a single
+ // Access-Control-Allow-Credentials header with the case-sensitive
+ // string "true" as value.
+ if (supportsCredentials) {
+ response.addHeader(
+ CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_CREDENTIALS,
+ "true");
+ }
+
+ // If the list of exposed headers is not empty add one or more
+ // Access-Control-Expose-Headers headers, with as values the header
+ // field names given in the list of exposed headers.
+ if ((exposedHeaders != null) && (exposedHeaders.size() > 0)) {
+ String exposedHeadersString = join(exposedHeaders, ",");
+ response.addHeader(CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_EXPOSE_HEADERS,
+ exposedHeadersString);
+ }
+
+ if ("OPTIONS".equals(method)) {
+ // For an OPTIONS request, the response will vary based on the
+ // value or absence of the following headers. Hence they need be be
+ // included in the Vary header.
+ ResponseUtil.addVaryFieldName(
+ response, CorsFilter.REQUEST_HEADER_ACCESS_CONTROL_REQUEST_METHOD);
+ ResponseUtil.addVaryFieldName(
+ response, CorsFilter.REQUEST_HEADER_ACCESS_CONTROL_REQUEST_HEADERS);
+
+ // CORS PRE_FLIGHT (OPTIONS) requests set the following headers although
+ // non-CORS OPTIONS requests do not need to. The headers are always set
+ // as a) they do no harm in the non-CORS case and b) it allows the same
+ // response to be cached for CORS and non-CORS requests.
+
+ if (preflightMaxAge > 0) {
+ response.addHeader(
+ CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_MAX_AGE,
+ String.valueOf(preflightMaxAge));
+ }
+
+ if ((allowedHttpMethods != null & !allowedHttpMethods.isEmpty())) {
+ response.addHeader(
+ CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_METHODS,
+ join(allowedHttpMethods, ","));
+ }
+
+ if ((allowedHttpHeaders != null) && (!allowedHttpHeaders.isEmpty())) {
+ response.addHeader(
+ CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_HEADERS,
+ join(allowedHttpHeaders, ","));
+ }
+ }
+ }
+
/**
* Decorates the {@link HttpServletRequest}, with CORS attributes.
Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1835427&r1=1835426&r2=1835427&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Mon Jul 9 14:30:22 2018
@@ -61,6 +61,11 @@
improve the tracking of changes to the default host as hosts are added
and removed while Tomcat is running. (markt)
</fix>
+ <fix>
+ Ensure that the HTTP Vary header is set correctly when using the CORS
+ filter and improve the cacheability of requests that pass through the
+ COPRS filter. (markt)
+ </fix>
</changelog>
</subsection>
<subsection name="Coyote">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org