diff --git a/common/src/main/java/org/opensearch/sql/common/setting/Settings.java b/common/src/main/java/org/opensearch/sql/common/setting/Settings.java index 5ab82f6b6c9..4f05682a2db 100644 --- a/common/src/main/java/org/opensearch/sql/common/setting/Settings.java +++ b/common/src/main/java/org/opensearch/sql/common/setting/Settings.java @@ -53,6 +53,9 @@ public enum Key { QUERY_MEMORY_LIMIT("plugins.query.memory_limit"), QUERY_SIZE_LIMIT("plugins.query.size_limit"), MAX_EXPRESSION_DEPTH("plugins.query.max_expression_depth"), + DESERIALIZATION_MAX_DEPTH("plugins.query.deserialization.max_depth"), + DESERIALIZATION_MAX_REFS("plugins.query.deserialization.max_refs"), + DESERIALIZATION_MAX_BYTES("plugins.query.deserialization.max_bytes"), QUERY_BUCKET_SIZE("plugins.query.buckets"), SEARCH_MAX_BUCKETS("search.max_buckets"), ENCYRPTION_MASTER_KEY("plugins.query.datasources.encryption.masterkey"), diff --git a/core/src/main/java/org/opensearch/sql/executor/pagination/PlanSerializer.java b/core/src/main/java/org/opensearch/sql/executor/pagination/PlanSerializer.java index 71ca80fdbf0..f7bd97966e9 100644 --- a/core/src/main/java/org/opensearch/sql/executor/pagination/PlanSerializer.java +++ b/core/src/main/java/org/opensearch/sql/executor/pagination/PlanSerializer.java @@ -17,7 +17,7 @@ import java.util.zip.Deflater; import java.util.zip.GZIPInputStream; import java.util.zip.GZIPOutputStream; -import lombok.RequiredArgsConstructor; +import org.opensearch.sql.common.setting.Settings; import org.opensearch.sql.exception.NoCursorException; import org.opensearch.sql.planner.SerializablePlan; import org.opensearch.sql.planner.physical.PhysicalPlan; @@ -28,12 +28,23 @@ * This class is entry point to paged requests. It is responsible to cursor serialization and * deserialization. */ -@RequiredArgsConstructor public class PlanSerializer { public static final String CURSOR_PREFIX = "n:"; private final StorageEngine engine; + /** Cluster settings supplying deserialization structural limits; null falls back to defaults. */ + private final Settings settings; + + public PlanSerializer(StorageEngine engine) { + this(engine, null); + } + + public PlanSerializer(StorageEngine engine, Settings settings) { + this.engine = engine; + this.settings = settings; + } + /** Converts a physical plan tree to a cursor. */ public Cursor convertToCursor(PhysicalPlan plan) { try { @@ -89,14 +100,17 @@ protected Serializable deserialize(String code) { new GZIPInputStream(new ByteArrayInputStream(HashCode.fromString(code).asBytes())); ObjectInputStream objectInput = new CursorDeserializationStream(new ByteArrayInputStream(gzip.readAllBytes())); + String additionalPatterns = + "org.opensearch.sql.planner.physical.*;" + + "org.opensearch.sql.opensearch.storage.scan.*;" + + "org.opensearch.sql.opensearch.data.type.*;" + + "org.opensearch.sql.executor.pagination.*;" + + "org.opensearch.sql.executor.QueryType;" + + "org.opensearch.sql.utils.*;"; objectInput.setObjectInputFilter( - DeserializationFilterUtil.createFilter( - "org.opensearch.sql.planner.physical.*;" - + "org.opensearch.sql.opensearch.storage.scan.*;" - + "org.opensearch.sql.opensearch.data.type.*;" - + "org.opensearch.sql.executor.pagination.*;" - + "org.opensearch.sql.executor.QueryType;" - + "org.opensearch.sql.utils.*;")); + settings == null + ? DeserializationFilterUtil.createFilter(additionalPatterns) + : DeserializationFilterUtil.createFilter(settings, additionalPatterns)); return (Serializable) objectInput.readObject(); } catch (Exception e) { throw new IllegalStateException("Failed to deserialize object", e); diff --git a/core/src/main/java/org/opensearch/sql/planner/DefaultImplementor.java b/core/src/main/java/org/opensearch/sql/planner/DefaultImplementor.java index 6332f98063f..56a3167fc73 100644 --- a/core/src/main/java/org/opensearch/sql/planner/DefaultImplementor.java +++ b/core/src/main/java/org/opensearch/sql/planner/DefaultImplementor.java @@ -5,6 +5,7 @@ package org.opensearch.sql.planner; +import org.opensearch.sql.common.setting.Settings; import org.opensearch.sql.executor.pagination.PlanSerializer; import org.opensearch.sql.planner.logical.LogicalAggregation; import org.opensearch.sql.planner.logical.LogicalCloseCursor; @@ -57,6 +58,17 @@ */ public class DefaultImplementor extends LogicalPlanNodeVisitor { + /** Cluster settings supplying deserialization structural limits; null falls back to defaults. */ + private final Settings settings; + + public DefaultImplementor() { + this(null); + } + + public DefaultImplementor(Settings settings) { + this.settings = settings; + } + @Override public PhysicalPlan visitRareTopN(LogicalRareTopN node, C context) { return new RareTopNOperator( @@ -164,7 +176,7 @@ public PhysicalPlan visitRelation(LogicalRelation node, C context) { @Override public PhysicalPlan visitFetchCursor(LogicalFetchCursor plan, C context) { - return new PlanSerializer(plan.getEngine()).convertToPlan(plan.getCursor()); + return new PlanSerializer(plan.getEngine(), settings).convertToPlan(plan.getCursor()); } @Override diff --git a/core/src/main/java/org/opensearch/sql/planner/Planner.java b/core/src/main/java/org/opensearch/sql/planner/Planner.java index 8a015bc072b..ef5dc8ede62 100644 --- a/core/src/main/java/org/opensearch/sql/planner/Planner.java +++ b/core/src/main/java/org/opensearch/sql/planner/Planner.java @@ -6,8 +6,8 @@ package org.opensearch.sql.planner; import java.util.List; -import lombok.RequiredArgsConstructor; import org.apache.calcite.rel.RelNode; +import org.opensearch.sql.common.setting.Settings; import org.opensearch.sql.planner.logical.LogicalPlan; import org.opensearch.sql.planner.logical.LogicalPlanNodeVisitor; import org.opensearch.sql.planner.logical.LogicalRelation; @@ -17,11 +17,22 @@ import org.opensearch.sql.storage.read.TableScanBuilder; /** Planner that plans and chooses the optimal physical plan. */ -@RequiredArgsConstructor public class Planner { private final LogicalPlanOptimizer logicalOptimizer; + /** Cluster settings supplying deserialization structural limits; null falls back to defaults. */ + private final Settings settings; + + public Planner(LogicalPlanOptimizer logicalOptimizer) { + this(logicalOptimizer, null); + } + + public Planner(LogicalPlanOptimizer logicalOptimizer, Settings settings) { + this.logicalOptimizer = logicalOptimizer; + this.settings = settings; + } + /** * Generate optimal physical plan for logical plan. If no table involved, translate logical plan * to physical by default implementor.
@@ -33,7 +44,7 @@ public class Planner { public PhysicalPlan plan(LogicalPlan plan) { Table table = findTable(plan); if (table == null) { - return plan.accept(new DefaultImplementor<>(), null); + return plan.accept(new DefaultImplementor<>(settings), null); } LogicalPlan optimized = table.optimize(optimize(plan)); // Give scan builders a chance to reject shapes that push-down alone cannot express safely diff --git a/core/src/main/java/org/opensearch/sql/utils/DeserializationFilterUtil.java b/core/src/main/java/org/opensearch/sql/utils/DeserializationFilterUtil.java index 0ae84bb35a2..d59ceb5bea2 100644 --- a/core/src/main/java/org/opensearch/sql/utils/DeserializationFilterUtil.java +++ b/core/src/main/java/org/opensearch/sql/utils/DeserializationFilterUtil.java @@ -8,6 +8,7 @@ import java.io.ObjectInputFilter; import org.apache.logging.log4j.LogManager; import org.apache.logging.log4j.Logger; +import org.opensearch.sql.common.setting.Settings; /** Utility class for creating deserialization filters with logging. */ public class DeserializationFilterUtil { @@ -46,6 +47,15 @@ public class DeserializationFilterUtil { + "java.time.**;" + "com.google.common.collect.**;"; + /** + * Default structural limits on the deserialized object graph, used when a setting is unset or no + * {@link Settings} is available (serialize-only call sites and tests). + */ + public static final int DEFAULT_MAX_DEPTH = 20; + + public static final int DEFAULT_MAX_REFS = 1000; + public static final int DEFAULT_MAX_BYTES = 15000; + /** * Creates a logging filter that wraps the provided filter and logs rejected classes. * @@ -55,21 +65,53 @@ public class DeserializationFilterUtil { public static ObjectInputFilter createLoggingFilter(ObjectInputFilter filter) { return info -> { ObjectInputFilter.Status status = filter.checkInput(info); - if (status == ObjectInputFilter.Status.REJECTED && info.serialClass() != null) { - LOG.warn("Deserialization filter rejected class: {}", info.serialClass().getName()); + if (status == ObjectInputFilter.Status.REJECTED) { + if (info.serialClass() != null) { + LOG.warn("Deserialization filter rejected class: {}", info.serialClass().getName()); + } else { + LOG.warn( + "Deserialization filter rejected: depth={}, refs={}, bytes={}", + info.depth(), + info.references(), + info.streamBytes()); + } } return status; }; } /** - * Creates a filter with the base allowlist plus additional patterns. + * Creates a filter with the base allowlist, the built-in default structural limits, and + * additional patterns. Used by serialize-only call sites and tests that have no {@link Settings}. * * @param additionalPatterns Additional patterns to append to the base allowlist. - * @return A logging filter with the combined allowlist. + * @return A logging filter with the combined allowlist and default structural limits. */ public static ObjectInputFilter createFilter(String additionalPatterns) { - String fullPattern = BASE_ALLOWLIST + additionalPatterns + "!*"; + return createFilter(DEFAULT_MAX_DEPTH, DEFAULT_MAX_REFS, DEFAULT_MAX_BYTES, additionalPatterns); + } + + /** + * Creates a filter with the base allowlist, the structural limits from the {@code + * plugins.query.deserialization.*} cluster settings, and additional patterns. + * + * @param settings cluster settings supplying the structural limits (must be non-null) + * @param additionalPatterns Additional patterns to append to the base allowlist. + * @return A logging filter with the combined allowlist and configured structural limits. + */ + public static ObjectInputFilter createFilter(Settings settings, String additionalPatterns) { + return createFilter( + settings.getSettingValue(Settings.Key.DESERIALIZATION_MAX_DEPTH), + settings.getSettingValue(Settings.Key.DESERIALIZATION_MAX_REFS), + settings.getSettingValue(Settings.Key.DESERIALIZATION_MAX_BYTES), + additionalPatterns); + } + + private static ObjectInputFilter createFilter( + int maxDepth, int maxRefs, int maxBytes, String additionalPatterns) { + String structuralLimits = + String.format("maxdepth=%d;maxrefs=%d;maxbytes=%d;", maxDepth, maxRefs, maxBytes); + String fullPattern = BASE_ALLOWLIST + additionalPatterns + structuralLimits + "!*"; return createLoggingFilter(ObjectInputFilter.Config.createFilter(fullPattern)); } } diff --git a/core/src/test/java/org/opensearch/sql/utils/DeserializationFilterUtilTest.java b/core/src/test/java/org/opensearch/sql/utils/DeserializationFilterUtilTest.java new file mode 100644 index 00000000000..4e5a868c556 --- /dev/null +++ b/core/src/test/java/org/opensearch/sql/utils/DeserializationFilterUtilTest.java @@ -0,0 +1,130 @@ +/* + * Copyright OpenSearch Contributors + * SPDX-License-Identifier: Apache-2.0 + */ + +package org.opensearch.sql.utils; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +import java.io.ObjectInputFilter; +import java.util.List; +import java.util.Map; +import org.junit.jupiter.api.DisplayNameGeneration; +import org.junit.jupiter.api.DisplayNameGenerator; +import org.junit.jupiter.api.Test; +import org.opensearch.sql.common.setting.Settings; + +@DisplayNameGeneration(DisplayNameGenerator.ReplaceUnderscores.class) +public class DeserializationFilterUtilTest { + + @Test + void allowlisted_class_is_allowed() { + ObjectInputFilter filter = DeserializationFilterUtil.createFilter(""); + assertEquals( + ObjectInputFilter.Status.ALLOWED, + filter.checkInput(info(String.class, /*depth*/ 1, /*refs*/ 1, /*bytes*/ 100))); + } + + @Test + void disallowed_class_is_rejected() { + ObjectInputFilter filter = DeserializationFilterUtil.createFilter(""); + assertEquals( + ObjectInputFilter.Status.REJECTED, + filter.checkInput(info(java.net.URL.class, /*depth*/ 1, /*refs*/ 1, /*bytes*/ 100))); + } + + @Test + void null_settings_fall_back_to_default_limits() { + // depth 21 > DEFAULT_MAX_DEPTH (20); refs 1001 > DEFAULT_MAX_REFS (1000); + // bytes 15001 > DEFAULT_MAX_BYTES (15000). + ObjectInputFilter filter = DeserializationFilterUtil.createFilter(""); + assertEquals( + ObjectInputFilter.Status.REJECTED, + filter.checkInput(info(/*class*/ null, /*depth*/ 21, /*refs*/ 1, /*bytes*/ 100))); + assertEquals( + ObjectInputFilter.Status.REJECTED, + filter.checkInput(info(/*class*/ null, /*depth*/ 1, /*refs*/ 1001, /*bytes*/ 100))); + assertEquals( + ObjectInputFilter.Status.REJECTED, + filter.checkInput(info(/*class*/ null, /*depth*/ 1, /*refs*/ 1, /*bytes*/ 15001))); + } + + @Test + void limits_are_read_from_settings() { + // Settings override the defaults; the filter must enforce the configured values. + Settings settings = settingsWith(/*depth*/ 5, /*refs*/ 10, /*bytes*/ 100); + ObjectInputFilter filter = DeserializationFilterUtil.createFilter(settings, ""); + assertEquals( + ObjectInputFilter.Status.REJECTED, + filter.checkInput(info(/*class*/ null, /*depth*/ 6, /*refs*/ 1, /*bytes*/ 50))); + assertEquals( + ObjectInputFilter.Status.REJECTED, + filter.checkInput(info(/*class*/ null, /*depth*/ 1, /*refs*/ 11, /*bytes*/ 50))); + assertEquals( + ObjectInputFilter.Status.REJECTED, + filter.checkInput(info(/*class*/ null, /*depth*/ 1, /*refs*/ 1, /*bytes*/ 101))); + assertEquals( + ObjectInputFilter.Status.ALLOWED, + filter.checkInput(info(String.class, /*depth*/ 1, /*refs*/ 1, /*bytes*/ 50))); + } + + @Test + void additional_pattern_is_honored() { + // Patterns passed in should extend the base allowlist. + ObjectInputFilter filter = DeserializationFilterUtil.createFilter("java.net.URI;"); + assertEquals( + ObjectInputFilter.Status.ALLOWED, + filter.checkInput(info(java.net.URI.class, /*depth*/ 1, /*refs*/ 1, /*bytes*/ 100))); + } + + private static Settings settingsWith(int depth, int refs, int bytes) { + Map values = + Map.of( + Settings.Key.DESERIALIZATION_MAX_DEPTH, depth, + Settings.Key.DESERIALIZATION_MAX_REFS, refs, + Settings.Key.DESERIALIZATION_MAX_BYTES, bytes); + return new Settings() { + @Override + @SuppressWarnings("unchecked") + public T getSettingValue(Settings.Key key) { + return (T) values.get(key); + } + + @Override + public List getSettings() { + return List.of(); + } + }; + } + + private static ObjectInputFilter.FilterInfo info( + Class cls, long depth, long refs, long bytes) { + return new ObjectInputFilter.FilterInfo() { + @Override + public Class serialClass() { + return cls; + } + + @Override + public long arrayLength() { + return -1; + } + + @Override + public long depth() { + return depth; + } + + @Override + public long references() { + return refs; + } + + @Override + public long streamBytes() { + return bytes; + } + }; + } +} diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/setting/OpenSearchSettings.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/setting/OpenSearchSettings.java index 0a7cf512210..daa71e44629 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/setting/OpenSearchSettings.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/setting/OpenSearchSettings.java @@ -31,6 +31,7 @@ import org.opensearch.search.aggregations.MultiBucketConsumerService; import org.opensearch.sql.common.antlr.AstBuildGuard; import org.opensearch.sql.common.setting.Settings; +import org.opensearch.sql.utils.DeserializationFilterUtil; /** Setting implementation on OpenSearch. */ @Log4j2 @@ -217,6 +218,30 @@ public class OpenSearchSettings extends Settings { Setting.Property.NodeScope, Setting.Property.Dynamic); + public static final Setting DESERIALIZATION_MAX_DEPTH_SETTING = + Setting.intSetting( + Key.DESERIALIZATION_MAX_DEPTH.getKeyValue(), + DeserializationFilterUtil.DEFAULT_MAX_DEPTH, + 1, + Setting.Property.NodeScope, + Setting.Property.Dynamic); + + public static final Setting DESERIALIZATION_MAX_REFS_SETTING = + Setting.intSetting( + Key.DESERIALIZATION_MAX_REFS.getKeyValue(), + DeserializationFilterUtil.DEFAULT_MAX_REFS, + 1, + Setting.Property.NodeScope, + Setting.Property.Dynamic); + + public static final Setting DESERIALIZATION_MAX_BYTES_SETTING = + Setting.intSetting( + Key.DESERIALIZATION_MAX_BYTES.getKeyValue(), + DeserializationFilterUtil.DEFAULT_MAX_BYTES, + 1, + Setting.Property.NodeScope, + Setting.Property.Dynamic); + // Set the default value to QUERY_SIZE_LIMIT_SETTING public static final Setting QUERY_BUCKET_SIZE_SETTING = Setting.intSetting( @@ -513,6 +538,24 @@ public OpenSearchSettings(ClusterSettings clusterSettings) { Key.MAX_EXPRESSION_DEPTH, MAX_EXPRESSION_DEPTH_SETTING, new Updater(Key.MAX_EXPRESSION_DEPTH)); + register( + settingBuilder, + clusterSettings, + Key.DESERIALIZATION_MAX_DEPTH, + DESERIALIZATION_MAX_DEPTH_SETTING, + new Updater(Key.DESERIALIZATION_MAX_DEPTH)); + register( + settingBuilder, + clusterSettings, + Key.DESERIALIZATION_MAX_REFS, + DESERIALIZATION_MAX_REFS_SETTING, + new Updater(Key.DESERIALIZATION_MAX_REFS)); + register( + settingBuilder, + clusterSettings, + Key.DESERIALIZATION_MAX_BYTES, + DESERIALIZATION_MAX_BYTES_SETTING, + new Updater(Key.DESERIALIZATION_MAX_BYTES)); register( settingBuilder, clusterSettings, @@ -693,6 +736,9 @@ public static List> pluginSettings() { .add(SQL_SLOWLOG_SETTING) .add(SQL_CURSOR_KEEP_ALIVE_SETTING) .add(MAX_EXPRESSION_DEPTH_SETTING) + .add(DESERIALIZATION_MAX_DEPTH_SETTING) + .add(DESERIALIZATION_MAX_REFS_SETTING) + .add(DESERIALIZATION_MAX_BYTES_SETTING) .add(PPL_ENABLED_SETTING) .add(PPL_QUERY_TIMEOUT_SETTING) .add(PPL_SYNTAX_LEGACY_PREFERRED_SETTING) diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/CalciteScriptEngine.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/CalciteScriptEngine.java index 6e2240909b0..6ac0a72c83e 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/CalciteScriptEngine.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/CalciteScriptEngine.java @@ -38,6 +38,7 @@ import java.util.Map; import java.util.Set; import java.util.function.BiFunction; +import java.util.function.Supplier; import lombok.Getter; import lombok.RequiredArgsConstructor; import org.apache.calcite.DataContext; @@ -78,6 +79,7 @@ import org.opensearch.script.StringSortScript; import org.opensearch.search.lookup.SourceLookup; import org.opensearch.sql.calcite.utils.CalciteClassLoaderHelper; +import org.opensearch.sql.common.setting.Settings; import org.opensearch.sql.data.model.ExprTimestampValue; import org.opensearch.sql.opensearch.storage.script.aggregation.CalciteAggregationScriptFactory; import org.opensearch.sql.opensearch.storage.script.field.CalciteFieldScriptFactory; @@ -96,7 +98,11 @@ public class CalciteScriptEngine implements ScriptEngine { private final RelJsonSerializer relJsonSerializer; public CalciteScriptEngine(RelOptCluster relOptCluster) { - this.relJsonSerializer = new RelJsonSerializer(relOptCluster); + this(relOptCluster, (Supplier) null); + } + + public CalciteScriptEngine(RelOptCluster relOptCluster, Supplier settingsSupplier) { + this.relJsonSerializer = new RelJsonSerializer(relOptCluster, settingsSupplier); } /** Expression script language name. */ diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/CompoundedScriptEngine.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/CompoundedScriptEngine.java index 1beffca6f61..9fb8b749cd2 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/CompoundedScriptEngine.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/script/CompoundedScriptEngine.java @@ -9,7 +9,7 @@ import com.fasterxml.jackson.annotation.JsonValue; import java.util.Map; import java.util.Set; -import lombok.RequiredArgsConstructor; +import java.util.function.Supplier; import org.apache.calcite.plan.RelOptCluster; import org.apache.calcite.plan.volcano.VolcanoPlanner; import org.apache.calcite.rex.RexBuilder; @@ -18,6 +18,7 @@ import org.opensearch.script.ScriptContext; import org.opensearch.script.ScriptEngine; import org.opensearch.sql.calcite.utils.OpenSearchTypeFactory; +import org.opensearch.sql.common.setting.Settings; import org.opensearch.sql.opensearch.storage.serde.DefaultExpressionSerializer; import org.opensearch.sql.opensearch.storage.serde.SerializationWrapper; import org.opensearch.sql.opensearch.storage.serde.SerializationWrapper.LangScriptWrapper; @@ -26,21 +27,25 @@ * Custom expression script engine that supports using core engine expression code in DSL as a new * script language just like built-in Painless language. */ -@RequiredArgsConstructor public class CompoundedScriptEngine implements ScriptEngine { /** Expression script language name. */ public static final String COMPOUNDED_LANG_NAME = "opensearch_compounded_script"; - private static final ExpressionScriptEngine v2ExpressionScriptEngine = - new ExpressionScriptEngine(new DefaultExpressionSerializer()); + private final ExpressionScriptEngine v2ExpressionScriptEngine; private final CalciteScriptEngine calciteScriptEngine; public CompoundedScriptEngine() { + this(null); + } + + public CompoundedScriptEngine(Supplier settingsSupplier) { + this.v2ExpressionScriptEngine = + new ExpressionScriptEngine(new DefaultExpressionSerializer(settingsSupplier)); RexBuilder rexBuilder = new RexBuilder(OpenSearchTypeFactory.TYPE_FACTORY); RelOptCluster cluster = RelOptCluster.create(new VolcanoPlanner(), rexBuilder); - this.calciteScriptEngine = new CalciteScriptEngine(cluster); + this.calciteScriptEngine = new CalciteScriptEngine(cluster, settingsSupplier); } @Override diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/serde/DefaultExpressionSerializer.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/serde/DefaultExpressionSerializer.java index b504f758926..c93664ad828 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/serde/DefaultExpressionSerializer.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/serde/DefaultExpressionSerializer.java @@ -11,12 +11,28 @@ import java.io.ObjectInputStream; import java.io.ObjectOutputStream; import java.util.Base64; +import java.util.function.Supplier; +import org.opensearch.sql.common.setting.Settings; import org.opensearch.sql.expression.Expression; import org.opensearch.sql.utils.DeserializationFilterUtil; /** Default serializer that (de-)serialize expressions by JDK serialization. */ public class DefaultExpressionSerializer implements ExpressionSerializer { + /** + * Supplies cluster settings for deserialization structural limits, resolved lazily because the + * script engine is created before plugin settings are initialized. Null falls back to defaults. + */ + private final Supplier settingsSupplier; + + public DefaultExpressionSerializer() { + this(null); + } + + public DefaultExpressionSerializer(Supplier settingsSupplier) { + this.settingsSupplier = settingsSupplier; + } + @Override public String serialize(Expression expr) { try { @@ -35,7 +51,11 @@ public Expression deserialize(String code) { try { ByteArrayInputStream input = new ByteArrayInputStream(Base64.getDecoder().decode(code)); ObjectInputStream objectInput = new ObjectInputStream(input); - objectInput.setObjectInputFilter(DeserializationFilterUtil.createFilter("")); + Settings settings = settingsSupplier == null ? null : settingsSupplier.get(); + objectInput.setObjectInputFilter( + settings == null + ? DeserializationFilterUtil.createFilter("") + : DeserializationFilterUtil.createFilter(settings, "")); return (Expression) objectInput.readObject(); } catch (Exception e) { throw new IllegalStateException("Failed to deserialize expression code: " + code, e); diff --git a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/serde/RelJsonSerializer.java b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/serde/RelJsonSerializer.java index e787114868a..4343d7b06ae 100644 --- a/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/serde/RelJsonSerializer.java +++ b/opensearch/src/main/java/org/opensearch/sql/opensearch/storage/serde/RelJsonSerializer.java @@ -13,6 +13,7 @@ import java.util.Base64; import java.util.LinkedHashMap; import java.util.Map; +import java.util.function.Supplier; import lombok.Getter; import org.apache.calcite.plan.RelOptCluster; import org.apache.calcite.rel.externalize.RelJson; @@ -24,6 +25,7 @@ import org.apache.calcite.sql.util.SqlOperatorTables; import org.apache.calcite.util.JsonBuilder; import org.opensearch.sql.calcite.CalcitePlanContext; +import org.opensearch.sql.common.setting.Settings; import org.opensearch.sql.expression.function.PPLBuiltinOperators; import org.opensearch.sql.opensearch.executor.OpenSearchExecutionEngine.OperatorTable; import org.opensearch.sql.utils.DeserializationFilterUtil; @@ -44,6 +46,13 @@ public class RelJsonSerializer { private final RelOptCluster cluster; + + /** + * Supplies cluster settings for deserialization structural limits, resolved lazily because the + * script engine is created before plugin settings are initialized. Null falls back to defaults. + */ + private final Supplier settingsSupplier; + private static final ObjectMapper mapper = JsonMapper.builder() .configure(DeserializationFeature.USE_BIG_DECIMAL_FOR_FLOATS, true) @@ -53,7 +62,12 @@ public class RelJsonSerializer { private static volatile SqlOperatorTable pplSqlOperatorTable; public RelJsonSerializer(RelOptCluster cluster) { + this(cluster, null); + } + + public RelJsonSerializer(RelOptCluster cluster, Supplier settingsSupplier) { this.cluster = cluster; + this.settingsSupplier = settingsSupplier; } private static SqlOperatorTable getPplSqlOperatorTable() { @@ -121,7 +135,11 @@ public RexNode deserialize(String struct) { try { ByteArrayInputStream input = new ByteArrayInputStream(Base64.getDecoder().decode(struct)); ObjectInputStream objectInput = new ObjectInputStream(input); - objectInput.setObjectInputFilter(DeserializationFilterUtil.createFilter("")); + Settings settings = settingsSupplier == null ? null : settingsSupplier.get(); + objectInput.setObjectInputFilter( + settings == null + ? DeserializationFilterUtil.createFilter("") + : DeserializationFilterUtil.createFilter(settings, "")); exprStr = (String) objectInput.readObject(); // Deserialize RelDataType and RexNode by JSON diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/setting/OpenSearchSettingsTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/setting/OpenSearchSettingsTest.java index 4d77df2c992..9303e29752d 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/setting/OpenSearchSettingsTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/setting/OpenSearchSettingsTest.java @@ -30,6 +30,7 @@ import org.opensearch.common.settings.Setting; import org.opensearch.core.common.unit.ByteSizeValue; import org.opensearch.sql.common.setting.Settings; +import org.opensearch.sql.utils.DeserializationFilterUtil; @ExtendWith(MockitoExtension.class) class OpenSearchSettingsTest { @@ -124,6 +125,37 @@ void testPplValuesMaxLimitSetting() { assertEquals(5000, newLimit); } + @Test + void testDeserializationStructuralLimitSettings() { + when(clusterSettings.get(ClusterName.CLUSTER_NAME_SETTING)).thenReturn(ClusterName.DEFAULT); + when(clusterSettings.get(not((eq(ClusterName.CLUSTER_NAME_SETTING))))).thenReturn(null); + OpenSearchSettings settings = new OpenSearchSettings(clusterSettings); + + // Defaults match DeserializationFilterUtil + Integer maxDepth = settings.getSettingValue(Settings.Key.DESERIALIZATION_MAX_DEPTH); + Integer maxRefs = settings.getSettingValue(Settings.Key.DESERIALIZATION_MAX_REFS); + Integer maxBytes = settings.getSettingValue(Settings.Key.DESERIALIZATION_MAX_BYTES); + assertEquals(DeserializationFilterUtil.DEFAULT_MAX_DEPTH, maxDepth); + assertEquals(DeserializationFilterUtil.DEFAULT_MAX_REFS, maxRefs); + assertEquals(DeserializationFilterUtil.DEFAULT_MAX_BYTES, maxBytes); + + // Dynamically updatable + settings.new Updater(Settings.Key.DESERIALIZATION_MAX_REFS).accept(2000); + Integer updatedRefs = settings.getSettingValue(Settings.Key.DESERIALIZATION_MAX_REFS); + assertEquals(2000, updatedRefs); + } + + @Test + void deserializationStructuralLimitSettingsAreDynamicAndRegistered() { + assertTrue(OpenSearchSettings.DESERIALIZATION_MAX_DEPTH_SETTING.isDynamic()); + assertTrue(OpenSearchSettings.DESERIALIZATION_MAX_REFS_SETTING.isDynamic()); + assertTrue(OpenSearchSettings.DESERIALIZATION_MAX_BYTES_SETTING.isDynamic()); + List> pluginSettings = OpenSearchSettings.pluginSettings(); + assertTrue(pluginSettings.contains(OpenSearchSettings.DESERIALIZATION_MAX_DEPTH_SETTING)); + assertTrue(pluginSettings.contains(OpenSearchSettings.DESERIALIZATION_MAX_REFS_SETTING)); + assertTrue(pluginSettings.contains(OpenSearchSettings.DESERIALIZATION_MAX_BYTES_SETTING)); + } + @Test void getSparkExecutionEngineConfigSetting() { // Default is empty string diff --git a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/serde/DefaultExpressionSerializerTest.java b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/serde/DefaultExpressionSerializerTest.java index 084fabb17b3..c3eac492c3f 100644 --- a/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/serde/DefaultExpressionSerializerTest.java +++ b/opensearch/src/test/java/org/opensearch/sql/opensearch/storage/serde/DefaultExpressionSerializerTest.java @@ -12,9 +12,12 @@ import static org.opensearch.sql.expression.DSL.literal; import static org.opensearch.sql.expression.DSL.ref; +import java.util.List; +import java.util.Map; import org.junit.jupiter.api.DisplayNameGeneration; import org.junit.jupiter.api.DisplayNameGenerator; import org.junit.jupiter.api.Test; +import org.opensearch.sql.common.setting.Settings; import org.opensearch.sql.data.model.ExprValue; import org.opensearch.sql.data.type.ExprType; import org.opensearch.sql.expression.DSL; @@ -84,6 +87,44 @@ public void cannot_deserialize_illegal_expression_code() { assertThrows(IllegalStateException.class, () -> serializer.deserialize("hello world")); } + @Test + public void deserialize_honors_configured_structural_limits() { + // A serializer wired with a very tight refs limit must reject an otherwise-valid expression, + // proving the injected Settings supplier actually drives the deserialization filter. + Settings tightLimits = settingsWith(/*depth*/ 20, /*refs*/ 1, /*bytes*/ 15000); + ExpressionSerializer limited = new DefaultExpressionSerializer(() -> tightLimits); + + Expression original = DSL.or(literal(true), DSL.less(literal(1), literal(2))); + String code = serializer.serialize(original); + + // Default limits (no override) round-trip the same payload fine. + assertEquals(original, serializer.deserialize(code)); + + // maxrefs=1 rejects the multi-object graph. + var exception = assertThrows(IllegalStateException.class, () -> limited.deserialize(code)); + assertTrue(exception.getMessage().contains("Failed to deserialize")); + } + + private static Settings settingsWith(int depth, int refs, int bytes) { + Map values = + Map.of( + Settings.Key.DESERIALIZATION_MAX_DEPTH, depth, + Settings.Key.DESERIALIZATION_MAX_REFS, refs, + Settings.Key.DESERIALIZATION_MAX_BYTES, bytes); + return new Settings() { + @Override + @SuppressWarnings("unchecked") + public T getSettingValue(Settings.Key key) { + return (T) values.get(key); + } + + @Override + public List getSettings() { + return List.of(); + } + }; + } + @Test public void deserialize_rejects_disallowed_class() throws Exception { java.io.ByteArrayOutputStream output = new java.io.ByteArrayOutputStream(); diff --git a/plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java b/plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java index e04566df590..cdbc0d5b3be 100644 --- a/plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java +++ b/plugin/src/main/java/org/opensearch/sql/plugin/SQLPlugin.java @@ -510,7 +510,8 @@ public List> getSettings() { @Override public ScriptEngine getScriptEngine(Settings settings, Collection> contexts) { - return new CompoundedScriptEngine(); + // Resolve plugin settings lazily: getScriptEngine runs before createComponents sets them. + return new CompoundedScriptEngine(() -> pluginSettings); } private DataSourceServiceImpl createDataSourceService() { diff --git a/plugin/src/main/java/org/opensearch/sql/plugin/config/OpenSearchPluginModule.java b/plugin/src/main/java/org/opensearch/sql/plugin/config/OpenSearchPluginModule.java index ebf0196c77a..33c95855d70 100644 --- a/plugin/src/main/java/org/opensearch/sql/plugin/config/OpenSearchPluginModule.java +++ b/plugin/src/main/java/org/opensearch/sql/plugin/config/OpenSearchPluginModule.java @@ -94,8 +94,8 @@ public ExecutionProtector protector(ResourceMonitor resourceMonitor) { } @Provides - public PlanSerializer planSerializer(StorageEngine storageEngine) { - return new PlanSerializer(storageEngine); + public PlanSerializer planSerializer(StorageEngine storageEngine, Settings settings) { + return new PlanSerializer(storageEngine, settings); } @Provides @@ -132,7 +132,7 @@ public QueryPlanFactory queryPlanFactory( Analyzer analyzer = new Analyzer( new ExpressionAnalyzer(functionRepository), dataSourceService, functionRepository); - Planner planner = new Planner(LogicalPlanOptimizer.create()); + Planner planner = new Planner(LogicalPlanOptimizer.create(), settings); ExecutionDispatcher executionDispatcher = new ThreadPoolExecutionDispatcher(nodeClient.threadPool(), settings); QueryService queryService =