You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by GitBox <gi...@apache.org> on 2020/11/13 22:16:40 UTC
[GitHub] [tomcat] akurth opened a new pull request #377: Set "Secure" session cookie attribute if called under SSL.
akurth opened a new pull request #377:
URL: https://github.com/apache/tomcat/pull/377
Missing "Secure" flag may lead to redirect loops under certain
conditions, e.g. when sameSiteCookies=none has been set at the
CookieProcessor and browser is Chrome.
Fixes <https://bz.apache.org/bugzilla/show_bug.cgi?id=64921>.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] markt-asf closed pull request #377: Set "Secure" session cookie attribute if called under SSL.
Posted by GitBox <gi...@apache.org>.
markt-asf closed pull request #377:
URL: https://github.com/apache/tomcat/pull/377
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] markt-asf commented on pull request #377: Set "Secure" session cookie attribute if called under SSL.
Posted by GitBox <gi...@apache.org>.
markt-asf commented on pull request #377:
URL: https://github.com/apache/tomcat/pull/377#issuecomment-730424887
Thanks for the PR. I have implemented a variation of this in 10.0.x and will back-port to 9.0.x and 8.5.x.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] akurth commented on a change in pull request #377: Set "Secure" session cookie attribute if called under SSL.
Posted by GitBox <gi...@apache.org>.
akurth commented on a change in pull request #377:
URL: https://github.com/apache/tomcat/pull/377#discussion_r524478033
##########
File path: test/org/apache/catalina/valves/TestLoadBalancerDrainingValve.java
##########
@@ -238,6 +238,8 @@ private void runValve(String jkActivation,
expectedCookie.setPath(cookieConfig.getPath());
expectedCookie.setMaxAge(0);
+ EasyMock.expect(request.isSecure()).andReturn(true);
Review comment:
Feel free to add it. Makes no difference though, since || is short-circuit.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] ChristopherSchultz commented on a change in pull request #377: Set "Secure" session cookie attribute if called under SSL.
Posted by GitBox <gi...@apache.org>.
ChristopherSchultz commented on a change in pull request #377:
URL: https://github.com/apache/tomcat/pull/377#discussion_r524294255
##########
File path: test/org/apache/catalina/valves/TestLoadBalancerDrainingValve.java
##########
@@ -238,6 +238,8 @@ private void runValve(String jkActivation,
expectedCookie.setPath(cookieConfig.getPath());
expectedCookie.setMaxAge(0);
+ EasyMock.expect(request.isSecure()).andReturn(true);
Review comment:
Should this be `expect(request.isSecure() || cookieConfig.isSecure())`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
[GitHub] [tomcat] martin-g commented on a change in pull request #377: Set "Secure" session cookie attribute if called under SSL.
Posted by GitBox <gi...@apache.org>.
martin-g commented on a change in pull request #377:
URL: https://github.com/apache/tomcat/pull/377#discussion_r526015818
##########
File path: test/org/apache/catalina/valves/TestLoadBalancerDrainingValve.java
##########
@@ -238,6 +238,8 @@ private void runValve(String jkActivation,
expectedCookie.setPath(cookieConfig.getPath());
expectedCookie.setMaxAge(0);
+ EasyMock.expect(request.isSecure()).andReturn(true);
Review comment:
Actually, I think we need a second test method to have coverage for the `cookieConfig.isSecure()` case. E.g. `runValve()` could have an additional boolean/enum parameter that decides whether to prepare `request.isSecure()` or `cookieConfig.isSecure()`.
In addition I don't see any new code that verifies that the `expectedCookie` has `isSecure() == true`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org