Using SecureRandom makes digesting the ID unnecessary. Dropping the digest gives...

git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1039648 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/java/org/apache/catalina/session/ManagerBase.java b/java/org/apache/catalina/session/ManagerBase.java
index b523c43..779ffe6 100644 (file)
--- a/java/org/apache/catalina/session/ManagerBase.java
+++ b/java/org/apache/catalina/session/ManagerBase.java
@@ -30,8 +30,6 @@ import java.io.IOException;
 import java.io.InputStream;
 import java.lang.reflect.Method;
 import java.security.AccessController;
-import java.security.MessageDigest;
-import java.security.NoSuchAlgorithmException;
 import java.security.PrivilegedAction;
 import java.security.SecureRandom;
 import java.util.ArrayList;
@@ -88,38 +86,12 @@ public abstract class ManagerBase extends LifecycleMBeanBase
     protected volatile boolean randomFileCurrentIsValid = true;
 
     /**
-     * The default message digest algorithm to use if we cannot use
-     * the requested one.
-     */
-    protected static final String DEFAULT_ALGORITHM = "MD5";
-
-
-    /**
-     * The message digest algorithm to be used when generating session
-     * identifiers.  This must be an algorithm supported by the
-     * <code>java.security.MessageDigest</code> class on your platform.
-     */
-    protected String algorithm = DEFAULT_ALGORITHM;
-
-
-    /**
      * The Container with which this Manager is associated.
      */
     protected Container container;
 
 
     /**
-     * Queue of MessageDigest objects to be used when creating session
-     * identifiers. If the queue is empty when a MessageDigest is required, a
-     * new MessageDigest object is created. This is designed this way since
-     * MessageDigest objects are not thread-safe and sync'ing on a single object
-     * is slow(er).
-     */
-    protected Queue<MessageDigest> digests =
-        new ConcurrentLinkedQueue<MessageDigest>();
-
-
-    /**
      * The distributable flag for Sessions created by this Manager.  If this
      * flag is set to <code>true</code>, any user attributes added to a
      * session controlled by this Manager must be Serializable.
@@ -293,30 +265,6 @@ public abstract class ManagerBase extends LifecycleMBeanBase
     // ------------------------------------------------------------- Properties
 
     /**
-     * Return the message digest algorithm for this Manager.
-     */
-    public String getAlgorithm() {
-
-        return (this.algorithm);
-
-    }
-
-
-    /**
-     * Set the message digest algorithm for this Manager.
-     *
-     * @param algorithm The new message digest algorithm
-     */
-    public void setAlgorithm(String algorithm) {
-
-        String oldAlgorithm = this.algorithm;
-        this.algorithm = algorithm;
-        support.firePropertyChange("algorithm", oldAlgorithm, this.algorithm);
-
-    }
-
-
-    /**
      * Return the Container with which this Manager is associated.
      */
     @Override
@@ -361,40 +309,6 @@ public abstract class ManagerBase extends LifecycleMBeanBase
 
 
     /**
-     * Return the MessageDigest object to be used for calculating
-     * session identifiers.  If none has been created yet, initialize
-     * one the first time this method is called.
-     */
-    protected MessageDigest createDigest() {
-
-        MessageDigest result;
-        
-        long t1=System.currentTimeMillis();
-        if (log.isDebugEnabled())
-            log.debug(sm.getString("managerBase.getting", algorithm));
-        try {
-            result = MessageDigest.getInstance(algorithm);
-        } catch (NoSuchAlgorithmException e) {
-            log.error(sm.getString("managerBase.digest", algorithm), e);
-            try {
-                result = MessageDigest.getInstance(DEFAULT_ALGORITHM);
-            } catch (NoSuchAlgorithmException f) {
-                log.error(sm.getString("managerBase.digest",
-                                 DEFAULT_ALGORITHM), e);
-                result = null;
-            }
-        }
-        if (log.isDebugEnabled())
-            log.debug(sm.getString("managerBase.gotten"));
-        long t2=System.currentTimeMillis();
-        if( log.isDebugEnabled() )
-            log.debug("getDigest() " + (t2-t1));
-
-        return result;
-    }
-
-
-    /**
      * Return the distributable flag for the sessions supported by
      * this Manager.
      */
