You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by rj...@apache.org on 2016/02/12 21:35:26 UTC
svn commit: r1730101 - in /tomcat/trunk:
java/org/apache/catalina/loader/WebappClassLoaderBase.java
test/org/apache/catalina/loader/TestWebappClassLoader.java
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[]{
+ "",
"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("")) {
- 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));
}
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1730101 - in /tomcat/trunk: java/org/apache/catalina/loader/WebappClassLoaderBase.java
test/org/apache/catalina/loader/TestWebappClassLoader.java
Posted by Violeta Georgieva <mi...@gmail.com>.
2016-02-13 0:40 GMT+02:00 Rainer Jung <ra...@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-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
Re: svn commit: r1730101 - in /tomcat/trunk:
java/org/apache/catalina/loader/WebappClassLoaderBase.java
test/org/apache/catalina/loader/TestWebappClassLoader.java
Posted by Rainer Jung <ra...@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?
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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r1730101 - in /tomcat/trunk: java/org/apache/catalina/loader/WebappClassLoaderBase.java
test/org/apache/catalina/loader/TestWebappClassLoader.java
Posted by Violeta Georgieva <mi...@gmail.com>.
Hi,
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-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>