From 2aee696a11713cbfbb9b5ea9c618792dd0f8d44d Mon Sep 17 00:00:00 2001 From: Georgios Andrianakis Date: Mon, 11 Apr 2022 13:52:55 +0300 Subject: [PATCH 1/3] Avoid XStream causing illegal access issues for internal JDK collections Relates to: #24492 --- .../junit/internal/CustomListConverter.java | 43 +++++++++++++++++++ .../junit/internal/CustomMapConverter.java | 41 ++++++++++++++++++ .../junit/internal/CustomSetConverter.java | 40 +++++++++++++++++ .../test/junit/internal/XStreamDeepClone.java | 3 ++ 4 files changed, 127 insertions(+) create mode 100644 test-framework/junit5/src/main/java/io/quarkus/test/junit/internal/CustomListConverter.java create mode 100644 test-framework/junit5/src/main/java/io/quarkus/test/junit/internal/CustomMapConverter.java create mode 100644 test-framework/junit5/src/main/java/io/quarkus/test/junit/internal/CustomSetConverter.java diff --git a/test-framework/junit5/src/main/java/io/quarkus/test/junit/internal/CustomListConverter.java b/test-framework/junit5/src/main/java/io/quarkus/test/junit/internal/CustomListConverter.java new file mode 100644 index 0000000000000..b55a0c77607a4 --- /dev/null +++ b/test-framework/junit5/src/main/java/io/quarkus/test/junit/internal/CustomListConverter.java @@ -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 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<>(); + } +} diff --git a/test-framework/junit5/src/main/java/io/quarkus/test/junit/internal/CustomMapConverter.java b/test-framework/junit5/src/main/java/io/quarkus/test/junit/internal/CustomMapConverter.java new file mode 100644 index 0000000000000..ada99f29fa331 --- /dev/null +++ b/test-framework/junit5/src/main/java/io/quarkus/test/junit/internal/CustomMapConverter.java @@ -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 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<>(); + } +} diff --git a/test-framework/junit5/src/main/java/io/quarkus/test/junit/internal/CustomSetConverter.java b/test-framework/junit5/src/main/java/io/quarkus/test/junit/internal/CustomSetConverter.java new file mode 100644 index 0000000000000..88d434cfaf34a --- /dev/null +++ b/test-framework/junit5/src/main/java/io/quarkus/test/junit/internal/CustomSetConverter.java @@ -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 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<>(); + } +} diff --git a/test-framework/junit5/src/main/java/io/quarkus/test/junit/internal/XStreamDeepClone.java b/test-framework/junit5/src/main/java/io/quarkus/test/junit/internal/XStreamDeepClone.java index dc945e5bf7f7c..d5486335b7669 100644 --- a/test-framework/junit5/src/main/java/io/quarkus/test/junit/internal/XStreamDeepClone.java +++ b/test-framework/junit5/src/main/java/io/quarkus/test/junit/internal/XStreamDeepClone.java @@ -18,6 +18,9 @@ class XStreamDeepClone implements DeepClone { 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; }; } From f07f14e63f3e04cf2c4639a9a01c5fde241884c2 Mon Sep 17 00:00:00 2001 From: Georgios Andrianakis Date: Mon, 11 Apr 2022 13:53:50 +0300 Subject: [PATCH 2/3] Removed deprecated (and also useless) method from XStream call This method was deprecated in XStream 1.4.18 and in practice is a no-op --- .../java/io/quarkus/test/junit/internal/XStreamDeepClone.java | 1 - 1 file changed, 1 deletion(-) diff --git a/test-framework/junit5/src/main/java/io/quarkus/test/junit/internal/XStreamDeepClone.java b/test-framework/junit5/src/main/java/io/quarkus/test/junit/internal/XStreamDeepClone.java index d5486335b7669..23aba9ec4ace3 100644 --- a/test-framework/junit5/src/main/java/io/quarkus/test/junit/internal/XStreamDeepClone.java +++ b/test-framework/junit5/src/main/java/io/quarkus/test/junit/internal/XStreamDeepClone.java @@ -15,7 +15,6 @@ 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())); From 7852ea9752703e477626d6e1494edcc19a8a0294 Mon Sep 17 00:00:00 2001 From: Robert Stupp Date: Fri, 25 Mar 2022 19:37:39 +0100 Subject: [PATCH 3/3] Add tests around "new-ish" collection types used with @ParameterizedTest See #24492 --- .../io/quarkus/it/extension/ParamsTest.java | 126 ++++++++++++++++++ 1 file changed, 126 insertions(+) create mode 100644 integration-tests/test-extension/tests/src/test/java/io/quarkus/it/extension/ParamsTest.java diff --git a/integration-tests/test-extension/tests/src/test/java/io/quarkus/it/extension/ParamsTest.java b/integration-tests/test-extension/tests/src/test/java/io/quarkus/it/extension/ParamsTest.java new file mode 100644 index 0000000000000..8460db8b12324 --- /dev/null +++ b/integration-tests/test-extension/tests/src/test/java/io/quarkus/it/extension/ParamsTest.java @@ -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. + * + *

+ * This test will run into x-stream/xstream#253 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 ignore) { + } + + static Stream> testDataStreamList() { + return Stream.of(Arrays.asList("a"), Arrays.asList("b")); + } + + @ParameterizedTest + @MethodSource("testDataStreamListOf") + public void methodStreamListOf(List ignore) { + } + + static Stream> testDataStreamListOf() { + return Stream.of(List.of("a")); + } + + @ParameterizedTest + @MethodSource("testDataStreamSetOf") + public void methodStreamListOf(Set ignore) { + } + + static Stream> testDataStreamSetOf() { + return Stream.of(Set.of("a")); + } + + @ParameterizedTest + @MethodSource("testDataStreamArrayList") + public void methodStreamArrayList(List ignore) { + } + + static Stream> testDataStreamArrayList() { + return Stream.of(Collections.emptyList()); + } + + @ParameterizedTest + @MethodSource("testDataStream") + public void methodStream(TestData ignore) { + } + + static Stream testDataStream() { + return Stream.of(new TestData()); + } + + @ParameterizedTest + @MethodSource("testDataList") + public void methodList(TestData ignore) { + } + + static List testDataList() { + return List.of(new TestData()); + } + + @ParameterizedTest + @MethodSource("testDataStreamArguments") + public void methodList(TestData ignore, String ignored) { + } + + static Stream testDataStreamArguments() { + return Stream.of(Arguments.of(new TestData(), "foo")); + } + + @ParameterizedTest + @MethodSource("testDataListArguments") + public void methodListArguments(TestData ignore, String ignored) { + } + + static List testDataListArguments() { + return Arrays.asList(Arguments.of(new TestData(), "foo"), Arguments.of(new TestData(), "foo")); + } + + @SuppressWarnings("unused") + static class TestData { + final List foo = Arrays.asList("one", "two", "three"); + } +}