Skip to content

Commit

Permalink
[FIXED JENKINS-21024] Catch a full range of XStreamException’s during…
Browse files Browse the repository at this point in the history 
… deserialization, including rethrown exceptions from readResolve.

(cherry picked from commit c350811)

Conflicts:
	changelog.html
	core/src/main/java/hudson/util/RobustReflectionConverter.java
  • Loading branch information
@jglick @olivergondza
jglick authored and olivergondza committed Feb 15, 2014
1 parent 14214d9 commit 15fcf7c
Show file tree
Hide file tree
Showing 7 changed files with 71 additions and 29 deletions.
9 changes: 2 additions & 7 deletions core/src/main/java/hudson/util/CopyOnWriteList.java
View file
Expand Up @@ -23,7 +23,7 @@
*/
package hudson.util;

import com.thoughtworks.xstream.mapper.CannotResolveClassException;
import com.thoughtworks.xstream.XStreamException;
import com.thoughtworks.xstream.converters.Converter;
import com.thoughtworks.xstream.converters.MarshallingContext;
import com.thoughtworks.xstream.converters.UnmarshallingContext;
Expand All @@ -38,8 +38,6 @@
import java.util.Iterator;
import java.util.List;
import java.util.Arrays;
import static java.util.logging.Level.FINE;
import java.util.logging.Logger;


/**
Expand Down Expand Up @@ -192,11 +190,9 @@ public CopyOnWriteList unmarshal(HierarchicalStreamReader reader, UnmarshallingC
try {
Object item = readItem(reader, context, items);
items.add(item);
} catch (CannotResolveClassException e) {
LOGGER.log(FINE, "Failed to resolve class", e);
} catch (XStreamException e) {
RobustReflectionConverter.addErrorInContext(context, e);
} catch (LinkageError e) {
LOGGER.log(FINE, "Failed to resolve class", e);
RobustReflectionConverter.addErrorInContext(context, e);
}
reader.moveUp();
Expand All @@ -206,5 +202,4 @@ public CopyOnWriteList unmarshal(HierarchicalStreamReader reader, UnmarshallingC
}
}

private static final Logger LOGGER = Logger.getLogger(CopyOnWriteList.class.getName());
}
11 changes: 3 additions & 8 deletions core/src/main/java/hudson/util/RobustCollectionConverter.java
View file
Expand Up @@ -23,24 +23,22 @@
*/
package hudson.util;

import com.thoughtworks.xstream.mapper.CannotResolveClassException;
import com.thoughtworks.xstream.converters.UnmarshallingContext;
import com.thoughtworks.xstream.converters.collections.CollectionConverter;
import com.thoughtworks.xstream.converters.reflection.ReflectionProvider;
import com.thoughtworks.xstream.converters.reflection.SerializableConverter;
import com.thoughtworks.xstream.io.HierarchicalStreamReader;
import com.thoughtworks.xstream.mapper.Mapper;
import com.thoughtworks.xstream.XStream;
import com.thoughtworks.xstream.XStreamException;

import java.util.Collection;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.CopyOnWriteArraySet;
import static java.util.logging.Level.FINE;
import java.util.logging.Logger;


/**
* {@link CollectionConverter} that ignores {@link CannotResolveClassException}.
* {@link CollectionConverter} that ignores {@link XStreamException}.
*
* <p>
* This allows Hudson to load XML files that contain non-existent classes
Expand Down Expand Up @@ -84,16 +82,13 @@ protected void populateCollection(HierarchicalStreamReader reader, Unmarshalling
try {
Object item = readItem(reader, context, collection);
collection.add(item);
} catch (CannotResolveClassException e) {
LOGGER.log(FINE, "Failed to resolve class", e);
} catch (XStreamException e) {
RobustReflectionConverter.addErrorInContext(context, e);
} catch (LinkageError e) {
LOGGER.log(FINE, "Failed to resolve class", e);
RobustReflectionConverter.addErrorInContext(context, e);
}
reader.moveUp();
}
}

private static final Logger LOGGER = Logger.getLogger(RobustCollectionConverter.class.getName());
}
14 changes: 3 additions & 11 deletions core/src/main/java/hudson/util/RobustReflectionConverter.java
View file
Expand Up @@ -23,12 +23,12 @@
*/
package hudson.util;

import com.thoughtworks.xstream.XStreamException;
import com.thoughtworks.xstream.converters.ConversionException;
import com.thoughtworks.xstream.converters.Converter;
import com.thoughtworks.xstream.converters.MarshallingContext;
import com.thoughtworks.xstream.converters.SingleValueConverter;
import com.thoughtworks.xstream.converters.UnmarshallingContext;
import com.thoughtworks.xstream.converters.reflection.MissingFieldException;
import com.thoughtworks.xstream.converters.reflection.ObjectAccessException;
import com.thoughtworks.xstream.converters.reflection.PureJavaReflectionProvider;
import com.thoughtworks.xstream.converters.reflection.ReflectionConverter;
Expand All @@ -38,7 +38,6 @@
import com.thoughtworks.xstream.io.ExtendedHierarchicalStreamWriterHelper;
import com.thoughtworks.xstream.io.HierarchicalStreamReader;
import com.thoughtworks.xstream.io.HierarchicalStreamWriter;
import com.thoughtworks.xstream.mapper.CannotResolveClassException;
import com.thoughtworks.xstream.mapper.Mapper;
import hudson.diagnosis.OldDataMonitor;
import hudson.model.Saveable;
Expand Down Expand Up @@ -308,23 +307,15 @@ public Object doUnmarshal(final Object result, final HierarchicalStreamReader re
implicitCollectionsForCurrentObject = writeValueToImplicitCollection(context, value, implicitCollectionsForCurrentObject, result, fieldName);
}
}
} catch (MissingFieldException e) {
} catch (XStreamException e) {
if (critical) {
throw e;
}
LOGGER.log(FINE, "Skipping a non-existent field " + e.getFieldName(), e);
addErrorInContext(context, e);
} catch (CannotResolveClassException e) {
if (critical) {
throw e;
}
LOGGER.log(FINE, "Skipping a non-existent type", e);
addErrorInContext(context, e);
} catch (LinkageError e) {
if (critical) {
throw e;
}
LOGGER.log(FINE, "Failed to resolve a type", e);
addErrorInContext(context, e);
}