@@ -1148,14 +1062,6 @@ public abstract class ManagerBase extends LifecycleMBeanBase
 
             while (resultLenBytes < this.sessionIdLength) {
                 getRandomBytes(random);
-                MessageDigest md = digests.poll();
-                if (md == null) {
-                    // If this fails, NPEs will follow. This should never fail
-                    // since if it falls back to the default digest
-                    md = createDigest();
-                }
-                random = md.digest(random);
-                digests.add(md);
                 for (int j = 0;
                 j < random.length && resultLenBytes < this.sessionIdLength;
                 j++) {

diff --git a/java/org/apache/catalina/session/mbeans-descriptors.xml b/java/org/apache/catalina/session/mbeans-descriptors.xml
index bbf4da1..447adad 100644 (file)
--- a/java/org/apache/catalina/session/mbeans-descriptors.xml
+++ b/java/org/apache/catalina/session/mbeans-descriptors.xml
@@ -28,11 +28,6 @@
                  type="int" 
             writeable="false"/>
 
-    <attribute   name="algorithm"
-          description="The message digest algorithm to be used when generating
-                       session identifiers"
-                 type="java.lang.String"/>
-
     <attribute   name="className"
           description="Fully qualified class name of the managed object"
                  type="java.lang.String"
@@ -226,11 +221,6 @@
                  type="int" 
             writeable="false"/>
 
-    <attribute   name="algorithm"
-          description="The message digest algorithm to be used when generating
-                       session identifiers"
-                 type="java.lang.String"/>
-
     <attribute   name="className"
           description="Fully qualified class name of the managed object"
                  type="java.lang.String"

diff --git a/test/org/apache/catalina/session/Benchmarks.java b/test/org/apache/catalina/session/Benchmarks.java
index 16a11cf..5396265 100644 (file)
--- a/test/org/apache/catalina/session/Benchmarks.java
+++ b/test/org/apache/catalina/session/Benchmarks.java
@@ -37,10 +37,10 @@ public class Benchmarks extends TestCase {
      * 16 threads - ~21,300ms
      * 
      * Results on markt's 2-core OSX dev box
-     *  1 thread  -   ~5,600ms
-     *  2 threads -  ~ 7,300ms
-     *  4 threads -  ~14,400ms
-     * 16 threads -  ~57,559ms
+     *  1 thread  -   ~4,700ms
+     *  2 threads -  ~ 6,000ms
+     *  4 threads -  ~11,900ms
+     * 16 threads -  ~48,659ms
      */
     public void testManagerBaseGenerateSessionId() throws Exception {
         doTestManagerBaseGenerateSessionId(1, 1000000);
@@ -105,8 +105,6 @@ public class Benchmarks extends TestCase {
         result.append(threadCount);
         result.append(", Time(ms): ");
         result.append(end-start);
-        result.append(", Digests: ");
-        result.append(mgr.digests.size());
         result.append(", Randoms: ");
         result.append(mgr.randoms.size());
         result.append(", RandomInputStreams: ");
@@ -203,8 +201,6 @@ public class Benchmarks extends TestCase {
         result.append(threadCount);
         result.append(", Time(ms): ");
         result.append(end-start);
-        result.append(", Digests: ");
-        result.append(mgr.digests.size());
         result.append(", Randoms: ");
         result.append(mgr.randoms.size());
         result.append(", RandomInputStreams: ");

diff --git a/webapps/docs/config/manager.xml b/webapps/docs/config/manager.xml
index 140db6b..4c354b1 100644 (file)
--- a/webapps/docs/config/manager.xml
+++ b/webapps/docs/config/manager.xml
@@ -99,13 +99,6 @@
 
     <attributes>
 
-      <attribute name="algorithm" required="false">
-        <p>Name of the <em>Message Digest</em> algorithm used to calculate
-        session identifiers produced by this Manager.  This value must
-        be supported by the <code>java.security.MessageDigest</code> class.
-        If not specified, the default value is "MD5".</p>
-      </attribute>
-
       <attribute name="entropy" required="false">
         <p>A String value that is utilized when seeding the random number
         generator used to create session identifiers for this Manager.