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 2019/11/30 11:54:49 UTC
[tomcat] branch 8.5.x updated (ca2be3c -> cb70be5)
This is an automated email from the ASF dual-hosted git repository.
markt pushed a change to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git.
from ca2be3c Fix case inconsistency
new 638e606 Refactor to reduce code volume and to allow for other method tests
new cb70be5 Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63939 same origin
The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
java/org/apache/catalina/filters/CorsFilter.java | 33 +----
java/org/apache/tomcat/util/http/RequestUtil.java | 51 +++++++
.../apache/tomcat/util/http/TestRequestUtil.java | 162 ---------------------
.../tomcat/util/http/TestRequestUtilNormalize.java | 77 ++++++++++
.../util/http/TestRequestUtilSameOrigin.java | 113 ++++++++++++++
5 files changed, 243 insertions(+), 193 deletions(-)
delete mode 100644 test/org/apache/tomcat/util/http/TestRequestUtil.java
create mode 100644 test/org/apache/tomcat/util/http/TestRequestUtilNormalize.java
create mode 100644 test/org/apache/tomcat/util/http/TestRequestUtilSameOrigin.java
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 02/02: Fix
https://bz.apache.org/bugzilla/show_bug.cgi?id=63939 same origin
Posted by ma...@apache.org.
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
commit cb70be5a723dbca99ff92a31320afabbdad8794c
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 c32bd30..185cfaa 100644
--- a/java/org/apache/catalina/filters/CorsFilter.java
+++ b/java/org/apache/catalina/filters/CorsFilter.java
@@ -38,6 +38,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;
@@ -618,7 +619,7 @@ public class CorsFilter implements Filter {
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();
@@ -661,36 +662,6 @@ public class CorsFilter implements Filter {
}
- 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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[tomcat] 01/02: Refactor to reduce code volume and to allow for
other method tests
Posted by ma...@apache.org.
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
commit 638e606733c83ca468a73cf6d096ac85714f6ab1
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Fri Nov 29 22:55:15 2019 +0000
Refactor to reduce code volume and to allow for other method tests
---
.../apache/tomcat/util/http/TestRequestUtil.java | 162 ---------------------
.../tomcat/util/http/TestRequestUtilNormalize.java | 77 ++++++++++
2 files changed, 77 insertions(+), 162 deletions(-)
diff --git a/test/org/apache/tomcat/util/http/TestRequestUtil.java b/test/org/apache/tomcat/util/http/TestRequestUtil.java
deleted file mode 100644
index 02deb5e..0000000
--- a/test/org/apache/tomcat/util/http/TestRequestUtil.java
+++ /dev/null
@@ -1,162 +0,0 @@
-/*
- * 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 org.junit.Assert;
-import org.junit.Test;
-
-public class TestRequestUtil {
-
- @Test
- public void testNormalize01() {
- doTestNormalize("//something", "/something");
- }
-
- @Test
- public void testNormalize02() {
- doTestNormalize("some//thing", "/some/thing");
- }
-
- @Test
- public void testNormalize03() {
- doTestNormalize("something//", "/something/");
- }
-
- @Test
- public void testNormalize04() {
- doTestNormalize("//", "/");
- }
-
- @Test
- public void testNormalize05() {
- doTestNormalize("//", "/");
- }
-
- @Test
- public void testNormalize06() {
- doTestNormalize("///", "/");
- }
-
- @Test
- public void testNormalize07() {
- doTestNormalize("////", "/");
- }
-
- @Test
- public void testNormalize08() {
- doTestNormalize("/.", "/");
- }
-
- @Test
- public void testNormalize09() {
- doTestNormalize("/./", "/");
- }
-
- @Test
- public void testNormalize10() {
- doTestNormalize(".", "/");
- }
-
- @Test
- public void testNormalize11() {
- doTestNormalize("/..", null);
- }
-
- @Test
- public void testNormalize12() {
- doTestNormalize("/../", null);
- }
-
- @Test
- public void testNormalize13() {
- doTestNormalize("..", null);
- }
-
- @Test
- public void testNormalize14() {
- doTestNormalize("//..", null);
- }
-
- @Test
- public void testNormalize15() {
- doTestNormalize("//../", null);
- }
-
- @Test
- public void testNormalize16() {
- doTestNormalize("/./..", null);
- }
-
- @Test
- public void testNormalize17() {
- doTestNormalize("/./../", null);
- }
-
- @Test
- public void testNormalize18() {
- doTestNormalize("/a/../..", null);
- }
-
- @Test
- public void testNormalize19() {
- doTestNormalize("/a/../../", null);
- }
-
- @Test
- public void testNormalize20() {
- doTestNormalize("/a/..", "/");
- }
-
- @Test
- public void testNormalize21() {
- doTestNormalize("/a/.", "/a");
- }
-
- @Test
- public void testNormalize22() {
- doTestNormalize("/a/../", "/");
- }
-
- @Test
- public void testNormalize23() {
- doTestNormalize("/a/./", "/a/");
- }
-
- @Test
- public void testNormalize24() {
- doTestNormalize("/a/b/..", "/a");
- }
-
- @Test
- public void testNormalize25() {
- doTestNormalize("/a/b/.", "/a/b");
- }
-
- @Test
- public void testNormalize26() {
- doTestNormalize("/a/b/../", "/a/");
- }
-
- @Test
- public void testNormalize27() {
- doTestNormalize("/a/b/./", "/a/b/");
- }
-
- private void doTestNormalize(String input, String expected) {
- Assert.assertEquals(expected,RequestUtil.normalize(input));
- }
-}
diff --git a/test/org/apache/tomcat/util/http/TestRequestUtilNormalize.java b/test/org/apache/tomcat/util/http/TestRequestUtilNormalize.java
new file mode 100644
index 0000000..b642868
--- /dev/null
+++ b/test/org/apache/tomcat/util/http/TestRequestUtilNormalize.java
@@ -0,0 +1,77 @@
+/*
+ * 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 org.junit.Assert;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.junit.runners.Parameterized.Parameter;
+
+@RunWith(Parameterized.class)
+public class TestRequestUtilNormalize {
+
+ @Parameterized.Parameters(name = "{index}: input[{0}]")
+ public static Collection<Object[]> parameters() {
+ List<Object[]> parameterSets = new ArrayList<>();
+
+ parameterSets.add(new String[] { "//something", "/something" });
+ parameterSets.add(new String[] { "some//thing", "/some/thing" });
+ parameterSets.add(new String[] { "something//", "/something/" });
+ parameterSets.add(new String[] { "//", "/" });
+ parameterSets.add(new String[] { "///", "/" });
+ parameterSets.add(new String[] { "////", "/" });
+ parameterSets.add(new String[] { "/.", "/" });
+ parameterSets.add(new String[] { "/./", "/" });
+ parameterSets.add(new String[] { ".", "/" });
+ parameterSets.add(new String[] { "/..", null });
+ parameterSets.add(new String[] { "/../", null });
+ parameterSets.add(new String[] { "..", null });
+ parameterSets.add(new String[] { "//..", null });
+ parameterSets.add(new String[] { "//../", null });
+ parameterSets.add(new String[] { "/./..", null });
+ parameterSets.add(new String[] { "/./../", null });
+ parameterSets.add(new String[] { "/a/../..", null });
+ parameterSets.add(new String[] { "/a/../../", null });
+ parameterSets.add(new String[] { "/a/..", "/" });
+ parameterSets.add(new String[] { "/a/.", "/a" });
+ parameterSets.add(new String[] { "/a/../", "/" });
+ parameterSets.add(new String[] { "/a/./", "/a/" });
+ parameterSets.add(new String[] { "/a/b/..", "/a" });
+ parameterSets.add(new String[] { "/a/b/.", "/a/b" });
+ parameterSets.add(new String[] { "/a/b/../", "/a/" });
+ parameterSets.add(new String[] { "/a/b/./", "/a/b/" });
+
+ return parameterSets;
+ }
+
+
+ @Parameter(0)
+ public String input;
+ @Parameter(1)
+ public String expected;
+
+
+ @Test
+ public void testNormalize() {
+ Assert.assertEquals(expected,RequestUtil.normalize(input));
+ }
+}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org