svn commit: r1848348 - in /tomcat/trunk: java/org/apache/catalina/valves/LoadBalancerDrainingValve.java test/org/apache/catalina/valves/TestLoadBalancerDrainingValve.java webapps/docs/changelog.xml

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

svn commit: r1848348 - in /tomcat/trunk: java/org/apache/catalina/valves/LoadBalancerDrainingValve.java test/org/apache/catalina/valves/TestLoadBalancerDrainingValve.java webapps/docs/changelog.xml

markt
Author: markt
Date: Thu Dec  6 19:10:36 2018
New Revision: 1848348

URL: http://svn.apache.org/viewvc?rev=1848348&view=rev
Log:
Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=62988
Fix the LoadBalancerDrainingValve so it works when the session cookie configuration is not explicitly declared.
Based on a patch provided by Andreas Kurth.

Modified:
    tomcat/trunk/java/org/apache/catalina/valves/LoadBalancerDrainingValve.java
    tomcat/trunk/test/org/apache/catalina/valves/TestLoadBalancerDrainingValve.java
    tomcat/trunk/webapps/docs/changelog.xml

Modified: tomcat/trunk/java/org/apache/catalina/valves/LoadBalancerDrainingValve.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/valves/LoadBalancerDrainingValve.java?rev=1848348&r1=1848347&r2=1848348&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/valves/LoadBalancerDrainingValve.java (original)
+++ tomcat/trunk/java/org/apache/catalina/valves/LoadBalancerDrainingValve.java Thu Dec  6 19:10:36 2018
@@ -36,9 +36,6 @@ import org.apache.catalina.util.SessionC
  * and the client should be redirected to the same URI. This will cause the
  * load-balanced to re-balance the client to another server.</p>
  *
- * <p>A request parameter is added to the redirect URI in order to avoid
- * repeated redirects in the event of an error or misconfiguration.</p>
- *
  * <p>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 +56,8 @@ import org.apache.catalina.util.SessionC
  * @see <a href="https://tomcat.apache.org/connectors-doc/generic_howto/loadbalancers.html">Load
  *      balancer documentation</a>
  */
-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 +86,7 @@ public class LoadBalancerDrainingValve
      * 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;
@@ -175,45 +171,45 @@ public class LoadBalancerDrainingValve
 
     @Override
     public void invoke(Request request, Response response) throws IOException, ServletException {
-        if("DIS".equals(request.getAttribute(ATTRIBUTE_KEY_JK_LB_ACTIVATION))
-           && !request.isRequestedSessionIdValid()) {
+        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) {
+            if (null != cookies) {
+                for (Cookie cookie : cookies) {
                     final String cookieName = cookie.getName();
-                    if(containerLog.isTraceEnabled())
+                    if (containerLog.isTraceEnabled()) {
                         containerLog.trace("Checking cookie " + cookieName + "=" + cookie.getValue());
+                    }
 
-                    if(sessionCookieName.equals(cookieName)
-                       && request.getRequestedSessionId().equals(cookie.getValue())) {
+                    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())) {
+                    } else if (null != _ignoreCookieName &&
+                            _ignoreCookieName.equals(cookieName) &&
+                            null != _ignoreCookieValue &&
+                            _ignoreCookieValue.equals(cookie.getValue())) {
+                        // The client presenting a valid ignore-cookie value?
                         ignoreRebalance = true;
                     }
                 }
             }
 
-            if(ignoreRebalance) {
-                if(containerLog.isDebugEnabled())
-                    containerLog.debug("Client is presenting a valid " + _ignoreCookieName
-                                 + " cookie, re-balancing is being skipped");
+            if (ignoreRebalance) {
+                if (containerLog.isDebugEnabled()) {
+                    containerLog.debug("Client is presenting a valid " + _ignoreCookieName +
+                            " cookie, re-balancing is being skipped");
+                }
 
                 getNext().invoke(request, response);
 
@@ -222,41 +218,32 @@ public class LoadBalancerDrainingValve
 
             // 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);
-                }
+            if (null != sessionCookie) {
+                sessionCookie.setPath(SessionConfig.getSessionCookiePath(request.getContext()));
+                sessionCookie.setMaxAge(0); // Delete
+                sessionCookie.setValue(""); // Purge the cookie's value
+                response.addCookie(sessionCookie);
             }
 
             // 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
             response.setHeader("Location", uri);
             response.setStatus(_redirectStatusCode);
-        }
-        else
+        } else {
             getNext().invoke(request, response);
+        }
     }
 }

Modified: tomcat/trunk/test/org/apache/catalina/valves/TestLoadBalancerDrainingValve.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/valves/TestLoadBalancerDrainingValve.java?rev=1848348&r1=1848347&r2=1848348&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/valves/TestLoadBalancerDrainingValve.java (original)
+++ tomcat/trunk/test/org/apache/catalina/valves/TestLoadBalancerDrainingValve.java Thu Dec  6 19:10:36 2018
@@ -224,14 +224,15 @@ public class TestLoadBalancerDrainingVal
             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());

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1848348&r1=1848347&r2=1848348&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Thu Dec  6 19:10:36 2018
@@ -133,6 +133,11 @@
         Filter out tomcat-web.xml from the watched resources list in
         storeconfig. (remm)
       </fix>
+      <fix>
+        <bug>62988</bug>: Fix the <code>LoadBalancerDrainingValve</code> so it
+        works when the session cookie configuration is not explicitly declared.
+        Based on a patch provided by Andreas Kurth. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Coyote">



---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]