Author: markt
Date: Fri Oct  5 11:44:10 2012
New Revision: 1394455

URL: http://svn.apache.org/viewvc?rev=1394455&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=53830
Better handling of Manager.randomFile default value on Windows

Modified:
    tomcat/tc6.0.x/trunk/STATUS.txt
    tomcat/tc6.0.x/trunk/java/org/apache/catalina/session/ManagerBase.java
    tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml
    tomcat/tc6.0.x/trunk/webapps/docs/config/manager.xml

Modified: tomcat/tc6.0.x/trunk/STATUS.txt
URL: 
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=1394455&r1=1394454&r2=1394455&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/STATUS.txt (original)
+++ tomcat/tc6.0.x/trunk/STATUS.txt Fri Oct  5 11:44:10 2012
@@ -92,12 +92,6 @@ PATCHES PROPOSED TO BACKPORT:
         requires an update to tcnative:
         https://issues.apache.org/bugzilla/show_bug.cgi?id=53969
 
-* Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=53830
-  Better handling of Manager.randomFile default value on Windows
-  https://issues.apache.org/bugzilla/attachment.cgi?id=29330
-  +1: kkolinko, schultz, markt
-  -1:
-
 * Improve session management in CsrfPreventionFilter
   (Backport of r1393071 from Tomcat 7)
   
http://people.apache.org/~kkolinko/patches/2012-10-03_tc6_CsrfPreventionFilter.patch

Modified: tomcat/tc6.0.x/trunk/java/org/apache/catalina/session/ManagerBase.java
URL: 
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/catalina/session/ManagerBase.java?rev=1394455&r1=1394454&r2=1394455&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/java/org/apache/catalina/session/ManagerBase.java 
(original)
+++ tomcat/tc6.0.x/trunk/java/org/apache/catalina/session/ManagerBase.java Fri 
Oct  5 11:44:10 2012
@@ -75,8 +75,20 @@ public abstract class ManagerBase implem
 
     // ----------------------------------------------------- Instance Variables
 
+    private static final String devRandomSourceDefault;
+    static {
+        // - Use the default value only if it is a Unix-like system
+        // - Check that it exists 
+        File f = new File("/dev/urandom");
+        if (f.isAbsolute() && f.exists()) {
+            devRandomSourceDefault = f.getPath();
+        } else {
+            devRandomSourceDefault = null;
+        }
+    }
+
     protected DataInputStream randomIS=null;
