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 2021/07/26 15:56:49 UTC
[tomcat] branch 8.5.x updated: Fix BZ 65443. Make CorsFilter more
extensible
This is an automated email from the ASF dual-hosted git repository.
markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git
The following commit(s) were added to refs/heads/8.5.x by this push:
new e896827 Fix BZ 65443. Make CorsFilter more extensible
e896827 is described below
commit e8968273d3822376c6f73550f8d179332bcc1c4b
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Mon Jul 26 16:50:12 2021 +0100
Fix BZ 65443. Make CorsFilter more extensible
https://bz.apache.org/bugzilla/show_bug.cgi?id=65443
Allows the getters to be overridden. I didn't go as far as adding
setters as that creates 'interesting' concurrency issues.
---
java/org/apache/catalina/filters/CorsFilter.java | 57 ++++++++++++++++++++----
webapps/docs/changelog.xml | 4 ++
2 files changed, 53 insertions(+), 8 deletions(-)
diff --git a/java/org/apache/catalina/filters/CorsFilter.java b/java/org/apache/catalina/filters/CorsFilter.java
index 4f1091a..60911af 100644
--- a/java/org/apache/catalina/filters/CorsFilter.java
+++ b/java/org/apache/catalina/filters/CorsFilter.java
@@ -77,6 +77,11 @@ import org.apache.tomcat.util.res.StringManager;
*
* @see <a href="http://www.w3.org/TR/cors/">CORS specification</a>
*
+ * If you extend this class and override one or more of the getXxx() methods,
+ * consider whether you also need to override
+ * {@link CorsFilter#doFilter(ServletRequest, ServletResponse, FilterChain)} and
+ * add appropriate locking so that the {@code doFilter()} method executes with a
+ * consistent configuration.
*/
public class CorsFilter implements Filter {
@@ -150,7 +155,7 @@ public class CorsFilter implements Filter {
CorsFilter.CORSRequestType requestType = checkRequestType(request);
// Adds CORS specific attributes to request.
- if (decorateRequest) {
+ if (isDecorateRequest()) {
CorsFilter.decorateCORSProperties(request, requestType);
}
switch (requestType) {
@@ -259,7 +264,7 @@ public class CorsFilter implements Filter {
return;
}
- if (!allowedHttpMethods.contains(method)) {
+ if (!getAllowedHttpMethods().contains(method)) {
handleInvalidCORS(request, response, filterChain);
return;
}
@@ -321,7 +326,7 @@ public class CorsFilter implements Filter {
}
// Section 6.2.5
- if (!allowedHttpMethods.contains(accessControlRequestMethod)) {
+ if (!getAllowedHttpMethods().contains(accessControlRequestMethod)) {
handleInvalidCORS(request, response, filterChain);
return;
}
@@ -329,7 +334,7 @@ public class CorsFilter implements Filter {
// Section 6.2.6
if (!accessControlRequestHeaders.isEmpty()) {
for (String header : accessControlRequestHeaders) {
- if (!allowedHttpHeaders.contains(header)) {
+ if (!getAllowedHttpHeaders().contains(header)) {
handleInvalidCORS(request, response, filterChain);
return;
}
@@ -407,6 +412,9 @@ public class CorsFilter implements Filter {
final String method = request.getMethod();
final String origin = request.getHeader(CorsFilter.REQUEST_HEADER_ORIGIN);
+ // Local copy to avoid concurrency issues if isAnyOriginAllowed()
+ // is overridden.
+ boolean anyOriginAllowed = isAnyOriginAllowed();
if (!anyOriginAllowed) {
// If only specific origins are allowed, the response will vary by
// origin
@@ -431,13 +439,16 @@ public class CorsFilter implements Filter {
// If the resource supports credentials, add a single
// Access-Control-Allow-Credentials header with the case-sensitive
// string "true" as value.
- if (supportsCredentials) {
+ if (isSupportsCredentials()) {
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.
+ // Local copy to avoid concurrency issues if getExposedHeaders()
+ // is overridden.
+ Collection<String> exposedHeaders = getExposedHeaders();
if ((exposedHeaders != null) && (exposedHeaders.size() > 0)) {
String exposedHeadersString = join(exposedHeaders, ",");
response.addHeader(CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_EXPOSE_HEADERS,
@@ -457,19 +468,27 @@ public class CorsFilter implements Filter {
// 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.
-
+ // Local copy to avoid concurrency issues if getPreflightMaxAge()
+ // is overridden.
+ long preflightMaxAge = getPreflightMaxAge();
if (preflightMaxAge > 0) {
response.addHeader(
CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_MAX_AGE,
String.valueOf(preflightMaxAge));
}
+ // Local copy to avoid concurrency issues if getAllowedHttpMethods()
+ // is overridden.
+ Collection<String> allowedHttpMethods = getAllowedHttpMethods();
if ((allowedHttpMethods != null) && (!allowedHttpMethods.isEmpty())) {
response.addHeader(
CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_METHODS,
join(allowedHttpMethods, ","));
}
+ // Local copy to avoid concurrency issues if getAllowedHttpHeaders()
+ // is overridden.
+ Collection<String> allowedHttpHeaders = getAllowedHttpHeaders();
if ((allowedHttpHeaders != null) && (!allowedHttpHeaders.isEmpty())) {
response.addHeader(
CorsFilter.RESPONSE_HEADER_ACCESS_CONTROL_ALLOW_HEADERS,
@@ -677,13 +696,13 @@ public class CorsFilter implements Filter {
* otherwise.
*/
private boolean isOriginAllowed(final String origin) {
- if (anyOriginAllowed) {
+ if (isAnyOriginAllowed()) {
return true;
}
// If 'Origin' header is a case-sensitive match of any of allowed
// origins, then return true, else return false.
- return allowedOrigins.contains(origin);
+ return getAllowedOrigins().contains(origin);
}
@@ -881,6 +900,28 @@ public class CorsFilter implements Filter {
}
+ /**
+ * Should CORS specific attributes be added to the request.
+ *
+ * @return {@code true} if the request should be decorated, otherwise
+ * {@code false}
+ */
+ public boolean isDecorateRequest() {
+ return decorateRequest;
+ }
+
+
+ /*
+ * Log objects are not Serializable but this Filter is because it extends
+ * GenericFilter. Tomcat won't serialize a Filter but in case something else
+ * does...
+ */
+ private void readObject(ObjectInputStream ois) throws ClassNotFoundException, IOException {
+ ois.defaultReadObject();
+ log = LogFactory.getLog(CorsFilter.class);
+ }
+
+
// -------------------------------------------------- CORS Response Headers
/**
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 98c415c..1aad55a 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -119,6 +119,10 @@
canonical path of the directory in which the symlink had been created.
Patch provided by Cedomir Igaly. (markt)
</fix>
+ <add>
+ <bug>65443</bug>: Refactor the <code>CorsFilter</code> to make it easier
+ to extend. (markt)
+ </add>
</changelog>
</subsection>
<subsection name="Coyote">
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org