Martin, On 6/22/17 3:04 AM, Martin Grigorov wrote: > On Wed, Jun 21, 2017 at 9:05 PM, <[email protected]> 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