-    protected String devRandomSource="/dev/urandom";
+    protected String devRandomSource = devRandomSourceDefault;
 
     /**
      * The default message digest algorithm to use if we cannot use
@@ -238,34 +250,17 @@ public abstract class ManagerBase implem
 
 
     private class PrivilegedSetRandomFile
-            implements PrivilegedAction<DataInputStream>{
-        
+            implements PrivilegedAction<Void>{
+
+        private final String s;
+
         public PrivilegedSetRandomFile(String s) {
-            devRandomSource = s;
+            this.s = s;
         }
-        
-        public DataInputStream run(){
-            try {
-                File f=new File( devRandomSource );
-                if( ! f.exists() ) return null;
-                randomIS= new DataInputStream( new FileInputStream(f));
-                randomIS.readLong();
-                if( log.isDebugEnabled() )
-                    log.debug( "Opening " + devRandomSource );
-                return randomIS;
-            } catch (IOException ex){
-                log.warn("Error reading " + devRandomSource, ex);
-                if (randomIS != null) {
-                    try {
-                        randomIS.close();
-                    } catch (Exception e) {
-                        log.warn("Failed to close randomIS.");
-                    }
-                }
-                devRandomSource = null;
-                randomIS=null;
-                return null;
-            }
+
+        public Void run(){
+            doSetRandomFile(s);
+            return null;
         }
     }
 
@@ -544,27 +539,49 @@ public abstract class ManagerBase implem
         // as a hack, you can use a static file - and generate the same
         // session ids ( good for strange debugging )
         if (Globals.IS_SECURITY_ENABLED){
-            randomIS = AccessController.doPrivileged(new 
PrivilegedSetRandomFile(s));
+            AccessController.doPrivileged(new PrivilegedSetRandomFile(s));
         } else {
-            try{
-                devRandomSource=s;
-                File f=new File( devRandomSource );
-                if( ! f.exists() ) return;
-                randomIS= new DataInputStream( new FileInputStream(f));
-                randomIS.readLong();
-                if( log.isDebugEnabled() )
-                    log.debug( "Opening " + devRandomSource );
-            } catch( IOException ex ) {
-                log.warn("Error reading " + devRandomSource, ex);
-                if (randomIS != null) {
-                    try {
-                        randomIS.close();
-                    } catch (Exception e) {
-                        log.warn("Failed to close randomIS.");
-                    }
+            doSetRandomFile(s);
+        }
+    }
+
+    private void doSetRandomFile(String s) {
+        DataInputStream is = null;
+        try {
+            if (s == null || s.length() == 0) {
+                return;
+            }
+            File f = new File(s);
+            if( ! f.exists() ) return;
+            if( log.isDebugEnabled() ) {
+                log.debug( "Opening " + s );
+            }
+            is = new DataInputStream( new FileInputStream(f));
+            is.readLong();
+        } catch( IOException ex ) {
+            log.warn("Error reading " + s, ex);
+            if (is != null) {
+                try {
+                    is.close();
+                } catch (Exception ex2) {
+                    log.warn("Failed to close " + s, ex2);
                 }
+                is = null;
+            }
+        } finally {
+            DataInputStream oldIS = randomIS;
+            if (is != null) {
+                devRandomSource = s;
+            } else {
                 devRandomSource = null;
-                randomIS=null;
+            }
+            randomIS = is;
+            if (oldIS != null) {
+                try {
+                    oldIS.close();
+                } catch (Exception ex) {
+                    log.warn("Failed to close RandomIS", ex);
+                }
             }
         }
     }

Modified: tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml
URL: 
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml?rev=1394455&r1=1394454&r2=1394455&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml Fri Oct  5 11:44:10 2012
@@ -210,6 +210,10 @@
         correct paths for subdirectories. Patch provided by Kevin Wooten.
         (kkolinko)
       </fix>
+      <fix>
+        <bug>53830</bug>: Better handling of <code>Manager.randomFile</code>
+        default value on Windows. (kkolinko)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Coyote">

Modified: tomcat/tc6.0.x/trunk/webapps/docs/config/manager.xml
URL: 
http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/webapps/docs/config/manager.xml?rev=1394455&r1=1394454&r2=1394455&view=diff
==============================================================================
--- tomcat/tc6.0.x/trunk/webapps/docs/config/manager.xml (original)
+++ tomcat/tc6.0.x/trunk/webapps/docs/config/manager.xml Fri Oct  5 11:44:10 
2012
@@ -157,6 +157,13 @@
         <code>java.security.SecureRandom</code>.</p>
       </attribute>
 
+      <attribute name="randomFile" required="false">
+        <p>Name of a file that provides random data that are used to generate
+        session ids. If not specified, the default value is
+        <code>/dev/urandom</code> on Unix-like systems (those where such
+        file path is absolute) and empty on others.</p>
+      </attribute>
+
       <attribute name="sessionIdLength" required="false">
        <p>The length of session ids created by this Manager, measured in bytes,
         excluding subsequent conversion to a hexadecimal string and
@@ -265,6 +272,13 @@
         <code>java.security.SecureRandom</code>.</p>
       </attribute>
 
+      <attribute name="randomFile" required="false">
+        <p>Name of a file that provides random data that are used to generate
+        session ids. If not specified, the default value is
+        <code>/dev/urandom</code> on Unix-like systems (those where such
+        file path is absolute) and empty on others.</p>
+      </attribute>
+
       <attribute name="saveOnRestart" required="false">
         <p>Should all sessions be persisted and reloaded when Tomcat is shut
         down and restarted (or when this application is reloaded)?  By default,



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

Reply via email to