You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2019/12/07 22:30:45 UTC

[tomcat] 13/14: Add an atomic method to rotate session ID and return new value.

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

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

commit c06674e09e9f3f43dc0e5c022dc8c311a4285cfd
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Fri Dec 6 12:13:15 2019 +0000

    Add an atomic method to rotate session ID and return new value.
    
    Use it where possible.
---
 java/org/apache/catalina/connector/Request.java   | 27 +++++++++++++++++++++++
 java/org/apache/catalina/session/ManagerBase.java | 15 +++++++++++--
 2 files changed, 40 insertions(+), 2 deletions(-)

diff --git a/java/org/apache/catalina/connector/Request.java b/java/org/apache/catalina/connector/Request.java
index a0726ee..ab4e5f0 100644
--- a/java/org/apache/catalina/connector/Request.java
+++ b/java/org/apache/catalina/connector/Request.java
@@ -74,6 +74,7 @@ import org.apache.catalina.core.ApplicationPart;
 import org.apache.catalina.core.ApplicationSessionCookieConfig;
 import org.apache.catalina.core.AsyncContextImpl;
 import org.apache.catalina.realm.GenericPrincipal;
+import org.apache.catalina.session.ManagerBase;
 import org.apache.catalina.util.ParameterMap;
 import org.apache.catalina.util.RequestUtil;
 import org.apache.catalina.util.StringParser;
@@ -2702,6 +2703,32 @@ public class Request implements HttpServletRequest {
     }
 
 
