Bug 64023

Summary: Session serialization does not support values that (de)serialize to null
Product: Tomcat 9 Reporter: Bernhard Frauendienst <apache>
Component: ClusterAssignee: Tomcat Developers Mailing List <dev>
Status: RESOLVED FIXED    
Severity: normal    
Priority: P2    
Version: unspecified   
Target Milestone: -----   
Hardware: PC   
OS: Linux   
Attachments: Don't put null-valued attributes into the session attributes when deserializing

Description Bernhard Frauendienst 2019-12-20 12:51:01 UTC
For caching purposes, we put some attributes in the session that are not supposed to be distributed to backup nodes. We wanted to keep the webapp distributable, and exclude these values explicitly from serialization, by wrapping them in a serialization proxy that serializes to null (using writeReplace).

While null values in the session are excluded from serialization in the first place, null values returned from deserialization cause a NullPointerException when the object is put into the ConcurrentHashMap:

> java.lang.NullPointerException
>         at java.base/java.util.concurrent.ConcurrentHashMap.putVal(ConcurrentHashMap.java:1011)
>         at java.base/java.util.concurrent.ConcurrentHashMap.put(ConcurrentHashMap.java:1006)
>         at org.apache.catalina.ha.session.DeltaSession.doReadObject(DeltaSession.java:849)
>         at org.apache.catalina.ha.session.DeltaSession.readObjectData(DeltaSession.java:618)
>         at org.apache.catalina.ha.session.DeltaSession.readExternal(DeltaSession.java:593)
>         at java.base/java.io.ObjectInputStream.readExternalData(ObjectInputStream.java:2136)
>         at java.base/java.io.ObjectInputStream.readOrdinaryObject(ObjectInputStream.java:2085)
>         at java.base/java.io.ObjectInputStream.readObject0(ObjectInputStream.java:1594)
>         at java.base/java.io.ObjectInputStream.readObject(ObjectInputStream.java:430)
>         at org.apache.catalina.tribes.io.XByteBuffer.deserialize(XByteBuffer.java:560)
>         at org.apache.catalina.tribes.tipis.AbstractReplicatedMap$MapMessage.value(AbstractReplicatedMap.java:1507)
>         at org.apache.catalina.tribes.tipis.AbstractReplicatedMap$MapMessage.deserialize(AbstractReplicatedMap.java:1465)
>         at org.apache.catalina.tribes.tipis.AbstractReplicatedMap.messageReceived(AbstractReplicatedMap.java:663)
>         at org.apache.catalina.tribes.group.GroupChannel.messageReceived(GroupChannel.java:336)
>         at org.apache.catalina.tribes.group.ChannelInterceptorBase.messageReceived(ChannelInterceptorBase.java:91)
>         at org.apache.catalina.tribes.group.interceptors.TcpFailureDetector.messageReceived(TcpFailureDetector.java:117)
>         at org.apache.catalina.tribes.group.ChannelInterceptorBase.messageReceived(ChannelInterceptorBase.java:91)
>         at org.apache.catalina.tribes.group.ChannelInterceptorBase.messageReceived(ChannelInterceptorBase.java:91)
>         at org.apache.catalina.tribes.group.interceptors.ThroughputInterceptor.messageReceived(ThroughputInterceptor.java:86)
>         at org.apache.catalina.tribes.group.ChannelInterceptorBase.messageReceived(ChannelInterceptorBase.java:91)
>         at org.apache.catalina.tribes.group.ChannelCoordinator.messageReceived(ChannelCoordinator.java:274)
>         at org.apache.catalina.tribes.transport.ReceiverBase.messageDataReceived(ReceiverBase.java:261)
>         at org.apache.catalina.tribes.transport.nio.NioReplicationTask.drainChannel(NioReplicationTask.java:213)
>         at org.apache.catalina.tribes.transport.nio.NioReplicationTask.run(NioReplicationTask.java:101)
>         at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
>         at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
>         at java.base/java.lang.Thread.run(Thread.java:834)

While this might be an unconventional solution, it should be possible to have null values in the object stream. A simple null-check after deserialization (in both DeltaSession and StandardSession) looks sufficient.
Comment 1 Christopher Schultz 2019-12-20 14:38:19 UTC
We'll look at this, as this should not fail, but there is another way to fix this perhaps faster: use a wrapper object that has a single transient member instead of writing null in writereplace().
Comment 2 Bernhard Frauendienst 2019-12-20 14:43:15 UTC
I'm afraid that won't help, because the attribute in question belongs to a session-scoped spring bean, and I need the session to contain null in order for the factory method to be invoked anew after session migration.
Comment 3 Christopher Schultz 2019-12-20 15:31:56 UTC
Gotcha. I have a patch for this ready to go that I'll attach to the bugin case you want to build your own for testing.
Comment 4 Christopher Schultz 2019-12-20 15:32:30 UTC
Created attachment 36926 [details]
Don't put null-valued attributes into the session attributes when deserializing
Comment 5 Christopher Schultz 2019-12-20 15:39:29 UTC
Fixed in d9f243aa49.

I'm leaving this open because I think it makes sense to avoid serializing the null values in the first place. Having the null-check in the deserialization process is a good sanity check, anyways, but there is no sense in writing the null-valued attribute to the serialized session, ever.
Comment 6 Christopher Schultz 2019-12-20 15:47:13 UTC
(In reply to Christopher Schultz from comment #5)
> [...] there is no sense in writing the null-valued attribute to the 
> serialized session, ever.


... except that in your case, it's writeReplace which is causing the null to be written. There is no way for the session manager to know that will happen without speculatively writing every session value to a scratch-area before writing the object to the real output stream. This could be very expensive, so it's best to simply leave the nulls in the serialized output.

Fixed in 9f98e7126a26283c9cc260232fd3fbd59900ef29 for 9.0.31.
Fixed in 50dacc525502387a326c3f47de53c0742b8a478b for 8.5.51.
Fixed in 9f98e7126a26283c9cc260232fd3fbd59900ef29 for Tomcat 7.0.100.