Skip to content

Commit 628587e

Browse files
authored
Avoid type pollution in StringCollectionDeserializer (#4848)
This patch contains test to guard against type pollution issue when deserializing a `List<String>` property, by adding explicit type checks for common subclasses (ArrayList, HashSet). (actual fix merged separately via #4862)
1 parent 6b2cb81 commit 628587e

File tree

4 files changed

+145
-0
lines changed

4 files changed

+145
-0
lines changed

pom.xml

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -154,6 +154,22 @@
154154
<version>${version.mockito}</version>
155155
<scope>test</scope>
156156
</dependency>
157+
158+
<!-- Dependencies for testing "type pollution", see:
159+
https://github.com/FasterXML/jackson-databind/pull/4848
160+
-->
161+
<dependency>
162+
<groupId>org.junit.platform</groupId>
163+
<artifactId>junit-platform-suite-engine</artifactId>
164+
<version>1.10.2</version>
165+
<scope>test</scope>
166+
</dependency>
167+
<dependency>
168+
<groupId>io.micronaut.test</groupId>
169+
<artifactId>micronaut-test-type-pollution</artifactId>
170+
<version>4.6.2</version>
171+
<scope>test</scope>
172+
</dependency>
157173
</dependencies>
158174

159175
<!-- Alas, need to include snapshot reference since otherwise can not find
@@ -213,6 +229,7 @@
213229
<excludes>
214230
<exclude>com.fasterxml.jackson.databind.MapperFootprintTest</exclude>
215231
</excludes>
232+
<test>com.fasterxml.jackson.databind.PrimarySuite</test>
216233
<!-- 26-Nov-2019, tatu: moar parallelism! Per-class basis, safe, efficient enough
217234
... although not 100% sure this makes much difference TBH
218235
-->
@@ -386,6 +403,19 @@
386403
<configuration>
387404
<argLine>--add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED</argLine>
388405
</configuration>
406+
<executions>
407+
<execution>
408+
<id>type-pollution-test</id>
409+
<phase>test</phase>
410+
<goals>
411+
<goal>test</goal>
412+
</goals>
413+
<configuration>
414+
<test>com.fasterxml.jackson.databind.typepollution.TypePollutionSuite</test>
415+
<threadCount>1</threadCount>
416+
</configuration>
417+
</execution>
418+
</executions>
389419
</plugin>
390420
</plugins>
391421
</build>
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
package com.fasterxml.jackson.databind.typepollution;
2+
3+
import org.junit.platform.suite.api.SelectPackages;
4+
import org.junit.platform.suite.api.Suite;
5+
6+
@Suite
7+
@SelectPackages("com.fasterxml.jackson.databind.typepollution")
8+
public class TypePollutionSuite {
9+
}
Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
package com.fasterxml.jackson.databind.typepollution;
2+
3+
import java.util.List;
4+
import java.util.Set;
5+
import java.util.concurrent.atomic.AtomicInteger;
6+
7+
import io.micronaut.test.typepollution.FocusListener;
8+
import io.micronaut.test.typepollution.ThresholdFocusListener;
9+
import io.micronaut.test.typepollution.TypePollutionTransformer;
10+
import org.junit.jupiter.api.*;
11+
import org.junit.jupiter.api.function.Executable;
12+
13+
import com.fasterxml.jackson.databind.ObjectMapper;
14+
import com.fasterxml.jackson.databind.json.JsonMapper;
15+
16+
public class TypePollutionTest {
17+
private static final int THRESHOLD = 1000;
18+
private ThresholdFocusListener focusListener;
19+
20+
@BeforeAll
21+
static void setupAgent() {
22+
TypePollutionTransformer.install(net.bytebuddy.agent.ByteBuddyAgent.install());
23+
}
24+
25+
@BeforeEach
26+
void setUp() {
27+
focusListener = new ThresholdFocusListener();
28+
FocusListener.setFocusListener(focusListener);
29+
}
30+
31+
@AfterEach
32+
void verifyNoTypeThrashing() {
33+
FocusListener.setFocusListener(null);
34+
Assertions.assertTrue(focusListener.checkThresholds(THRESHOLD), "Threshold exceeded, check logs.");
35+
}
36+
37+
private void doTest(Executable r) throws Throwable {
38+
for (int i = 0; i < THRESHOLD * 2; i++) {
39+
r.execute();
40+
}
41+
}
42+
43+
@Test
44+
@Disabled
45+
public void sample() throws Throwable {
46+
// example test. If you enable this, it will fail, and you can see what a type pollution error looks like.
47+
48+
interface A {
49+
}
50+
51+
interface B {
52+
}
53+
54+
class Concrete implements A, B {
55+
}
56+
57+
Object c = new Concrete();
58+
AtomicInteger j = new AtomicInteger();
59+
doTest(() -> {
60+
if (c instanceof A) {
61+
j.incrementAndGet();
62+
}
63+
if (c instanceof B) {
64+
j.incrementAndGet();
65+
}
66+
});
67+
System.out.println(j);
68+
}
69+
70+
@Test
71+
public void deserializeRecordWithList() throws Throwable {
72+
ObjectMapper mapper = JsonMapper.builder().build();
73+
ListRecord input = new ListRecord(List.of("foo"));
74+
String json = mapper.writeValueAsString(input);
75+
doTest(() -> Assertions.assertEquals(input, mapper.readValue(json, ListRecord.class)));
76+
}
77+
78+
record ListRecord(List<String> list) {
79+
}
80+
81+
@Test
82+
public void deserializeRecordWithSet() throws Throwable {
83+
ObjectMapper mapper = JsonMapper.builder().build();
84+
SetRecord input = new SetRecord(Set.of("foo"));
85+
String json = mapper.writeValueAsString(input);
86+
doTest(() -> Assertions.assertEquals(input, mapper.readValue(json, SetRecord.class)));
87+
}
88+
89+
record SetRecord(Set<String> list) {
90+
}
91+
}
Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,15 @@
1+
package com.fasterxml.jackson.databind;
2+
3+
import org.junit.platform.suite.api.ExcludeClassNamePatterns;
4+
import org.junit.platform.suite.api.ExcludePackages;
5+
import org.junit.platform.suite.api.SelectPackages;
6+
import org.junit.platform.suite.api.Suite;
7+
8+
@Suite
9+
@SelectPackages("com.fasterxml.jackson.databind")
10+
@ExcludePackages("com.fasterxml.jackson.databind.typepollution")
11+
@ExcludeClassNamePatterns({
12+
"com\\.fasterxml\\.jackson\\.databind\\.MapperFootprintTest"
13+
})
14+
public class PrimarySuite {
15+
}

0 commit comments

Comments
 (0)