Skip to content

Prevent modifications of static JsonFactory instances #17950

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

Merged
merged 3 commits into from
Jun 21, 2023
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
Expand Up @@ -13,6 +13,7 @@
*/
package io.trino.operator.scalar;

import com.fasterxml.jackson.core.JsonFactory;
import com.fasterxml.jackson.core.JsonGenerator;
import com.google.common.collect.ImmutableList;
import io.airlift.slice.DynamicSliceOutput;
Expand All @@ -30,7 +31,6 @@
import java.io.IOException;
import java.lang.invoke.MethodHandle;

import static io.trino.operator.scalar.JsonOperators.JSON_FACTORY;
import static io.trino.spi.StandardErrorCode.INVALID_CAST_ARGUMENT;
import static io.trino.spi.function.InvocationConvention.InvocationArgumentConvention.NEVER_NULL;
import static io.trino.spi.function.InvocationConvention.InvocationReturnConvention.FAIL_ON_NULL;
Expand All @@ -39,6 +39,7 @@
import static io.trino.type.JsonType.JSON;
import static io.trino.util.Failures.checkCondition;
import static io.trino.util.JsonUtil.canCastToJson;
import static io.trino.util.JsonUtil.createJsonFactory;
import static io.trino.util.JsonUtil.createJsonGenerator;
import static io.trino.util.Reflection.methodHandle;

Expand All @@ -49,6 +50,8 @@ public class ArrayToJsonCast

private static final MethodHandle METHOD_HANDLE = methodHandle(ArrayToJsonCast.class, "toJson", JsonGeneratorWriter.class, Block.class);

private static final JsonFactory JSON_FACTORY = createJsonFactory();

