--- java/org/apache/catalina/valves/LoadBalancerDrainingValve.java (Revision 1848314) +++ java/org/apache/catalina/valves/LoadBalancerDrainingValve.java (Arbeitskopie) @@ -19,9 +19,11 @@ import java.io.IOException; import javax.servlet.ServletException; +import javax.servlet.SessionCookieConfig; import javax.servlet.http.Cookie; import javax.servlet.http.HttpServletResponse; +import org.apache.catalina.Context; import org.apache.catalina.connector.Request; import org.apache.catalina.connector.Response; import org.apache.catalina.util.SessionConfig; @@ -36,9 +38,6 @@ * and the client should be redirected to the same URI. This will cause the * load-balanced to re-balance the client to another server.

* - *

A request parameter is added to the redirect URI in order to avoid - * repeated redirects in the event of an error or misconfiguration.

- * *

All this work is required because when the activation state of a node is * DISABLED, the load-balancer will still send requests to the node if they * appear to have a session on that node. Since mod_jk doesn't actually know @@ -59,9 +58,8 @@ * @see Load * balancer documentation */ -public class LoadBalancerDrainingValve - extends ValveBase -{ +public class LoadBalancerDrainingValve extends ValveBase { + /** * The request attribute key where the load-balancer's activation state * can be found. @@ -90,7 +88,7 @@ * The value of the cookie which can be set to ignore the "draining" action * of this Filter. This will allow a client to contact the server without * being re-balanced to another server. The expected cookie name can be set - * in the {@link #_ignoreCookieValue}. The cookie name and value must match + * in the {@link #_ignoreCookieName}. The cookie name and value must match * to avoid being re-balanced. */ private String _ignoreCookieValue; @@ -115,19 +113,6 @@ } /** - * Gets the name of the cookie that can be used to override the - * re-balancing behavior of this Valve when the current node is - * in the DISABLED activation state. - * - * @return The cookie name used to ignore normal processing rules. - * - * @see #setIgnoreCookieValue - */ - public String getIgnoreCookieName() { - return _ignoreCookieName; - } - - /** * Sets the name of the cookie that can be used to override the * re-balancing behavior of this Valve when the current node is * in the DISABLED activation state. @@ -137,8 +122,6 @@ * * @param cookieName The cookie name to use to ignore normal * processing rules. - * - * @see #getIgnoreCookieValue */ public void setIgnoreCookieName(String cookieName) { _ignoreCookieName = cookieName; @@ -145,19 +128,6 @@ } /** - * Gets the expected value of the cookie that can be used to override the - * re-balancing behavior of this Valve when the current node is - * in the DISABLED activation state. - * - * @return The cookie value used to ignore normal processing rules. - * - * @see #setIgnoreCookieValue - */ - public String getIgnoreCookieValue() { - return _ignoreCookieValue; - } - - /** * Sets the expected value of the cookie that can be used to override the * re-balancing behavior of this Valve when the current node is * in the DISABLED activation state. The "ignore" cookie's value @@ -166,8 +136,6 @@ * * @param cookieValue The cookie value to use to ignore normal * processing rules. - * - * @see #getIgnoreCookieValue */ public void setIgnoreCookieValue(String cookieValue) { _ignoreCookieValue = cookieValue; @@ -178,18 +146,17 @@ if("DIS".equals(request.getAttribute(ATTRIBUTE_KEY_JK_LB_ACTIVATION)) && !request.isRequestedSessionIdValid()) { - if(containerLog.isDebugEnabled()) + if(containerLog.isDebugEnabled()) { containerLog.debug("Load-balancer is in DISABLED state; draining this node"); + } - boolean ignoreRebalance = false; // Allow certain clients + boolean ignoreRebalance = false; Cookie sessionCookie = null; - // Kill any session cookie present final Cookie[] cookies = request.getCookies(); - final String sessionCookieName = request.getServletContext().getSessionCookieConfig().getName(); + final String sessionCookieName = SessionConfig.getSessionCookieName(request.getContext()); - // Kill any session cookie present if(null != cookies) { for(Cookie cookie : cookies) { final String cookieName = cookie.getName(); @@ -199,21 +166,23 @@ if(sessionCookieName.equals(cookieName) && request.getRequestedSessionId().equals(cookie.getValue())) { sessionCookie = cookie; - } else - // Is the client presenting a valid ignore-cookie value? - if(null != _ignoreCookieName - && _ignoreCookieName.equals(cookieName) - && null != _ignoreCookieValue - && _ignoreCookieValue.equals(cookie.getValue())) { - ignoreRebalance = true; + } else { + // Is the client presenting a valid ignore-cookie value? + if (null != _ignoreCookieName + && _ignoreCookieName.equals(cookieName) + && null != _ignoreCookieValue + && _ignoreCookieValue.equals(cookie.getValue())) { + ignoreRebalance = true; + } } } } if(ignoreRebalance) { - if(containerLog.isDebugEnabled()) + if(containerLog.isDebugEnabled()) { containerLog.debug("Client is presenting a valid " + _ignoreCookieName - + " cookie, re-balancing is being skipped"); + + " cookie, re-balancing is being skipped"); + } getNext().invoke(request, response); @@ -220,43 +189,70 @@ return; } - // Kill any session cookie that was found - // TODO: Consider implications of SSO cookies - if(null != sessionCookie) { - String cookiePath = request.getServletContext().getSessionCookieConfig().getPath(); - - if(request.getContext().getSessionCookiePathUsesTrailingSlash()) { - // Handle special case of ROOT context where cookies require a path of - // '/' but the servlet spec uses an empty string - // Also ensure the cookies for a context with a path of /foo don't get - // sent for requests with a path of /foobar - if (!cookiePath.endsWith("/")) - cookiePath = cookiePath + "/"; - - sessionCookie.setPath(cookiePath); - sessionCookie.setMaxAge(0); // Delete - sessionCookie.setValue(""); // Purge the cookie's value - response.addCookie(sessionCookie); - } + // Kill any session cookie that was found. + // TODO: Consider implications of SSO cookies. + if (null != sessionCookie) { + sessionCookie.setPath(determineSessionCookiePath(request)); + sessionCookie.setMaxAge(0); // Delete + sessionCookie.setValue(""); // Purge the cookie's value + response.addCookie(sessionCookie); } - // Re-write the URI if it contains a ;jsessionid parameter + // Re-write the URI if it contains a ;jsessionid parameter. String uri = request.getRequestURI(); String sessionURIParamName = SessionConfig.getSessionUriParamName(request.getContext()); - if(uri.contains(";" + sessionURIParamName + "=")) + if(uri.contains(";" + sessionURIParamName + "=")) { uri = uri.replaceFirst(";" + sessionURIParamName + "=[^&?]*", ""); + } String queryString = request.getQueryString(); - if(null != queryString) + if(null != queryString) { uri = uri + "?" + queryString; + } // NOTE: Do not call response.encodeRedirectURL or the bad - // sessionid will be restored + // sessionid will be restored. response.setHeader("Location", uri); response.setStatus(_redirectStatusCode); + + } else { + getNext().invoke(request, response); } - else - getNext().invoke(request, response); } + + // TODO This is basically copied from ApplicationSessionCookieConfig. Should be + // refactored to avoid code duplication. + private String determineSessionCookiePath(final Request request) { + Context context = request.getContext(); + SessionCookieConfig scc = context.getServletContext().getSessionCookieConfig(); + String contextPath = context.getSessionCookiePath(); + if (!have(contextPath)) { + contextPath = scc.getPath(); + } + if (!have(contextPath)) { + contextPath = context.getEncodedPath(); + } + if (context.getSessionCookiePathUsesTrailingSlash()) { + // Handle special case of ROOT context where cookies require a path of + // '/' but the servlet spec uses an empty string + // Also ensure the cookies for a context with a path of /foo don't get + // sent for requests with a path of /foobar + if (!contextPath.endsWith("/")) { + contextPath = contextPath + "/"; + } + } else { + // Only handle special case of ROOT context where cookies require a + // path of '/' but the servlet spec uses an empty string + if (contextPath.length() == 0) { + contextPath = "/"; + } + } + return contextPath; + } + + private boolean have(final String s) { + return s != null && !s.isEmpty(); + } + } --- test/org/apache/catalina/valves/TestLoadBalancerDrainingValve.java (Revision 1848314) +++ test/org/apache/catalina/valves/TestLoadBalancerDrainingValve.java (Arbeitskopie) @@ -22,6 +22,7 @@ import javax.servlet.SessionCookieConfig; import javax.servlet.http.Cookie; +import org.apache.catalina.util.SessionConfig; import org.junit.Test; import org.apache.catalina.Context; @@ -224,22 +225,21 @@ EasyMock.expect(request.getRequestedSessionId()).andStubReturn(sessionId); EasyMock.expect(request.getRequestURI()).andStubReturn(requestURI); EasyMock.expect(request.getCookies()).andStubReturn(cookies.toArray(new Cookie[cookies.size()])); - EasyMock.expect(servletContext.getSessionCookieConfig()).andStubReturn(cookieConfig); - EasyMock.expect(request.getServletContext()).andStubReturn(servletContext); EasyMock.expect(request.getContext()).andStubReturn(ctx); - EasyMock.expect(Boolean.valueOf(ctx.getSessionCookiePathUsesTrailingSlash())).andStubReturn(Boolean.TRUE); + EasyMock.expect(ctx.getSessionCookieName()).andStubReturn(sessionCookieName); EasyMock.expect(servletContext.getSessionCookieConfig()).andStubReturn(cookieConfig); - EasyMock.expect(request.getQueryString()).andStubReturn(queryString); + EasyMock.expect(ctx.getSessionCookiePath()).andStubReturn("/"); - if(!enableIgnore) { + if(!enableIgnore) { + EasyMock.expect(Boolean.valueOf(ctx.getSessionCookiePathUsesTrailingSlash())).andStubReturn(Boolean.TRUE); + EasyMock.expect(request.getQueryString()).andStubReturn(queryString); + // Response will have cookie deleted MyCookie expectedCookie = new MyCookie(cookieConfig.getName(), ""); expectedCookie.setPath(cookieConfig.getPath()); expectedCookie.setMaxAge(0); - // These two lines just mean EasyMock.expect(response.addCookie) but for a void method response.addCookie(expectedCookie); - EasyMock.expect(ctx.getSessionCookieName()).andReturn(sessionCookieName); // Indirect call String expectedRequestURI = requestURI; if(null != queryString) expectedRequestURI = expectedRequestURI + '?' + queryString;