Don't remove unserializable attributes from active sessions (with default/java serialization)

[GoogleCodeExporter]

Observed for the TC 7 branch, but likely not restricted to that.

The JavaSerializationTranscoder removes any attributes that do not implement 
Serializable from the active session.

Of course it can not serialize and push attribiutes to memcached if they are 
not serializable. But in addition now it smashes the consistency of the local 
session by removing those attributes.

One can imagine a situation, where the webapp developpers only want to keep 
certain attributes from the session in memcached and take their own 
responsibility for recreating the remaining data after failover (e.g. getting 
them from some database or other backend).

My current patch looks like this:

--- a/core/src/main/java/de/javakaffee/web/msm/JavaSerializationTranscoder.java
+++ b/core/src/main/java/de/javakaffee/web/msm/JavaSerializationTranscoder.java
@@ -126,11 +126,11 @@ public class JavaSerializationTranscoder implements 
SessionAttributesTranscoder
             final Object value = attributes.get( keys[i] );
             if ( value == null ) {
                 continue;
-            } else if ( ( value instanceof Serializable ) && ( 
!session.exclude( keys[i] ) ) ) {
+            } else if ( session.exclude( keys[i] ) ) {
+                session.removeAttributeInternal( keys[i], true );
+            } else if ( ( value instanceof Serializable ) ) {
                 saveNames.add( keys[i] );
                 saveValues.add( value );
-            } else {
-                session.removeAttributeInternal( keys[i], true );
             }
         }

I'm not sure whether the "exclude()" attributes should get removed, but I 
didn't want to change the behaviour for those.

Original issue reported on code.google.com by rainer.j...@kippdata.de on 23 Mar 2011 at 2:14

[GoogleCodeExporter]

The current implementation was taken from StandardSession.writeObject.

Your patch actually removes an attribute if it matches session.exclude( keys[i] 
), which in StandardSession is returning true for an attribute 
"javax.security.auth.subject" - this would drop auth information from the 
session AFAICS.

If you don't want to remove unserializable attributes from the session, I'd 
suggest this (more or less how DeltaSession.writeObject is implemented):

--- a/core/src/main/java/de/javakaffee/web/msm/JavaSerializationTranscoder.java
+++ b/core/src/main/java/de/javakaffee/web/msm/JavaSerializationTranscoder.java
@@ -126,11 +126,11 @@ public class JavaSerializationTranscoder implements 
SessionAttributesTranscoder
             final Object value = attributes.get( keys[i] );
-            if ( value == null ) {
+            if ( value == null || session.exclude( keys[i] ) ) {
                 continue;
-            } else if ( ( value instanceof Serializable ) && ( 
!session.exclude( keys[i] ) ) ) {
+            } else if ( value instanceof Serializable ) {
                 saveNames.add( keys[i] );
                 saveValues.add( value );
             } else {
-                session.removeAttributeInternal( keys[i], true );
+                if ( LOG.isDebugEnabled() ) {
+                    LOG.debug( "Ignoring attribute '" + keys[i] + "' as it 
does not implement Serializable" );
+                }
             }
         }

The only potential problem I see with this is that it might break servlet 
compliance, as 7.7.2 "Distributed Environments" says:
"The distributed servlet container must throw an IllegalArgumentException for 
objects where the container cannot support the mechanism necessary for 
migration of the session storing them."
(which would require a different patch for throwing an IllegalArgumentException)

However, 7.7.2 states also that "The container may choose to support storage of 
other designated objects in the
HttpSession, such as references to Enterprise JavaBeans components and 
transactions."
which is a conflict with the previous statement - from my understatement.

AFAICS tomcat follows the last quotation as it does not throw an 
IllegalArgumentException for non serializable session attributes, so it should 
be ok ;-)

Original comment by martin.grotzke on 23 Mar 2011 at 8:52

• Changed state: Accepted
• Added labels: Type-Enhancement
• Removed labels: Type-Defect

[GoogleCodeExporter]

I fully agree with your patch. I kept removing the exclude attributes because 
that was what your original code did. I think the patch you proposed is fine.

Concerning servlet spec: unfortunately it doesn't help a lot when thinking 
about scaling and replication. More precisely: I'd say when adding a 
replication mechanism you want as little negative influence as possible as long 
as you are not failing over (sticky case) and a good way of finding out, 
whether fail over will actually work.

So logging replication problems is helpful, breaking the local app not. Since 
on a production system the problems will happen often - if they happen at all - 
e.g. because of several session attributes not being serializable, those kind 
of messages should have a low log level or a special logger in order to be able 
to silence them without removing real (unexpected) error logging.

Thanks for caring!

Rainer

Original comment by rainer.j...@kippdata.de on 24 Mar 2011 at 8:18

[GoogleCodeExporter]

Changed in master and tomcat7 branch.

Original comment by martin.grotzke on 24 Mar 2011 at 5:49

• Changed title: Don't remove unserializable attributes from active sessions (with default/java serialization)
• Changed state: Fixed
• Added labels: Milestone-1.4.1