Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=53830
Better handling of Manager.randomFile default value on Windows

git-svn-id: https://svn.apache.org/repos/asf/tomcat/tc5.5.x/trunk@1392256 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/STATUS.txt b/STATUS.txt
index bae7a2f..c2e842e 100644
--- a/STATUS.txt
+++ b/STATUS.txt
@@ -27,9 +27,3 @@
 
 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:
diff --git a/container/catalina/src/share/org/apache/catalina/session/ManagerBase.java b/container/catalina/src/share/org/apache/catalina/session/ManagerBase.java
index 97e6261..9183125 100644
--- a/container/catalina/src/share/org/apache/catalina/session/ManagerBase.java
+++ b/container/catalina/src/share/org/apache/catalina/session/ManagerBase.java
@@ -67,8 +67,20 @@
 
     // ----------------------------------------------------- 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 @@
 
 
     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 @@
         // 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);
+                }
             }
         }
     }
diff --git a/container/webapps/docs/changelog.xml b/container/webapps/docs/changelog.xml
index fc9f848..9ccb0f3 100644
--- a/container/webapps/docs/changelog.xml
+++ b/container/webapps/docs/changelog.xml
@@ -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">
diff --git a/container/webapps/docs/config/manager.xml b/container/webapps/docs/config/manager.xml
index 837bf1c..0238abe 100644
--- a/container/webapps/docs/config/manager.xml
+++ b/container/webapps/docs/config/manager.xml
@@ -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,