Skip to content

Commit

Permalink
Merge branch 'master' into improve-view-settings-2
Browse files Browse the repository at this point in the history
  • Loading branch information
@janfaracik
janfaracik authored Oct 12, 2024
2 parents 952873d + 925de03 commit 96d317e
Show file tree
Hide file tree
Showing 15 changed files with 479 additions and 93 deletions.
2 changes: 1 addition & 1 deletion ath.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
48 changes: 44 additions & 4 deletions core/src/main/java/hudson/util/RobustCollectionConverter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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}.
Expand All @@ -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.
* <p>Elements with invalid types will be omitted from deserialized collections and may result in an
* {@link OldDataMonitor} warning.
* <p>This type checking currently uses the erasure of the type argument, so for example, the element type for a
* {@code List<Optional<Integer>>} 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
Expand All @@ -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) {
Expand Down
57 changes: 53 additions & 4 deletions core/src/main/java/hudson/util/RobustMapConverter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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.
* <p>Map entries whose key or value has an invalid type will be omitted from deserialized maps and may result in
* an {@link OldDataMonitor} warning.
* <p>This type checking currently uses the erasure of the type argument, so for example, the value type for a
* {@code Map<String, Optional<Integer>>} 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();
Expand All @@ -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);
Expand All @@ -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) {
Expand Down
43 changes: 33 additions & 10 deletions core/src/main/java/hudson/util/RobustReflectionConverter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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.
Expand All @@ -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;
Expand Down Expand Up @@ -324,7 +326,8 @@ public Object doUnmarshal(final Object result, final HierarchicalStreamReader re
}
}

Map implicitCollectionsForCurrentObject = null;
Map<String, Collection<Object>> implicitCollectionsForCurrentObject = new HashMap<>();
Map<String, Class<?>> implicitCollectionElementTypesForCurrentObject = new HashMap<>();
while (reader.hasMoreChildren()) {
reader.moveDown();

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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<String, Collection<Object>> implicitCollections, Map<String, Class<?>> 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());
Expand All @@ -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) {
Expand Down
4 changes: 1 addition & 3 deletions core/src/main/java/jenkins/model/queue/ItemDeletion.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
2 changes: 1 addition & 1 deletion core/src/site/site.xml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
</breadcrumbs>
<head>
<![CDATA[
<script src="https://cdn.jsdelivr.net/npm/[email protected].0/polyfill-support.js"></script>
<script src="https://cdn.jsdelivr.net/npm/[email protected].1/polyfill-support.js"></script>
<script src="https://cdn.jsdelivr.net/npm/@webcomponents/[email protected]/webcomponents-loader.js"></script>
<script data="jio" src="https://cdn.jsdelivr.net/npm/@jenkinsci/jenkins-io-components/+esm" type="module"></script>
<script data="jio" nomodule="" src="https://cdn.jsdelivr.net/npm/@jenkinsci/jenkins-io-components"></script>
Expand Down
Loading

0 comments on commit 96d317e

Please sign in to comment.