Skip to content
This repository has been archived by the owner on Nov 7, 2020. It is now read-only.

pop unmarshalled type on exception #2

Closed
wants to merge 1 commit into from

Conversation

mbakhoff
Copy link

@mbakhoff mbakhoff commented Jun 6, 2017

bugfix for the following scenario:

deserializing the following xml from jenkins:

<matrix-project plugin="[email protected]">
  <publishers>
    <htmlpublisher.HtmlPublisher plugin="[email protected]">
    </htmlpublisher.HtmlPublisher>
    <hudson.plugins.performance.PerformancePublisher plugin="[email protected]">
    </hudson.plugins.performance.PerformancePublisher>
  </publishers>
</matrix-project>
  • start deserializing hudson.util.DescribableList (publishers)
    • push DescribableList on TreeUnmarshaller.types
    • DescribableList$ConverterImpl tries to collects the children into a list and creates the real DescribableList instance after
  • deserialize HtmlPublisher
  • start deserializing PerformancePublisher
    • push PerformancePublisher on TreeUnmarshaller.types
    • deserializing PerformancePublisher fails, but PerformancePublisher is not removed from TreeUnmarshaller.types
  • DescribableList$ConverterImpl tries to create DescribableList using (DescribableList)context.getRequiredType().newInstance() which tries to new PerformancePublisher()

Copy link
Member

@jglick jglick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Solves JENKINS-49054.

@@ -70,7 +70,6 @@ protected Object convert(Object parent, Class type, Converter converter) {
try {
types.push(type);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Best style to move this line outside the try block.

@@ -80,6 +79,9 @@ protected Object convert(Object parent, Class type, Converter converter) {
addInformationTo(conversionException, type, converter, parent);
throw conversionException;
}
finally {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: should not be on a new line.

@jglick
Copy link
Member

jglick commented Oct 23, 2020

To be closed once jenkinsci/jenkins#4944 is merged.

@daniel-beck daniel-beck closed this Nov 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants