This is an automated email from the ASF dual-hosted git repository. markt pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/tomcat.git
commit afe40851efbdddc44862a7b314a07d86ca04f06d Author: Mark Thomas <ma...@apache.org> AuthorDate: Fri Nov 29 23:19:00 2019 +0000 Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63939 same origin Refactor isSameOrigin test into utility class (for re-use in AuthenticatorBase) and fix two bugs: - comparison should be case-sensitive - origin may or may not include default port --- java/org/apache/catalina/filters/CorsFilter.java | 33 +----- java/org/apache/tomcat/util/http/RequestUtil.java | 51 ++++++++++ .../util/http/TestRequestUtilSameOrigin.java | 113 +++++++++++++++++++++ 3 files changed, 166 insertions(+), 31 deletions(-) diff --git a/java/org/apache/catalina/filters/CorsFilter.java b/java/org/apache/catalina/filters/CorsFilter.java index ad5a1f4..4213fb4 100644 --- a/java/org/apache/catalina/filters/CorsFilter.java +++ b/java/org/apache/catalina/filters/CorsFilter.java @@ -39,6 +39,7 @@ import javax.servlet.http.HttpServletResponse; import org.apache.juli.logging.Log; import org.apache.juli.logging.LogFactory; +import org.apache.tomcat.util.http.RequestUtil; import org.apache.tomcat.util.http.ResponseUtil; import org.apache.tomcat.util.res.StringManager; @@ -591,7 +592,7 @@ public class CorsFilter extends GenericFilter { requestType = CORSRequestType.INVALID_CORS; } else if (!isValidOrigin(originHeader)) { requestType = CORSRequestType.INVALID_CORS; - } else if (isLocalOrigin(request, originHeader)) { + } else if (RequestUtil.isSameOrigin(request, originHeader)) { return CORSRequestType.NOT_CORS; } else { String method = request.getMethod(); @@ -634,36 +635,6 @@ public class CorsFilter extends GenericFilter { } - private boolean isLocalOrigin(HttpServletRequest request, String origin) { - - // Build scheme://host:port from request - StringBuilder target = new StringBuilder(); - String scheme = request.getScheme(); - if (scheme == null) { - return false; - } else { - scheme = scheme.toLowerCase(Locale.ENGLISH); - } - target.append(scheme); - target.append("://"); - - String host = request.getServerName(); - if (host == null) { - return false; - } - target.append(host); - - int port = request.getServerPort(); - if ("http".equals(scheme) && port != 80 || - "https".equals(scheme) && port != 443) { - target.append(':'); - target.append(port); - } - - return origin.equalsIgnoreCase(target.toString()); - } - - /** * Return the lower case, trimmed value of the media type from the content * type. diff --git a/java/org/apache/tomcat/util/http/RequestUtil.java b/java/org/apache/tomcat/util/http/RequestUtil.java index 28922c4..cfa9c57 100644 --- a/java/org/apache/tomcat/util/http/RequestUtil.java +++ b/java/org/apache/tomcat/util/http/RequestUtil.java @@ -16,6 +16,10 @@ */ package org.apache.tomcat.util.http; +import java.util.Locale; + +import javax.servlet.http.HttpServletRequest; + public class RequestUtil { private RequestUtil() { @@ -113,4 +117,51 @@ public class RequestUtil { // Return the normalized path that we have completed return normalized; } + + + public static boolean isSameOrigin(HttpServletRequest request, String origin) { + // Build scheme://host:port from request + StringBuilder target = new StringBuilder(); + String scheme = request.getScheme(); + if (scheme == null) { + return false; + } else { + scheme = scheme.toLowerCase(Locale.ENGLISH); + } + target.append(scheme); + target.append("://"); + + String host = request.getServerName(); + if (host == null) { + return false; + } + target.append(host); + + int port = request.getServerPort(); + // Origin may or may not include the (default) port. + // At this point target doesn't include a port. + if (target.length() == origin.length()) { + // origin and target can only be equal if both are using default + // ports. Therefore only append the port to the target if a + // non-default port is used. + if (("http".equals(scheme) || "ws".equals(scheme)) && port != 80 || + ("https".equals(scheme) || "wss".equals(scheme)) && port != 443) { + target.append(':'); + target.append(port); + } + } else { + // origin and target can only be equal if: + // a) origin includes an explicit default port + // b) origin is using a non-default port + // Either way, add the port to the target so it can be compared + target.append(':'); + target.append(port); + } + + + // Both scheme and host are case-insensitive but the CORS spec states + // this check should be case-sensitive + return origin.equals(target.toString()); + } + } diff --git a/test/org/apache/tomcat/util/http/TestRequestUtilSameOrigin.java b/test/org/apache/tomcat/util/http/TestRequestUtilSameOrigin.java new file mode 100644 index 0000000..5b4c0d0 --- /dev/null +++ b/test/org/apache/tomcat/util/http/TestRequestUtilSameOrigin.java @@ -0,0 +1,113 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); 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 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.tomcat.util.http; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.List; + +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletRequestWrapper; + +import org.junit.Assert; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.Parameterized; +import org.junit.runners.Parameterized.Parameter; + +import org.apache.catalina.connector.Request; + +@RunWith(Parameterized.class) +public class TestRequestUtilSameOrigin { + + @Parameterized.Parameters(name = "{index}: request[{0}], origin[{1}]") + public static Collection<Object[]> parameters() { + List<Object[]> parameterSets = new ArrayList<>(); + + TesterRequest request1 = new TesterRequest("http", "example.com", 80); + TesterRequest request2 = new TesterRequest("ws", "example.com", 80); + TesterRequest request3 = new TesterRequest("http", "example.com", 443); + TesterRequest request4 = new TesterRequest("http", "example.com", 8080); + + parameterSets.add(new Object[] { request1, "http://example.com", Boolean.TRUE }); + parameterSets.add(new Object[] { request1, "http://example.com:80", Boolean.TRUE }); + parameterSets.add(new Object[] { request1, "http://example.com:8080", Boolean.FALSE}); + + parameterSets.add(new Object[] { request2, "ws://example.com", Boolean.TRUE }); + parameterSets.add(new Object[] { request2, "ws://example.com:80", Boolean.TRUE }); + parameterSets.add(new Object[] { request2, "ws://example.com:8080", Boolean.FALSE}); + + parameterSets.add(new Object[] { request3, "http://example.com", Boolean.FALSE }); + parameterSets.add(new Object[] { request3, "http://example.com:80", Boolean.FALSE }); + parameterSets.add(new Object[] { request3, "http://example.com:443", Boolean.TRUE}); + + parameterSets.add(new Object[] { request4, "http://example.com", Boolean.FALSE }); + parameterSets.add(new Object[] { request4, "http://example.com:80", Boolean.FALSE }); + parameterSets.add(new Object[] { request4, "http://example.com:8080", Boolean.TRUE}); + + return parameterSets; + } + + + @Parameter(0) + public HttpServletRequest request; + @Parameter(1) + public String origin; + @Parameter(2) + public Boolean same; + + + @Test + public void testSameOrigin() { + Assert.assertEquals(same, Boolean.valueOf(RequestUtil.isSameOrigin(request, origin))); + } + + + private static class TesterRequest extends HttpServletRequestWrapper { + + private final String scheme; + private final String host; + private final int port; + + public TesterRequest(String scheme, String host, int port) { + super(new Request(null)); + this.scheme = scheme; + this.host = host; + this.port = port; + } + + @Override + public String getScheme() { + return scheme; + } + + @Override + public String getServerName() { + return host; + } + + @Override + public int getServerPort() { + return port; + } + + @Override + public String toString() { + return scheme + "://" + host + ":" + port; + } + } +} --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org