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 2020/05/15 16:43:35 UTC

[tomcat] branch 7.0.x updated: Always use DeploymentException for invalid paths and add more checks

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

markt pushed a commit to branch 7.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/7.0.x by this push:
     new 2e3aa16  Always use DeploymentException for invalid paths and add more checks
2e3aa16 is described below

commit 2e3aa161041f56d0251ed62e8fa7a92975438b93
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Fri May 15 17:36:29 2020 +0100

    Always use DeploymentException for invalid paths and add more checks
    
    The additional paths should have been rejected later or would have never
    have worked anyway.
---
 .../websocket/server/LocalStrings.properties       |  2 +-
 .../tomcat/websocket/server/UriTemplate.java       |  9 +++---
 .../tomcat/websocket/server/TestUriTemplate.java   | 37 +++++++++++++++++-----
 webapps/docs/changelog.xml                         |  9 ++++++
 4 files changed, 44 insertions(+), 13 deletions(-)

diff --git a/java/org/apache/tomcat/websocket/server/LocalStrings.properties b/java/org/apache/tomcat/websocket/server/LocalStrings.properties
index 7fad34c..6616e33 100644
--- a/java/org/apache/tomcat/websocket/server/LocalStrings.properties
+++ b/java/org/apache/tomcat/websocket/server/LocalStrings.properties
@@ -27,7 +27,7 @@ serverContainer.threadGroupNotDestroyed=Unable to destroy WebSocket thread group
 upgradeUtil.incompatibleRsv=Extensions were specified that have incompatible RSV bit usage
 
 uriTemplate.duplicateParameter=The parameter [{0}] appears more than once in the path which is not permitted
-uriTemplate.emptySegment=The path [{0}] contains one or more empty segments which are is not permitted
+uriTemplate.emptySegment=The path [{0}] contains one or more empty segments which is not permitted
 uriTemplate.invalidPath=The path [{0}] is not valid.
 uriTemplate.invalidSegment=The segment [{0}] is not valid in the provided path [{1}]
 
