From 23cafa3a21a23d69fc42b2f90c2f0ec2b2210060 Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Wed, 22 Jul 2026 09:32:22 +0100 Subject: [PATCH 01/12] DRILL-8549. Add Jackson validation of polymorphic types. --- .../org/apache/drill/common/util/JacksonUtils.java | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/common/src/main/java/org/apache/drill/common/util/JacksonUtils.java b/common/src/main/java/org/apache/drill/common/util/JacksonUtils.java index e0cb0dee805..39bab80d322 100644 --- a/common/src/main/java/org/apache/drill/common/util/JacksonUtils.java +++ b/common/src/main/java/org/apache/drill/common/util/JacksonUtils.java @@ -20,6 +20,8 @@ import com.fasterxml.jackson.core.JsonFactory; import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.json.JsonMapper; +import com.fasterxml.jackson.databind.jsontype.BasicPolymorphicTypeValidator; +import com.fasterxml.jackson.databind.jsontype.PolymorphicTypeValidator; /** * Utility class which contain methods for interacting with Jackson. @@ -50,7 +52,8 @@ public static ObjectMapper createObjectMapper(final JsonFactory factory) { * @return an {@link JsonMapper.Builder} instance */ public static JsonMapper.Builder createJsonMapperBuilder() { - return JsonMapper.builder(); + return JsonMapper.builder() + .polymorphicTypeValidator(createPolymorphicTypeValidator()); } /** @@ -59,6 +62,13 @@ public static JsonMapper.Builder createJsonMapperBuilder() { * @return an {@link JsonMapper.Builder} instance */ public static JsonMapper.Builder createJsonMapperBuilder(final JsonFactory factory) { - return JsonMapper.builder(factory); + return JsonMapper.builder(factory) + .polymorphicTypeValidator(createPolymorphicTypeValidator()); + } + + private static final PolymorphicTypeValidator createPolymorphicTypeValidator() { + return BasicPolymorphicTypeValidator.builder() + .allowIfSubType("org.apache.drill.") + .build(); } } From ca6c4bb9e46615918c1fa70a5c705f59519d0648 Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Wed, 22 Jul 2026 09:44:42 +0100 Subject: [PATCH 02/12] Update pom.xml --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 229a50405f9..5568fa6d251 100644 --- a/pom.xml +++ b/pom.xml @@ -96,7 +96,7 @@ 5.11.0 0.12.1 1.3.1 - 2.18.3 + 2.18.9 3.1.12 3.29.2-GA 3.0.0 From 4c721fc8b5aa30baadad5910cc64592cb2a1bb00 Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Wed, 22 Jul 2026 10:45:26 +0100 Subject: [PATCH 03/12] Update JacksonUtils.java --- .../java/org/apache/drill/common/util/JacksonUtils.java | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/common/src/main/java/org/apache/drill/common/util/JacksonUtils.java b/common/src/main/java/org/apache/drill/common/util/JacksonUtils.java index 39bab80d322..f98fc0b2911 100644 --- a/common/src/main/java/org/apache/drill/common/util/JacksonUtils.java +++ b/common/src/main/java/org/apache/drill/common/util/JacksonUtils.java @@ -66,9 +66,12 @@ public static JsonMapper.Builder createJsonMapperBuilder(final JsonFactory facto .polymorphicTypeValidator(createPolymorphicTypeValidator()); } - private static final PolymorphicTypeValidator createPolymorphicTypeValidator() { + private static PolymorphicTypeValidator createPolymorphicTypeValidator() { + // only use case appears to be org.apache.drill.metastore.statistics.StatisticsHolder + // which holds a number, so allow only subtypes of Number + // the more restrictive this validator is, the better for security return BasicPolymorphicTypeValidator.builder() - .allowIfSubType("org.apache.drill.") + .allowIfSubType(Number.class) .build(); } } From 0dd6c73c4d510a26a02341836db072b71ab8fbbd Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Wed, 22 Jul 2026 11:03:02 +0100 Subject: [PATCH 04/12] Update JacksonUtils.java --- .../org/apache/drill/common/util/JacksonUtils.java | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/common/src/main/java/org/apache/drill/common/util/JacksonUtils.java b/common/src/main/java/org/apache/drill/common/util/JacksonUtils.java index f98fc0b2911..1cedf6f6d6f 100644 --- a/common/src/main/java/org/apache/drill/common/util/JacksonUtils.java +++ b/common/src/main/java/org/apache/drill/common/util/JacksonUtils.java @@ -67,11 +67,18 @@ public static JsonMapper.Builder createJsonMapperBuilder(final JsonFactory facto } private static PolymorphicTypeValidator createPolymorphicTypeValidator() { - // only use case appears to be org.apache.drill.metastore.statistics.StatisticsHolder - // which holds a number, so allow only subtypes of Number - // the more restrictive this validator is, the better for security + // Only use case appears to be org.apache.drill.metastore.statistics.StatisticsHolder + // which can hold a number or in theory, any type. The problem is that it is a security hole + // to accept any type because a hacker could use it to load a gadget. + // The main use case appears to be for Parquet and I've made a best guess as to what types to + // restrict to. + // The more restrictive this validator is, the better for security. return BasicPolymorphicTypeValidator.builder() .allowIfSubType(Number.class) + .allowIfSubType(Boolean.class) + .allowIfSubType(String.class) + .allowIfSubType(byte[].class) + .allowIfSubType("java.time.") .build(); } } From 3f6c8409e2b38e9b79a1f3c6b6a601f823db9f26 Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Wed, 22 Jul 2026 12:07:49 +0100 Subject: [PATCH 05/12] refactor --- .../drill/common/util/JacksonUtils.java | 29 +++++++------------ .../statistics/ColumnStatistics.java | 8 ++--- .../statistics/StatisticsHolder.java | 27 ++++++++++++++++- 3 files changed, 40 insertions(+), 24 deletions(-) diff --git a/common/src/main/java/org/apache/drill/common/util/JacksonUtils.java b/common/src/main/java/org/apache/drill/common/util/JacksonUtils.java index 1cedf6f6d6f..30542ea622a 100644 --- a/common/src/main/java/org/apache/drill/common/util/JacksonUtils.java +++ b/common/src/main/java/org/apache/drill/common/util/JacksonUtils.java @@ -21,7 +21,6 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.json.JsonMapper; import com.fasterxml.jackson.databind.jsontype.BasicPolymorphicTypeValidator; -import com.fasterxml.jackson.databind.jsontype.PolymorphicTypeValidator; /** * Utility class which contain methods for interacting with Jackson. @@ -52,8 +51,12 @@ public static ObjectMapper createObjectMapper(final JsonFactory factory) { * @return an {@link JsonMapper.Builder} instance */ public static JsonMapper.Builder createJsonMapperBuilder() { + // it is deliberate to have polymorphicTypeValidator that allows nothing + // for security reasons + // org.apache.drill.metastore.statistics.StatisticsHolder replaces this with + // a polymorphicTypeValidator that allows only the types it needs return JsonMapper.builder() - .polymorphicTypeValidator(createPolymorphicTypeValidator()); + .polymorphicTypeValidator(BasicPolymorphicTypeValidator.builder().build()); } /** @@ -62,23 +65,11 @@ public static JsonMapper.Builder createJsonMapperBuilder() { * @return an {@link JsonMapper.Builder} instance */ public static JsonMapper.Builder createJsonMapperBuilder(final JsonFactory factory) { + // it is deliberate to have polymorphicTypeValidator that allows nothing + // for security reasons + // org.apache.drill.metastore.statistics.StatisticsHolder replaces this with + // a polymorphicTypeValidator that allows only the types it needs return JsonMapper.builder(factory) - .polymorphicTypeValidator(createPolymorphicTypeValidator()); - } - - private static PolymorphicTypeValidator createPolymorphicTypeValidator() { - // Only use case appears to be org.apache.drill.metastore.statistics.StatisticsHolder - // which can hold a number or in theory, any type. The problem is that it is a security hole - // to accept any type because a hacker could use it to load a gadget. - // The main use case appears to be for Parquet and I've made a best guess as to what types to - // restrict to. - // The more restrictive this validator is, the better for security. - return BasicPolymorphicTypeValidator.builder() - .allowIfSubType(Number.class) - .allowIfSubType(Boolean.class) - .allowIfSubType(String.class) - .allowIfSubType(byte[].class) - .allowIfSubType("java.time.") - .build(); + .polymorphicTypeValidator(BasicPolymorphicTypeValidator.builder().build()); } } diff --git a/metastore/metastore-api/src/main/java/org/apache/drill/metastore/statistics/ColumnStatistics.java b/metastore/metastore-api/src/main/java/org/apache/drill/metastore/statistics/ColumnStatistics.java index b909280a5e6..b28a6088d3b 100644 --- a/metastore/metastore-api/src/main/java/org/apache/drill/metastore/statistics/ColumnStatistics.java +++ b/metastore/metastore-api/src/main/java/org/apache/drill/metastore/statistics/ColumnStatistics.java @@ -28,7 +28,6 @@ import com.fasterxml.jackson.databind.ObjectWriter; import com.fasterxml.jackson.datatype.joda.JodaModule; import org.apache.drill.common.types.TypeProtos; -import org.apache.drill.common.util.JacksonUtils; import org.apache.drill.metastore.util.TableMetadataUtils; import java.io.IOException; @@ -67,9 +66,10 @@ @JsonPropertyOrder({"statistics", "comparator"}) public class ColumnStatistics { - private static final ObjectMapper MAPPER = JacksonUtils.createJsonMapperBuilder() - .addModule(new JodaModule()) - .build(); + private static final ObjectMapper MAPPER = + StatisticsHolder.createJsonMapperBuilder() + .addModule(new JodaModule()) + .build(); private static final ObjectWriter OBJECT_WRITER = MAPPER.writerFor(ColumnStatistics.class); diff --git a/metastore/metastore-api/src/main/java/org/apache/drill/metastore/statistics/StatisticsHolder.java b/metastore/metastore-api/src/main/java/org/apache/drill/metastore/statistics/StatisticsHolder.java index a7964a7c230..9d2a0991cec 100644 --- a/metastore/metastore-api/src/main/java/org/apache/drill/metastore/statistics/StatisticsHolder.java +++ b/metastore/metastore-api/src/main/java/org/apache/drill/metastore/statistics/StatisticsHolder.java @@ -25,6 +25,9 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.ObjectReader; import com.fasterxml.jackson.databind.ObjectWriter; +import com.fasterxml.jackson.databind.json.JsonMapper; +import com.fasterxml.jackson.databind.jsontype.BasicPolymorphicTypeValidator; +import com.fasterxml.jackson.databind.jsontype.PolymorphicTypeValidator; import org.apache.drill.common.util.JacksonUtils; import java.io.IOException; @@ -39,7 +42,7 @@ @JsonInclude(JsonInclude.Include.NON_DEFAULT) public class StatisticsHolder { - private static final ObjectMapper OBJECT_MAPPER = JacksonUtils.createObjectMapper(); + private static final ObjectMapper OBJECT_MAPPER = createJsonMapperBuilder().build(); private static final ObjectWriter OBJECT_WRITER = OBJECT_MAPPER.writerFor(StatisticsHolder.class); private static final ObjectReader OBJECT_READER = OBJECT_MAPPER.readerFor(StatisticsHolder.class); @@ -110,4 +113,26 @@ public static StatisticsHolder of(String serialized) { throw new IllegalArgumentException("Unable to convert statistics holder from json string: " + serialized, e); } } + + static JsonMapper.Builder createJsonMapperBuilder() { + JacksonUtils.createJsonMapperBuilder() + .polymorphicTypeValidator(createPolymorphicTypeValidator()); + } + + // Only use case appears to be org.apache.drill.metastore.statistics.StatisticsHolder + // which can hold a number or in theory, any type. The problem is that it is a security hole + // to accept any type because a hacker could use it to load a gadget. + // The main use case appears to be for Parquet and I've made a best guess as to what types to + // restrict to. + // The more restrictive this validator is, the better for security. + private static PolymorphicTypeValidator createPolymorphicTypeValidator() { + return BasicPolymorphicTypeValidator.builder() + .allowIfSubType(Number.class) + .allowIfSubType(Boolean.class) + .allowIfSubType(String.class) + .allowIfSubType(byte[].class) + .allowIfSubType("java.time.") + .allowIfSubType("org.joda.time.") // Joda used by ColumnStatistics + .build(); + } } From 9825faf2ed03341162d0a2c8c93e228adb1e1830 Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Wed, 22 Jul 2026 12:18:09 +0100 Subject: [PATCH 06/12] Update StatisticsHolder.java --- .../apache/drill/metastore/statistics/StatisticsHolder.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/metastore/metastore-api/src/main/java/org/apache/drill/metastore/statistics/StatisticsHolder.java b/metastore/metastore-api/src/main/java/org/apache/drill/metastore/statistics/StatisticsHolder.java index 9d2a0991cec..5d61637e4ff 100644 --- a/metastore/metastore-api/src/main/java/org/apache/drill/metastore/statistics/StatisticsHolder.java +++ b/metastore/metastore-api/src/main/java/org/apache/drill/metastore/statistics/StatisticsHolder.java @@ -115,7 +115,8 @@ public static StatisticsHolder of(String serialized) { } static JsonMapper.Builder createJsonMapperBuilder() { - JacksonUtils.createJsonMapperBuilder() + return JacksonUtils + .createJsonMapperBuilder() .polymorphicTypeValidator(createPolymorphicTypeValidator()); } From 30939aa53d24c4fb76938d1a25086809a563c8e9 Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Wed, 22 Jul 2026 14:30:10 +0100 Subject: [PATCH 07/12] Update StatisticsHolder.java --- .../org/apache/drill/metastore/statistics/StatisticsHolder.java | 1 + 1 file changed, 1 insertion(+) diff --git a/metastore/metastore-api/src/main/java/org/apache/drill/metastore/statistics/StatisticsHolder.java b/metastore/metastore-api/src/main/java/org/apache/drill/metastore/statistics/StatisticsHolder.java index 5d61637e4ff..3e5a21770a8 100644 --- a/metastore/metastore-api/src/main/java/org/apache/drill/metastore/statistics/StatisticsHolder.java +++ b/metastore/metastore-api/src/main/java/org/apache/drill/metastore/statistics/StatisticsHolder.java @@ -134,6 +134,7 @@ private static PolymorphicTypeValidator createPolymorphicTypeValidator() { .allowIfSubType(byte[].class) .allowIfSubType("java.time.") .allowIfSubType("org.joda.time.") // Joda used by ColumnStatistics + .allowIfSubType("org.apache.drill.metastore.") .build(); } } From 14cf573fa9d85762599261e97f5fe37dd642071a Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Wed, 22 Jul 2026 18:23:17 +0100 Subject: [PATCH 08/12] Update StatisticsHolder.java --- .../org/apache/drill/metastore/statistics/StatisticsHolder.java | 1 + 1 file changed, 1 insertion(+) diff --git a/metastore/metastore-api/src/main/java/org/apache/drill/metastore/statistics/StatisticsHolder.java b/metastore/metastore-api/src/main/java/org/apache/drill/metastore/statistics/StatisticsHolder.java index 3e5a21770a8..eea3ea25b73 100644 --- a/metastore/metastore-api/src/main/java/org/apache/drill/metastore/statistics/StatisticsHolder.java +++ b/metastore/metastore-api/src/main/java/org/apache/drill/metastore/statistics/StatisticsHolder.java @@ -134,6 +134,7 @@ private static PolymorphicTypeValidator createPolymorphicTypeValidator() { .allowIfSubType(byte[].class) .allowIfSubType("java.time.") .allowIfSubType("org.joda.time.") // Joda used by ColumnStatistics + .allowIfSubType("org.apache.drill.exec.") .allowIfSubType("org.apache.drill.metastore.") .build(); } From 8fda034a493529a669d472b6d9bc33f2db925fda Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Wed, 22 Jul 2026 20:33:24 +0100 Subject: [PATCH 09/12] refactor --- .../drill/common/util/JacksonUtils.java | 21 +++++++++++++ .../exec/planner/common/DrillStatsTable.java | 2 +- .../planner/common/DrillValuesRelBase.java | 3 +- .../statistics/ColumnStatistics.java | 3 +- .../statistics/StatisticsHolder.java | 31 ++----------------- 5 files changed, 28 insertions(+), 32 deletions(-) diff --git a/common/src/main/java/org/apache/drill/common/util/JacksonUtils.java b/common/src/main/java/org/apache/drill/common/util/JacksonUtils.java index 30542ea622a..e7e7161ff56 100644 --- a/common/src/main/java/org/apache/drill/common/util/JacksonUtils.java +++ b/common/src/main/java/org/apache/drill/common/util/JacksonUtils.java @@ -21,6 +21,7 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.json.JsonMapper; import com.fasterxml.jackson.databind.jsontype.BasicPolymorphicTypeValidator; +import com.fasterxml.jackson.databind.jsontype.PolymorphicTypeValidator; /** * Utility class which contain methods for interacting with Jackson. @@ -72,4 +73,24 @@ public static JsonMapper.Builder createJsonMapperBuilder(final JsonFactory facto return JsonMapper.builder(factory) .polymorphicTypeValidator(BasicPolymorphicTypeValidator.builder().build()); } + + public static JsonMapper.Builder createJsonMapperBuilderWithPolymorphicTypeValidator() { + return createJsonMapperBuilder() + .polymorphicTypeValidator(createPolymorphicTypeValidator()); + } + + // The more restrictive this validator is, the better for security. + private static PolymorphicTypeValidator createPolymorphicTypeValidator() { + return BasicPolymorphicTypeValidator.builder() + .allowIfSubType(Number.class) + .allowIfSubType(Boolean.class) + .allowIfSubType(String.class) + .allowIfSubType(byte[].class) + .allowIfSubType("java.time.") + .allowIfSubType("org.joda.time.") // Joda used by ColumnStatistics + .allowIfSubType("org.apache.drill.exec.") + .allowIfSubType("org.apache.drill.metastore.") + .build(); + } + } diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillStatsTable.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillStatsTable.java index bfb58cb5148..f5d713f58ca 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillStatsTable.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillStatsTable.java @@ -470,7 +470,7 @@ public static ObjectMapper getMapper() { .addSerializer(TypeProtos.MajorType.class, new MajorTypeSerDe.Se()) .addDeserializer(TypeProtos.MajorType.class, new MajorTypeSerDe.De()) .addDeserializer(SchemaPath.class, new SchemaPath.De()); - ObjectMapper mapper = JacksonUtils.createJsonMapperBuilder() + ObjectMapper mapper = JacksonUtils.createJsonMapperBuilderWithPolymorphicTypeValidator() .addModule(deModule) .build(); mapper.registerSubtypes(new NamedType(NumericEquiDepthHistogram.class, "numeric-equi-depth")); diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillValuesRelBase.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillValuesRelBase.java index 233414c20b3..3a4e0ba6827 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillValuesRelBase.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillValuesRelBase.java @@ -51,7 +51,8 @@ */ public abstract class DrillValuesRelBase extends Values implements DrillRelNode { - private static final ObjectMapper MAPPER = JacksonUtils.createObjectMapper(); + private static final ObjectMapper MAPPER = + JacksonUtils.createJsonMapperBuilderWithPolymorphicTypeValidator().build(); protected final String content; diff --git a/metastore/metastore-api/src/main/java/org/apache/drill/metastore/statistics/ColumnStatistics.java b/metastore/metastore-api/src/main/java/org/apache/drill/metastore/statistics/ColumnStatistics.java index b28a6088d3b..cef21cd0133 100644 --- a/metastore/metastore-api/src/main/java/org/apache/drill/metastore/statistics/ColumnStatistics.java +++ b/metastore/metastore-api/src/main/java/org/apache/drill/metastore/statistics/ColumnStatistics.java @@ -28,6 +28,7 @@ import com.fasterxml.jackson.databind.ObjectWriter; import com.fasterxml.jackson.datatype.joda.JodaModule; import org.apache.drill.common.types.TypeProtos; +import org.apache.drill.common.util.JacksonUtils; import org.apache.drill.metastore.util.TableMetadataUtils; import java.io.IOException; @@ -67,7 +68,7 @@ public class ColumnStatistics { private static final ObjectMapper MAPPER = - StatisticsHolder.createJsonMapperBuilder() + JacksonUtils.createJsonMapperBuilderWithPolymorphicTypeValidator() .addModule(new JodaModule()) .build(); diff --git a/metastore/metastore-api/src/main/java/org/apache/drill/metastore/statistics/StatisticsHolder.java b/metastore/metastore-api/src/main/java/org/apache/drill/metastore/statistics/StatisticsHolder.java index eea3ea25b73..6b2d2b34667 100644 --- a/metastore/metastore-api/src/main/java/org/apache/drill/metastore/statistics/StatisticsHolder.java +++ b/metastore/metastore-api/src/main/java/org/apache/drill/metastore/statistics/StatisticsHolder.java @@ -25,9 +25,6 @@ import com.fasterxml.jackson.databind.ObjectMapper; import com.fasterxml.jackson.databind.ObjectReader; import com.fasterxml.jackson.databind.ObjectWriter; -import com.fasterxml.jackson.databind.json.JsonMapper; -import com.fasterxml.jackson.databind.jsontype.BasicPolymorphicTypeValidator; -import com.fasterxml.jackson.databind.jsontype.PolymorphicTypeValidator; import org.apache.drill.common.util.JacksonUtils; import java.io.IOException; @@ -42,7 +39,8 @@ @JsonInclude(JsonInclude.Include.NON_DEFAULT) public class StatisticsHolder { - private static final ObjectMapper OBJECT_MAPPER = createJsonMapperBuilder().build(); + private static final ObjectMapper OBJECT_MAPPER = + JacksonUtils.createJsonMapperBuilderWithPolymorphicTypeValidator().build(); private static final ObjectWriter OBJECT_WRITER = OBJECT_MAPPER.writerFor(StatisticsHolder.class); private static final ObjectReader OBJECT_READER = OBJECT_MAPPER.readerFor(StatisticsHolder.class); @@ -113,29 +111,4 @@ public static StatisticsHolder of(String serialized) { throw new IllegalArgumentException("Unable to convert statistics holder from json string: " + serialized, e); } } - - static JsonMapper.Builder createJsonMapperBuilder() { - return JacksonUtils - .createJsonMapperBuilder() - .polymorphicTypeValidator(createPolymorphicTypeValidator()); - } - - // Only use case appears to be org.apache.drill.metastore.statistics.StatisticsHolder - // which can hold a number or in theory, any type. The problem is that it is a security hole - // to accept any type because a hacker could use it to load a gadget. - // The main use case appears to be for Parquet and I've made a best guess as to what types to - // restrict to. - // The more restrictive this validator is, the better for security. - private static PolymorphicTypeValidator createPolymorphicTypeValidator() { - return BasicPolymorphicTypeValidator.builder() - .allowIfSubType(Number.class) - .allowIfSubType(Boolean.class) - .allowIfSubType(String.class) - .allowIfSubType(byte[].class) - .allowIfSubType("java.time.") - .allowIfSubType("org.joda.time.") // Joda used by ColumnStatistics - .allowIfSubType("org.apache.drill.exec.") - .allowIfSubType("org.apache.drill.metastore.") - .build(); - } } From a4f5d9540fc062737fa5f71b751af09e57106416 Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Wed, 22 Jul 2026 22:34:16 +0100 Subject: [PATCH 10/12] try to fix broken test --- .../apache/drill/common/util/JacksonUtils.java | 18 ++++++++++++++++++ .../store/DrillbitPluginRegistryContext.java | 2 +- .../common/config/LogicalPlanPersistence.java | 10 +++++----- 3 files changed, 24 insertions(+), 6 deletions(-) diff --git a/common/src/main/java/org/apache/drill/common/util/JacksonUtils.java b/common/src/main/java/org/apache/drill/common/util/JacksonUtils.java index e7e7161ff56..017ab0061bb 100644 --- a/common/src/main/java/org/apache/drill/common/util/JacksonUtils.java +++ b/common/src/main/java/org/apache/drill/common/util/JacksonUtils.java @@ -74,11 +74,29 @@ public static JsonMapper.Builder createJsonMapperBuilder(final JsonFactory facto .polymorphicTypeValidator(BasicPolymorphicTypeValidator.builder().build()); } + /** + * Creates a new instance of the Jackson {@link JsonMapper.Builder} that has a + * PolymorphicTypeValidator applied that allows a curated set of classes + * that can be loaded. + * @return an {@link JsonMapper.Builder} instance + */ public static JsonMapper.Builder createJsonMapperBuilderWithPolymorphicTypeValidator() { return createJsonMapperBuilder() .polymorphicTypeValidator(createPolymorphicTypeValidator()); } + /** + * Creates a new instance of the Jackson {@link JsonMapper.Builder} that has a + * PolymorphicTypeValidator applied that allows a curated set of classes + * that can be loaded. + * @param factory a {@link JsonFactory} instance + * @return an {@link JsonMapper.Builder} instance + */ + public static JsonMapper.Builder createJsonMapperBuilderWithPolymorphicTypeValidator(JsonFactory factory) { + return createJsonMapperBuilder(factory) + .polymorphicTypeValidator(createPolymorphicTypeValidator()); + } + // The more restrictive this validator is, the better for security. private static PolymorphicTypeValidator createPolymorphicTypeValidator() { return BasicPolymorphicTypeValidator.builder() diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/store/DrillbitPluginRegistryContext.java b/exec/java-exec/src/main/java/org/apache/drill/exec/store/DrillbitPluginRegistryContext.java index 4565b55fa77..91cff4492fa 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/store/DrillbitPluginRegistryContext.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/store/DrillbitPluginRegistryContext.java @@ -45,7 +45,7 @@ public DrillbitPluginRegistryContext(DrillbitContext drillbitContext) { // to handle HOCON format in the override file LogicalPlanPersistence persistence = new LogicalPlanPersistence(drillbitContext.getConfig(), drillbitContext.getClasspathScan(), - JacksonUtils.createObjectMapper(new HoconFactory())); + JacksonUtils.createJsonMapperBuilderWithPolymorphicTypeValidator(new HoconFactory()).build()); hoconMapper = persistence.getMapper(); } diff --git a/logical/src/main/java/org/apache/drill/common/config/LogicalPlanPersistence.java b/logical/src/main/java/org/apache/drill/common/config/LogicalPlanPersistence.java index 903b8f014f6..4f9e3cb4263 100644 --- a/logical/src/main/java/org/apache/drill/common/config/LogicalPlanPersistence.java +++ b/logical/src/main/java/org/apache/drill/common/config/LogicalPlanPersistence.java @@ -46,7 +46,7 @@ public class LogicalPlanPersistence { private final ObjectMapper mapper; public LogicalPlanPersistence(DrillConfig conf, ScanResult scanResult) { - this(conf, scanResult, JacksonUtils.createObjectMapper()); + this(conf, scanResult, JacksonUtils.createJsonMapperBuilderWithPolymorphicTypeValidator().build()); } public LogicalPlanPersistence(DrillConfig conf, ScanResult scanResult, ObjectMapper mapper) { @@ -65,9 +65,9 @@ public LogicalPlanPersistence(DrillConfig conf, ScanResult scanResult, ObjectMap mapper.setInjectableValues(injectables); mapper.registerModule(deserModule); mapper.enable(SerializationFeature.INDENT_OUTPUT); - mapper.configure(Feature.ALLOW_UNQUOTED_FIELD_NAMES, true); - mapper.configure(JsonGenerator.Feature.QUOTE_FIELD_NAMES, true); - mapper.configure(Feature.ALLOW_COMMENTS, true); + mapper.enable(Feature.ALLOW_UNQUOTED_FIELD_NAMES); + mapper.enable(JsonGenerator.Feature.QUOTE_FIELD_NAMES); + mapper.enable(Feature.ALLOW_COMMENTS); mapper.setFilterProvider(new SimpleFilterProvider().setFailOnUnknownId(false)); // For LogicalOperatorBase registerSubtypes(getSubTypes(scanResult, LogicalOperator.class)); @@ -92,7 +92,7 @@ private void registerSubtypes(Set> types) { * Scan for implementations of the given interface. * * @param classpathScan Drill configuration object used to find the packages to scan - * @return list of classes that implement the interface. + * @return set of classes that implement the interface. */ public static Set> getSubTypes(final ScanResult classpathScan, Class parent) { Set> subclasses = classpathScan.getImplementations(parent); From 7fced2916ef8ae0d23d89a7eed9ec3c7e2d8eb72 Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Thu, 23 Jul 2026 09:57:13 +0100 Subject: [PATCH 11/12] refactor --- .../drill/common/util/JacksonUtils.java | 29 +------------------ .../exec/planner/common/DrillStatsTable.java | 2 +- .../planner/common/DrillValuesRelBase.java | 3 +- .../store/DrillbitPluginRegistryContext.java | 2 +- .../common/config/LogicalPlanPersistence.java | 2 +- .../statistics/ColumnStatistics.java | 2 +- .../statistics/StatisticsHolder.java | 3 +- 7 files changed, 7 insertions(+), 36 deletions(-) diff --git a/common/src/main/java/org/apache/drill/common/util/JacksonUtils.java b/common/src/main/java/org/apache/drill/common/util/JacksonUtils.java index 017ab0061bb..70ee21109e8 100644 --- a/common/src/main/java/org/apache/drill/common/util/JacksonUtils.java +++ b/common/src/main/java/org/apache/drill/common/util/JacksonUtils.java @@ -57,7 +57,7 @@ public static JsonMapper.Builder createJsonMapperBuilder() { // org.apache.drill.metastore.statistics.StatisticsHolder replaces this with // a polymorphicTypeValidator that allows only the types it needs return JsonMapper.builder() - .polymorphicTypeValidator(BasicPolymorphicTypeValidator.builder().build()); + .polymorphicTypeValidator(createPolymorphicTypeValidator()); } /** @@ -66,34 +66,7 @@ public static JsonMapper.Builder createJsonMapperBuilder() { * @return an {@link JsonMapper.Builder} instance */ public static JsonMapper.Builder createJsonMapperBuilder(final JsonFactory factory) { - // it is deliberate to have polymorphicTypeValidator that allows nothing - // for security reasons - // org.apache.drill.metastore.statistics.StatisticsHolder replaces this with - // a polymorphicTypeValidator that allows only the types it needs return JsonMapper.builder(factory) - .polymorphicTypeValidator(BasicPolymorphicTypeValidator.builder().build()); - } - - /** - * Creates a new instance of the Jackson {@link JsonMapper.Builder} that has a - * PolymorphicTypeValidator applied that allows a curated set of classes - * that can be loaded. - * @return an {@link JsonMapper.Builder} instance - */ - public static JsonMapper.Builder createJsonMapperBuilderWithPolymorphicTypeValidator() { - return createJsonMapperBuilder() - .polymorphicTypeValidator(createPolymorphicTypeValidator()); - } - - /** - * Creates a new instance of the Jackson {@link JsonMapper.Builder} that has a - * PolymorphicTypeValidator applied that allows a curated set of classes - * that can be loaded. - * @param factory a {@link JsonFactory} instance - * @return an {@link JsonMapper.Builder} instance - */ - public static JsonMapper.Builder createJsonMapperBuilderWithPolymorphicTypeValidator(JsonFactory factory) { - return createJsonMapperBuilder(factory) .polymorphicTypeValidator(createPolymorphicTypeValidator()); } diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillStatsTable.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillStatsTable.java index f5d713f58ca..bfb58cb5148 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillStatsTable.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillStatsTable.java @@ -470,7 +470,7 @@ public static ObjectMapper getMapper() { .addSerializer(TypeProtos.MajorType.class, new MajorTypeSerDe.Se()) .addDeserializer(TypeProtos.MajorType.class, new MajorTypeSerDe.De()) .addDeserializer(SchemaPath.class, new SchemaPath.De()); - ObjectMapper mapper = JacksonUtils.createJsonMapperBuilderWithPolymorphicTypeValidator() + ObjectMapper mapper = JacksonUtils.createJsonMapperBuilder() .addModule(deModule) .build(); mapper.registerSubtypes(new NamedType(NumericEquiDepthHistogram.class, "numeric-equi-depth")); diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillValuesRelBase.java b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillValuesRelBase.java index 3a4e0ba6827..233414c20b3 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillValuesRelBase.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/planner/common/DrillValuesRelBase.java @@ -51,8 +51,7 @@ */ public abstract class DrillValuesRelBase extends Values implements DrillRelNode { - private static final ObjectMapper MAPPER = - JacksonUtils.createJsonMapperBuilderWithPolymorphicTypeValidator().build(); + private static final ObjectMapper MAPPER = JacksonUtils.createObjectMapper(); protected final String content; diff --git a/exec/java-exec/src/main/java/org/apache/drill/exec/store/DrillbitPluginRegistryContext.java b/exec/java-exec/src/main/java/org/apache/drill/exec/store/DrillbitPluginRegistryContext.java index 91cff4492fa..44e9bb98d00 100644 --- a/exec/java-exec/src/main/java/org/apache/drill/exec/store/DrillbitPluginRegistryContext.java +++ b/exec/java-exec/src/main/java/org/apache/drill/exec/store/DrillbitPluginRegistryContext.java @@ -45,7 +45,7 @@ public DrillbitPluginRegistryContext(DrillbitContext drillbitContext) { // to handle HOCON format in the override file LogicalPlanPersistence persistence = new LogicalPlanPersistence(drillbitContext.getConfig(), drillbitContext.getClasspathScan(), - JacksonUtils.createJsonMapperBuilderWithPolymorphicTypeValidator(new HoconFactory()).build()); + JacksonUtils.createJsonMapperBuilder(new HoconFactory()).build()); hoconMapper = persistence.getMapper(); } diff --git a/logical/src/main/java/org/apache/drill/common/config/LogicalPlanPersistence.java b/logical/src/main/java/org/apache/drill/common/config/LogicalPlanPersistence.java index 4f9e3cb4263..48548714967 100644 --- a/logical/src/main/java/org/apache/drill/common/config/LogicalPlanPersistence.java +++ b/logical/src/main/java/org/apache/drill/common/config/LogicalPlanPersistence.java @@ -46,7 +46,7 @@ public class LogicalPlanPersistence { private final ObjectMapper mapper; public LogicalPlanPersistence(DrillConfig conf, ScanResult scanResult) { - this(conf, scanResult, JacksonUtils.createJsonMapperBuilderWithPolymorphicTypeValidator().build()); + this(conf, scanResult, JacksonUtils.createObjectMapper()); } public LogicalPlanPersistence(DrillConfig conf, ScanResult scanResult, ObjectMapper mapper) { diff --git a/metastore/metastore-api/src/main/java/org/apache/drill/metastore/statistics/ColumnStatistics.java b/metastore/metastore-api/src/main/java/org/apache/drill/metastore/statistics/ColumnStatistics.java index cef21cd0133..beb3e4eb2df 100644 --- a/metastore/metastore-api/src/main/java/org/apache/drill/metastore/statistics/ColumnStatistics.java +++ b/metastore/metastore-api/src/main/java/org/apache/drill/metastore/statistics/ColumnStatistics.java @@ -68,7 +68,7 @@ public class ColumnStatistics { private static final ObjectMapper MAPPER = - JacksonUtils.createJsonMapperBuilderWithPolymorphicTypeValidator() + JacksonUtils.createJsonMapperBuilder() .addModule(new JodaModule()) .build(); diff --git a/metastore/metastore-api/src/main/java/org/apache/drill/metastore/statistics/StatisticsHolder.java b/metastore/metastore-api/src/main/java/org/apache/drill/metastore/statistics/StatisticsHolder.java index 6b2d2b34667..a7964a7c230 100644 --- a/metastore/metastore-api/src/main/java/org/apache/drill/metastore/statistics/StatisticsHolder.java +++ b/metastore/metastore-api/src/main/java/org/apache/drill/metastore/statistics/StatisticsHolder.java @@ -39,8 +39,7 @@ @JsonInclude(JsonInclude.Include.NON_DEFAULT) public class StatisticsHolder { - private static final ObjectMapper OBJECT_MAPPER = - JacksonUtils.createJsonMapperBuilderWithPolymorphicTypeValidator().build(); + private static final ObjectMapper OBJECT_MAPPER = JacksonUtils.createObjectMapper(); private static final ObjectWriter OBJECT_WRITER = OBJECT_MAPPER.writerFor(StatisticsHolder.class); private static final ObjectReader OBJECT_READER = OBJECT_MAPPER.readerFor(StatisticsHolder.class); From 215e67fec639cf6ac8f956d9065d3793c56399ae Mon Sep 17 00:00:00 2001 From: PJ Fanning Date: Thu, 23 Jul 2026 13:39:35 +0100 Subject: [PATCH 12/12] Clean up comments in createJsonMapperBuilder method Removed comments explaining the polymorphicTypeValidator usage. --- .../main/java/org/apache/drill/common/util/JacksonUtils.java | 4 ---- 1 file changed, 4 deletions(-) diff --git a/common/src/main/java/org/apache/drill/common/util/JacksonUtils.java b/common/src/main/java/org/apache/drill/common/util/JacksonUtils.java index 70ee21109e8..e3fdb32eb04 100644 --- a/common/src/main/java/org/apache/drill/common/util/JacksonUtils.java +++ b/common/src/main/java/org/apache/drill/common/util/JacksonUtils.java @@ -52,10 +52,6 @@ public static ObjectMapper createObjectMapper(final JsonFactory factory) { * @return an {@link JsonMapper.Builder} instance */ public static JsonMapper.Builder createJsonMapperBuilder() { - // it is deliberate to have polymorphicTypeValidator that allows nothing - // for security reasons - // org.apache.drill.metastore.statistics.StatisticsHolder replaces this with - // a polymorphicTypeValidator that allows only the types it needs return JsonMapper.builder() .polymorphicTypeValidator(createPolymorphicTypeValidator()); }