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

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


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

commit fe8472d62b11a027d0fc1ec10c8849fc4ef639a9
Author: Mark Thomas <ma...@apache.org>
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 1d6b8be..7ef9386 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 jakarta.servlet.ServletException;
+import jakarta.servlet.SessionCookieConfig;
 import jakarta.servlet.http.Cookie;
 import jakarta.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 a45d274..7c670bc 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 8d615bc..3ad87f3 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 10.0.0-M11 (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: dev-unsubscr...@tomcat.apache.org
For additional commands, e-mail: dev-h...@tomcat.apache.org

Reply via email to