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