+    public String changeSessionId() {
+
+        Session session = this.getSessionInternal(false);
+        if (session == null) {
+            throw new IllegalStateException(
+                sm.getString("coyoteRequest.changeSessionId"));
+        }
+
+        Manager manager = this.getContext().getManager();
+
+        String newSessionId = rotateSessionId(manager, session);
+        this.changeSessionId(newSessionId);
+
+        return newSessionId;
+    }
+
+    private String rotateSessionId(Manager manager, Session session) {
+        if (manager instanceof ManagerBase) {
+            return ((ManagerBase) manager).rotateSessionId(session);
+        } else {
+            // Best we do with the current interface
+            manager.changeSessionId(session);
+            return session.getId();
+        }
+    }
+
     /**
      * @return the session associated with this Request, creating one
      * if necessary and requested.
diff --git a/java/org/apache/catalina/session/ManagerBase.java b/java/org/apache/catalina/session/ManagerBase.java
index e4121a6..8022d08 100644
--- a/java/org/apache/catalina/session/ManagerBase.java
+++ b/java/org/apache/catalina/session/ManagerBase.java
@@ -851,9 +851,20 @@ public abstract class ManagerBase extends LifecycleMBeanBase implements Manager
 
     @Override
     public void changeSessionId(Session session) {
+        rotateSessionId(session);
+    }
+
+
+    public String rotateSessionId(Session session) {
+        String newId = generateSessionId();
+        changeSessionId(session, newId);
+        return newId;
+    }
+
+
+    public void changeSessionId(Session session, String newId) {
         String oldId = session.getIdInternal();
-        session.setId(generateSessionId(), false);
-        String newId = session.getIdInternal();
+        session.setId(newId, false);
         container.fireContainerEvent(Context.CHANGE_SESSION_ID_EVENT,
                 new String[] {oldId, newId});
     }


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: [tomcat] 13/14: Add an atomic method to rotate session ID and return new value.

Posted by Christopher Schultz <ch...@christopherschultz.net>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Mark,

On 12/11/19 12:52, Mark Thomas wrote:
> On 11/12/2019 17:46, Christopher Schultz wrote: Mark,
> 
> On 12/7/19 17:30, markt@apache.org wrote:
>>>> This is an automated email from the ASF dual-hosted git 
>>>> repository.
>>>> 
>>>> markt pushed a commit to branch 7.0.x in repository 
>>>> https://gitbox.apache.org/repos/asf/tomcat.git
>>>> 
>>>> commit c06674e09e9f3f43dc0e5c022dc8c311a4285cfd Author: Mark
>>>> Thomas <ma...@apache.org> AuthorDate: Fri Dec 6 12:13:15 2019
>>>> +0000
>>>> 
>>>> Add an atomic method to rotate session ID and return new
>>>> value.
>>>> 
>>>> Use it where possible.
> 
> Shouldn't there be a "synchronized" keyword somewhere in there?
> 
>> Not to solve the problem the commit was intended to solve.
> 
>> Maybe the commit message wasn't worded in the best way to
>> describe what was going on.
> 
>> The key thing here is that the new session ID that was created as
>> a result of this call is the one that is returned to the caller.

Ack.

Thanks,
- -chris

>>>> --- java/org/apache/catalina/connector/Request.java   | 27 
>>>> +++++++++++++++++++++++ 
>>>> java/org/apache/catalina/session/ManagerBase.java | 15 
>>>> +++++++++++-- 2 files changed, 40 insertions(+), 2
>>>> deletions(-)
>>>> 
>>>> diff --git a/java/org/apache/catalina/connector/Request.java 
>>>> b/java/org/apache/catalina/connector/Request.java index 
>>>> a0726ee..ab4e5f0 100644 --- 
>>>> a/java/org/apache/catalina/connector/Request.java +++ 
>>>> b/java/org/apache/catalina/connector/Request.java @@ -74,6
>>>> +74,7 @@ import org.apache.catalina.core.ApplicationPart;
>>>> import 
>>>> org.apache.catalina.core.ApplicationSessionCookieConfig;
>>>> import org.apache.catalina.core.AsyncContextImpl; import 
>>>> org.apache.catalina.realm.GenericPrincipal; +import 
>>>> org.apache.catalina.session.ManagerBase; import 
>>>> org.apache.catalina.util.ParameterMap; import 
>>>> org.apache.catalina.util.RequestUtil; import 
>>>> org.apache.catalina.util.StringParser; @@ -2702,6 +2703,32
>>>> @@ public class Request implements HttpServletRequest { }
>>>> 
>>>> 
>>>> +    public String changeSessionId() { + +        Session
>>>> session = this.getSessionInternal(false); +        if
>>>> (session == null) { + throw new IllegalStateException( + 
>>>> sm.getString("coyoteRequest.changeSessionId")); +        } +
>>>> + Manager manager = this.getContext().getManager(); + +
>>>> String newSessionId = rotateSessionId(manager, session); + 
>>>> this.changeSessionId(newSessionId); + +        return 
>>>> newSessionId; +    } + +    private String
>>>> rotateSessionId(Manager manager, Session session) { +
>>>> if (manager instanceof ManagerBase) { +            return
>>>> ((ManagerBase) manager).rotateSessionId(session); +        }
>>>> else { + // Best we do with the current interface + 
>>>> manager.changeSessionId(session); +            return 
>>>> session.getId(); +        } +    } + /** * @return the
>>>> session associated with this Request, creating one * if
>>>> necessary and requested. diff --git 
>>>> a/java/org/apache/catalina/session/ManagerBase.java 
>>>> b/java/org/apache/catalina/session/ManagerBase.java index 
>>>> e4121a6..8022d08 100644 --- 
>>>> a/java/org/apache/catalina/session/ManagerBase.java +++ 
>>>> b/java/org/apache/catalina/session/ManagerBase.java @@
>>>> -851,9 +851,20 @@ public abstract class ManagerBase extends 
>>>> LifecycleMBeanBase implements Manager
>>>> 
>>>> @Override public void changeSessionId(Session session) { + 
>>>> rotateSessionId(session); +    } + + +    public String 
>>>> rotateSessionId(Session session) { +        String newId = 
>>>> generateSessionId(); +        changeSessionId(session,
>>>> newId); + return newId; +    } + + +    public void
>>>> changeSessionId(Session session, String newId) { String oldId
>>>> = session.getIdInternal(); - 
>>>> session.setId(generateSessionId(), false); -        String
>>>> newId = session.getIdInternal(); +
>>>> session.setId(newId, false); 
>>>> container.fireContainerEvent(Context.CHANGE_SESSION_ID_EVENT,
>>>> new String[] {oldId, newId}); }
>>>> 
>>>> 
>>>> -------------------------------------------------------------------
- --
>>>>
>>>>
>
>>>> 
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
>>>> For additional commands, e-mail: dev-help@tomcat.apache.org
>>>> 
>> 
>> ---------------------------------------------------------------------
>>
>> 
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
>> For additional commands, e-mail: dev-help@tomcat.apache.org
>> 
> 
> ---------------------------------------------------------------------
>
> 
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
> 
-----BEGIN PGP SIGNATURE-----
Comment: Using GnuPG with Thunderbird - https://www.enigmail.net/

iQIzBAEBCAAdFiEEMmKgYcQvxMe7tcJcHPApP6U8pFgFAl3xQToACgkQHPApP6U8
pFg/5BAApLUOHwQXbBxEkEshvmLw/slDMXihnR/ZP5tQuujjnwcjMmGNCTEgdxtI
cMDfRLYBiXhF3T+lOGrdFCnnTHEvXFg3Pbspy5FZobK8vvgR4JOqZa5liYGPG6iq
62b9U2zrMPDihwBQ/qOCD5TwP+r6MeOC1MrLEwWZfCAM+TZlTSZqg6A+M2HMWdj9
X0BR+H2vaUy8m9MrC0TQXYDfYKsJXTOWLHNG7+WviWNN6LJXywxVeaie7X4x687g
hQOvZIu5wU4dcsHUbcH+8Eo1G2HeG9Y61YL2EvzWtDwJ9CF3sX68EV12wDl1oV5M
hEljbWrMuoZrt4/WDIW4QndUUsVJNxTJDnhYxGjocD3o58CI2QcUbhjN7qT3rAWN
4MJtOIGr4HoPDhKbPskibi+yJvK7DLSp5C0xwE+mJFC7vwng3NTchvkhjls7YMs5
MAEUgDOgSP+Nipa6h89WCtzxO557D10dLhdVuqfCeiXcDMOMR8cHz9zel2hfmtHI
4d4WSY+laNnJOxlB1oXMkgmnGicFWz09Hk5jvth+0gkge9+32z9AQ16kcCUGvT5M
i61Vat/tAy9FLf32TFXRVubYLprsRFJRYy/GypBKMIA3Ob3JJ9392bDkcT9F8t9o
6ob43RjrYz1rU7TN13yBz5lyfmkml5BdQiXSm1jm7lgy+gaA9k8=
=zJ3K
-----END PGP SIGNATURE-----

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: [tomcat] 13/14: Add an atomic method to rotate session ID and return new value.

Posted by Mark Thomas <ma...@apache.org>.
On 11/12/2019 17:46, Christopher Schultz wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA256
> 
> Mark,
> 
> On 12/7/19 17:30, markt@apache.org wrote:
>> This is an automated email from the ASF dual-hosted git
>> repository.
>>
>> markt pushed a commit to branch 7.0.x in repository
>> https://gitbox.apache.org/repos/asf/tomcat.git
>>
>> commit c06674e09e9f3f43dc0e5c022dc8c311a4285cfd Author: Mark Thomas
>> <ma...@apache.org> AuthorDate: Fri Dec 6 12:13:15 2019 +0000
>>
>> Add an atomic method to rotate session ID and return new value.
>>
>> Use it where possible.
> 
> Shouldn't there be a "synchronized" keyword somewhere in there?

Not to solve the problem the commit was intended to solve.

Maybe the commit message wasn't worded in the best way to describe what
was going on.

The key thing here is that the new session ID that was created as a
result of this call is the one that is returned to the caller.

Mark


> 
> - -chris
> 
>> --- java/org/apache/catalina/connector/Request.java   | 27
>> +++++++++++++++++++++++ 
>> java/org/apache/catalina/session/ManagerBase.java | 15
>> +++++++++++-- 2 files changed, 40 insertions(+), 2 deletions(-)
>>
>> diff --git a/java/org/apache/catalina/connector/Request.java
>> b/java/org/apache/catalina/connector/Request.java index
>> a0726ee..ab4e5f0 100644 ---
>> a/java/org/apache/catalina/connector/Request.java +++
>> b/java/org/apache/catalina/connector/Request.java @@ -74,6 +74,7 @@
>> import org.apache.catalina.core.ApplicationPart; import
>> org.apache.catalina.core.ApplicationSessionCookieConfig; import
>> org.apache.catalina.core.AsyncContextImpl; import
>> org.apache.catalina.realm.GenericPrincipal; +import
>> org.apache.catalina.session.ManagerBase; import
>> org.apache.catalina.util.ParameterMap; import
>> org.apache.catalina.util.RequestUtil; import
>> org.apache.catalina.util.StringParser; @@ -2702,6 +2703,32 @@
>> public class Request implements HttpServletRequest { }
>>
>>
>> +    public String changeSessionId() { + +        Session session =
>> this.getSessionInternal(false); +        if (session == null) { +
>> throw new IllegalStateException( +
>> sm.getString("coyoteRequest.changeSessionId")); +        } + +
>> Manager manager = this.getContext().getManager(); + +        String
>> newSessionId = rotateSessionId(manager, session); +
>> this.changeSessionId(newSessionId); + +        return
>> newSessionId; +    } + +    private String rotateSessionId(Manager
>> manager, Session session) { +        if (manager instanceof
>> ManagerBase) { +            return ((ManagerBase)
>> manager).rotateSessionId(session); +        } else { +
>> // Best we do with the current interface +
>> manager.changeSessionId(session); +            return
>> session.getId(); +        } +    } + /** * @return the session
>> associated with this Request, creating one * if necessary and
>> requested. diff --git
>> a/java/org/apache/catalina/session/ManagerBase.java
>> b/java/org/apache/catalina/session/ManagerBase.java index
>> e4121a6..8022d08 100644 ---
>> a/java/org/apache/catalina/session/ManagerBase.java +++
>> b/java/org/apache/catalina/session/ManagerBase.java @@ -851,9
>> +851,20 @@ public abstract class ManagerBase extends
>> LifecycleMBeanBase implements Manager
>>
>> @Override public void changeSessionId(Session session) { +
>> rotateSessionId(session); +    } + + +    public String
>> rotateSessionId(Session session) { +        String newId =
>> generateSessionId(); +        changeSessionId(session, newId); +
>> return newId; +    } + + +    public void changeSessionId(Session
>> session, String newId) { String oldId = session.getIdInternal(); -
>> session.setId(generateSessionId(), false); -        String newId =
>> session.getIdInternal(); +        session.setId(newId, false); 
>> container.fireContainerEvent(Context.CHANGE_SESSION_ID_EVENT, new
>> String[] {oldId, newId}); }
>>
>>
>> ---------------------------------------------------------------------
>>
>>
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
>> For additional commands, e-mail: dev-help@tomcat.apache.org
>>
> -----BEGIN PGP SIGNATURE-----
> Comment: Using GnuPG with Thunderbird - https://www.enigmail.net/
> 
> iQIzBAEBCAAdFiEEMmKgYcQvxMe7tcJcHPApP6U8pFgFAl3xK2kACgkQHPApP6U8
> pFgyKxAAs1tW1HjrlofSFAD1hPFG8iWITjAF7zuFzH3+gTCyuaQT/V6mD6Ad6xwR
> qrwdM1X4pZLkGYmIz1QOQtpzjAdZyuFRjlLLIlIVOdEj+d2vth3O0GgwSfRcU0PZ
> o4ars/2xeLyK3BD7iU79FfqtzrWlHuzXNtoBoBvy7YSJPvHqJh0Jd7faiiNZtUQ9
> H4zDlKQdBCyuehf5LOCV18iL0FhvwFZBzs09P8BXAwdKjuI5SEj9Tc3DYTAMM6yS
> yc7EztgAg/YXDtV6dDfwHZ5T32apMxqOqH1iTZl6cjUcKlKSvTMoH6EeakyF+DDJ
> W0edlmP9rUTj8Gwu13L5I8T/3G4qu6dV3RGV7BxQmha7gJoafte/TL64v20RkXYw
> qidZ0asGu96d4/VQsCSTmBVIpBDMhxUpmm62dPQpO4aD6bWLOCrAChSvouk0uDCO
> 6eBZhSfFWRo/I3SmPrLpy4/bO8L/JlBWEGr2Oen84iNSBX3K1xqWY35/weOPZhaN
> uoU0xJzICU9umbnSwSECUmNFIfdhhfQ80rc4RWFovCAfcvfbH9V7TU+LQk4WGuhU
> oaezHcAarYvfYXRAtl7ypxwDjeOT0oNonDi8WHkaPdkFo1ZNS2aXxm7fh5jxDIbF
> QSLyDjm35hX2+pJkdgNoKCfVnlWOm2QJUQhhc+i3EiNdksuET8U=
> =qerJ
> -----END PGP SIGNATURE-----
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: [tomcat] 13/14: Add an atomic method to rotate session ID and return new value.

Posted by Christopher Schultz <ch...@christopherschultz.net>.
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

Mark,

On 12/7/19 17:30, markt@apache.org wrote:
> This is an automated email from the ASF dual-hosted git
> repository.
> 
> markt pushed a commit to branch 7.0.x in repository
> https://gitbox.apache.org/repos/asf/tomcat.git
> 
> commit c06674e09e9f3f43dc0e5c022dc8c311a4285cfd Author: Mark Thomas
> <ma...@apache.org> AuthorDate: Fri Dec 6 12:13:15 2019 +0000
> 
> Add an atomic method to rotate session ID and return new value.
> 
> Use it where possible.

Shouldn't there be a "synchronized" keyword somewhere in there?

- -chris

> --- java/org/apache/catalina/connector/Request.java   | 27
> +++++++++++++++++++++++ 
> java/org/apache/catalina/session/ManagerBase.java | 15
> +++++++++++-- 2 files changed, 40 insertions(+), 2 deletions(-)
> 
> diff --git a/java/org/apache/catalina/connector/Request.java
> b/java/org/apache/catalina/connector/Request.java index
> a0726ee..ab4e5f0 100644 ---
> a/java/org/apache/catalina/connector/Request.java +++
> b/java/org/apache/catalina/connector/Request.java @@ -74,6 +74,7 @@
> import org.apache.catalina.core.ApplicationPart; import
> org.apache.catalina.core.ApplicationSessionCookieConfig; import
> org.apache.catalina.core.AsyncContextImpl; import
> org.apache.catalina.realm.GenericPrincipal; +import
> org.apache.catalina.session.ManagerBase; import
> org.apache.catalina.util.ParameterMap; import
> org.apache.catalina.util.RequestUtil; import
> org.apache.catalina.util.StringParser; @@ -2702,6 +2703,32 @@
> public class Request implements HttpServletRequest { }
> 
> 
> +    public String changeSessionId() { + +        Session session =
> this.getSessionInternal(false); +        if (session == null) { +
> throw new IllegalStateException( +
> sm.getString("coyoteRequest.changeSessionId")); +        } + +
> Manager manager = this.getContext().getManager(); + +        String
> newSessionId = rotateSessionId(manager, session); +
> this.changeSessionId(newSessionId); + +        return
> newSessionId; +    } + +    private String rotateSessionId(Manager
> manager, Session session) { +        if (manager instanceof
> ManagerBase) { +            return ((ManagerBase)
> manager).rotateSessionId(session); +        } else { +
> // Best we do with the current interface +
> manager.changeSessionId(session); +            return
> session.getId(); +        } +    } + /** * @return the session
> associated with this Request, creating one * if necessary and
> requested. diff --git
> a/java/org/apache/catalina/session/ManagerBase.java
> b/java/org/apache/catalina/session/ManagerBase.java index
> e4121a6..8022d08 100644 ---
> a/java/org/apache/catalina/session/ManagerBase.java +++
> b/java/org/apache/catalina/session/ManagerBase.java @@ -851,9
> +851,20 @@ public abstract class ManagerBase extends
> LifecycleMBeanBase implements Manager
> 
> @Override public void changeSessionId(Session session) { +
> rotateSessionId(session); +    } + + +    public String
> rotateSessionId(Session session) { +        String newId =
> generateSessionId(); +        changeSessionId(session, newId); +
> return newId; +    } + + +    public void changeSessionId(Session
> session, String newId) { String oldId = session.getIdInternal(); -
> session.setId(generateSessionId(), false); -        String newId =
> session.getIdInternal(); +        session.setId(newId, false); 
> container.fireContainerEvent(Context.CHANGE_SESSION_ID_EVENT, new
> String[] {oldId, newId}); }
> 
> 
> ---------------------------------------------------------------------
>
> 
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
> 
-----BEGIN PGP SIGNATURE-----
Comment: Using GnuPG with Thunderbird - https://www.enigmail.net/

iQIzBAEBCAAdFiEEMmKgYcQvxMe7tcJcHPApP6U8pFgFAl3xK2kACgkQHPApP6U8
pFgyKxAAs1tW1HjrlofSFAD1hPFG8iWITjAF7zuFzH3+gTCyuaQT/V6mD6Ad6xwR
qrwdM1X4pZLkGYmIz1QOQtpzjAdZyuFRjlLLIlIVOdEj+d2vth3O0GgwSfRcU0PZ
o4ars/2xeLyK3BD7iU79FfqtzrWlHuzXNtoBoBvy7YSJPvHqJh0Jd7faiiNZtUQ9
H4zDlKQdBCyuehf5LOCV18iL0FhvwFZBzs09P8BXAwdKjuI5SEj9Tc3DYTAMM6yS
yc7EztgAg/YXDtV6dDfwHZ5T32apMxqOqH1iTZl6cjUcKlKSvTMoH6EeakyF+DDJ
W0edlmP9rUTj8Gwu13L5I8T/3G4qu6dV3RGV7BxQmha7gJoafte/TL64v20RkXYw
qidZ0asGu96d4/VQsCSTmBVIpBDMhxUpmm62dPQpO4aD6bWLOCrAChSvouk0uDCO
6eBZhSfFWRo/I3SmPrLpy4/bO8L/JlBWEGr2Oen84iNSBX3K1xqWY35/weOPZhaN
uoU0xJzICU9umbnSwSECUmNFIfdhhfQ80rc4RWFovCAfcvfbH9V7TU+LQk4WGuhU
oaezHcAarYvfYXRAtl7ypxwDjeOT0oNonDi8WHkaPdkFo1ZNS2aXxm7fh5jxDIbF
QSLyDjm35hX2+pJkdgNoKCfVnlWOm2QJUQhhc+i3EiNdksuET8U=
=qerJ
-----END PGP SIGNATURE-----

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org