Skip to content

Commit

Permalink
Merge pull request #24872 from geoand/#24492
Browse files Browse the repository at this point in the history
Avoid XStream causing illegal access issues for internal JDK collections
  • Loading branch information
geoand authored Apr 11, 2022
2 parents 1df0015 + 7852ea9 commit aa1f7ef
Show file tree
Hide file tree
Showing 5 changed files with 253 additions and 1 deletion.
Original file line number Diff line number Diff line change
@@ -0,0 +1,126 @@
package io.quarkus.it.extension;

import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Set;
import java.util.stream.Stream;

import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.Arguments;
import org.junit.jupiter.params.provider.MethodSource;
import org.junit.jupiter.params.provider.ValueSource;

import io.quarkus.test.junit.QuarkusTest;

/**
* Exercise {@link ParameterizedTest @ParameterizedTest}s.
*
* <p>
* This test will run into <a href="https://github.com/x-stream/xstream/issues/253">x-stream/xstream#253</a> on Java 16 and
* newer if the custom XStream converters are not used
*/
@QuarkusTest
public class ParamsTest {
@ParameterizedTest
@ValueSource(booleans = { true, false })
public void valuesBooleans(boolean ignore) {
}

@ParameterizedTest
@ValueSource(strings = { "true", "false" })
public void valuesStrings(String ignore) {
}

@ParameterizedTest
@ValueSource(classes = { String.class, TestData.class })
public void valuesClasses(Class<?> ignore) {
}

@ParameterizedTest
@ValueSource(chars = { 'a', 'b', 'c' })
public void valuesChars(char ignore) {
}

@ParameterizedTest
@ValueSource(bytes = { (byte) 1, (byte) 2, (byte) 3 })
public void valuesBytes(byte ignore) {
}

@ParameterizedTest
@MethodSource("testDataStreamList")
public void methodStreamList(List<String> ignore) {
}

static Stream<List<String>> testDataStreamList() {
return Stream.of(Arrays.asList("a"), Arrays.asList("b"));
}

@ParameterizedTest
@MethodSource("testDataStreamListOf")
public void methodStreamListOf(List<String> ignore) {
}

static Stream<List<String>> testDataStreamListOf() {
return Stream.of(List.of("a"));
}

@ParameterizedTest
@MethodSource("testDataStreamSetOf")
public void methodStreamListOf(Set<String> ignore) {
}

static Stream<Set<String>> testDataStreamSetOf() {
return Stream.of(Set.of("a"));
}

@ParameterizedTest
@MethodSource("testDataStreamArrayList")
public void methodStreamArrayList(List<String> ignore) {
}

static Stream<List<String>> testDataStreamArrayList() {
return Stream.of(Collections.emptyList());
}

@ParameterizedTest
@MethodSource("testDataStream")
public void methodStream(TestData ignore) {
}

static Stream<TestData> testDataStream() {
return Stream.of(new TestData());
}

@ParameterizedTest
@MethodSource("testDataList")
public void methodList(TestData ignore) {
}

static List<TestData> testDataList() {
return List.of(new TestData());
}

@ParameterizedTest
@MethodSource("testDataStreamArguments")
public void methodList(TestData ignore, String ignored) {
}

static Stream<Arguments> testDataStreamArguments() {
return Stream.of(Arguments.of(new TestData(), "foo"));
}

@ParameterizedTest
@MethodSource("testDataListArguments")
public void methodListArguments(TestData ignore, String ignored) {
}

static List<Arguments> testDataListArguments() {
return Arrays.asList(Arguments.of(new TestData(), "foo"), Arguments.of(new TestData(), "foo"));
}

@SuppressWarnings("unused")
static class TestData {
final List<String> foo = Arrays.asList("one", "two", "three");
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
package io.quarkus.test.junit.internal;

import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
import java.util.List;
import java.util.Set;

import com.thoughtworks.xstream.converters.collections.CollectionConverter;
import com.thoughtworks.xstream.mapper.Mapper;

/**
* A custom List converter that always uses ArrayList for unmarshalling.
* This is probably not semantically correct 100% of the time, but it's likely fine
* for all the cases where we are using marshalling / unmarshalling.
*
* The reason for doing this is to avoid XStream causing illegal access issues
* for internal JDK lists
*/
public class CustomListConverter extends CollectionConverter {

// if we wanted to be 100% sure, we'd list all the List.of methods, but I think it's pretty safe to say
// that the JDK won't add custom implementations for the other classes
private final Set<String> SUPPORTED_CLASS_NAMES = Set.of(
List.of().getClass().getName(),
List.of(Integer.MAX_VALUE).getClass().getName(),
Arrays.asList(Integer.MAX_VALUE).getClass().getName(),
Collections.emptyList().getClass().getName());

public CustomListConverter(Mapper mapper) {
super(mapper);
}

@Override
public boolean canConvert(Class type) {
return (type != null) && SUPPORTED_CLASS_NAMES.contains(type.getName());
}

@Override
protected Object createCollection(Class type) {
return new ArrayList<>();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package io.quarkus.test.junit.internal;

import java.util.Collections;
import java.util.HashMap;
import java.util.Map;
import java.util.Set;

import com.thoughtworks.xstream.converters.collections.CollectionConverter;
import com.thoughtworks.xstream.mapper.Mapper;

/**
* A custom Map converter that always uses HashMap for unmarshalling.
* This is probably not semantically correct 100% of the time, but it's likely fine
* for all the cases where we are using marshalling / unmarshalling.
*
* The reason for doing this is to avoid XStream causing illegal access issues
* for internal JDK maps
*/
public class CustomMapConverter extends CollectionConverter {

// if we wanted to be 100% sure, we'd list all the Set.of methods, but I think it's pretty safe to say
// that the JDK won't add custom implementations for the other classes
private final Set<String> SUPPORTED_CLASS_NAMES = Set.of(
Map.of().getClass().getName(),
Map.of(Integer.MAX_VALUE, Integer.MAX_VALUE).getClass().getName(),
Collections.emptyMap().getClass().getName());

public CustomMapConverter(Mapper mapper) {
super(mapper);
}

@Override
public boolean canConvert(Class type) {
return (type != null) && SUPPORTED_CLASS_NAMES.contains(type.getName());
}

@Override
protected Object createCollection(Class type) {
return new HashMap<>();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
package io.quarkus.test.junit.internal;

import java.util.Collections;
import java.util.HashSet;
import java.util.Set;

import com.thoughtworks.xstream.converters.collections.CollectionConverter;
import com.thoughtworks.xstream.mapper.Mapper;

/**
* A custom Set converter that always uses HashSet for unmarshalling.
* This is probably not semantically correct 100% of the time, but it's likely fine
* for all the cases where we are using marshalling / unmarshalling.
*
* The reason for doing this is to avoid XStream causing illegal access issues
* for internal JDK sets
*/
public class CustomSetConverter extends CollectionConverter {

// if we wanted to be 100% sure, we'd list all the Set.of methods, but I think it's pretty safe to say
// that the JDK won't add custom implementations for the other classes
private final Set<String> SUPPORTED_CLASS_NAMES = Set.of(
Set.of().getClass().getName(),
Set.of(Integer.MAX_VALUE).getClass().getName(),
Collections.emptySet().getClass().getName());

public CustomSetConverter(Mapper mapper) {
super(mapper);
}

@Override
public boolean canConvert(Class type) {
return (type != null) && SUPPORTED_CLASS_NAMES.contains(type.getName());
}

@Override
protected Object createCollection(Class type) {
return new HashSet<>();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,11 @@ class XStreamDeepClone implements DeepClone {
// avoid doing any work eagerly since the cloner is rarely used
xStreamSupplier = () -> {
XStream result = new XStream();
XStream.setupDefaultSecurity(result);
result.allowTypesByRegExp(new String[] { ".*" });
result.setClassLoader(classLoader);
result.registerConverter(new CustomListConverter(result.getMapper()));
result.registerConverter(new CustomSetConverter(result.getMapper()));
result.registerConverter(new CustomMapConverter(result.getMapper()));
return result;
};
}
Expand Down

0 comments on commit aa1f7ef

Please sign in to comment.