Skip to content

Commit

Permalink
Avoid type pollution in StringCollectionDeserializer (#4848)
Browse files Browse the repository at this point in the history
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)
  • Loading branch information
yawkat authored Dec 21, 2024
1 parent 6b2cb81 commit 628587e
Show file tree
Hide file tree
Showing 4 changed files with 145 additions and 0 deletions.
30 changes: 30 additions & 0 deletions pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,22 @@
<version>${version.mockito}</version>
<scope>test</scope>
</dependency>

<!-- Dependencies for testing "type pollution", see:
https://github.com/FasterXML/jackson-databind/pull/4848
-->
<dependency>
<groupId>org.junit.platform</groupId>
<artifactId>junit-platform-suite-engine</artifactId>
<version>1.10.2</version>
<scope>test</scope>
</dependency>
<dependency>
<groupId>io.micronaut.test</groupId>
<artifactId>micronaut-test-type-pollution</artifactId>
<version>4.6.2</version>
<scope>test</scope>
</dependency>
</dependencies>

<!-- Alas, need to include snapshot reference since otherwise can not find
Expand Down Expand Up @@ -213,6 +229,7 @@
<excludes>
<exclude>com.fasterxml.jackson.databind.MapperFootprintTest</exclude>
</excludes>
<test>com.fasterxml.jackson.databind.PrimarySuite</test>
<!-- 26-Nov-2019, tatu: moar parallelism! Per-class basis, safe, efficient enough
... although not 100% sure this makes much difference TBH
-->
Expand Down Expand Up @@ -386,6 +403,19 @@
<configuration>
<argLine>--add-opens=java.base/java.lang=ALL-UNNAMED --add-opens=java.base/java.util=ALL-UNNAMED</argLine>
</configuration>
<executions>
<execution>
<id>type-pollution-test</id>
<phase>test</phase>
<goals>
<goal>test</goal>
</goals>
<configuration>
<test>com.fasterxml.jackson.databind.typepollution.TypePollutionSuite</test>
<threadCount>1</threadCount>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
package com.fasterxml.jackson.databind.typepollution;

import org.junit.platform.suite.api.SelectPackages;
import org.junit.platform.suite.api.Suite;

@Suite
@SelectPackages("com.fasterxml.jackson.databind.typepollution")
public class TypePollutionSuite {
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
package com.fasterxml.jackson.databind.typepollution;

import java.util.List;
import java.util.Set;
import java.util.concurrent.atomic.AtomicInteger;

import io.micronaut.test.typepollution.FocusListener;
import io.micronaut.test.typepollution.ThresholdFocusListener;
import io.micronaut.test.typepollution.TypePollutionTransformer;
import org.junit.jupiter.api.*;
import org.junit.jupiter.api.function.Executable;

import com.fasterxml.jackson.databind.ObjectMapper;
import com.fasterxml.jackson.databind.json.JsonMapper;

public class TypePollutionTest {
private static final int THRESHOLD = 1000;
private ThresholdFocusListener focusListener;

@BeforeAll
static void setupAgent() {
TypePollutionTransformer.install(net.bytebuddy.agent.ByteBuddyAgent.install());
}

@BeforeEach
void setUp() {
focusListener = new ThresholdFocusListener();
FocusListener.setFocusListener(focusListener);
}

@AfterEach
void verifyNoTypeThrashing() {
FocusListener.setFocusListener(null);
Assertions.assertTrue(focusListener.checkThresholds(THRESHOLD), "Threshold exceeded, check logs.");
}

private void doTest(Executable r) throws Throwable {
for (int i = 0; i < THRESHOLD * 2; i++) {
r.execute();
}
}

@Test
@Disabled
public void sample() throws Throwable {
// example test. If you enable this, it will fail, and you can see what a type pollution error looks like.

interface A {
}

interface B {
}

class Concrete implements A, B {
}

Object c = new Concrete();
AtomicInteger j = new AtomicInteger();
doTest(() -> {
if (c instanceof A) {
j.incrementAndGet();
}
if (c instanceof B) {
j.incrementAndGet();
}
});
System.out.println(j);
}

@Test
public void deserializeRecordWithList() throws Throwable {
ObjectMapper mapper = JsonMapper.builder().build();
ListRecord input = new ListRecord(List.of("foo"));
String json = mapper.writeValueAsString(input);
doTest(() -> Assertions.assertEquals(input, mapper.readValue(json, ListRecord.class)));
}

record ListRecord(List<String> list) {
}

@Test
public void deserializeRecordWithSet() throws Throwable {
ObjectMapper mapper = JsonMapper.builder().build();
SetRecord input = new SetRecord(Set.of("foo"));
String json = mapper.writeValueAsString(input);
doTest(() -> Assertions.assertEquals(input, mapper.readValue(json, SetRecord.class)));
}

record SetRecord(Set<String> list) {
}
}
15 changes: 15 additions & 0 deletions src/test/java/com/fasterxml/jackson/databind/PrimarySuite.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
package com.fasterxml.jackson.databind;

import org.junit.platform.suite.api.ExcludeClassNamePatterns;
import org.junit.platform.suite.api.ExcludePackages;
import org.junit.platform.suite.api.SelectPackages;
import org.junit.platform.suite.api.Suite;

@Suite
@SelectPackages("com.fasterxml.jackson.databind")
@ExcludePackages("com.fasterxml.jackson.databind.typepollution")
@ExcludeClassNamePatterns({
"com\\.fasterxml\\.jackson\\.databind\\.MapperFootprintTest"
})
public class PrimarySuite {
}

0 comments on commit 628587e

Please sign in to comment.