Author: markt
Date: Mon Oct  1 09:50:57 2012
New Revision: 1392256

URL: http://svn.apache.org/viewvc?rev=1392256&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/tc5.5.x/trunk/STATUS.txt
    
tomcat/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/session/ManagerBase.java
    tomcat/tc5.5.x/trunk/container/webapps/docs/changelog.xml
    tomcat/tc5.5.x/trunk/container/webapps/docs/config/manager.xml

Modified: tomcat/tc5.5.x/trunk/STATUS.txt
URL: 
http://svn.apache.org/viewvc/tomcat/tc5.5.x/trunk/STATUS.txt?rev=1392256&r1=1392255&r2=1392256&view=diff
==============================================================================
--- tomcat/tc5.5.x/trunk/STATUS.txt (original)
+++ tomcat/tc5.5.x/trunk/STATUS.txt Mon Oct  1 09:50:57 2012
@@ -27,9 +27,3 @@ PATCHES ACCEPTED TO BACKPORT FROM TRUNK/
 
 PATCHES PROPOSED TO BACKPORT:
   [ New proposals should be added at the end of the list ]
-
-* 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=29331
-  +1: kkolinko, markt, kfujino
-  -1:

Modified: 
tomcat/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/session/ManagerBase.java
URL: 
http://svn.apache.org/viewvc/tomcat/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/session/ManagerBase.java?rev=1392256&r1=1392255&r2=1392256&view=diff
==============================================================================
--- 
tomcat/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/session/ManagerBase.java
 (original)
+++ 
tomcat/tc5.5.x/trunk/container/catalina/src/share/org/apache/catalina/session/ManagerBase.java
 Mon Oct  1 09:50:57 2012
@@ -67,8 +67,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
@@ -220,32 +232,16 @@ public abstract class ManagerBase implem
 
 
     private class PrivilegedSetRandomFile implements PrivilegedAction{
+
+        private final String s;
+
         public PrivilegedSetRandomFile(String s) {
-            devRandomSource = s;
+            this.s = s;
         }
 
         public Object 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;
-            }
+            doSetRandomFile(s);
+            return null;
         }
     }
 
@@ -524,28 +520,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 (System.getSecurityManager() != null){
-            randomIS = (DataInputStream) 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/tc5.5.x/trunk/container/webapps/docs/changelog.xml
URL: 
http://svn.apache.org/viewvc/tomcat/tc5.5.x/trunk/container/webapps/docs/changelog.xml?rev=1392256&r1=1392255&r2=1392256&view=diff
==============================================================================
--- tomcat/tc5.5.x/trunk/container/webapps/docs/changelog.xml (original)
+++ tomcat/tc5.5.x/trunk/container/webapps/docs/changelog.xml Mon Oct  1 
09:50:57 2012
@@ -71,6 +71,10 @@
       <scode>
         Remove unneeded handling of FORM authentication in RealmBase. 
(kkolinko)
       </scode>
+      <fix>
+        <bug>53830</bug>: Better handling of Manager.randomFile default value 
on
+        Windows. (kkolinko)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Coyote">

Modified: tomcat/tc5.5.x/trunk/container/webapps/docs/config/manager.xml
URL: 
http://svn.apache.org/viewvc/tomcat/tc5.5.x/trunk/container/webapps/docs/config/manager.xml?rev=1392256&r1=1392255&r2=1392256&view=diff
==============================================================================
--- tomcat/tc5.5.x/trunk/container/webapps/docs/config/manager.xml (original)
+++ tomcat/tc5.5.x/trunk/container/webapps/docs/config/manager.xml Mon Oct  1 
09:50:57 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, excluding any
         JVM route information used for load balancing. 
@@ -263,6 +270,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