httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Guernsey, Byron \(GE Consumer & Industrial\)" <BYRON.GUERN...@GE.COM>
Subject RE: PATCH - cookie expire #28391 (Was E: I'd like to make some contributions)
Date Thu, 17 Jun 2004 15:49:14 GMT
Thanks for the advice.  I'm waiting patiently to see if the 'bugid'  (#28391) gets assigned
at some point.  I realize its a feature patch and not an important bug fix, which is why I
labeled it as normal priority and have not been persistently nagging.  The patch that was
posted to #28391 and the patch I'm now using differ, so I likely need to update the report
with the newer, more elegant, patch.
 
I did post a followup message with the newer patch, hoping to raise some discussion about
the patch and the CO flag in general, but there were no takers at that time.
 
One general question is how to properly extend the CO flag parameters such that either expires
or the path are optional- because its valid to not set a expire time (which implies it lives
as long as the brower is open) and its valid to not set a domain (which means the browser
applies the current domain)  I changed the patch to look for null in the parameter list and
leave the expire time or domain as NULL if found.  One of the load balancing rewrite rules
look something like (this is not a complete implementation by itself):
 
<Location /ACE>
    Options FollowSymLinks
    RewriteEngine On
    RewriteCond %{HTTP_COOKIE}                  (^|;\s*)GEBalance=(\w+)($|;)
    RewriteRule ^.*                             "http://${SERVERS:%2}%{REQUEST_URI}"  [NS,env=GETEMP:blah,P,L]
    RewriteCond %{HTTP_COOKIE}                  !(^|;)\s*GEBalance=\w+($|;)
    RewriteRule ^.*                             - [env=GETEMP:${PERLMAP:ACE}]
    RewriteCond %{HTTP_COOKIE}                  !(^|;)\s*GEBalance=\w+($|;)

    RewriteRule ^.*                             - [CO=GEBalance:%{ENV:GETEMP}:null:null:/ACE]
    RewriteCond %{HTTP_COOKIE}                  !(^|;)\s*GEBalance=\w+($|;)
    RewriteRule ^.*                             "http://${SERVERS:%{ENV:GETEMP}}%{REQUEST_URI}"
 [P,L]
</Location>
 
A quick description for those interested- if the cookie is set, it puts the cookie value into
%2 and Proxies the request to a Server coming out of a perl map based upon the cookie value.
 If the cookie is not set in the request, it sets the environment variable GETEMP to a randomly
selected value from another perl map and then puts that value into the Cookie and then proxies
the request to that server.  Some other logic handles failure with a ErrorDocument perl script
that flags entries in the maps as unavailable and redirects to get a new cookie.  There is
also some recursion fallout code that fails out when all of the available servers fail, and
there are some scripts that generate all of the rewrite rules from enhydra_director.conf files.
Voila! Enhydra version 3 app server support in Apache 2 without the Enhydra Director Module.
 This same solution could work for web server sticky load balancing and failover- but it requires
session cookies. 
 
The CO implementation above remains consistent with the old CO flag parameter syntax - but
I can't help but feel that hard coded fields are bad and that the parameters should have some
kind of descriptor- ie:
 
[CO=name;GEBalance:value;%{ENV:GETEMP}:path;/ACE]
 
where domain and expires are not specified at all and some field separator is used.  But this
changes the syntax described in the apache 2 documentation and I'm assuming that a goal for
the developers is to maintain the same interfaces once they are established.  My current patch,
which checks for "null", accomplishes that goal, but its an ugly interface.
 
Byron

	-----Original Message----- 
	From: Dirk-Willem van Gulik [mailto:dirkx@webweaving.org] 
	Sent: Thu 6/17/2004 10:21 AM 
	To: dev@httpd.apache.org 
	Cc: 
	Subject: PATCH - cookie expire (Was E: I'd like to make some contributions)
	
	



	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,
	

Mime
View raw message