2016-02-13 0:40 GMT+02:00 Rainer Jung <rainer.j...@kippdata.de>: > > 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?
Thanks, Violeta > 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 >
