Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid XStream causing illegal access issues for internal JDK collections #24872

Merged
merged 3 commits into from
Apr 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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);
geoand marked this conversation as resolved.
Show resolved Hide resolved
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