JsonMappingException not Serializable due to 2.7 reference to underlying parser

[mjustin]

JsonMappingExceptions thrown by the Jackson parser cannot be serialized. This breaks any code that assumes the exception (per the Serializable interface it implements) is serializable.

Since the exception contains a reference to a non-serializable parser (such as com.fasterxml.jackson.core.json.ReaderBasedJsonParser), serialization will throw a NotSerializableException. Furthermore, the parser may contain a reference to the class being parsed; if that class is not itself serializable, that too will prevent serialization.

The expected behavior would be that these exceptions would be serializable.

I ran across this issue when testing some code with a missing subtype. The following code will replicate this issue:

import com.fasterxml.jackson.annotation.JsonSubTypes;
import com.fasterxml.jackson.annotation.JsonTypeInfo;
import com.fasterxml.jackson.databind.JsonMappingException;
import com.fasterxml.jackson.databind.ObjectMapper;

import java.io.ByteArrayOutputStream;
import java.io.NotSerializableException;
import java.io.ObjectOutputStream;

public class SerializationTest {
    public static void main(String[] args) throws Exception {
        try {
            new ObjectMapper().readValue("{\"type\": \"B\"}", ClassToRead.class);
        } catch (JsonMappingException e) {
            try (ObjectOutputStream stream = new ObjectOutputStream(new ByteArrayOutputStream())) {
                stream.writeObject(e);
            } catch (NotSerializableException e2) {
                // java.io.NotSerializableException: com.fasterxml.jackson.core.json.ReaderBasedJsonParser
                System.err.println("Unable to serialize exception " + e);
                e2.printStackTrace();
            }
        }
        try {
            new ObjectMapper().readValue("{\"classToRead\": {\"type\": \"B\"}}", ContainerClassToRead.class);
        } catch (JsonMappingException e) {
            try (ObjectOutputStream stream = new ObjectOutputStream(new ByteArrayOutputStream())) {
                stream.writeObject(e);
            } catch (NotSerializableException e2) {
                // java.io.NotSerializableException: jackson.ContainerClassToRead
                System.err.println("Unable to serialize exception " + e);
                e2.printStackTrace();
            }
        }
    }
}

@JsonTypeInfo(use = JsonTypeInfo.Id.NAME, include = JsonTypeInfo.As.PROPERTY, property="type")
@JsonSubTypes({
        @JsonSubTypes.Type(value = SubclassToRead.class, name = "A")
})
abstract class ClassToRead {}
class SubclassToRead extends ClassToRead {}

class ContainerClassToRead {
    public ClassToRead classToRead;
}

[cowtowncoder]

Thank you for reporting this. I'll see if making source transient would help here.

[cowtowncoder]

Changed _processor to be transient, and also added handling to replace non-serializable object refs as their Class within Reference path. Latter part could probably benefit from some re-thinking, as the main use case (local information for catch clause) does require/benefit from access to actual POJOs in question; but remote usage (via JDK serialization) doesn't and this is likely to even be problematic (for POJOs, size of serialization; or maybe even Class that client does not have).

But at least now it serializes similar to 2.6 and earlier (_processor was added in 2.7).

[mjustin]

It looks like this fix doesn't always work. For example, if the unserializable class is in a list, the ArrayList is determined to be serializable even though the element inside is not.

Example failing code:

    try {
        new ObjectMapper().readValue("[{\"type\": \"A\"}, {\"type\": \"B\"}]",
                new TypeReference<List<ClassToRead>>(){});
    } catch (JsonMappingException e) {
        try (ObjectOutputStream stream = new ObjectOutputStream(new ByteArrayOutputStream())) {
            stream.writeObject(e);
        } catch (NotSerializableException e2) {
            // java.io.NotSerializableException: SubclassToRead
            e2.printStackTrace();
        }
    }

    try {
        new ObjectMapper().readValue("{\"classesToRead\": [{\"type\": \"A\"}, {\"type\": \"B\"}]}",
                ContainerClassToRead2.class);
    } catch (JsonMappingException e) {
        try (ObjectOutputStream stream = new ObjectOutputStream(new ByteArrayOutputStream())) {
            stream.writeObject(e);
        } catch (NotSerializableException e2) {
            // java.io.NotSerializableException: SubclassToRead
            e2.printStackTrace();
        }
    }

...
class ContainerClassToRead2 {
    public List<ClassToRead> classesToRead;
}

[cowtowncoder]

Hmmh. Yes, that is tricky, serializability is a transitive problem. Probably need to simply replace all non-Class objects with classes, for now. Eventually should figure out bit more elaborate set up, where there's distinction between transient actual value, and then serializable/printable representation (non-transient).

Thank you for your help here in improving this aspect; there is clearly friction between getting more local information and allowing some level of remote access.

[cowtowncoder]

@mjustin Improved further, please let me know if you find other holes. :)

[mjustin]

Confirmed that this is now working for me. Thanks!

Minor implementation question. Now that the _from field in Reference is not deserialized, why not mark it transient (as you did when you fixed the _processor issue), rather than have a custom writeReplace method?

On a related note, since _toString is only generated the once even if the underlying fields were changed, would it make sense to drop the setters and make _from, _fieldName, and _index final, making the object immutable? As far as I can tell, it doesn't look like the mutability of Reference is actually used.

[cowtowncoder]

@mjustin I was going back and forth with that one, and transient would mostly work. However, writeReplace is needed to force representation serialization (that is, creation of the _toString).

As to mutability: if I remember correctly fields are non-final only because there was no way of making them final -- they are not meant to be mutable aside from being set right after construction. If this is not the case any more (or I am mixing this with something else), I would be happy to make them properly final. So: the only reason for non-final would be inability to set them in constructor.

[mjustin]

Thanks for the explanation; I haven't had to work much with implementing serialization behavior, so I was wondering if that was intentional or not.

Six of one, half dozen of the other, I suspect; but it occurs to me that if you made the _from field transient, you could just initialize the _asString property and let the serialization do its thing with the same object. I don't think it buys you anything, though:

Object writeReplace() {
    toString(); // Force creation of _asString if it has not been initialized yet
    return this;
}

As to the mutability thing, as long as the final fields are not initialized inline, you should be able to set them in the constructor. In fact, it wouldn't compile unless the constructor was setting all the final fields.

[cowtowncoder]

Yes final fields must be set from within constructor, but what I meant was that I thought whoever constructed instances was not always in position to pass in all the information; and that this was why there were setters (setIndex() etc). I use final fields and immutable objects wherever possible otherwise, so I must have had some reason not to do that here. Another possible reason may have been that instances would be Jackson serializable/deserializable, not just JDK serializable (which in a way is secondary concern).

But looking at call chains it does not actually look like anything was actually calling setters so there does not seem current need to do that. I'll clean this up for 2.8; for 2.7 I will leave things mostly as are just to keep even low likelihood problems at bay.