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 2009/12/16 17:56:22 UTC

svn commit: r891310 - in /tomcat/tc6.0.x/trunk: STATUS.txt java/org/apache/catalina/loader/WebappClassLoader.java java/org/apache/catalina/security/SecurityClassLoad.java

Author: markt
Date: Wed Dec 16 16:56:22 2009
New Revision: 891310

URL: http://svn.apache.org/viewvc?rev=891310&view=rev
Log:
* Additional fix for https://issues.apache.org/bugzilla/show_bug.cgi?id=48097
  1) Code cleanup: Remove use of WebappClassLoader$PrivilegedFindResource,
  because all findResourceInternal(String,String) calls are now already
  wrapped with AccessController.doPrivileged, so additional wrapping is not
  needed.
  2) Add preloading of WebappClassLoader$PrivilegedFindResourceByName,
  to fix https://issues.apache.org/bugzilla/show_bug.cgi?id=48097#c13

Modified:
    tomcat/tc6.0.x/trunk/STATUS.txt
    tomcat/tc6.0.x/trunk/java/org/apache/catalina/loader/WebappClassLoader.java
    tomcat/tc6.0.x/trunk/java/org/apache/catalina/security/SecurityClassLoad.java

Modified: tomcat/tc6.0.x/trunk/STATUS.txt
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=891310&r1=891309&r2=891310&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/STATUS.txt (original)
+++ tomcat/tc6.0.x/trunk/STATUS.txt Wed Dec 16 16:56:22 2009
@@ -285,17 +285,6 @@
   -1:
   +0: markt Combined patch needs to have 834047 removed and 881765 added
 
-* Additional fix for https://issues.apache.org/bugzilla/show_bug.cgi?id=48097
-  1) Code cleanup: Remove use of WebappClassLoader$PrivilegedFindResource,
-  because all findResourceInternal(String,String) calls are now already
-  wrapped with AccessController.doPrivileged, so additional wrapping is not
-  needed.
-  2) Add preloading of WebappClassLoader$PrivilegedFindResourceByName,
-  to fix https://issues.apache.org/bugzilla/show_bug.cgi?id=48097#c13
-  http://people.apache.org/~kkolinko/patches/2009-11-12_PrivilegedFindResource_tc6.patch
-  +1: kkolinko, markt, jim
-  -1:
-
 * Update to commons-pool 1.5.4
   http://svn.apache.org/viewvc?rev=881412&view=rev
   +1: markt, jim
@@ -420,7 +409,6 @@
   http://svn.apache.org/viewvc?rev=891185&view=rev (better log messages)
   +1: markt
   -1: 
-
   
 * Add support for a connectionTimeout attribute to the JNDIRealm
   http://svn.apache.org/viewvc?rev=891186&view=rev

Modified: tomcat/tc6.0.x/trunk/java/org/apache/catalina/loader/WebappClassLoader.java
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/catalina/loader/WebappClassLoader.java?rev=891310&r1=891309&r2=891310&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/catalina/loader/WebappClassLoader.java (original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/catalina/loader/WebappClassLoader.java Wed Dec 16 16:56:22 2009
@@ -111,7 +111,10 @@
 
     public static final boolean ENABLE_CLEAR_REFERENCES = 
         Boolean.valueOf(System.getProperty("org.apache.catalina.loader.WebappClassLoader.ENABLE_CLEAR_REFERENCES", "true")).booleanValue();
-    
+
+    /**
+     * @deprecated Not used
+     */
     protected class PrivilegedFindResource
         implements PrivilegedAction {
 
@@ -1977,8 +1980,7 @@
     }
 
     /**
-     * Find specified resource in local repositories. This block
-     * will execute under an AccessControl.doPrivilege block.
+     * Find specified resource in local repositories.
      *
      * @return the loaded resource, or null if the resource isn't found
      */
@@ -2037,13 +2039,7 @@
 
                 // Note : Not getting an exception here means the resource was
                 // found
-                 if (securityManager != null) {
-                    PrivilegedAction dp =
-                        new PrivilegedFindResource(files[i], path);
-                    entry = (ResourceEntry)AccessController.doPrivileged(dp);
-                 } else {
-                    entry = findResourceInternal(files[i], path);
-                 }
+                entry = findResourceInternal(files[i], path);
 
                 ResourceAttributes attributes =
                     (ResourceAttributes) resources.getAttributes(fullPath);

Modified: tomcat/tc6.0.x/trunk/java/org/apache/catalina/security/SecurityClassLoad.java
URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/catalina/security/SecurityClassLoad.java?rev=891310&r1=891309&r2=891310&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/catalina/security/SecurityClassLoad.java (original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/catalina/security/SecurityClassLoad.java Wed Dec 16 16:56:22 2009
@@ -77,7 +77,7 @@
         String basePackage = "org.apache.catalina.";
         loader.loadClass
             (basePackage +
-             "loader.WebappClassLoader$PrivilegedFindResource");
+             "loader.WebappClassLoader$PrivilegedFindResourceByName");
     }
     
     



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


Re: svn commit: r891310 - in /tomcat/tc6.0.x/trunk: STATUS.txt java/org/apache/catalina/loader/WebappClassLoader.java java/org/apache/catalina/security/SecurityClassLoad.java

Posted by Konstantin Kolinko <kn...@gmail.com>.
2009/12/16 sebb <se...@gmail.com>:
> On 16/12/2009, markt@apache.org <ma...@apache.org> wrote:
>> Author: markt
>>  Date: Wed Dec 16 16:56:22 2009
>>  New Revision: 891310
>>
>>  URL: http://svn.apache.org/viewvc?rev=891310&view=rev
>
>>  +
>>  +    /**
>>  +     * @deprecated Not used
>>  +     */
>
> Doesn't this need the annotation:
>
>       @Deprecated ?
>
>>      protected class PrivilegedFindResource
>>          implements PrivilegedAction {
>>

I think that nobody cares.

In TC 6 there are 6 occurrences of @Deprecated and 150+ of /* @deprecated */.

When preparing the patch, that Mark applied, I went for the most
frequent option. I do not think that this inner class needs any
emphasis, as it is not some public API.

If anyone wants to add @Deprecated there, here is my +1 for that, but
I do not really care.

Best regards,
Konstantin Kolinko

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


Re: svn commit: r891310 - in /tomcat/tc6.0.x/trunk: STATUS.txt java/org/apache/catalina/loader/WebappClassLoader.java java/org/apache/catalina/security/SecurityClassLoad.java

Posted by sebb <se...@gmail.com>.
On 16/12/2009, markt@apache.org <ma...@apache.org> wrote:
> Author: markt
>  Date: Wed Dec 16 16:56:22 2009
>  New Revision: 891310
>
>  URL: http://svn.apache.org/viewvc?rev=891310&view=rev

<...>

>  --- tomcat/tc6.0.x/trunk/java/org/apache/catalina/loader/WebappClassLoader.java (original)
>  +++ tomcat/tc6.0.x/trunk/java/org/apache/catalina/loader/WebappClassLoader.java Wed Dec 16 16:56:22 2009
>  @@ -111,7 +111,10 @@
>
>      public static final boolean ENABLE_CLEAR_REFERENCES =
>          Boolean.valueOf(System.getProperty("org.apache.catalina.loader.WebappClassLoader.ENABLE_CLEAR_REFERENCES", "true")).booleanValue();
>  -
>  +
>  +    /**
>  +     * @deprecated Not used
>  +     */

Doesn't this need the annotation:

       @Deprecated ?

>      protected class PrivilegedFindResource
>          implements PrivilegedAction {
>

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