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:05 UTC

[tomcat] branch master updated (6e25e58 -> afe4085)

This is an automated email from the ASF dual-hosted git repository.

markt pushed a change to branch master
in repository https://gitbox.apache.org/repos/asf/tomcat.git.


    from 6e25e58  Improve Graal doc again
     new 4a8ad6d  Refactor to reduce code volume and to allow for other method tests
     new afe4085  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 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-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 master
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit 4a8ad6d0769a4f6335886b4175d317fc513b723a
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