diff --git a/ath.sh b/ath.sh
index 12eca9c947fb..7009d692b8d0 100644
--- a/ath.sh
+++ b/ath.sh
@@ -6,7 +6,7 @@ set -o xtrace
cd "$(dirname "$0")"
# https://github.com/jenkinsci/acceptance-test-harness/releases
-export ATH_VERSION=6016.v3a_e3864eb_993
+export ATH_VERSION=6038.v190f938efc87
if [[ $# -eq 0 ]]; then
export JDK=17
diff --git a/core/src/main/java/hudson/util/RobustCollectionConverter.java b/core/src/main/java/hudson/util/RobustCollectionConverter.java
index 64dbbc7d9e9a..f914d909be27 100644
--- a/core/src/main/java/hudson/util/RobustCollectionConverter.java
+++ b/core/src/main/java/hudson/util/RobustCollectionConverter.java
@@ -26,6 +26,7 @@
import com.thoughtworks.xstream.XStream;
import com.thoughtworks.xstream.XStreamException;
+import com.thoughtworks.xstream.converters.ConversionException;
import com.thoughtworks.xstream.converters.UnmarshallingContext;
import com.thoughtworks.xstream.converters.collections.CollectionConverter;
import com.thoughtworks.xstream.converters.reflection.ReflectionProvider;
@@ -34,11 +35,15 @@
import com.thoughtworks.xstream.io.HierarchicalStreamReader;
import com.thoughtworks.xstream.mapper.Mapper;
import com.thoughtworks.xstream.security.InputManipulationException;
+import edu.umd.cs.findbugs.annotations.CheckForNull;
+import hudson.diagnosis.OldDataMonitor;
+import java.lang.reflect.Type;
import java.util.Collection;
import java.util.concurrent.CopyOnWriteArrayList;
import java.util.concurrent.CopyOnWriteArraySet;
import java.util.logging.Logger;
import jenkins.util.xstream.CriticalXStreamException;
+import org.jvnet.tiger_types.Types;
/**
* {@link CollectionConverter} that ignores {@link XStreamException}.
@@ -52,14 +57,39 @@
@SuppressWarnings({"rawtypes", "unchecked"})
public class RobustCollectionConverter extends CollectionConverter {
private final SerializableConverter sc;
+ /**
+ * When available, this field holds the declared type of the collection being deserialized.
+ */
+ private final @CheckForNull Class> elementType;
public RobustCollectionConverter(XStream xs) {
- this(xs.getMapper(), xs.getReflectionProvider());
+ this(xs.getMapper(), xs.getReflectionProvider(), null);
}
public RobustCollectionConverter(Mapper mapper, ReflectionProvider reflectionProvider) {
+ this(mapper, reflectionProvider, null);
+ }
+
+ /**
+ * Creates a converter that will validate the types of collection elements during deserialization.
+ *
Elements with invalid types will be omitted from deserialized collections and may result in an
+ * {@link OldDataMonitor} warning.
+ *
This type checking currently uses the erasure of the type argument, so for example, the element type for a
+ * {@code List>} is just a raw {@code Optional}, so non-integer values inside of the optional
+ * would still deserialize successfully and the resulting optional would be included in the list.
+ *
+ * @see RobustReflectionConverter#unmarshalField
+ */
+ public RobustCollectionConverter(Mapper mapper, ReflectionProvider reflectionProvider, Type collectionType) {
super(mapper);
sc = new SerializableConverter(mapper, reflectionProvider, new ClassLoaderReference(null));
+ if (collectionType != null && Collection.class.isAssignableFrom(Types.erasure(collectionType))) {
+ var baseType = Types.getBaseClass(collectionType, Collection.class);
+ var typeArg = Types.getTypeArgument(baseType, 0, Object.class);
+ this.elementType = Types.erasure(typeArg);
+ } else {
+ this.elementType = null;
+ }
}
@Override
@@ -85,9 +115,19 @@ protected void populateCollection(HierarchicalStreamReader reader, Unmarshalling
reader.moveDown();
try {
Object item = readBareItem(reader, context, collection);
- long nanoNow = System.nanoTime();
- collection.add(item);
- XStream2SecurityUtils.checkForCollectionDoSAttack(context, nanoNow);
+ if (elementType != null && item != null && !elementType.isInstance(item)) {
+ var exception = new ConversionException("Invalid type for collection element");
+ // c.f. TreeUnmarshaller.addInformationTo
+ exception.add("required-type", elementType.getName());
+ exception.add("class", item.getClass().getName());
+ exception.add("converter-type", getClass().getName());
+ reader.appendErrors(exception);
+ RobustReflectionConverter.addErrorInContext(context, exception);
+ } else {
+ long nanoNow = System.nanoTime();
+ collection.add(item);
+ XStream2SecurityUtils.checkForCollectionDoSAttack(context, nanoNow);
+ }
} catch (CriticalXStreamException e) {
throw e;
} catch (InputManipulationException e) {
diff --git a/core/src/main/java/hudson/util/RobustMapConverter.java b/core/src/main/java/hudson/util/RobustMapConverter.java
index f845e38771cc..c802959d0d09 100644
--- a/core/src/main/java/hudson/util/RobustMapConverter.java
+++ b/core/src/main/java/hudson/util/RobustMapConverter.java
@@ -31,9 +31,13 @@
import com.thoughtworks.xstream.io.HierarchicalStreamReader;
import com.thoughtworks.xstream.mapper.Mapper;
import com.thoughtworks.xstream.security.InputManipulationException;
+import edu.umd.cs.findbugs.annotations.CheckForNull;
+import hudson.diagnosis.OldDataMonitor;
+import java.lang.reflect.Type;
import java.util.Map;
import java.util.logging.Logger;
import jenkins.util.xstream.CriticalXStreamException;
+import org.jvnet.tiger_types.Types;
/**
* Loads a {@link Map} while tolerating read errors on its keys and values.
@@ -42,13 +46,47 @@
final class RobustMapConverter extends MapConverter {
private static final Object ERROR = new Object();
+ /**
+ * When available, this field holds the declared type of the keys of the map being deserialized.
+ */
+ private final @CheckForNull Class> keyType;
+
+ /**
+ * When available, this field holds the declared type of the values of the map being deserialized.
+ */
+ private final @CheckForNull Class> valueType;
+
RobustMapConverter(Mapper mapper) {
+ this(mapper, null);
+ }
+
+ /**
+ * Creates a converter that will validate the types of map entry keys and values during deserialization.
+ *
Map entries whose key or value has an invalid type will be omitted from deserialized maps and may result in
+ * an {@link OldDataMonitor} warning.
+ *
This type checking currently uses the erasure of the type argument, so for example, the value type for a
+ * {@code Map>} is just a raw {@code Optional}, so non-integer values inside of the
+ * optional would still deserialize successfully and the resulting map entry would be included in the map.
+ *
+ * @see RobustReflectionConverter#unmarshalField
+ */
+ RobustMapConverter(Mapper mapper, Type mapType) {
super(mapper);
+ if (mapType != null && Map.class.isAssignableFrom(Types.erasure(mapType))) {
+ var baseType = Types.getBaseClass(mapType, Map.class);
+ var keyTypeArg = Types.getTypeArgument(baseType, 0, Object.class);
+ this.keyType = Types.erasure(keyTypeArg);
+ var valueTypeArg = Types.getTypeArgument(baseType, 1, Object.class);
+ this.valueType = Types.erasure(valueTypeArg);
+ } else {
+ this.keyType = null;
+ this.valueType = null;
+ }
}
@Override protected void putCurrentEntryIntoMap(HierarchicalStreamReader reader, UnmarshallingContext context, Map map, Map target) {
- Object key = read(reader, context, map);
- Object value = read(reader, context, map);
+ Object key = read(reader, context, map, keyType);
+ Object value = read(reader, context, map, valueType);
if (key != ERROR && value != ERROR) {
try {
long nanoNow = System.nanoTime();
@@ -64,7 +102,7 @@ final class RobustMapConverter extends MapConverter {
}
}
- private Object read(HierarchicalStreamReader reader, UnmarshallingContext context, Map map) {
+ private Object read(HierarchicalStreamReader reader, UnmarshallingContext context, Map map, @CheckForNull Class> expectedType) {
if (!reader.hasMoreChildren()) {
var exception = new ConversionException("Invalid map entry");
reader.appendErrors(exception);
@@ -73,7 +111,18 @@ private Object read(HierarchicalStreamReader reader, UnmarshallingContext contex
}
reader.moveDown();
try {
- return readBareItem(reader, context, map);
+ var object = readBareItem(reader, context, map);
+ if (expectedType != null && object != null && !expectedType.isInstance(object)) {
+ var exception = new ConversionException("Invalid type for map entry key/value");
+ // c.f. TreeUnmarshaller.addInformationTo
+ exception.add("required-type", expectedType.getName());
+ exception.add("class", object.getClass().getName());
+ exception.add("converter-type", getClass().getName());
+ reader.appendErrors(exception);
+ RobustReflectionConverter.addErrorInContext(context, exception);
+ return ERROR;
+ }
+ return object;
} catch (CriticalXStreamException x) {
throw x;
} catch (XStreamException | LinkageError x) {
diff --git a/core/src/main/java/hudson/util/RobustReflectionConverter.java b/core/src/main/java/hudson/util/RobustReflectionConverter.java
index d1bc500003e1..686aad13c342 100644
--- a/core/src/main/java/hudson/util/RobustReflectionConverter.java
+++ b/core/src/main/java/hudson/util/RobustReflectionConverter.java
@@ -48,6 +48,7 @@
import hudson.model.Saveable;
import hudson.security.ACL;
import java.lang.reflect.Field;
+import java.lang.reflect.Type;
import java.util.ArrayList;
import java.util.Collection;
import java.util.HashMap;
@@ -65,6 +66,7 @@
import jenkins.util.xstream.CriticalXStreamException;
import net.jcip.annotations.GuardedBy;
import org.acegisecurity.Authentication;
+import org.jvnet.tiger_types.Types;
/**
* Custom {@link ReflectionConverter} that handle errors more gracefully.
@@ -80,7 +82,7 @@
@SuppressWarnings({"rawtypes", "unchecked"})
public class RobustReflectionConverter implements Converter {
- private static /* non-final for Groovy */ boolean RECORD_FAILURES_FOR_ALL_AUTHENTICATIONS = SystemProperties.getBoolean(RobustReflectionConverter.class.getName() + ".recordFailuresForAllAuthentications", false);
+ static /* non-final for Groovy */ boolean RECORD_FAILURES_FOR_ALL_AUTHENTICATIONS = SystemProperties.getBoolean(RobustReflectionConverter.class.getName() + ".recordFailuresForAllAuthentications", false);
private static /* non-final for Groovy */ boolean RECORD_FAILURES_FOR_ADMINS = SystemProperties.getBoolean(RobustReflectionConverter.class.getName() + ".recordFailuresForAdmins", false);
protected final ReflectionProvider reflectionProvider;
@@ -324,7 +326,8 @@ public Object doUnmarshal(final Object result, final HierarchicalStreamReader re
}
}
- Map implicitCollectionsForCurrentObject = null;
+ Map> implicitCollectionsForCurrentObject = new HashMap<>();
+ Map> implicitCollectionElementTypesForCurrentObject = new HashMap<>();
while (reader.hasMoreChildren()) {
reader.moveDown();
@@ -365,7 +368,7 @@ public Object doUnmarshal(final Object result, final HierarchicalStreamReader re
reflectionProvider.writeField(result, fieldName, value, classDefiningField);
seenFields.add(classDefiningField, fieldName);
} else {
- implicitCollectionsForCurrentObject = writeValueToImplicitCollection(context, value, implicitCollectionsForCurrentObject, result, fieldName);
+ writeValueToImplicitCollection(reader, context, value, implicitCollectionsForCurrentObject, implicitCollectionElementTypesForCurrentObject, result, fieldName);
}
}
} catch (CriticalXStreamException e) {
@@ -451,18 +454,23 @@ private boolean fieldDefinedInClass(Object result, String attrName) {
protected Object unmarshalField(final UnmarshallingContext context, final Object result, Class type, Field field) {
Converter converter = mapper.getLocalConverter(field.getDeclaringClass(), field.getName());
+ if (converter == null) {
+ if (new RobustCollectionConverter(mapper, reflectionProvider).canConvert(type)) {
+ converter = new RobustCollectionConverter(mapper, reflectionProvider, field.getGenericType());
+ } else if (new RobustMapConverter(mapper).canConvert(type)) {
+ converter = new RobustMapConverter(mapper, field.getGenericType());
+ }
+ }
return context.convertAnother(result, type, converter);
}
- private Map writeValueToImplicitCollection(UnmarshallingContext context, Object value, Map implicitCollections, Object result, String itemFieldName) {
+ private void writeValueToImplicitCollection(HierarchicalStreamReader reader, UnmarshallingContext context, Object value, Map> implicitCollections, Map> implicitCollectionElementTypes, Object result, String itemFieldName) {
String fieldName = mapper.getFieldNameForItemTypeAndName(context.getRequiredType(), value.getClass(), itemFieldName);
if (fieldName != null) {
- if (implicitCollections == null) {
- implicitCollections = new HashMap(); // lazy instantiation
- }
- Collection collection = (Collection) implicitCollections.get(fieldName);
+ Collection collection = implicitCollections.get(fieldName);
if (collection == null) {
- Class fieldType = mapper.defaultImplementationOf(reflectionProvider.getFieldType(result, fieldName, null));
+ Field field = reflectionProvider.getField(result.getClass(), fieldName);
+ Class> fieldType = mapper.defaultImplementationOf(field.getType());
if (!Collection.class.isAssignableFrom(fieldType)) {
throw new ObjectAccessException("Field " + fieldName + " of " + result.getClass().getName() +
" is configured for an implicit Collection, but field is of type " + fieldType.getName());
@@ -473,10 +481,25 @@ private Map writeValueToImplicitCollection(UnmarshallingContext context, Object
collection = (Collection) pureJavaReflectionProvider.newInstance(fieldType);
reflectionProvider.writeField(result, fieldName, collection, null);
implicitCollections.put(fieldName, collection);
+ Type fieldGenericType = field.getGenericType();
+ Type elementGenericType = Types.getTypeArgument(Types.getBaseClass(fieldGenericType, Collection.class), 0, Object.class);
+ Class> elementType = Types.erasure(elementGenericType);
+ implicitCollectionElementTypes.put(fieldName, elementType);
+ }
+ Class> elementType = implicitCollectionElementTypes.getOrDefault(fieldName, Object.class);
+ if (!elementType.isInstance(value)) {
+ var exception = new ConversionException("Invalid element type for implicit collection for field: " + fieldName);
+ // c.f. TreeUnmarshaller.addInformationTo
+ exception.add("required-type", elementType.getName());
+ exception.add("class", value.getClass().getName());
+ exception.add("converter-type", getClass().getName());
+ reader.appendErrors(exception);
+ throw exception;
}
collection.add(value);
+ } else {
+ // TODO: Should we warn in this case? The value will be ignored.
}
- return implicitCollections;
}
private Class determineWhichClassDefinesField(HierarchicalStreamReader reader) {
diff --git a/core/src/main/java/jenkins/model/queue/ItemDeletion.java b/core/src/main/java/jenkins/model/queue/ItemDeletion.java
index a2d954fbc459..b278f4d24c93 100644
--- a/core/src/main/java/jenkins/model/queue/ItemDeletion.java
+++ b/core/src/main/java/jenkins/model/queue/ItemDeletion.java
@@ -266,12 +266,10 @@ public static void cancelBuildsInProgress(@NonNull Item initiatingItem) throws F
// comparison with executor.getCurrentExecutable() == executable currently should always be
// true as we no longer recycle Executors, but safer to future-proof in case we ever
// revisit recycling.
- if (!entry.getKey().isAlive()
+ if (!entry.getKey().isActive()
|| entry.getValue() != entry.getKey().getCurrentExecutable()) {
iterator.remove();
}
- // I don't know why, but we have to keep interrupting
- entry.getKey().interrupt(Result.ABORTED);
}
Thread.sleep(50L);
}
diff --git a/core/src/site/site.xml b/core/src/site/site.xml
index 56902cecd2a1..8b17895170df 100644
--- a/core/src/site/site.xml
+++ b/core/src/site/site.xml
@@ -6,7 +6,7 @@
+
diff --git a/core/src/test/java/hudson/util/RobustCollectionConverterTest.java b/core/src/test/java/hudson/util/RobustCollectionConverterTest.java
index 7786fb0833f7..57f2ad42af39 100644
--- a/core/src/test/java/hudson/util/RobustCollectionConverterTest.java
+++ b/core/src/test/java/hudson/util/RobustCollectionConverterTest.java
@@ -32,6 +32,8 @@
import static org.junit.Assert.assertTrue;
import com.thoughtworks.xstream.security.InputManipulationException;
+import hudson.model.Saveable;
+import java.io.IOException;
import java.time.Instant;
import java.util.ArrayList;
import java.util.Arrays;
@@ -42,10 +44,24 @@
import java.util.Map;
import java.util.Set;
import jenkins.util.xstream.CriticalXStreamException;
+import org.junit.After;
+import org.junit.Before;
import org.junit.Test;
import org.jvnet.hudson.test.Issue;
public class RobustCollectionConverterTest {
+ private final boolean originalRecordFailures = RobustReflectionConverter.RECORD_FAILURES_FOR_ALL_AUTHENTICATIONS;
+
+ @Before
+ public void before() {
+ RobustReflectionConverter.RECORD_FAILURES_FOR_ALL_AUTHENTICATIONS = true;
+ }
+
+ @After
+ public void after() {
+ RobustReflectionConverter.RECORD_FAILURES_FOR_ALL_AUTHENTICATIONS = originalRecordFailures;
+ }
+
@Test
public void workingByDefaultWithSimplePayload() {
XStream2 xstream2 = new XStream2();
@@ -173,4 +189,58 @@ private Set