diff --git a/java/org/apache/tomcat/websocket/server/UriTemplate.java b/java/org/apache/tomcat/websocket/server/UriTemplate.java
index 172359a..523d3fc 100644
--- a/java/org/apache/tomcat/websocket/server/UriTemplate.java
+++ b/java/org/apache/tomcat/websocket/server/UriTemplate.java
@@ -43,7 +43,8 @@ public class UriTemplate {
 
     public UriTemplate(String path) throws DeploymentException {
 
-        if (path == null || path.length() ==0 || !path.startsWith("/")) {
+        if (path == null || path.length() == 0 || !path.startsWith("/") || path.contains("/../") ||
+                path.contains("/./") || path.contains("//")) {
             throw new DeploymentException(
                     sm.getString("uriTemplate.invalidPath", path));
         }
@@ -68,7 +69,7 @@ public class UriTemplate {
                 } else {
                     // As per EG discussion, all other empty segments are
                     // invalid
-                    throw new IllegalArgumentException(sm.getString(
+                    throw new DeploymentException(sm.getString(
                             "uriTemplate.emptySegment", path));
                 }
             }
@@ -81,12 +82,12 @@ public class UriTemplate {
                 normalized.append(paramCount++);
                 normalized.append('}');
                 if (!paramNames.add(segment)) {
-                    throw new IllegalArgumentException(sm.getString(
+                    throw new DeploymentException(sm.getString(
                             "uriTemplate.duplicateParameter", segment));
                 }
             } else {
                 if (segment.contains("{") || segment.contains("}")) {
-                    throw new IllegalArgumentException(sm.getString(
+                    throw new DeploymentException(sm.getString(
                             "uriTemplate.invalidSegment", segment, path));
                 }
                 normalized.append(segment);
diff --git a/test/org/apache/tomcat/websocket/server/TestUriTemplate.java b/test/org/apache/tomcat/websocket/server/TestUriTemplate.java
index f0b1c4e..697b48f 100644
--- a/test/org/apache/tomcat/websocket/server/TestUriTemplate.java
+++ b/test/org/apache/tomcat/websocket/server/TestUriTemplate.java
@@ -44,35 +44,35 @@ public class TestUriTemplate {
     }
 
 
-    @Test(expected=java.lang.IllegalArgumentException.class)
+    @Test(expected=javax.websocket.DeploymentException.class)
     public void testBasicPrefix() throws Exception {
         @SuppressWarnings("unused")
         UriTemplate t = new UriTemplate("/x{a}/y{b}");
     }
 
 
-    @Test(expected=java.lang.IllegalArgumentException.class)
+    @Test(expected=javax.websocket.DeploymentException.class)
     public void testPrefixOneOfTwo() throws Exception {
         UriTemplate t = new UriTemplate("/x{a}/y{b}");
         t.match(new UriTemplate("/xfoo"));
     }
 
 
-    @Test(expected=java.lang.IllegalArgumentException.class)
+    @Test(expected=javax.websocket.DeploymentException.class)
     public void testPrefixTwoOfTwo() throws Exception {
         UriTemplate t = new UriTemplate("/x{a}/y{b}");
         t.match(new UriTemplate("/ybar"));
     }
 
 
-    @Test(expected=java.lang.IllegalArgumentException.class)
+    @Test(expected=javax.websocket.DeploymentException.class)
     public void testQuote1() throws Exception {
         UriTemplate t = new UriTemplate("/.{a}");
         t.match(new UriTemplate("/yfoo"));
     }
 
 
-    @Test(expected=java.lang.IllegalArgumentException.class)
+    @Test(expected=javax.websocket.DeploymentException.class)
     public void testQuote2() throws Exception {
         @SuppressWarnings("unused")
         UriTemplate t = new UriTemplate("/.{a}");
@@ -153,7 +153,7 @@ public class TestUriTemplate {
     }
 
 
-    @Test(expected=java.lang.IllegalArgumentException.class)
+    @Test(expected=javax.websocket.DeploymentException.class)
     public void testDuplicate01() throws Exception {
         @SuppressWarnings("unused")
         UriTemplate t = new UriTemplate("/{var}/{var}");
@@ -196,7 +196,7 @@ public class TestUriTemplate {
     }
 
 
-    @Test(expected=java.lang.IllegalArgumentException.class)
+    @Test(expected=javax.websocket.DeploymentException.class)
     public void testEgMailingList04() throws Exception {
         UriTemplate t = new UriTemplate("/a/{var1}/{var2}");
         @SuppressWarnings("unused")
@@ -204,10 +204,31 @@ public class TestUriTemplate {
     }
 
 
-    @Test(expected=java.lang.IllegalArgumentException.class)
+    @Test(expected=javax.websocket.DeploymentException.class)
     public void testEgMailingList05() throws Exception {
         UriTemplate t = new UriTemplate("/a/{var}/");
         @SuppressWarnings("unused")
         Map<String,String> result = t.match(new UriTemplate("/a/b/"));
     }
+
+
+    @Test(expected=javax.websocket.DeploymentException.class)
+    public void testSpecIssue194a() throws Exception {
+        @SuppressWarnings("unused")
+        UriTemplate t = new UriTemplate("/a/../b");
+    }
+
+
+    @Test(expected=javax.websocket.DeploymentException.class)
+    public void testSpecIssue194b() throws Exception {
+        @SuppressWarnings("unused")
+        UriTemplate t = new UriTemplate("/./b");
+    }
+
+
+    @Test(expected=javax.websocket.DeploymentException.class)
+    public void testSpecIssue194c() throws Exception {
+        @SuppressWarnings("unused")
+        UriTemplate t = new UriTemplate("//b");
+    }
 }
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 29a389f..1339557 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -60,6 +60,15 @@
   issues do not "pop up" wrt. others).
 -->
 <section name="Tomcat 7.0.105 (violetagg)" rtext="in development">
+  <subsection name="WebSocket">
+    <changelog>
+      <fix>
+        Consistently throw a <code>DeploymentException</code> when an invalid
+        endpoint path is specified and catch invalid endpoint paths earlier.
+        (markt)
+      </fix>
+    </changelog>
+  </subsection>
 </section>
 <section name="Tomcat 7.0.104 (violetagg)" rtext="release in progress">
   <subsection name="Catalina">


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org