[tomcat] branch 9.0.x updated: Fix BZ 64921. LoadBalancerDrainingValve sets correct cookie secure attr

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

[tomcat] branch 9.0.x updated: Fix BZ 64921. LoadBalancerDrainingValve sets correct cookie secure attr

markt
This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 9.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/9.0.x by this push:
     new 9c659dd  Fix BZ 64921. LoadBalancerDrainingValve sets correct cookie secure attr
9c659dd is described below

commit 9c659dd2ae3b617517539e647ca7fd1ab03276fe
Author: Mark Thomas <[hidden email]>
AuthorDate: Thu Nov 19 14:49:05 2020 +0000

    Fix BZ 64921. LoadBalancerDrainingValve sets correct cookie secure attr
   
    https://bz.apache.org/bugzilla/show_bug.cgi?id=64921
    Based on a pull request by Andreas Kurth
---
 .../catalina/valves/LoadBalancerDrainingValve.java     |  4 ++++
 .../catalina/valves/TestLoadBalancerDrainingValve.java | 18 ++++++++++++++++--
 webapps/docs/changelog.xml                             |  9 +++++++++
 3 files changed, 29 insertions(+), 2 deletions(-)

diff --git a/java/org/apache/catalina/valves/LoadBalancerDrainingValve.java b/java/org/apache/catalina/valves/LoadBalancerDrainingValve.java
index 002013b..d2bf9d4 100644
--- a/java/org/apache/catalina/valves/LoadBalancerDrainingValve.java
+++ b/java/org/apache/catalina/valves/LoadBalancerDrainingValve.java
@@ -19,6 +19,7 @@ package org.apache.catalina.valves;
 import java.io.IOException;
 
 import javax.servlet.ServletException;
+import javax.servlet.SessionCookieConfig;
 import javax.servlet.http.Cookie;
 import javax.servlet.http.HttpServletResponse;
 
@@ -222,6 +223,9 @@ public class LoadBalancerDrainingValve extends ValveBase {
                 sessionCookie.setPath(SessionConfig.getSessionCookiePath(request.getContext()));
                 sessionCookie.setMaxAge(0); // Delete
                 sessionCookie.setValue(""); // Purge the cookie's value
+                // Replicate logic used to set secure attribute for session cookies
+                SessionCookieConfig sessionCookieConfig = request.getContext().getServletContext().getSessionCookieConfig();
+                sessionCookie.setSecure(request.isSecure() || sessionCookieConfig.isSecure());
                 response.addCookie(sessionCookie);
             }
 
diff --git a/test/org/apache/catalina/valves/TestLoadBalancerDrainingValve.java b/test/org/apache/catalina/valves/TestLoadBalancerDrainingValve.java
index 9f32f11..a7b710c 100644
--- a/test/org/apache/catalina/valves/TestLoadBalancerDrainingValve.java
+++ b/test/org/apache/catalina/valves/TestLoadBalancerDrainingValve.java
@@ -55,8 +55,12 @@ public class TestLoadBalancerDrainingValve {
                         Boolean expectInvokeNext = Boolean.valueOf("ACT".equals(jkActivation) || enableIgnore.booleanValue() ||
                                 validSessionId.booleanValue());
                         for (String queryString : queryStrings) {
-                            parameterSets.add(new Object[] { jkActivation, validSessionId, expectInvokeNext,
-                                    enableIgnore, queryString});
+                            for (Boolean secureRequest : booleans) {
+                                for (Boolean secureSessionConfig : booleans) {
+                                    parameterSets.add(new Object[] { jkActivation, validSessionId, expectInvokeNext,
+                                            enableIgnore, queryString, secureRequest, secureSessionConfig});
+                                }
+                            }
                         }
                     }
                 }
@@ -80,6 +84,13 @@ public class TestLoadBalancerDrainingValve {
     @Parameter(4)
     public String queryString;
 
+    @Parameter(5)
+    public Boolean secureRequest;
+
+    @Parameter(6)
+    public boolean secureSessionConfig;
+
+
     @Test
     public void runValve() throws Exception {
         IMocksControl control = EasyMock.createControl();
@@ -95,6 +106,7 @@ public class TestLoadBalancerDrainingValve {
         cookieConfig.setDomain("example.com");
         cookieConfig.setName(sessionCookieName);
         cookieConfig.setPath("/");
+        cookieConfig.setSecure(secureSessionConfig);
 
         // Valve.init requires all of this stuff
         EasyMock.expect(ctx.getMBeanKeyProperties()).andStubReturn("");
@@ -137,6 +149,8 @@ public class TestLoadBalancerDrainingValve {
                 expectedCookie.setPath(cookieConfig.getPath());
                 expectedCookie.setMaxAge(0);
 
+                EasyMock.expect(Boolean.valueOf(request.isSecure())).andReturn(secureRequest);
+
                 // 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
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 5857785..1555f20 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -104,6 +104,15 @@
   issues do not "pop up" wrt. others).
 -->
 <section name="Tomcat 9.0.41 (markt)" rtext="in development">
+  <subsection name="Catalina">
+    <changelog>
+      <fix>
+        <bug>64921</bug>: Ensure that the <code>LoadBalancerDrainingValve</code>
+        uses the correct setting for the secure attribute for any session
+        cookies it creates. Based on a pull request by Andreas Kurth. (markt)
+      </fix>
+    </changelog>
+  </subsection>
   <subsection name="Other">
     <changelog>
       <update>


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