Expand All @@ -340,6 +331,7 @@ public Object doUnmarshal(final Object result, final HierarchicalStreamReader re
}

public static void addErrorInContext(UnmarshallingContext context, Throwable e) {
LOGGER.log(FINE, "Failed to load", e);
ArrayList<Throwable> list = (ArrayList<Throwable>)context.get("ReadError");
if (list == null)
context.put("ReadError", list = new ArrayList<Throwable>());
Expand Down
4 changes: 2 additions & 2 deletions core/src/main/java/hudson/util/xstream/ImmutableListConverter.java
View file
Expand Up @@ -26,12 +26,12 @@

import com.google.common.collect.ImmutableList;
import com.thoughtworks.xstream.XStream;
import com.thoughtworks.xstream.XStreamException;
import com.thoughtworks.xstream.converters.UnmarshallingContext;
import com.thoughtworks.xstream.converters.collections.CollectionConverter;
import com.thoughtworks.xstream.converters.reflection.ReflectionProvider;
import com.thoughtworks.xstream.converters.reflection.SerializableConverter;
import com.thoughtworks.xstream.io.HierarchicalStreamReader;
import com.thoughtworks.xstream.mapper.CannotResolveClassException;
import com.thoughtworks.xstream.mapper.Mapper;

import hudson.util.RobustReflectionConverter;
Expand Down Expand Up @@ -76,7 +76,7 @@ public Object unmarshal(HierarchicalStreamReader reader, UnmarshallingContext co
try {
Object item = readItem(reader, context, items);
items.add(item);
} catch (CannotResolveClassException e) {
} catch (XStreamException e) {
RobustReflectionConverter.addErrorInContext(context, e);
} catch (LinkageError e) {
RobustReflectionConverter.addErrorInContext(context, e);
Expand Down
6 changes: 5 additions & 1 deletion test/src/test/groovy/hudson/model/RunMapTest.groovy
View file
Expand Up @@ -53,7 +53,11 @@ class RunMapTest extends HudsonTestCase {
b.save()

p._getRuns().purgeCache()
assert p.getBuildByNumber(b.number)==null
b = p.getBuildByNumber(b.number)
// Original test assumed that b == null, but after JENKINS-21024 this is no longer true,
// so this may not really be testing anything interesting:
assert b != null
assert b.getAction(BombAction.class) == null
assert bombed
}

Expand Down
56 changes: 56 additions & 0 deletions test/src/test/java/hudson/util/RobustReflectionConverterTest.java
View file
@@ -0,0 +1,56 @@
/*
* The MIT License
*
* Copyright 2013 Jesse Glick.
*
* Permission is hereby granted, free of charge, to any person obtaining a copy
* of this software and associated documentation files (the "Software"), to deal
* in the Software without restriction, including without limitation the rights
* to use, copy, modify, merge, publish, distribute, sublicense, and/or sell
* copies of the Software, and to permit persons to whom the Software is
* furnished to do so, subject to the following conditions:
*
* The above copyright notice and this permission notice shall be included in
* all copies or substantial portions of the Software.
*
* THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
* IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
* FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE
* AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER
* LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM,
* OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN
* THE SOFTWARE.
*/

package hudson.util;

import hudson.diagnosis.OldDataMonitor;
import hudson.model.FreeStyleProject;
import hudson.model.Saveable;
import java.util.Collections;
import java.util.Map;
import static org.junit.Assert.*;
import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.Bug;
import org.jvnet.hudson.test.JenkinsRule;
import org.jvnet.hudson.test.recipes.LocalData;

public class RobustReflectionConverterTest {

@Rule public JenkinsRule r = new JenkinsRule();

@Bug(21024)
@LocalData
@Test public void randomExceptionsReported() throws Exception {
FreeStyleProject p = r.jenkins.getItemByFullName("j", FreeStyleProject.class);
assertNotNull(p);
assertEquals(Collections.emptyMap(), p.getTriggers());
OldDataMonitor odm = (OldDataMonitor) r.jenkins.getAdministrativeMonitor("OldData");
Map<Saveable,OldDataMonitor.VersionRange> data = odm.getData();
assertEquals(Collections.singleton(p), data.keySet());
String text = data.values().iterator().next().extra;
assertTrue(text, text.contains("Could not call hudson.triggers.TimerTrigger.readResolve"));
}

}
Binary file added ...src/test/resources/hudson/util/RobustReflectionConverterTest/randomExceptionsReported.zip
View file
Binary file not shown.

0 comments on commit 15fcf7c

Please sign in to comment.