FIX for serializing java.io.Serializable

[mP]

Before i point out the fix i will describe what led me to finding it.

Originally all my dtos used in my rpcs implemented IsSerializable in
an old project at work.

I thought id try the latest build 1163 ( i think thats it) against it.
Im not sure why but i searched + replaced all the implements
IsSerializable with java.io.Serializable. I then recompiled to
javascript and tried my app ( didnt bother trying in no server mode ).

I got a few strange errors in the deserialization process on the
client - the rpc worked. It was obvious that the serialized string
wasnt valid and didnt match what should be there. On top of that the
string was too short. After more checking on the server i tracked down
the problem.

ServerSerializationStreamWriter was asking the oracle if that object
should be serialized and it returned false because the class
implemented java.io.Serializable and not Gwt's IsSerializable.

Taking a look at the oracle which is asked if a class should have its
fields serialized its obvious what the bug is.

com.google.gwt.user.server.rpc.impl.ServerSerializableTypeOracleImpl

public boolean isSerializable(Class instanceType) {
if (instanceType.isArray()) {
return isSerializable(instanceType.getComponentType());
}
if (instanceType.isPrimitive()) {
return true;
}
if (IsSerializable.class.isAssignableFrom(instanceType)) {
return true;
}

/// FIX START
if (java.io.Serializable.class.isAssignableFrom(instanceType)) {
return true;
}
/// FIX END

return hasCustomFieldSerializer(instanceType) != null;
}

[Miroslav Pokorny]

Nowhere does the code test and throw an exception if an object is not serializable. WIth that in mind it is also possible to make the serialization process a bit safer and faster with the following changes.

public void writeObject(Object instance) throws SerializationException {
    if (instance == null) {
      // write a null string
      writeString(null);
      return;
    }

SUGGESTION
why not ask the oracle here if the instance is serializable and throw an exception if the object isnt.


private void serializeClass(Object instance, Class instanceClass){
...blha blah..

    if (superClass != null && serializableTypeOracle.isSerializable(superClass)) {
      serializeImpl(instance, superClass);
    }
}

SUGGESTION:
Why is the if above testing if the instance is super class is serializable ? If the sub-class impls serializable ( GWT or java.io) does it make sense to also serialize the super's fields.
Why not simply the if statement to only test superclass != null.

[Miguel Méndez]

Good comments Miroslav.

My comments inline.

On 5/29/07, Miroslav Pokorny <miroslav...@gmail.com > wrote:

Nowhere does the code test and throw an exception if an object is not serializable. WIth that in mind it is also possible to make the serialization process a bit safer and faster with the following changes.

public void writeObject(Object instance) throws SerializationException {
    if (instance == null) {
      // write a null string
      writeString(null);
      return;
    }

SUGGESTION
why not ask the oracle here if the instance is serializable and throw an exception if the object isnt.

private void serializeClass(Object instance, Class instanceClass){
...blha blah..

    if (superClass != null && serializableTypeOracle.isSerializable(superClass)) {
      serializeImpl(instance, superClass);
    }
}

The original (pre-1.0 thinking) was that the check on the server for serializability was not complete so we left it open.  We had discussed adding this clamp but I opted to leave it the way that it is for now and deal with it in the next version of RPC.

SUGGESTION:
Why is the if above testing if the instance is super class is serializable ? If the sub-class impls serializable ( GWT or java.io) does it make sense to also serialize the super's fields.
Why not simply the if statement to only test superclass != null.

We were trying to match the behavior of Java's serialization.  We should debate this point in the next version of RPC.

--
Miguel

[Miroslav Pokorny]

Could you at least fix the oracle test mentioned in this post. :)

--
Miguel

--
mP

[Miroslav Pokorny]

Its beneficial to remove the if (  not null and ask oracle ) because it produces broken streams which are not unserilizable by the client. This leads to obscure jsni exceptions where the client is trying to unmarshall one thing as another ( in my case it was trying to unmarshall one of the tokens but found undefined because the token array was cut short ). Whats the point of sending down a broken stream that can be potentially broken ? I cant see how not doing what i mentioned breaks anything. If anything it can only help.

--
mP

[Miguel Méndez]

Already committed the fix.

Thanks,

--
Miguel

[Miroslav Pokorny]

Good :)

--
mP

[Miguel Méndez]

This problem keeps coming up.  Even though the architecture won't support a 100% solution, I added issue 1297 to 1.4 RC2.  I think it will resolve a fair number of the instances where this is a problem; certainly not all of them.

On 5/30/07, Miroslav Pokorny <miroslav...@gmail.com> wrote:

--
Miguel