[JENKINS-49054] Work around XStream bug by looking up list type before trying to unmarshal elements #3248
Conversation
…e trying to unmarshal elements.
|
This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation. |
| try { | ||
| DescribableList r = (DescribableList)context.getRequiredType().newInstance(); | ||
| DescribableList r = (DescribableList) context.getRequiredType().asSubclass(DescribableList.class).newInstance(); |
There was a problem hiding this comment.
asSubclass ensures that even if the problem occurs, we will get a ClassCastException, which is easier to diagnose than a NoSuchMethodException.
| try { | ||
| DescribableList r = (DescribableList)context.getRequiredType().newInstance(); | ||
| DescribableList r = (DescribableList) context.getRequiredType().asSubclass(DescribableList.class).newInstance(); | ||
| CopyOnWriteList core = copyOnWriteListConverter.unmarshal(reader, context); |
There was a problem hiding this comment.
The workaround consists of moving the unmarshal call after the getRequiredType call, so even if unmarshal corrupts this aspect of the context, we can still produce a meaningful error.
| XStream2 xs = new XStream2(); | ||
| xs.addCriticalField(Data.class, "list"); | ||
| String xml = xs.toXML(data); | ||
| data = (Data) xs.fromXML(xml); |
There was a problem hiding this comment.
Without the fix, we get something like
java.lang.InstantiationError
at hudson.util.DescribableList$ConverterImpl.unmarshal(DescribableList.java:280)
at com.thoughtworks.xstream.core.TreeUnmarshaller.convert(TreeUnmarshaller.java:72)
at com.thoughtworks.xstream.core.AbstractReferenceUnmarshaller.convert(AbstractReferenceUnmarshaller.java:65)
at com.thoughtworks.xstream.core.TreeUnmarshaller.convertAnother(TreeUnmarshaller.java:66)
at hudson.util.RobustReflectionConverter.unmarshalField(RobustReflectionConverter.java:393)
at hudson.util.RobustReflectionConverter.doUnmarshal(RobustReflectionConverter.java:331)
at hudson.util.RobustReflectionConverter.unmarshal(RobustReflectionConverter.java:270)
at com.thoughtworks.xstream.core.TreeUnmarshaller.convert(TreeUnmarshaller.java:72)
at com.thoughtworks.xstream.core.AbstractReferenceUnmarshaller.convert(AbstractReferenceUnmarshaller.java:65)
at com.thoughtworks.xstream.core.TreeUnmarshaller.convertAnother(TreeUnmarshaller.java:66)
at com.thoughtworks.xstream.core.TreeUnmarshaller.convertAnother(TreeUnmarshaller.java:50)
at com.thoughtworks.xstream.core.TreeUnmarshaller.start(TreeUnmarshaller.java:134)
at com.thoughtworks.xstream.core.AbstractTreeMarshallingStrategy.unmarshal(AbstractTreeMarshallingStrategy.java:32)
at com.thoughtworks.xstream.XStream.unmarshal(XStream.java:1189)
at hudson.util.XStream2.unmarshal(XStream2.java:147)
at hudson.util.XStream2.unmarshal(XStream2.java:118)
at com.thoughtworks.xstream.XStream.unmarshal(XStream.java:1173)
at com.thoughtworks.xstream.XStream.fromXML(XStream.java:1044)
at com.thoughtworks.xstream.XStream.fromXML(XStream.java:1035)
at hudson.util.DescribableListTest.unmarshalling(DescribableListTest.java:44)
at …
Caused by: java.lang.InstantiationException: hudson.util.DescribableListTest$Datum
at java.lang.Class.newInstance(Class.java:427)
at hudson.util.DescribableList$ConverterImpl.unmarshal(DescribableList.java:276)
... 43 more
Caused by: java.lang.NoSuchMethodException: hudson.util.DescribableListTest$Datum.<init>()
at java.lang.Class.getConstructor0(Class.java:3082)
at java.lang.Class.newInstance(Class.java:412)
... 44 more
| public Object fromString(String str) { | ||
| int val = Integer.parseInt(str); | ||
| if (val == 2) { | ||
| throw new IllegalStateException("oops"); |
There was a problem hiding this comment.
Just here to make for an artificial error in some element. Normally RobustReflectionConverter in Jenkins will handle these cases differently; I am not exactly sure how this happens in the field—because the original error is being masked by the NoSuchMethodException! Observed deserializing a FolderCredentialsProperty element of Folder.properties.
|
There is still some mess there BTW. @ndeloof forked XStream and deprecated the original https://github.com/jenkinsci/xstream repository, but he has not finished his work. Jenkins project still uses |
oleg-nenashev
added
the
ready-for-merge
See JENKINS-49054.
jenkinsci/xstream-fork#2 is the actual fix, and suffices to make the test pass. The change to
DescribableListis more of a workaround, and might not cover more exotic cases such as an error inside aDescribableListinside some other object inside anotherDescribableList.Proposed changelog entries
ClassCastExceptionorNoSuchMethodExceptioncould under certain circumstances mask the actual error when loading erroneous data from an XML file.Desired reviewers
@jenkinsci/code-reviewers @reviewbybees