diff --git a/pom.xml b/pom.xml index 0fe021f53..8179f8afc 100644 --- a/pom.xml +++ b/pom.xml @@ -673,6 +673,12 @@ ${version.org.ops4j.pax.exam} test + + io.github.classgraph + classgraph + ${version.io.github.classgraph} + test + @@ -1095,6 +1101,7 @@ 1.11 3.8.1 2.2.8 + 4.8.87 1.2.1 1.3.4 2.3.2 diff --git a/xstream/pom.xml b/xstream/pom.xml index 736367e37..e3155278b 100644 --- a/xstream/pom.xml +++ b/xstream/pom.xml @@ -137,6 +137,11 @@ commons-lang3 test + + io.github.classgraph + classgraph + test + diff --git a/xstream/src/java/com/thoughtworks/xstream/converters/collections/PropertiesConverter.java b/xstream/src/java/com/thoughtworks/xstream/converters/collections/PropertiesConverter.java index dd401251e..4a09e1a89 100644 --- a/xstream/src/java/com/thoughtworks/xstream/converters/collections/PropertiesConverter.java +++ b/xstream/src/java/com/thoughtworks/xstream/converters/collections/PropertiesConverter.java @@ -6,7 +6,7 @@ * The software in this package is published under the terms of the BSD * style license a copy of which has been included with this distribution in * the LICENSE.txt file. - * + * * Created on 23. February 2004 by Joe Walnes */ package com.thoughtworks.xstream.converters.collections; @@ -35,13 +35,12 @@ * permissions for SecurityManager.checkPackageAccess, SecurityManager.checkMemberAccess(this, EnumSet.MEMBER) and * ReflectPermission("suppressAccessChecks"). *

