Skip to content

Commit 0c0f86c

Browse files
wendigofindepi
authored andcommitted
Disable Jackson default JSON processing limits
Jackson 2.15 introduced read constraints that are meant to protect deserialization path from deeply nested/long string JSON documents. Introduced limits are too small for Trino to work properly. Airlift is already disabling those limits when ObjectMapperProvider is used. Additionaly ban direct usage of JsonFactory and JsonFactoryBuilder. Instead JsonUtils.jsonFactory() or JsonUtils.jsonFactoryBuilder() should be used.
1 parent bdc07e5 commit 0c0f86c

File tree

20 files changed

+120
-23
lines changed

20 files changed

+120
-23
lines changed

.mvn/modernizer/violations.xml

+12
Original file line numberDiff line numberDiff line change
@@ -313,4 +313,16 @@
313313
<version>1.8</version>
314314
<comment>Prefer org.testng.annotations.AfterClass</comment>
315315
</violation>
316+
317+
<violation>
318+
<name>com/fasterxml/jackson/core/JsonFactory."&lt;init&gt;":()V</name>
319+
<version>1.8</version>
320+
<comment>Use io.trino.plugin.base.util.JsonUtils.jsonFactory()</comment>
321+
</violation>
322+
323+
<violation>
324+
<name>com/fasterxml/jackson/core/JsonFactoryBuilder."&lt;init&gt;":()V</name>
325+
<version>1.8</version>
326+
<comment>Use io.trino.plugin.base.util.JsonUtils.jsonFactoryBuilder() instead</comment>
327+
</violation>
316328
</modernizer>

client/trino-cli/src/main/java/io/trino/cli/JsonPrinter.java

+18-1
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,11 @@
1414
package io.trino.cli;
1515

1616
import com.fasterxml.jackson.core.JsonFactory;
17+
import com.fasterxml.jackson.core.JsonFactoryBuilder;
1718
import com.fasterxml.jackson.core.JsonGenerator;
19+
import com.fasterxml.jackson.core.StreamReadConstraints;
1820
import com.google.common.collect.ImmutableList;
21+
import org.gaul.modernizer_maven_annotations.SuppressModernizer;
1922

2023
import java.io.IOException;
2124
import java.io.Writer;
@@ -40,7 +43,7 @@ public JsonPrinter(List<String> fieldNames, Writer writer)
4043
public void printRows(List<List<?>> rows, boolean complete)
4144
throws IOException
4245
{
43-
JsonFactory jsonFactory = new JsonFactory().configure(JsonGenerator.Feature.AUTO_CLOSE_TARGET, false);
46+
JsonFactory jsonFactory = jsonFactory();
4447
try (JsonGenerator jsonGenerator = jsonFactory.createGenerator(writer)) {
4548
jsonGenerator.setRootValueSeparator(null);
4649
for (List<?> row : rows) {
@@ -70,4 +73,18 @@ private static Object formatValue(Object o)
7073
}
7174
return o;
7275
}
76+
77+
@SuppressModernizer
78+
// JsonFactoryBuilder usage is intentional as we don't want to bring additional dependency on plugin-toolkit module
79+
private static JsonFactory jsonFactory()
80+
{
81+
return new JsonFactoryBuilder()
82+
.streamReadConstraints(StreamReadConstraints.builder()
83+
.maxNumberLength(Integer.MAX_VALUE)
84+
.maxNestingDepth(Integer.MAX_VALUE)
85+
.maxStringLength(Integer.MAX_VALUE)
86+
.build())
87+
.build()
88+
.configure(JsonGenerator.Feature.AUTO_CLOSE_TARGET, false);
89+
}
7390
}

core/trino-main/src/main/java/io/trino/operator/scalar/JsonExtract.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
package io.trino.operator.scalar;
1515

