Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

toby1984
Copy link
Contributor

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

@toby1984
Copy link
Contributor Author

Any chance this PR will get merged and XStream 1.5.x released ? We'd like to get rid of your in-house patched version.

@joehni
Copy link
Member

joehni commented Dec 21, 2024

As already said on the list: The RecordConverter needs a complete rewrite, since it does not support a large set of standard functionalities.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants