Re: svn commit: r1730101 - in /tomcat/trunk: java/org/apache/catalina/loader/WebappClassLoaderBase.java test/org/apache/catalina/loader/TestWebappClassLoader.java

[Rainer Jung]

Hi Violeta,

build breakage fixed in r1730137.
I adjusted the test to better reflect what's implemented currently:

- deny if name is something below the denied package. We don't care for 
the package names themselves without anything added.
- permit exclude rules work the same way, only permit for something 
below the permitted packages. Don't care for package name itself
- permit any other combination of prefix/suffix (here's the place for 
the "org" and "javax" test)

OK?

Regards,

Rainer

Am 12.02.2016 um 22:20 schrieb Violeta Georgieva:
2016-02-12 22:35 GMT+02:00 <rj...@apache.org>:

Author: rjung
Date: Fri Feb 12 20:35:26 2016
New Revision: 1730101

URL: http://svn.apache.org/viewvc?rev=1730101&view=rev
Log:
BZ 58999: Fix class and resource name
filtering in WebappClassLoader.

It throws a StringIndexOutOfBoundsException
if the name is "org" or "javax".

We currently do not filter class or resource
names which are exactly equals to one of the
package names of classes and resources to
filter. Only classes or resources underneath
that packages.

Example:
- "javax.servlet" will not be filtered
- "javax.servlet.Class" will be filtered

Modified:

tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoaderBase.java

tomcat/trunk/test/org/apache/catalina/loader/TestWebappClassLoader.java

Modified:
tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoaderBase.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoaderBase.java?rev=1730101&r1=1730100&r2=1730101&view=diff

==============================================================================
---
tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoaderBase.java
(original)
+++
tomcat/trunk/java/org/apache/catalina/loader/WebappClassLoaderBase.java Fri
Feb 12 20:35:26 2016
@@ -2765,6 +2765,9 @@ public abstract class WebappClassLoaderB
          char ch;
          if (name.startsWith("javax")) {
              /* 5 == length("javax") */
+            if (name.length() == 5) {
+                return false;
+            }
              ch = name.charAt(5);
              if (isClassName && ch == '.') {
                  /* 6 == length("javax.") */
@@ -2791,6 +2794,9 @@ public abstract class WebappClassLoaderB
              }
          } else if (name.startsWith("org")) {
              /* 3 == length("org") */
+            if (name.length() == 3) {
+                return false;
+            }
              ch = name.charAt(3);
              if (isClassName && ch == '.') {
                  /* 4 == length("org.") */

Modified:
tomcat/trunk/test/org/apache/catalina/loader/TestWebappClassLoader.java
URL:
http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/loader/TestWebappClassLoader.java?rev=1730101&r1=1730100&r2=1730101&view=diff

==============================================================================
---
tomcat/trunk/test/org/apache/catalina/loader/TestWebappClassLoader.java
(original)
+++
tomcat/trunk/test/org/apache/catalina/loader/TestWebappClassLoader.java Fri
Feb 12 20:35:26 2016
@@ -65,10 +65,12 @@ public class TestWebappClassLoader exten
      public void testFilter() throws IOException {

          String[] classSuffixes = new String[]{
+            "",


With this test we would like to test "org" and "javax", but then why we add
"." and "/" when the suffix is empty string?


              "some.package.Example"
          };

          String[] resourceSuffixes = new String[]{
+            "",
              "some/path/test.properties",
              "some/path/test"
          };
@@ -83,7 +85,7 @@ public class TestWebappClassLoader exten
              "org.apache",
              "org.apache.tomcat.jdbc",
              "javax",
-            "javax.jsp.jstl",
+            "javax.servlet.jsp.jstl",
              "com.mycorp"
          };

@@ -131,20 +133,13 @@ public class TestWebappClassLoader exten
              for (String prefix : prefixesDeny) {
                  for (String suffix : classSuffixes) {
                      if (prefix.equals("")) {

This one should be removed. Currently it breaks the build.

-                        name = suffix;
-                    } else {
-                        name = prefix + "." + suffix;
-                    }
+                    name = prefix + "." + suffix;
                      Assert.assertTrue("Class '" + name + "' failed deny
filter",
                                 loader.filter(name, true));
                  }
                  prefix = prefix.replace('.', '/');
                  for (String suffix : resourceSuffixes) {
-                    if (prefix.equals("")) {
-                        name = suffix;
-                    } else {
-                        name = prefix + "/" + suffix;
-                    }
+                    name = prefix + "/" + suffix;
                      Assert.assertTrue("Resource '" + name + "' failed
deny filter",
                                 loader.filter(name, false));
                  }




Regards,
Violeta

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