Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
Message-ID: <001101c2dd13$1e0c39b0$0200a8c0@FamHolthaus>
From: "Sander Holthaus - Orange XL" <info@orangexl.com>
To: <dev@httpd.apache.org>
References: <F8DE719985C66B489BFB893864C4F51D7CB413@EXBOS2.digitas.com>
Subject: Re: mod_usertrack bugfix patch
Date: Tue, 25 Feb 2003 22:16:09 +0100
Organization: Orange XL
MIME-Version: 1.0
Content-Type: text/plain;
	charset="iso-8859-1"
Content-Transfer-Encoding: 7bit

As a PERL-programmer, I cannot help you. What I can tell you is that your
patch does not fully comply with RFC 2109. The seperator for cookies is ;
BUT , should also be reserved for this purpose. The regex should include the
, as well as the ; as seperators.

Kind Regards,
Sander Holthaus

----- Original Message -----
From: "Manni Wood" <mwood@digitas.com>
To: <dev@httpd.apache.org>; <trawick@attglobal.net>
Sent: Tuesday, February 25, 2003 9:13 PM
Subject: RE: mod_usertrack bugfix patch


I suppose I should have put more details at
http://www.manniwood.net/mod_usertrack_patch.html about my results running a
patched vs. unpatched version of my code through a JMeter test, but as my
web page says, the patch adds a couple of milliseconds to the request time.
Is there any standard way the Apache group tests new code for performance? I
will happily run more tests for you if you'd like.

The patch that I sent is the best balance I could find between correctness
and speed. (The current mod_usertrack is fast but incorrect, unfortunately.)
Remember that in my patch, I compile the regexp only once (at config time),
so running the compiled regexp on the cookie header is really quite fast.

As a matter of fact, the use of regexps in mod_alias is what inspired me to
use a regexp in my mod_usertrack patch. I figured if the performance hit of
mod_alias was deemed worthwhile in the pursuit of correctness, a corrected
mod_usertrack would also be permitted the same indulgence. (But I *am* new
to this, so is my assumption incorrect?)

Finally, the use of strstr to find the cookie name and substr to find the
cookie value is, unfortunately, what leads to the bug in the current code. A
more refined use of strstr might look for the cookiename proceeded by a
space and followed by the '=' sign. However there's no reason why the space
separating cookienames can't actually be a tab, as the RFC only calls for
"whitespace" between cookie/value pairs. Also, the whitespace is not going
to be present if the cookiename happens to be the first cookie in the
header, so one would have to handle that exception. Finally, one has to deal
with the fact that the cookiename could actually have a space in it, as long
as it is in quotation marks, so that would break the algorithm I just
discussed. In the end, as with mod_alias, a regexp starts to make a whole
lot of sense for mod_usertrack instead of coding a baroque usage of strstr
and substr to take care of all these exceptions.

What do you think?

As I said, I'm willing to run more tests and give you numbers, if you'd like
(and encourage you to do the same, naturally). Is there a standard way the
Apache Group runs tests on new code to see if it meets performance
guidelines? I'm sure the performance hit won't be any worse than for
mod_alias.

-Manni

--------------------------------------------
Manni Wood, Programmer, Digitas
800 Boylston Street, Boston, MA, 02199
617 867 1881 mwood@digitas.com

"Most men would rather die than think. Many do."    --Bertrand Russell


-----Original Message-----
From: Jeff Trawick [mailto:trawick@attglobal.net]
Sent: Tuesday, February 25, 2003 6:29 AM
To: dev@httpd.apache.org
Subject: Re: mod_usertrack bugfix patch


Manni Wood wrote:

> I am submitting a patch to mod_usertrack for both Apache 2.0 and 1.3
> for your consideration.

> The patch fixes a bug where the use of strstr() to find the name of
> the cookie in the cookieheader can accidentally "find" the name of the
> cookie in what is actually the contents of a cookie if the contents
> happen to contain the name of the user tracking cookie.
>
> The patch relies on a robust regexp to find the cookiename in the
> header instead of strstr().

performance concern a.k.a. dumb question...  is a regexp required for
fixing this problem?

> More details are at http://www.manniwood.net/mod_usertrack_patch.html.


excellent resource...  I applaud you for taking the initiative to deal
with this so carefully