PATCH - cookie expire (Was E: I'd like to make some contributions)

[Dirk-Willem van Gulik <di...@webweaving.org>]

On Thu, 17 Jun 2004, Guernsey, Byron (GE Consumer & Industrial) wrote:

> Don't expect a quick response.  I submitted a feature patch to bugzilla
> over a month ago and it hasn't changed state from its "new" state or had

Of course it would help if you posted the number of the bug. Assuming it
is 28391 (http://nagoya.apache.org/bugzilla/show_bug.cgi?id=28391) then
keep prodding once in a while. It has merit. You can even reduce the
load/make it more attractive by adding a docs patch too :-). But then
again - be aware that there are countless features, lots of special needs
and just a few developers.

Dw.

--- mod_rewrite.c.orig	Wed Apr 14 14:15:33 2004
+++ mod_rewrite.c	Wed Apr 14 14:22:09 2004
@@ -4357,6 +4357,19 @@
             path = NULL;
         }

+	// Treat 0 or empty string in expires (ie: "::")  as session based
expire (use -1 to expire a cookie immediately)
+	if(expires && atol(expires) == 0) {
+	  // Below will handle the case where timeout is "::" in the
+	  // rewrite rule and the path is mistakenly loaded into expires
by strtok
+	  // it will fail if the path portion is a directory that starts
+          // with a numeric digit and no preceeding / (it should always
be a / for a cookie path)
+	  // This should be unicode/ebdic safe...
+	  if(path == NULL && !apr_isdigit(*expires)) {
+		path = expires;
+		expires = NULL;
+	  }
+	}
+
         if (var && val && domain) {
             /* FIX: use cached time similar to how logging does it */
             request_rec *rmain = r;


The cookie flag with mod_rewrite does not allow you to expire a cookie
when the
browser closes if you also need to specify a path.  For example:

RewriteRule (.*)   -   [CO=MyCookie:value::/somepath]

Actually results in /somepath being copied into the variable expires in
mod_rewrite.c:addcookie() and path being set to NULL.  This results in
expires
being set to 0, a cookie being generated for the current time/date, and
the path
being set to NULL, which seems like a bug.

likewise, the following line will result in a cookie being set that
expires
immediately:

RewriteRule (.*)   -   [CO=MyCookie:value:0:/somepath]

I've written a patch that allows 0 or "::" to indicate that the cookie is
valid
for the current session.  The only time I can imagine that a user would
want to
set a cookie to "now+0" is if he is trying to expire an existing cookie,
and in
that case "now+(-1)" will work much better, and I believe this is
supported by
specifying -1 in the CO flag field for the expire time.

Basically, I needed to set a cookie that is for a particular path and is
good
only while the browser is open.  So my patch checks for the improper
tokenization of the path into the expires field and moves the path into
the
proper variable and reassigns expires to NULL.  If determines this by
testing to
see if expires != NULL, atol(expires)==0, path==NULL and
!apr_isdigit(*expires).
 The only case this logic could fail is if the path is specified as a
number as
the first digit, but a cookie path must begin with a / to be well formed.

See the patch which I will attach. This is against 2.0.49.  I hope that
this can
be included in a future revision, or the concept of the patch can be
accomplished.

Thanks,