- * + * * @author Joe Walnes * @author Kevin Ring */ public class PropertiesConverter implements Converter { - private final static Field defaultsField = Fields.locate(Properties.class, Properties.class, false); private final boolean sort; public PropertiesConverter() { @@ -67,8 +66,8 @@ public void marshal(final Object source, final HierarchicalStreamWriter writer, writer.addAttribute("value", entry.getValue().toString()); writer.endNode(); } - if (defaultsField != null) { - final Properties defaults = (Properties)Fields.read(defaultsField, properties); + if (Reflections.defaultsField != null) { + final Properties defaults = (Properties)Fields.read(Reflections.defaultsField, properties); if (defaults != null) { writer.startNode("defaults"); marshal(defaults, writer, context); @@ -101,4 +100,7 @@ public Object unmarshal(final HierarchicalStreamReader reader, final Unmarshalli } } + private static class Reflections { + private final static Field defaultsField = Fields.locate(Properties.class, Properties.class, false); + } } diff --git a/xstream/src/java/com/thoughtworks/xstream/converters/collections/TreeMapConverter.java b/xstream/src/java/com/thoughtworks/xstream/converters/collections/TreeMapConverter.java index 02b7140a8..debaba7b0 100644 --- a/xstream/src/java/com/thoughtworks/xstream/converters/collections/TreeMapConverter.java +++ b/xstream/src/java/com/thoughtworks/xstream/converters/collections/TreeMapConverter.java @@ -49,7 +49,6 @@ public int compare(final Comparable o1, final Comparable o2) { @SuppressWarnings("rawtypes") private final static Comparator NULL_MARKER = new NullComparator(); - private final static Field comparatorField = Fields.locate(TreeMap.class, Comparator.class, false); public TreeMapConverter(final Mapper mapper) { super(mapper, TreeMap.class); @@ -75,7 +74,7 @@ protected void marshalComparator(final Comparator comparator, final Hierarchi @Override public Object unmarshal(final HierarchicalStreamReader reader, final UnmarshallingContext context) { - TreeMap result = comparatorField != null ? new TreeMap<>() : null; + TreeMap result = Reflections.comparatorField != null ? new TreeMap<>() : null; @SuppressWarnings("unchecked") final Comparator comparator = (Comparator)unmarshalComparator(reader, context, result); if (result == null) { @@ -125,14 +124,14 @@ protected void populateTreeMap(final HierarchicalStreamReader reader, final Unma final TreeMap typedResult = (TreeMap)result; try { if (JVM.hasOptimizedTreeMapPutAll()) { - if (comparator != null && comparatorField != null) { - comparatorField.set(result, comparator); + if (comparator != null && Reflections.comparatorField != null) { + Reflections.comparatorField.set(result, comparator); } typedResult.putAll(sortedMap); // internal optimization will not call comparator - } else if (comparatorField != null) { - comparatorField.set(result, sortedMap.comparator()); + } else if (Reflections.comparatorField != null) { + Reflections.comparatorField.set(result, sortedMap.comparator()); typedResult.putAll(sortedMap); // "sort" by index - comparatorField.set(result, comparator); + Reflections.comparatorField.set(result, comparator); } else { typedResult.putAll(sortedMap); // will use comparator for already sorted map } @@ -140,4 +139,8 @@ protected void populateTreeMap(final HierarchicalStreamReader reader, final Unma throw new ObjectAccessException("Cannot set comparator of TreeMap", e); } } + + private static class Reflections { + private final static Field comparatorField = Fields.locate(TreeMap.class, Comparator.class, false); + } } diff --git a/xstream/src/java/com/thoughtworks/xstream/converters/collections/TreeSetConverter.java b/xstream/src/java/com/thoughtworks/xstream/converters/collections/TreeSetConverter.java index 02c497281..1a6e03aed 100644 --- a/xstream/src/java/com/thoughtworks/xstream/converters/collections/TreeSetConverter.java +++ b/xstream/src/java/com/thoughtworks/xstream/converters/collections/TreeSetConverter.java @@ -42,43 +42,6 @@ */ public class TreeSetConverter extends CollectionConverter { private transient TreeMapConverter treeMapConverter; - private final static Field sortedMapField; - private final static Object constantValue; - - static { - Object value = null; - sortedMapField = JVM.hasOptimizedTreeSetAddAll() ? Fields.locate(TreeSet.class, SortedMap.class, false) : null; - if (sortedMapField != null) { - final TreeSet set = new TreeSet<>(); - set.add("1"); - set.add("2"); - - Map backingMap = null; - try { - @SuppressWarnings("unchecked") - final Map map = (Map)sortedMapField.get(set); - backingMap = map; - } catch (final IllegalAccessException e) { - // give up; - } - if (backingMap != null) { - final Object[] values = backingMap.values().toArray(); - if (values[0] == values[1]) { - value = values[0]; - } - } - } else { - final Field valueField = Fields.locate(TreeSet.class, Object.class, true); - if (valueField != null) { - try { - value = valueField.get(null); - } catch (final IllegalAccessException e) { - // give up; - } - } - } - constantValue = value; - } public TreeSetConverter(final Mapper mapper) { super(mapper, TreeSet.class); @@ -100,11 +63,11 @@ public Object unmarshal(final HierarchicalStreamReader reader, final Unmarshalli final boolean inFirstElement = unmarshalledComparator instanceof Mapper.Null; @SuppressWarnings("unchecked") final Comparator comparator = inFirstElement ? null : (Comparator)unmarshalledComparator; - if (sortedMapField != null) { + if (Reflections.sortedMapField != null) { final TreeSet possibleResult = comparator == null ? new TreeSet<>() : new TreeSet<>(comparator); Object backingMap = null; try { - backingMap = sortedMapField.get(possibleResult); + backingMap = Reflections.sortedMapField.get(possibleResult); } catch (final IllegalAccessException e) { throw new ObjectAccessException("Cannot get backing map of TreeSet", e); } @@ -146,7 +109,8 @@ protected void populateMap(final HierarchicalStreamReader reader, final Unmarsha public boolean add(final Object object) { @SuppressWarnings("unchecked") final Map collectionTarget = (Map)target; - return collectionTarget.put(object, constantValue != null ? constantValue : object) != null; + return collectionTarget.put( + object, Reflections.constantValue != null ? Reflections.constantValue : object) != null; } @Override @@ -173,4 +137,46 @@ protected void putCurrentEntryIntoMap(final HierarchicalStreamReader reader, }; return this; } + + private static class Reflections { + + private final static Field sortedMapField; + private final static Object constantValue; + + static { + Object value = null; + sortedMapField = JVM.hasOptimizedTreeSetAddAll() + ? Fields.locate(TreeSet.class, SortedMap.class, false) : null; + if (sortedMapField != null) { + final TreeSet set = new TreeSet<>(); + set.add("1"); + set.add("2"); + + Map backingMap = null; + try { + @SuppressWarnings("unchecked") + final Map map = (Map)sortedMapField.get(set); + backingMap = map; + } catch (final IllegalAccessException e) { + // give up; + } + if (backingMap != null) { + final Object[] values = backingMap.values().toArray(); + if (values[0] == values[1]) { + value = values[0]; + } + } + } else { + final Field valueField = Fields.locate(TreeSet.class, Object.class, true); + if (valueField != null) { + try { + value = valueField.get(null); + } catch (final IllegalAccessException e) { + // give up; + } + } + } + constantValue = value; + } + } } diff --git a/xstream/src/java/com/thoughtworks/xstream/converters/enums/EnumMapConverter.java b/xstream/src/java/com/thoughtworks/xstream/converters/enums/EnumMapConverter.java index ed1ac84ce..310310f6a 100644 --- a/xstream/src/java/com/thoughtworks/xstream/converters/enums/EnumMapConverter.java +++ b/xstream/src/java/com/thoughtworks/xstream/converters/enums/EnumMapConverter.java @@ -6,7 +6,7 @@ * The software in this package is published under the terms of the BSD * style license a copy of which has been included with this distribution in * the LICENSE.txt file. - * + * * Created on 06. April 2005 by Joe Walnes */ @@ -31,12 +31,11 @@ * If a {@link SecurityManager} is set, the converter will only work with permissions for SecurityManager.checkPackageAccess, * SecurityManager.checkMemberAccess(this, EnumSet.MEMBER) and ReflectPermission("suppressAccessChecks"). *

- * + * * @author Joe Walnes */ public class EnumMapConverter extends MapConverter { - private final static Field typeField = Fields.locate(EnumMap.class, Class.class, false); public EnumMapConverter(final Mapper mapper) { super(mapper); @@ -44,12 +43,12 @@ public EnumMapConverter(final Mapper mapper) { @Override public boolean canConvert(final Class type) { - return typeField != null && type == EnumMap.class; + return type == EnumMap.class && Reflections.typeField != null; } @Override public void marshal(final Object source, final HierarchicalStreamWriter writer, final MarshallingContext context) { - final Class type = (Class)Fields.read(typeField, source); + final Class type = (Class)Fields.read(Reflections.typeField, source); final String attributeName = mapper().aliasForSystemAttribute("enum-type"); if (attributeName != null) { writer.addAttribute(attributeName, mapper().serializedClass(type)); @@ -69,4 +68,8 @@ public Object unmarshal(final HierarchicalStreamReader reader, final Unmarshalli populateMap(reader, context, map); return map; } + + private static class Reflections { + private final static Field typeField = Fields.locate(EnumMap.class, Class.class, false); + } } diff --git a/xstream/src/java/com/thoughtworks/xstream/converters/enums/EnumSetConverter.java b/xstream/src/java/com/thoughtworks/xstream/converters/enums/EnumSetConverter.java index 0089fe43c..a20a04b75 100644 --- a/xstream/src/java/com/thoughtworks/xstream/converters/enums/EnumSetConverter.java +++ b/xstream/src/java/com/thoughtworks/xstream/converters/enums/EnumSetConverter.java @@ -6,7 +6,7 @@ * The software in this package is published under the terms of the BSD * style license a copy of which has been included with this distribution in * the LICENSE.txt file. - * + * * Created on 06. April 2005 by Joe Walnes */ @@ -31,13 +31,12 @@ * If a SecurityManager is set, the converter will only work with permissions for SecurityManager.checkPackageAccess, * SecurityManager.checkMemberAccess(this, EnumSet.MEMBER) and ReflectPermission("suppressAccessChecks"). *

- * + * * @author Joe Walnes * @author Jörg Schaible */ public class EnumSetConverter implements Converter { - private final static Field typeField = Fields.locate(EnumSet.class, Class.class, false); private final Mapper mapper; public EnumSetConverter(final Mapper mapper) { @@ -46,13 +45,13 @@ public EnumSetConverter(final Mapper mapper) { @Override public boolean canConvert(final Class type) { - return typeField != null && type != null && EnumSet.class.isAssignableFrom(type); + return type != null && EnumSet.class.isAssignableFrom(type) && Reflections.typeField != null; } @Override public void marshal(final Object source, final HierarchicalStreamWriter writer, final MarshallingContext context) { final EnumSet set = (EnumSet)source; - final Class enumTypeForSet = (Class)Fields.read(typeField, set); + final Class enumTypeForSet = (Class)Fields.read(Reflections.typeField, set); final String attributeName = mapper.aliasForSystemAttribute("enum-type"); if (attributeName != null) { writer.addAttribute(attributeName, mapper.serializedClass(enumTypeForSet)); @@ -99,4 +98,7 @@ private > EnumSet create(final Class type, final String return set; } + private static class Reflections { + private final static Field typeField = Fields.locate(EnumSet.class, Class.class, false); + } } diff --git a/xstream/src/java/com/thoughtworks/xstream/converters/extended/DynamicProxyConverter.java b/xstream/src/java/com/thoughtworks/xstream/converters/extended/DynamicProxyConverter.java index 868635264..a20e63725 100644 --- a/xstream/src/java/com/thoughtworks/xstream/converters/extended/DynamicProxyConverter.java +++ b/xstream/src/java/com/thoughtworks/xstream/converters/extended/DynamicProxyConverter.java @@ -6,7 +6,7 @@ * The software in this package is published under the terms of the BSD * style license a copy of which has been included with this distribution in * the LICENSE.txt file. - * + * * Created on 25. March 2004 by Joe Walnes */ package com.thoughtworks.xstream.converters.extended; @@ -32,20 +32,13 @@ /** * Converts a dynamic proxy to XML, storing the implemented interfaces and handler. - * + * * @author Joe Walnes */ public class DynamicProxyConverter implements Converter { private final ClassLoaderReference classLoaderReference; private final Mapper mapper; - private static final Field HANDLER = Fields.locate(Proxy.class, InvocationHandler.class, false); - private static final InvocationHandler DUMMY = new InvocationHandler() { - @Override - public Object invoke(final Object proxy, final Method method, final Object[] args) throws Throwable { - return null; - } - }; /** * @deprecated As of 1.4.5 use {@link #DynamicProxyConverter(Mapper, ClassLoaderReference)} @@ -57,7 +50,7 @@ public DynamicProxyConverter(final Mapper mapper) { /** * Construct a DynamicProxyConverter. - * + * * @param mapper the Mapper chain * @param classLoaderReference the reference to the {@link ClassLoader} of the XStream instance * @since 1.4.5 @@ -127,16 +120,27 @@ public Object unmarshal(final HierarchicalStreamReader reader, final Unmarshalli final Class[] interfacesAsArray = new Class[interfaces.size()]; interfaces.toArray(interfacesAsArray); Object proxy = null; - if (HANDLER != null) { // we will not be able to resolve references to the proxy - proxy = Proxy.newProxyInstance(classLoaderReference.getReference(), interfacesAsArray, DUMMY); + if (Reflections.HANDLER != null) { // we will not be able to resolve references to the proxy + proxy = Proxy.newProxyInstance(classLoaderReference.getReference(), interfacesAsArray, Reflections.DUMMY); } handler = (InvocationHandler)context.convertAnother(proxy, handlerType); reader.moveUp(); - if (HANDLER != null) { - Fields.write(HANDLER, proxy, handler); + if (Reflections.HANDLER != null) { + Fields.write(Reflections.HANDLER, proxy, handler); } else { proxy = Proxy.newProxyInstance(classLoaderReference.getReference(), interfacesAsArray, handler); } return proxy; } + + private static class Reflections { + + private static final Field HANDLER = Fields.locate(Proxy.class, InvocationHandler.class, false); + private static final InvocationHandler DUMMY = new InvocationHandler() { + @Override + public Object invoke(final Object proxy, final Method method, final Object[] args) throws Throwable { + return null; + } + }; + } } diff --git a/xstream/src/java/com/thoughtworks/xstream/converters/reflection/AbstractAttributedCharacterIteratorAttributeConverter.java b/xstream/src/java/com/thoughtworks/xstream/converters/reflection/AbstractAttributedCharacterIteratorAttributeConverter.java index e85bee6f0..b7645aa7b 100644 --- a/xstream/src/java/com/thoughtworks/xstream/converters/reflection/AbstractAttributedCharacterIteratorAttributeConverter.java +++ b/xstream/src/java/com/thoughtworks/xstream/converters/reflection/AbstractAttributedCharacterIteratorAttributeConverter.java @@ -17,6 +17,7 @@ import java.text.AttributedCharacterIterator; import java.util.HashMap; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; import com.thoughtworks.xstream.converters.ConversionException; import com.thoughtworks.xstream.converters.basic.AbstractSingleValueConverter; @@ -34,26 +35,9 @@ public class AbstractAttributedCharacterIteratorAttributeConverter> instanceMaps = - new HashMap<>(); - private static final Method getName; - - static { - Method method = null; - try { - method = AttributedCharacterIterator.Attribute.class.getDeclaredMethod("getName", (Class[])null); - if (!method.isAccessible()) { - method.setAccessible(true); - } - } catch (final SecurityException e) { - // ignore for now - } catch (final NoSuchMethodException e) { - // ignore for now - } - getName = method; - } + new ConcurrentHashMap<>(); private final Class type; - private transient Map attributeMap; public AbstractAttributedCharacterIteratorAttributeConverter(final Class type) { super(); @@ -63,12 +47,11 @@ public AbstractAttributedCharacterIteratorAttributeConverter(final Class type) { - return type == this.type && !attributeMap.isEmpty(); + return type == this.type && !getAttributeMap().isEmpty(); } @Override @@ -80,9 +63,9 @@ public String toString(final Object source) { private String getName(final AttributedCharacterIterator.Attribute attribute) { Exception ex = null; - if (getName != null) { + if (Reflections.getName != null) { try { - return (String)getName.invoke(attribute); + return (String)Reflections.getName.invoke(attribute); } catch (final IllegalAccessException e) { ex = e; } catch (final InvocationTargetException e) { @@ -101,8 +84,9 @@ private String getName(final AttributedCharacterIterator.Attribute attribute) { @Override public Object fromString(final String str) { - if (attributeMap.containsKey(str)) { - return attributeMap.get(str); + T attr = getAttributeMap().get(str); + if (attr != null) { + return attr; } final ConversionException exception = new ConversionException("Cannot find attribute"); exception.add("attribute-type", type.getName()); @@ -110,50 +94,69 @@ public Object fromString(final String str) { throw exception; } - private Object readResolve() { + private Map getAttributeMap() { @SuppressWarnings("unchecked") - final Map typedMap = (Map)instanceMaps.get(type.getName()); - attributeMap = typedMap; - if (attributeMap == null) { - attributeMap = new HashMap<>(); - final Field instanceMap = Fields.locate(type, Map.class, true); - if (instanceMap != null) { - try { - @SuppressWarnings("unchecked") - final Map map = (Map)Fields.read(instanceMap, null); - if (map != null) { - boolean valid = true; - for (final Map.Entry entry : map.entrySet()) { - valid = entry.getKey().getClass() == String.class && entry.getValue().getClass() == type; - } - if (valid) { - attributeMap.putAll(map); - } + final Map map = (Map) instanceMaps.computeIfAbsent(type.getName(), t -> buildAttributeMap(type)); + return map; + } + + private Map buildAttributeMap(Class type) { + final Map attributeMap = new HashMap<>(); + final Field instanceMap = Fields.locate(type, Map.class, true); + if (instanceMap != null) { + try { + @SuppressWarnings("unchecked") + final Map map = (Map)Fields.read(instanceMap, null); + if (map != null) { + boolean valid = true; + for (final Map.Entry entry : map.entrySet()) { + valid = entry.getKey().getClass() == String.class && entry.getValue().getClass() == type; + } + if (valid) { + attributeMap.putAll(map); } - } catch (final ObjectAccessException e) { } + } catch (final ObjectAccessException e) { } - if (attributeMap.isEmpty()) { - try { - final Field[] fields = type.getDeclaredFields(); - for (final Field field : fields) { - if (field.getType() == type == Modifier.isStatic(field.getModifiers())) { - @SuppressWarnings("unchecked") - final T attribute = (T)Fields.read(field, null); - attributeMap.put(toString(attribute), attribute); - } + } + if (attributeMap.isEmpty()) { + try { + final Field[] fields = type.getDeclaredFields(); + for (final Field field : fields) { + if (field.getType() == type == Modifier.isStatic(field.getModifiers())) { + @SuppressWarnings("unchecked") + final T attribute = (T)Fields.read(field, null); + attributeMap.put(toString(attribute), attribute); } - } catch (final SecurityException e) { - attributeMap.clear(); - } catch (final ObjectAccessException e) { - attributeMap.clear(); - } catch (final NoClassDefFoundError e) { - attributeMap.clear(); } + } catch (final SecurityException e) { + attributeMap.clear(); + } catch (final ObjectAccessException e) { + attributeMap.clear(); + } catch (final NoClassDefFoundError e) { + attributeMap.clear(); } - instanceMaps.put(type.getName(), attributeMap); } - return this; + return attributeMap; } -} + private static class Reflections { + + private static final Method getName; + + static { + Method method = null; + try { + method = AttributedCharacterIterator.Attribute.class.getDeclaredMethod("getName", (Class[])null); + if (!method.isAccessible()) { + method.setAccessible(true); + } + } catch (final SecurityException e) { + // ignore for now + } catch (final NoSuchMethodException e) { + // ignore for now + } + getName = method; + } + } +} \ No newline at end of file diff --git a/xstream/src/test/com/thoughtworks/xstream/converters/ConvertersArchTest.java b/xstream/src/test/com/thoughtworks/xstream/converters/ConvertersArchTest.java new file mode 100644 index 000000000..550dfc94a --- /dev/null +++ b/xstream/src/test/com/thoughtworks/xstream/converters/ConvertersArchTest.java @@ -0,0 +1,54 @@ +/* + * Copyright (C) 2006, 2007, 2016 XStream Committers. + * All rights reserved. + * + * The software in this package is published under the terms of the BSD + * style license a copy of which has been included with this distribution in + * the LICENSE.txt file. + * + * Created on 29. July 2020 by Falko Modler + */ +package com.thoughtworks.xstream.converters; + +import java.util.stream.Collectors; + +import io.github.classgraph.ClassGraph; +import io.github.classgraph.ScanResult; +import io.github.classgraph.FieldInfoList.FieldInfoFilter; +import junit.framework.Assert; +import junit.framework.TestCase; + +/** + * @author Falko Modler + */ +public class ConvertersArchTest extends TestCase { + + private static final FieldInfoFilter REFLECTION_FIELD = + fieldInfo -> fieldInfo.isStatic() && fieldInfo.getTypeDescriptor().toString().startsWith("java.lang.reflect."); + + /** + * Tests that no converter has a static field of type {@code java.lang.reflect.*} which hints at eager reflection access at construction time. + * Such eager access will cause unecessary "illegal reflective access" warnings or even failures when {@code XStream} is just instantiated (not even executed). + */ + public void testNoEagerStaticReflectionFields() { + String violatingFields = ""; + + try(ScanResult scanResult = new ClassGraph() + .acceptPackages(ConvertersArchTest.class.getPackage().getName()) + .enableFieldInfo() + .ignoreFieldVisibility() + .disableJarScanning() + .scan()) { + + violatingFields = scanResult.getAllStandardClasses().stream() + .flatMap(info -> info.getDeclaredFieldInfo().filter(REFLECTION_FIELD).stream()) + .map(info -> info.getClassInfo().getName() + "." + info.getName()) + .collect(Collectors.joining("\n\t")); + } + + if (!violatingFields.isEmpty()) { + Assert.fail("The following direct java.lang.reflect fields must be moved to static inner classes " + + " to avoid eager init:\n\t" + violatingFields); + } + } +}