Copilot commented on code in PR #2712:
URL: https://github.com/apache/shiro/pull/2712#discussion_r3295632653
##########
core/src/main/java/org/apache/shiro/session/mgt/SimpleSession.java:
##########
@@ -491,22 +503,30 @@ private void readObject(ObjectInputStream in) throws
IOException, ClassNotFoundE
this.startTimestamp = (Date) in.readObject();
}
if (isFieldPresent(bitMask, STOP_TIMESTAMP_BIT_MASK)) {
- this.stopTimestamp = (Date) in.readObject();
+ this.stopTimestamp = new AtomicReference<>((Date) in.readObject());
+ } else {
+ this.stopTimestamp = new AtomicReference<>();
}
if (isFieldPresent(bitMask, LAST_ACCESS_TIME_BIT_MASK)) {
- this.lastAccessTime = (Date) in.readObject();
+ this.lastAccessTime = new AtomicReference<>((Date)
in.readObject());
+ } else {
+ this.lastAccessTime = new AtomicReference<>();
}
if (isFieldPresent(bitMask, TIMEOUT_BIT_MASK)) {
- this.timeout = in.readLong();
+ this.timeout = new AtomicLong(in.readLong());
+ } else {
+ this.timeout = new AtomicLong();
}
if (isFieldPresent(bitMask, EXPIRED_BIT_MASK)) {
- this.expired = in.readBoolean();
+ this.expired = new AtomicBoolean(in.readBoolean());
+ } else {
+ this.expired = new AtomicBoolean();
}
if (isFieldPresent(bitMask, HOST_BIT_MASK)) {
this.host = in.readUTF();
}
if (isFieldPresent(bitMask, ATTRIBUTES_BIT_MASK)) {
- this.attributes = (Map<Object, Object>) in.readObject();
+ this.attributes = (ConcurrentHashMap<Object, Object>)
in.readObject();
Review Comment:
Deserialization currently casts the serialized attributes map to
ConcurrentHashMap. This will throw ClassCastException when reading sessions
serialized by older Shiro versions (which used HashMap) or any stream
containing a different Map implementation. Read into Map and wrap/convert to
ConcurrentHashMap (or delegate to setAttributes) to preserve serialization
backward compatibility; otherwise serialVersionUID should be bumped.
##########
core/src/main/java/org/apache/shiro/session/mgt/SimpleSession.java:
##########
@@ -166,45 +162,40 @@ public void setLastAccessTime(Date lastAccessTime) {
* @return true if this session has expired, false otherwise.
*/
public boolean isExpired() {
- return expired;
+ return expired.get();
}
public void setExpired(boolean expired) {
- this.expired = expired;
+ this.expired.set(expired);
}
public long getTimeout() {
- return timeout;
+ return timeout.get();
}
public void setTimeout(long timeout) {
- this.timeout = timeout;
+ this.timeout.set(timeout);
}
public String getHost() {
return host;
}
Review Comment:
The public SimpleSession#setHost(String) setter was removed, making the host
effectively immutable after construction. This is a breaking API change for
downstream users that previously set the host after instantiation; consider
keeping the setter (possibly deprecated) or providing an alternative migration
path if immutability is required.
##########
core/src/main/java/org/apache/shiro/session/mgt/SimpleSession.java:
##########
@@ -433,6 +429,10 @@ public String toString() {
return sb.toString();
}
+ void setStartTimestamp(Date startTimestamp) {
Review Comment:
setStartTimestamp(Date) was changed from public to package-private. If
SimpleSession is considered part of the public API, this is a breaking change;
consider retaining the public method (or deprecating it) unless the visibility
reduction is explicitly intended for 3.0 and documented.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