1616
import com.fasterxml.jackson.core.JsonFactory;
17-
import com.fasterxml.jackson.core.JsonFactoryBuilder;
1817
import com.fasterxml.jackson.core.JsonGenerator;
1918
import com.fasterxml.jackson.core.JsonParseException;
2019
import com.fasterxml.jackson.core.JsonParser;
@@ -36,6 +35,7 @@
3635
import static com.fasterxml.jackson.core.JsonToken.START_OBJECT;
3736
import static com.fasterxml.jackson.core.JsonToken.VALUE_NULL;
3837
import static io.airlift.slice.Slices.utf8Slice;
38+
import static io.trino.plugin.base.util.JsonUtils.jsonFactoryBuilder;
3939
import static io.trino.spi.StandardErrorCode.INVALID_FUNCTION_ARGUMENT;
4040
import static io.trino.util.JsonUtil.createJsonGenerator;
4141
import static io.trino.util.JsonUtil.createJsonParser;
@@ -118,7 +118,7 @@ public final class JsonExtract
118118
{
119119
private static final int ESTIMATED_JSON_OUTPUT_SIZE = 512;
120120

121-
private static final JsonFactory JSON_FACTORY = new JsonFactoryBuilder()
121+
private static final JsonFactory JSON_FACTORY = jsonFactoryBuilder()
122122
.disable(CANONICALIZE_FIELD_NAMES)
123123
.build();
124124

core/trino-main/src/main/java/io/trino/operator/scalar/JsonFunctions.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
package io.trino.operator.scalar;
1515

1616
import com.fasterxml.jackson.core.JsonFactory;
17-
import com.fasterxml.jackson.core.JsonFactoryBuilder;
1817
import com.fasterxml.jackson.core.JsonParser;
1918
import com.fasterxml.jackson.core.JsonToken;
2019
import com.fasterxml.jackson.databind.MappingJsonFactory;
@@ -47,14 +46,15 @@
4746
import static com.fasterxml.jackson.core.JsonToken.VALUE_STRING;
4847
import static com.fasterxml.jackson.core.JsonToken.VALUE_TRUE;
4948
import static io.airlift.slice.Slices.utf8Slice;
49+
import static io.trino.plugin.base.util.JsonUtils.jsonFactoryBuilder;
5050
import static io.trino.spi.StandardErrorCode.INVALID_FUNCTION_ARGUMENT;
5151
import static io.trino.spi.type.Chars.padSpaces;
5252
import static io.trino.util.JsonUtil.createJsonParser;
5353
import static io.trino.util.JsonUtil.truncateIfNecessaryForErrorMessage;
5454

5555
public final class JsonFunctions
5656
{
57-
private static final JsonFactory JSON_FACTORY = new JsonFactoryBuilder()
57+
private static final JsonFactory JSON_FACTORY = jsonFactoryBuilder()
5858
.disable(CANONICALIZE_FIELD_NAMES)
5959
.build();
6060

core/trino-main/src/main/java/io/trino/util/JsonUtil.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
package io.trino.util;
1515

1616
import com.fasterxml.jackson.core.JsonFactory;
17-
import com.fasterxml.jackson.core.JsonFactoryBuilder;
1817
import com.fasterxml.jackson.core.JsonGenerator;
1918
import com.fasterxml.jackson.core.JsonParser;
2019
import com.fasterxml.jackson.core.JsonToken;
@@ -77,6 +76,7 @@
7776
import static com.fasterxml.jackson.core.JsonToken.START_ARRAY;
7877
import static com.fasterxml.jackson.core.JsonToken.START_OBJECT;
7978
import static com.google.common.base.Verify.verify;
79+
import static io.trino.plugin.base.util.JsonUtils.jsonFactoryBuilder;
8080
import static io.trino.spi.StandardErrorCode.INVALID_CAST_ARGUMENT;
8181
import static io.trino.spi.StandardErrorCode.INVALID_FUNCTION_ARGUMENT;
8282
import static io.trino.spi.type.BigintType.BIGINT;
@@ -114,7 +114,7 @@ private JsonUtil() {}
114114
// Note: JsonFactory is mutable, instances cannot be shared openly.
115115
public static JsonFactory createJsonFactory()
116116
{
117-
return new JsonFactoryBuilder().disable(CANONICALIZE_FIELD_NAMES).build();
117+
return jsonFactoryBuilder().disable(CANONICALIZE_FIELD_NAMES).build();
118118
}
119119

120120
public static JsonParser createJsonParser(JsonFactory factory, Slice json)

core/trino-main/src/test/java/io/trino/operator/scalar/TestJsonExtract.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import static io.trino.operator.scalar.JsonExtract.ObjectFieldJsonExtractor;
3131
import static io.trino.operator.scalar.JsonExtract.ScalarValueJsonExtractor;
3232
import static io.trino.operator.scalar.JsonExtract.generateExtractor;
33+
import static io.trino.plugin.base.util.JsonUtils.jsonFactory;
3334
import static io.trino.spi.StandardErrorCode.INVALID_FUNCTION_ARGUMENT;
3435
import static io.trino.spi.type.VarcharType.VARCHAR;
3536
import static io.trino.testing.assertions.TrinoExceptionAssert.assertTrinoExceptionThrownBy;
@@ -341,7 +342,7 @@ public void testNoAutomaticEncodingDetection()
341342
private static String doExtract(JsonExtractor<Slice> jsonExtractor, String json)
342343
throws IOException
343344
{
344-
JsonFactory jsonFactory = new JsonFactory();
345+
JsonFactory jsonFactory = jsonFactory();
345346
JsonParser jsonParser = jsonFactory.createParser(json);
346347
jsonParser.nextToken(); // Advance to the first token
347348
Slice extract = jsonExtractor.extract(jsonParser);

lib/trino-hive-formats/src/main/java/io/trino/hive/formats/line/json/JsonDeserializer.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
package io.trino.hive.formats.line.json;
1515

1616
import com.fasterxml.jackson.core.JsonFactory;
17-
import com.fasterxml.jackson.core.JsonFactoryBuilder;
1817
import com.fasterxml.jackson.core.JsonLocation;
1918
import com.fasterxml.jackson.core.JsonParser;
2019
import com.fasterxml.jackson.core.JsonToken;
@@ -68,6 +67,7 @@
6867
import static io.trino.hive.formats.HiveFormatUtils.parseHiveDate;
6968
import static io.trino.hive.formats.HiveFormatUtils.writeDecimal;
7069
import static io.trino.plugin.base.type.TrinoTimestampEncoderFactory.createTimestampEncoder;
70+
import static io.trino.plugin.base.util.JsonUtils.jsonFactoryBuilder;
7171
import static io.trino.spi.type.BigintType.BIGINT;
7272
import static io.trino.spi.type.BooleanType.BOOLEAN;
7373
import static io.trino.spi.type.Chars.truncateToLengthAndTrimSpaces;
@@ -107,7 +107,7 @@
107107
public class JsonDeserializer
108108
implements LineDeserializer
109109
{
110-
private static final JsonFactory JSON_FACTORY = new JsonFactoryBuilder()
110+
private static final JsonFactory JSON_FACTORY = jsonFactoryBuilder()
111111
.disable(INTERN_FIELD_NAMES)
112112
.build();
113113

lib/trino-hive-formats/src/main/java/io/trino/hive/formats/line/json/JsonSerializer.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141

4242
import static com.google.common.base.Preconditions.checkArgument;
4343
import static com.google.common.collect.ImmutableList.toImmutableList;
44+
import static io.trino.plugin.base.util.JsonUtils.jsonFactory;
4445
import static io.trino.spi.type.BigintType.BIGINT;
4546
import static io.trino.spi.type.BooleanType.BOOLEAN;
4647
import static io.trino.spi.type.DateType.DATE;
@@ -67,7 +68,7 @@ public JsonSerializer(List<Column> columns)
6768
.map(column -> field(column.name().toLowerCase(Locale.ROOT), column.type()))
6869
.collect(toImmutableList()));
6970

70-
jsonFactory = new JsonFactory();
71+
jsonFactory = jsonFactory();
7172
}
7273

7374
@Override

lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/util/JsonTypeUtil.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
package io.trino.plugin.base.util;
1515

1616
import com.fasterxml.jackson.core.JsonFactory;
17-
import com.fasterxml.jackson.core.JsonFactoryBuilder;
1817
import com.fasterxml.jackson.core.JsonParser;
1918
import com.fasterxml.jackson.databind.ObjectMapper;
2019
import io.airlift.json.ObjectMapperProvider;
@@ -31,13 +30,14 @@
3130
import static com.fasterxml.jackson.core.JsonFactory.Feature.CANONICALIZE_FIELD_NAMES;
3231
import static com.fasterxml.jackson.databind.SerializationFeature.ORDER_MAP_ENTRIES_BY_KEYS;
3332
import static com.google.common.base.Preconditions.checkState;
33+
import static io.trino.plugin.base.util.JsonUtils.jsonFactoryBuilder;
3434
import static io.trino.spi.StandardErrorCode.INVALID_FUNCTION_ARGUMENT;
3535
import static java.lang.String.format;
3636
import static java.nio.charset.StandardCharsets.UTF_8;
3737

3838
public final class JsonTypeUtil
3939
{
40-
private static final JsonFactory JSON_FACTORY = new JsonFactoryBuilder().disable(CANONICALIZE_FIELD_NAMES).build();
40+
private static final JsonFactory JSON_FACTORY = jsonFactoryBuilder().disable(CANONICALIZE_FIELD_NAMES).build();
4141
private static final ObjectMapper SORTED_MAPPER = new ObjectMapperProvider().get().configure(ORDER_MAP_ENTRIES_BY_KEYS, true);
4242

4343
private JsonTypeUtil() {}

lib/trino-plugin-toolkit/src/main/java/io/trino/plugin/base/util/JsonUtils.java

+22
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,17 @@
1313
*/
1414
package io.trino.plugin.base.util;
1515

16+
import com.fasterxml.jackson.core.JsonFactory;
17+
import com.fasterxml.jackson.core.JsonFactoryBuilder;
1618
import com.fasterxml.jackson.core.JsonParser;
1719
import com.fasterxml.jackson.core.JsonProcessingException;
20+
import com.fasterxml.jackson.core.StreamReadConstraints;
1821
import com.fasterxml.jackson.databind.DeserializationFeature;
1922
import com.fasterxml.jackson.databind.JsonNode;
2023
import com.fasterxml.jackson.databind.MapperFeature;
2124
import com.fasterxml.jackson.databind.ObjectMapper;
2225
import io.airlift.json.ObjectMapperProvider;
26+
import org.gaul.modernizer_maven_annotations.SuppressModernizer;
2327

2428
import java.io.IOException;
2529
import java.io.InputStream;
@@ -144,6 +148,24 @@ private static <T> T parseJson(JsonNode node, String jsonPointer, Class<T> javaT
144148
return jsonTreeToValue(mappingsNode, javaType);
145149
}
146150

151+
public static JsonFactory jsonFactory()
152+
{
153+
return jsonFactoryBuilder().build();
154+
}
155+
156+
@SuppressModernizer
157+
// JsonFactoryBuilder usage is intentional as we need to disable read constraints
158+
// due to the limits introduced by Jackson 2.15
159+
public static JsonFactoryBuilder jsonFactoryBuilder()
160+
{
161+
return new JsonFactoryBuilder()
162+
.streamReadConstraints(StreamReadConstraints.builder()
163+
.maxStringLength(Integer.MAX_VALUE)
164+
.maxNestingDepth(Integer.MAX_VALUE)
165+
.maxNumberLength(Integer.MAX_VALUE)
166+
.build());
167+
}
168+
147169
private interface ParserConstructor<I>
148170
{
149171
JsonParser createParser(ObjectMapper mapper, I input)

lib/trino-plugin-toolkit/src/test/java/io/trino/plugin/base/util/TestJsonUtils.java

+27
Original file line numberDiff line numberDiff line change
@@ -14,12 +14,15 @@
1414
package io.trino.plugin.base.util;
1515

1616
import com.fasterxml.jackson.annotation.JsonProperty;
17+
import com.fasterxml.jackson.core.StreamReadConstraints;
1718
import com.fasterxml.jackson.databind.JsonNode;
1819
import org.testng.annotations.Test;
1920

2021
import java.io.IOException;
2122
import java.io.UncheckedIOException;
2223

24+
import static io.trino.plugin.base.util.JsonUtils.jsonFactory;
25+
import static io.trino.plugin.base.util.JsonUtils.jsonFactoryBuilder;
2326
import static io.trino.plugin.base.util.JsonUtils.parseJson;
2427
import static io.trino.plugin.base.util.TestJsonUtils.TestEnum.OPTION_A;
2528
import static java.nio.charset.StandardCharsets.US_ASCII;
@@ -70,4 +73,28 @@ public void testTrailingContent()
7073
.hasMessage("Could not parse JSON")
7174
.hasStackTraceContaining("Unrecognized token 'not': was expecting (JSON String, Number, Array, Object or token 'null', 'true' or 'false')");
7275
}
76+
77+
@Test
78+
public void testFactoryHasNoReadContraints()
79+
{
80+
assertReadConstraints(jsonFactory().streamReadConstraints());
81+
assertReadConstraints(jsonFactoryBuilder().build().streamReadConstraints());
82+
}
83+
84+
@Test
85+
public void testBuilderHasNoReadConstraints()
86+
{
87+
assertReadConstraints(jsonFactoryBuilder().build().streamReadConstraints());
88+
}
89+
90+
private static void assertReadConstraints(StreamReadConstraints constraints)
91+
{
92+
// Jackson 2.15 introduced read constraints limit that are too strict for
93+
// Trino use-cases. Ensure that those limits are no longer present for JsonFactories.
94+
//
95+
// https://github.com/trinodb/trino/issues/17843
96+
assertThat(constraints.getMaxStringLength()).isEqualTo(Integer.MAX_VALUE);
97+
assertThat(constraints.getMaxNestingDepth()).isEqualTo(Integer.MAX_VALUE);
98+
assertThat(constraints.getMaxNumberLength()).isEqualTo(Integer.MAX_VALUE);
99+
}
73100
}

plugin/trino-iceberg/src/main/java/io/trino/plugin/iceberg/PartitionData.java

+2-1
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import java.nio.ByteBuffer;
2929
import java.util.UUID;
3030

31+
import static io.trino.plugin.base.util.JsonUtils.jsonFactory;
3132
import static io.trino.spi.type.DecimalType.createDecimalType;
3233
import static io.trino.spi.type.Decimals.rescale;
3334
import static java.lang.String.format;
@@ -37,7 +38,7 @@ public class PartitionData
3738
implements StructLike
3839
{
3940
private static final String PARTITION_VALUES_FIELD = "partitionValues";
40-
private static final JsonFactory FACTORY = new JsonFactory();
41+
private static final JsonFactory FACTORY = jsonFactory();
4142
private static final ObjectMapper MAPPER = new ObjectMapper(FACTORY)
4243
.configure(DeserializationFeature.USE_BIG_DECIMAL_FOR_FLOATS, true);
4344

plugin/trino-prometheus/src/main/java/io/trino/plugin/prometheus/PrometheusQueryResponseParse.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,6 @@
1313
*/
1414
package io.trino.plugin.prometheus;
1515

16-
import com.fasterxml.jackson.core.JsonFactory;
1716
import com.fasterxml.jackson.core.JsonParser;
1817
import com.fasterxml.jackson.core.JsonToken;
1918
import com.fasterxml.jackson.core.type.TypeReference;
@@ -28,6 +27,7 @@
2827
import java.util.List;
2928
import java.util.Map;
3029

30+
import static io.trino.plugin.base.util.JsonUtils.jsonFactory;
3131
import static io.trino.plugin.prometheus.PrometheusErrorCode.PROMETHEUS_PARSE_ERROR;
3232
import static java.util.Collections.singletonList;
3333

@@ -46,7 +46,7 @@ public PrometheusQueryResponseParse(InputStream response)
4646
{
4747
ObjectMapper mapper = new ObjectMapper();
4848
mapper.registerModule(new JavaTimeModule());
49-
JsonParser parser = new JsonFactory().createParser(response);
49+
JsonParser parser = jsonFactory().createParser(response);
5050
while (!parser.isClosed()) {
5151
JsonToken jsonToken = parser.nextToken();
5252
if (JsonToken.FIELD_NAME.equals(jsonToken)) {

service/trino-proxy/pom.xml

+5
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,11 @@
1818
</properties>
1919

2020
<dependencies>
21+
<dependency>
22+
<groupId>io.trino</groupId>
23+
<artifactId>trino-plugin-toolkit</artifactId>
24+
</dependency>
25+
2126
<dependency>
2227
<groupId>io.airlift</groupId>
2328
<artifactId>bootstrap</artifactId>

service/trino-proxy/src/main/java/io/trino/proxy/ProxyResource.java

+2-2
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
package io.trino.proxy;
1515

1616
import com.fasterxml.jackson.core.JsonFactory;
17-
import com.fasterxml.jackson.core.JsonFactoryBuilder;
1817
import com.fasterxml.jackson.core.JsonGenerator;
1918
import com.fasterxml.jackson.core.JsonParser;
2019
import com.fasterxml.jackson.core.JsonToken;
@@ -72,6 +71,7 @@
7271
import static io.airlift.http.client.Request.Builder.preparePost;
7372
import static io.airlift.http.client.StaticBodyGenerator.createStaticBodyGenerator;
7473
import static io.airlift.jaxrs.AsyncResponseHandler.bindAsyncResponse;
74+
import static io.trino.plugin.base.util.JsonUtils.jsonFactoryBuilder;
7575
import static jakarta.ws.rs.core.MediaType.APPLICATION_JSON;
7676
import static jakarta.ws.rs.core.MediaType.TEXT_PLAIN_TYPE;
7777
import static jakarta.ws.rs.core.Response.Status.BAD_GATEWAY;
@@ -93,7 +93,7 @@ public class ProxyResource
9393

9494
private static final String X509_ATTRIBUTE = "jakarta.servlet.request.X509Certificate";
9595
private static final Duration ASYNC_TIMEOUT = new Duration(2, MINUTES);
96-
private static final JsonFactory JSON_FACTORY = new JsonFactoryBuilder().disable(CANONICALIZE_FIELD_NAMES).build();
96+
private static final JsonFactory JSON_FACTORY = jsonFactoryBuilder().disable(CANONICALIZE_FIELD_NAMES).build();
9797

9898
private final ExecutorService executor = newCachedThreadPool(daemonThreadsNamed("proxy-%s"));
9999
private final HttpClient httpClient;

service/trino-verifier/pom.xml

+5
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,11 @@
2828
<artifactId>trino-parser</artifactId>
2929
</dependency>
3030

31+
<dependency>
32+
<groupId>io.trino</groupId>
33+
<artifactId>trino-plugin-toolkit</artifactId>
34+
</dependency>
35+
3136
<dependency>
3237
<groupId>io.trino</groupId>
3338
<artifactId>trino-spi</artifactId>

0 commit comments

Comments
 (0)