About 1 in 200 sessions are *broken* (Tomcat 3.1)

[Tomas Rokicki <ro...@instantis.com>]

There are two substantial bugs in Tomcat 3.0 and Tomcat 3.1
that will in effect break about 1 in every 1000 sessions.
This has been reported by our users and finally debugged by
Joel Fowler and I.

Bug 1 is that the session IDs that Tomcat generates are broken
(contain a hyphen) about one in 1000 times.

Bug 2 is that Tomcat does not support double-quoted cookies
for session IDs.

The first bug is in org.apache.tomcat.util.SessionIdGenerator,
which code contains (approximately):

  String dString = Double.toString(Math.abs(Math.random()));
  ...
  buf.append(dString.substring(2, dString.length()));

so normally this creates something like 0.31431 which gets
converted into buf.append("31431") which is kosher.

But about 1/1000 times the value returned is < 10^-3 (see Java
Lang Spec), in which case it is converted to a string in
scientific notation, and thus contains something like
3.1431e-4.

And there is the possibility that Math.random() will return
a simple "0" in which case the String.subString() call will
throw an StringIndexOutOfBoundsException.

[I also question the need for Math.abs.]

This is kludgey code, and the simplest fix is probably to
use

  String dString = Double.toString(Math.abs(1+Math.random())) + "00";

The 1+ forces the result to be >= 1 and < 2, so the toString
part will work, and to avoid the exception for the case when
an exact 1 is returned we just stuff a 00 on the end.

The second bug is in org.apache.tomcat.util.RequestUtil:

               int i = token.indexOf("=");
                if (i > -1) {

                    // XXX
                    // the trims here are a *hack* -- this should
                    // be more properly fixed to be spec compliant

                    String name = token.substring(0, i).trim();
                    String value = token.substring(i+1,
token.length()).trim();
                    Cookie cookie = new Cookie(name, value);
                    cookies.addElement(cookie);

Here we don't take into account the possibility that the cookie
might be surrounded by double quotes as allowed in RFC 2109.
A 90% fix is to just add right after String value = the line

   if (value.length() > 1 && value.charAt(0) == '"' &&
       value.charAt(value.length()-1) == '"')
      value = value.substring(1, value.length()-1) ;

Careful consideration should be given with respect to that RFC
and others, and implementations, with regard to cookie values
that contain semicolons, equal signs, and double quotes, and
how these values may or may not be escaped.

I am sorry for not posting these bugs and fixes to the bugtracker
(or even looking them up in the bugtracker) but for some reason I
get a `not permitted to access that page' error.

I cannot keep up with the volume on this list in general, so
feel free to cc me directly on any responses or requests for
additional comments.

-tom