x-stream / xstream

Serialize Java objects to XML and back again.
http://x-stream.github.io
Other
749 stars 227 forks source link

RecordConverter: Do not throw ClassNotFoundException when "class" attribute is absent #351

Open toby1984 opened 1 year ago

toby1984 commented 1 year ago

This PR fixes a crash in RecordConverter during deserialization.

For some reason the RecordConverter implementation assumes that either the "class" attribute is present or the XML tag name is the record classname. This is not true in at least one case (see unit test added by this PR) and causes a ClassNotFoundException when RecordConverter tries to resolve the member field name as a class.

I'm not familiar with the XStream design but after stepping through the code it seems that the "class" attribute is not being omitted by bug/accident but intentionally (most likely to save memory/disk space) when the actual type of a value stored in a field matches the field's type... so to me it looks like the crash is caused by a bug in RecordConverter and not outside code.

I've fixed the issue by using UnmarshallingContext#getRequiredType() instead of trying to use the "class" attribute and falling back to HierarchicalStreamReader#getNodeName() ... not sure whether this is the right approach though or why the original RecordConverter contribution did not use that method in the first place.

Because I was unsure about using UnmarshallingContext#getRequiredType() I've added a couple additional test cases (Object field type, Object field containing an object array ) to my unit test but it still passes so at least my fix is not trivially wrong.

For the sake of completeness, this is the exception being thrown when running my test case without my RecordConverter change:

com.thoughtworks.xstream.converters.ConversionException: Class not found. ---- Debugging information ---- message : Class not found. cause-exception : java.lang.ClassNotFoundException cause-message : field1 class : com.thoughtworks.xstream.converters.extended.RecordConverterTest$SerializableRecord required-type : com.thoughtworks.xstream.converters.extended.RecordConverterTest$SerializableRecord converter-type : com.thoughtworks.xstream.converters.extended.RecordConverter path : /com.thoughtworks.xstream.converters.extended.RecordConverterTest$MyObj/field1 line number : 1 class[1] : com.thoughtworks.xstream.converters.extended.RecordConverterTest$MyObj required-type[1] : com.thoughtworks.xstream.converters.extended.RecordConverterTest$MyObj converter-type[1] : com.thoughtworks.xstream.converters.reflection.ReflectionConverter version : not available

    at com.thoughtworks.xstream.converters.extended.RecordConverter.classForName(RecordConverter.java:224)
    at com.thoughtworks.xstream.converters.extended.RecordConverter.findClass(RecordConverter.java:231)
    at com.thoughtworks.xstream.converters.extended.RecordConverter.unmarshal(RecordConverter.java:156)
    at com.thoughtworks.xstream.core.TreeUnmarshaller.convert(TreeUnmarshaller.java:75)
    at com.thoughtworks.xstream.core.AbstractReferenceUnmarshaller.convert(AbstractReferenceUnmarshaller.java:76)
    at com.thoughtworks.xstream.core.TreeUnmarshaller.convertAnother(TreeUnmarshaller.java:69)
    at com.thoughtworks.xstream.converters.reflection.AbstractReflectionConverter.unmarshallField(AbstractReflectionConverter.java:487)
    at com.thoughtworks.xstream.converters.reflection.AbstractReflectionConverter.doUnmarshal(AbstractReflectionConverter.java:419)
    at com.thoughtworks.xstream.converters.reflection.AbstractReflectionConverter.unmarshal(AbstractReflectionConverter.java:275)
    at com.thoughtworks.xstream.core.TreeUnmarshaller.convert(TreeUnmarshaller.java:75)
    at com.thoughtworks.xstream.core.AbstractReferenceUnmarshaller.convert(AbstractReferenceUnmarshaller.java:76)
    at com.thoughtworks.xstream.core.TreeUnmarshaller.convertAnother(TreeUnmarshaller.java:69)
    at com.thoughtworks.xstream.core.TreeUnmarshaller.convertAnother(TreeUnmarshaller.java:53)
    at com.thoughtworks.xstream.core.TreeUnmarshaller.start(TreeUnmarshaller.java:144)
    at com.thoughtworks.xstream.core.AbstractTreeMarshallingStrategy.unmarshal(AbstractTreeMarshallingStrategy.java:34)
    at com.thoughtworks.xstream.XStream.unmarshal(XStream.java:1420)
    at com.thoughtworks.xstream.XStream.unmarshal(XStream.java:1396)
    at com.thoughtworks.xstream.XStream.fromXML(XStream.java:1289)
    at com.thoughtworks.xstream.converters.extended.RecordConverterTest.testMissingClassAttributeDoesNotCauseCrash(RecordConverterTest.java:358)

Caused by: java.lang.ClassNotFoundException: field1 at java.base/jdk.internal.loader.BuiltinClassLoader.loadClass(BuiltinClassLoader.java:641) at java.base/jdk.internal.loader.ClassLoaders$AppClassLoader.loadClass(ClassLoaders.java:188) at java.base/java.lang.ClassLoader.loadClass(ClassLoader.java:525) at java.base/java.lang.Class.forName0(Native Method) at java.base/java.lang.Class.forName(Class.java:375) at com.thoughtworks.xstream.converters.extended.RecordConverter.classForName(RecordConverter.java:222) ... 40 more