Bug 34844

Created attachment 16701 [details]
Propose URL decoding of stickysession route

I suspect current httpd-2.2.x branch mod_proxy_balancer stickysession doesn't
work _at all_ when using cookie-based stickysessions, at least not when the
cookie value is URL encoded (as it was in my case).  Found this by load
balancing between two servers with different content, and I was sent back and
forth between them; definitely not sticky.

I got Found value <sessionid> for stickysession <stickysession>, but _not_ any
Found route <route> when looking at the debug log.

The <sessionid> was logged as "%2EW1" and not ".W1", though.  It seems as if
mod_proxy_balancer.c checks against '.', but doesn't URL decode the string
first (if it had, it shouldn't have logged it encoded):

    ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
			    "proxy: BALANCER: Found value %s for "
			    "stickysession %s", *route, balancer->sticky);
    /*
     * If we found a value for sticksession, find the first '.' within.
     * Everything after '.' (if present) is our route.
     */
    if ((*route) && ((*route = strchr(*route, '.')) != NULL ))
	(*route)++;
    if ((*route) && (**route)) {
	ap_log_error(APLOG_MARK, APLOG_DEBUG, 0, r->server,
				  "proxy: BALANCER: Found route %s", *route);

(Note that the backend server is running IIS.  I'm assuming it's doing the
right thing of encoding the cookie values; I'm not doing that explicitly.)

The attached patch seems to fix this in my case, and something like this should
probably be included in mod_proxy_balancer.c.

Note: I'm not sure if this is the proper Apache way of doing things.  I
borrowed decodeenc() from mod_proxy_ftp.c, so it should probably be refactored
into a common function, if it's not already in APR (which I guess it may be). 
Also, I had to change from (void)decodeenc(route) to (void)decodeenc(*route) -
not sure if that's a problem in mod_proxy_ftp.c, didn't check that closely.

Created attachment 16702 [details]
Proposal for url decoding cookie-based stickysession routes

This version hopefully also works properly for new sessions, and only tries to
decode the cookie value.

(In reply to comment #1)
> 
> The <sessionid> was logged as "%2EW1" and not ".W1", though.  It seems as if
> mod_proxy_balancer.c checks against '.', but doesn't URL decode the string
> first (if it had, it shouldn't have logged it encoded):

I am currently unsure if it is ok to expect that the '.' in the sessioncookie
that separates sessionid and route can be URL encoded. So lets see the opinion
of other developers on this topic. I tend to say: No.
Nevertheless, if it is found ok to be URL encoded, I think the following patch
would be better:

Index: modules/proxy/mod_proxy_balancer.c
===================================================================
--- modules/proxy/mod_proxy_balancer.c  (Revision 320868)
+++ modules/proxy/mod_proxy_balancer.c  (Arbeitskopie)
@@ -158,6 +158,7 @@
                         *end_cookie = '\0';
                     if((end_cookie = strchr(cookie, ',')) != NULL)
                         *end_cookie = '\0';
+                    ap_unescape_url_keep2f(cookie);
                     return cookie;
                 }
             }

(In reply to comment #0)

> the session moves back across to this server, not staying on the new server as
> would be expected. This leads to the client / backend server session being
> interupted twice, not once as would be expected.

I do not regard this as a failure of httpd. If you get a new sessioncookie when
you fall over to the second server, then this cookie should contain a correct
route to the second server and everything is fine (no fallback back once the
first server recovered). If the second server delivers the same routing
information with the new sessioncookie as the first server then either

- first and second server should have an internal clustering where it does
  not matter which one gets the session

- your backend configuration is wrong

(In reply to comment #3)
> I am currently unsure if it is ok to expect that the '.' in the sessioncookie
> that separates sessionid and route can be URL encoded. So lets see the opinion
> of other developers on this topic. I tend to say: No.

You're probably right, even if it means trouble for me :)  I'm not sure if this is a bug in ASP 3.0, but 
it seems that MS switched to _not_ encoding cookies in ASP.NET -- http://support.microsoft.com/
default.aspx?scid=kb;en-us;313282.  (Also, even when encoding chances are . should not have 
been touched in the first place.)

What's the best approach then?  Just keep this a local modification as long as one has ASP 3.0 
backends?