private ArrayToJsonCast()
{
super(FunctionMetadata.scalarBuilder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@
package io.trino.operator.scalar;

import com.fasterxml.jackson.core.JsonFactory;
import com.fasterxml.jackson.core.JsonFactoryBuilder;
import com.fasterxml.jackson.core.JsonGenerator;
import com.fasterxml.jackson.core.JsonParser;
import io.airlift.slice.DynamicSliceOutput;
Expand All @@ -30,7 +29,6 @@

import java.io.IOException;

import static com.fasterxml.jackson.core.JsonFactory.Feature.CANONICALIZE_FIELD_NAMES;
import static io.trino.spi.StandardErrorCode.INVALID_CAST_ARGUMENT;
import static io.trino.spi.StandardErrorCode.NUMERIC_VALUE_OUT_OF_RANGE;
import static io.trino.spi.function.OperatorType.CAST;
Expand All @@ -46,6 +44,7 @@
import static io.trino.spi.type.StandardTypes.VARCHAR;
import static io.trino.util.DateTimeUtils.printDate;
import static io.trino.util.Failures.checkCondition;
import static io.trino.util.JsonUtil.createJsonFactory;
import static io.trino.util.JsonUtil.createJsonGenerator;
import static io.trino.util.JsonUtil.createJsonParser;
import static io.trino.util.JsonUtil.currentTokenAsBigint;
Expand All @@ -61,7 +60,7 @@

public final class JsonOperators
{
public static final JsonFactory JSON_FACTORY = new JsonFactoryBuilder().disable(CANONICALIZE_FIELD_NAMES).build();
private static final JsonFactory JSON_FACTORY = createJsonFactory();

private JsonOperators()
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@
*/
package io.trino.operator.scalar;

import com.fasterxml.jackson.core.JsonFactory;
import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.JsonToken;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.collect.ImmutableList;
import io.airlift.slice.Slice;
import io.trino.annotation.UsedByGeneratedCode;
Expand All @@ -41,8 +43,8 @@
import static io.trino.spi.type.TypeSignature.arrayType;
import static io.trino.type.JsonType.JSON;
import static io.trino.util.Failures.checkCondition;
import static io.trino.util.JsonUtil.JSON_FACTORY;
import static io.trino.util.JsonUtil.canCastFromJson;
import static io.trino.util.JsonUtil.createJsonFactory;
import static io.trino.util.JsonUtil.createJsonParser;
import static io.trino.util.JsonUtil.truncateIfNecessaryForErrorMessage;
import static io.trino.util.Reflection.methodHandle;
Expand All @@ -54,6 +56,13 @@ public class JsonToArrayCast
public static final JsonToArrayCast JSON_TO_ARRAY = new JsonToArrayCast();
private static final MethodHandle METHOD_HANDLE = methodHandle(JsonToArrayCast.class, "toArray", ArrayType.class, BlockBuilderAppender.class, ConnectorSession.class, Slice.class);

private static final JsonFactory JSON_FACTORY = createJsonFactory();

static {
// Changes factory. Necessary for JsonParser.readValueAsTree to work.
new ObjectMapper(JSON_FACTORY);
}

private JsonToArrayCast()
{
super(FunctionMetadata.scalarBuilder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@
*/
package io.trino.operator.scalar;

import com.fasterxml.jackson.core.JsonFactory;
import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.JsonToken;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.collect.ImmutableList;
import io.airlift.slice.Slice;
import io.trino.annotation.UsedByGeneratedCode;
Expand Down Expand Up @@ -43,8 +45,8 @@
import static io.trino.type.JsonType.JSON;
import static io.trino.util.Failures.checkCondition;
import static io.trino.util.JsonUtil.BlockBuilderAppender.createBlockBuilderAppender;
import static io.trino.util.JsonUtil.JSON_FACTORY;
import static io.trino.util.JsonUtil.canCastFromJson;
import static io.trino.util.JsonUtil.createJsonFactory;
import static io.trino.util.JsonUtil.createJsonParser;
import static io.trino.util.JsonUtil.truncateIfNecessaryForErrorMessage;
import static io.trino.util.Reflection.methodHandle;
Expand All @@ -56,6 +58,13 @@ public class JsonToMapCast
public static final JsonToMapCast JSON_TO_MAP = new JsonToMapCast();
private static final MethodHandle METHOD_HANDLE = methodHandle(JsonToMapCast.class, "toMap", MapType.class, BlockBuilderAppender.class, ConnectorSession.class, Slice.class);

private static final JsonFactory JSON_FACTORY = createJsonFactory();

static {
// Changes factory. Necessary for JsonParser.readValueAsTree to work.
new ObjectMapper(JSON_FACTORY);
}

private JsonToMapCast()
{
super(FunctionMetadata.scalarBuilder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@
*/
package io.trino.operator.scalar;

import com.fasterxml.jackson.core.JsonFactory;
import com.fasterxml.jackson.core.JsonParser;
import com.fasterxml.jackson.core.JsonToken;
import com.fasterxml.jackson.databind.ObjectMapper;
import com.google.common.collect.ImmutableList;
import io.airlift.slice.Slice;
import io.trino.annotation.UsedByGeneratedCode;
Expand All @@ -41,8 +43,8 @@
import static io.trino.type.JsonType.JSON;
import static io.trino.util.Failures.checkCondition;
import static io.trino.util.JsonUtil.BlockBuilderAppender.createBlockBuilderAppender;
import static io.trino.util.JsonUtil.JSON_FACTORY;
import static io.trino.util.JsonUtil.canCastFromJson;
import static io.trino.util.JsonUtil.createJsonFactory;
import static io.trino.util.JsonUtil.createJsonParser;
import static io.trino.util.JsonUtil.truncateIfNecessaryForErrorMessage;
import static io.trino.util.Reflection.methodHandle;
Expand All @@ -54,6 +56,13 @@ public class JsonToRowCast
public static final JsonToRowCast JSON_TO_ROW = new JsonToRowCast();
private static final MethodHandle METHOD_HANDLE = methodHandle(JsonToRowCast.class, "toRow", RowType.class, BlockBuilderAppender.class, ConnectorSession.class, Slice.class);

private static final JsonFactory JSON_FACTORY = createJsonFactory();

static {
// Changes factory. Necessary for JsonParser.readValueAsTree to work.
new ObjectMapper(JSON_FACTORY);
}

private JsonToRowCast()
{
super(FunctionMetadata.scalarBuilder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
*/
package io.trino.operator.scalar;

import com.fasterxml.jackson.core.JsonFactory;
import com.fasterxml.jackson.core.JsonGenerator;
import com.google.common.collect.ImmutableList;
import io.airlift.slice.DynamicSliceOutput;
Expand All @@ -33,7 +34,6 @@
import java.util.Map;
import java.util.TreeMap;

import static io.trino.operator.scalar.JsonOperators.JSON_FACTORY;
import static io.trino.spi.StandardErrorCode.INVALID_CAST_ARGUMENT;
import static io.trino.spi.function.InvocationConvention.InvocationArgumentConvention.NEVER_NULL;
import static io.trino.spi.function.InvocationConvention.InvocationReturnConvention.FAIL_ON_NULL;
Expand All @@ -45,6 +45,7 @@
import static io.trino.util.JsonUtil.JsonGeneratorWriter;
import static io.trino.util.JsonUtil.ObjectKeyProvider;
import static io.trino.util.JsonUtil.canCastToJson;
import static io.trino.util.JsonUtil.createJsonFactory;
import static io.trino.util.JsonUtil.createJsonGenerator;
import static io.trino.util.Reflection.methodHandle;

Expand All @@ -54,6 +55,8 @@ public class MapToJsonCast
public static final MapToJsonCast MAP_TO_JSON = new MapToJsonCast();
private static final MethodHandle METHOD_HANDLE = methodHandle(MapToJsonCast.class, "toJson", ObjectKeyProvider.class, JsonGeneratorWriter.class, Block.class);

private static final JsonFactory JSON_FACTORY = createJsonFactory();

private MapToJsonCast()
{
super(FunctionMetadata.scalarBuilder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
*/
package io.trino.operator.scalar;

import com.fasterxml.jackson.core.JsonFactory;
import com.fasterxml.jackson.core.JsonGenerator;
import com.google.common.collect.ImmutableList;
import io.airlift.slice.DynamicSliceOutput;
Expand All @@ -35,7 +36,6 @@
import java.util.ArrayList;
import java.util.List;

import static io.trino.operator.scalar.JsonOperators.JSON_FACTORY;
import static io.trino.spi.StandardErrorCode.INVALID_CAST_ARGUMENT;
import static io.trino.spi.function.InvocationConvention.InvocationArgumentConvention.NEVER_NULL;
import static io.trino.spi.function.InvocationConvention.InvocationReturnConvention.FAIL_ON_NULL;
Expand All @@ -44,6 +44,7 @@
import static io.trino.util.Failures.checkCondition;
import static io.trino.util.JsonUtil.JsonGeneratorWriter.createJsonGeneratorWriter;
import static io.trino.util.JsonUtil.canCastToJson;
import static io.trino.util.JsonUtil.createJsonFactory;
import static io.trino.util.JsonUtil.createJsonGenerator;
import static io.trino.util.Reflection.methodHandle;

Expand All @@ -54,6 +55,8 @@ public class RowToJsonCast

private static final MethodHandle METHOD_HANDLE = methodHandle(RowToJsonCast.class, "toJsonObject", List.class, List.class, Block.class);

private static final JsonFactory JSON_FACTORY = createJsonFactory();

private RowToJsonCast()
{
super(FunctionMetadata.scalarBuilder()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
*/
package io.trino.operator.scalar.timestamp;

import com.fasterxml.jackson.core.JsonFactory;
import com.fasterxml.jackson.core.JsonGenerator;
import io.airlift.slice.DynamicSliceOutput;
import io.airlift.slice.Slice;
Expand All @@ -32,7 +33,7 @@
import static io.trino.spi.function.OperatorType.CAST;
import static io.trino.spi.type.StandardTypes.JSON;
import static io.trino.type.DateTimes.formatTimestamp;
import static io.trino.util.JsonUtil.JSON_FACTORY;
import static io.trino.util.JsonUtil.createJsonFactory;
import static io.trino.util.JsonUtil.createJsonGenerator;
import static java.lang.String.format;
import static java.time.ZoneOffset.UTC;
Expand All @@ -41,6 +42,7 @@
public final class TimestampToJsonCast
{
private static final DateTimeFormatter TIMESTAMP_FORMATTER = DateTimeFormatter.ofPattern("uuuu-MM-dd HH:mm:ss");
private static final JsonFactory JSON_FACTORY = createJsonFactory();

private TimestampToJsonCast() {}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
*/
package io.trino.type;

import com.fasterxml.jackson.core.JsonFactory;
import com.fasterxml.jackson.core.JsonGenerator;
import com.fasterxml.jackson.core.JsonParser;
import com.google.common.collect.ImmutableList;
Expand All @@ -39,7 +40,6 @@
import java.math.BigDecimal;

import static io.airlift.slice.Slices.utf8Slice;
import static io.trino.operator.scalar.JsonOperators.JSON_FACTORY;
import static io.trino.spi.StandardErrorCode.INVALID_CAST_ARGUMENT;
import static io.trino.spi.function.InvocationConvention.InvocationReturnConvention.FAIL_ON_NULL;
import static io.trino.spi.function.InvocationConvention.InvocationReturnConvention.NULLABLE_RETURN;
Expand All @@ -61,6 +61,7 @@
import static io.trino.spi.type.VarcharType.UNBOUNDED_LENGTH;
import static io.trino.type.JsonType.JSON;
import static io.trino.util.Failures.checkCondition;
import static io.trino.util.JsonUtil.createJsonFactory;
import static io.trino.util.JsonUtil.createJsonGenerator;
import static io.trino.util.JsonUtil.createJsonParser;
import static io.trino.util.JsonUtil.currentTokenAsLongDecimal;
Expand Down Expand Up @@ -92,6 +93,8 @@ public final class DecimalCasts
public static final SqlScalarFunction DECIMAL_TO_JSON_CAST = castFunctionFromDecimalTo(JSON.getTypeSignature(), "shortDecimalToJson", "longDecimalToJson");
public static final SqlScalarFunction JSON_TO_DECIMAL_CAST = castFunctionToDecimalFromBuilder(JSON.getTypeSignature(), true, "jsonToShortDecimal", "jsonToLongDecimal");

private static final JsonFactory JSON_FACTORY = createJsonFactory();

private static SqlScalarFunction castFunctionFromDecimalTo(TypeSignature to, String... methodNames)
{
Signature signature = Signature.builder()
Expand Down
10 changes: 7 additions & 3 deletions core/trino-main/src/main/java/io/trino/util/JsonUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -102,16 +102,20 @@

public final class JsonUtil
{
public static final JsonFactory JSON_FACTORY = new JsonFactoryBuilder().disable(CANONICALIZE_FIELD_NAMES).build();
private JsonUtil() {}

// This object mapper is constructed without .configure(ORDER_MAP_ENTRIES_BY_KEYS, true) because
// `OBJECT_MAPPER.writeValueAsString(parser.readValueAsTree());` preserves input order.
// Be aware. Using it arbitrarily can produce invalid json (ordered by key is required in Trino).
private static final ObjectMapper OBJECT_MAPPED_UNORDERED = new ObjectMapper(JSON_FACTORY);
private static final ObjectMapper OBJECT_MAPPED_UNORDERED = new ObjectMapper(createJsonFactory());

private static final int MAX_JSON_LENGTH_IN_ERROR_MESSAGE = 10_000;

private JsonUtil() {}
// Note: JsonFactory is mutable, instances cannot be shared openly.
public static JsonFactory createJsonFactory()
{
return new JsonFactoryBuilder().disable(CANONICALIZE_FIELD_NAMES).build();
}

public static JsonParser createJsonParser(JsonFactory factory, Slice json)
throws IOException
Expand Down