Martin, On 6/22/17 3:04 AM, Martin Grigorov wrote: > On Wed, Jun 21, 2017 at 9:05 PM, <schu...@apache.org> wrote: > >> + /** >> + * The request attribute key where the load-balancer's activation >> state >> + * can be found. >> + */ >> + static final String ATTRIBUTE_KEY_JK_LB_ACTIVATION = >> "JK_LB_ACTIVATION"; >> > > Any objection to make this constant public and visible from outside > ? I find it useful to be able to refer the constant by name than its > value when integrating.
No objections. It looks like I'll have a follow-on patch shortly, so I can change this at the same time. >> + // Kill any session cookie present >> + if(null != cookies) { >> + for(Cookie cookie : cookies) { >> + final String cookieName = cookie.getName(); >> + if(containerLog.isTraceEnabled()) >> + containerLog.trace("Checking cookie " + >> cookieName + "=" + cookie.getValue()); >> + >> + if(sessionCookieName.equals(cookieName) >> + && request.getRequestedSessionId().equals(cookie.getValue())) >> { >> + sessionCookie = cookie; >> > > Is it a good idea to 'break' here ? > Do you expect more than one session cookies ?! No, but I do expect that there may be more interesting cookies later on in the list... >> + } else >> + // Is the client presenting a valid ignore-cookie >> value? >> + if(null != _ignoreCookieName >> + && _ignoreCookieName.equals(cookieName) >> + && null != _ignoreCookieValue >> + && _ignoreCookieValue.equals(cookie.getValue())) >> { >> + ignoreRebalance = true; >> + } >> + } Like here ^^^^^. >> + // Re-write the URI if it contains a ;jsessionid parameter >> + String uri = request.getRequestURI(); >> + String sessionURIParamName = "jsessionid"; >> + SessionConfig.getSessionUriParamName(request.getContext()); >> > > It seems this bug has been inroduced during testing/debugging. > The return value of > "SessionConfig.getSessionUriParamName(request.getContext());" > is ignored and the sessionURIParamName is always "jsessionid". +1 I'll get that fixed. This Valve is a port of a Filter that I wrote earlier and evidently that got lost in the shuffle. Thanks for the review. -chris
signature.asc
Description: OpenPGP digital signature