From f73ca597bc1f509f81d492b3f5c88af0f17baf11 Mon Sep 17 00:00:00 2001 From: Jialiang Liang Date: Mon, 24 Aug 2026 22:46:41 +0000 Subject: [PATCH 1/7] Add structural limits for deserialization filter Follow-up to #5469. Bounds the deserialized object graph with maxdepth/maxrefs/maxbytes limits and logs structural-limit rejections alongside the existing class-rejection logging. Signed-off-by: Jialiang Liang --- .../sql/utils/DeserializationFilterUtil.java | 21 ++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) 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..e5229d66c1d 100644 --- a/core/src/main/java/org/opensearch/sql/utils/DeserializationFilterUtil.java +++ b/core/src/main/java/org/opensearch/sql/utils/DeserializationFilterUtil.java @@ -46,6 +46,9 @@ public class DeserializationFilterUtil { + "java.time.**;" + "com.google.common.collect.**;"; + /** Structural limits on the deserialized object graph. */ + private static final String STRUCTURAL_LIMITS = "maxdepth=20;maxrefs=300;maxbytes=15000;"; + /** * Creates a logging filter that wraps the provided filter and logs rejected classes. * @@ -55,21 +58,29 @@ 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, structural limits, and additional patterns. * * @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 structural limits. */ public static ObjectInputFilter createFilter(String additionalPatterns) { - String fullPattern = BASE_ALLOWLIST + additionalPatterns + "!*"; + String fullPattern = BASE_ALLOWLIST + additionalPatterns + STRUCTURAL_LIMITS + "!*"; return createLoggingFilter(ObjectInputFilter.Config.createFilter(fullPattern)); } } From ec32115a53aa43774dcce800c80f81f45b86567d Mon Sep 17 00:00:00 2001 From: Jialiang Liang Date: Tue, 25 Aug 2026 16:35:34 +0000 Subject: [PATCH 2/7] Add DeserializationFilterUtilTest for structural limits Directly exercises maxdepth, maxrefs, and maxbytes rejections plus the allowlist/additional-pattern paths via ObjectInputFilter.FilterInfo. Signed-off-by: Jialiang Liang --- .../utils/DeserializationFilterUtilTest.java | 99 +++++++++++++++++++ 1 file changed, 99 insertions(+) create mode 100644 core/src/test/java/org/opensearch/sql/utils/DeserializationFilterUtilTest.java 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..69a627bb67a --- /dev/null +++ b/core/src/test/java/org/opensearch/sql/utils/DeserializationFilterUtilTest.java @@ -0,0 +1,99 @@ +/* + * 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 org.junit.jupiter.api.DisplayNameGeneration; +import org.junit.jupiter.api.DisplayNameGenerator; +import org.junit.jupiter.api.Test; + +@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 depth_exceeding_limit_is_rejected() { + // Structural limit: maxdepth=20 → depth 21 must be rejected regardless of class. + ObjectInputFilter filter = DeserializationFilterUtil.createFilter(""); + assertEquals( + ObjectInputFilter.Status.REJECTED, + filter.checkInput(info(/*class*/ null, /*depth*/ 21, /*refs*/ 1, /*bytes*/ 100))); + } + + @Test + void refs_exceeding_limit_is_rejected() { + // Structural limit: maxrefs=300 → refs 301 must be rejected regardless of class. + ObjectInputFilter filter = DeserializationFilterUtil.createFilter(""); + assertEquals( + ObjectInputFilter.Status.REJECTED, + filter.checkInput(info(/*class*/ null, /*depth*/ 1, /*refs*/ 301, /*bytes*/ 100))); + } + + @Test + void bytes_exceeding_limit_are_rejected() { + // Structural limit: maxbytes=15000 → 15001 bytes must be rejected regardless of class. + ObjectInputFilter filter = DeserializationFilterUtil.createFilter(""); + assertEquals( + ObjectInputFilter.Status.REJECTED, + filter.checkInput(info(/*class*/ null, /*depth*/ 1, /*refs*/ 1, /*bytes*/ 15001))); + } + + @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 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; + } + }; + } +} From 6143db05aaeb5cb8d1e86b14cb1f637af8cbffea Mon Sep 17 00:00:00 2001 From: Jialiang Liang Date: Tue, 25 Aug 2026 17:27:50 +0000 Subject: [PATCH 3/7] Raise maxrefs from 300 to 1000 The previous cap rejected legitimate paginated cursors (a SELECT * ... ORDER BY query serializes to 301 refs), causing PaginationIT failures. 1000 leaves comfortable headroom for real cursors while remaining well below deserialization-bomb scale. Test updated to match the new bound. Signed-off-by: Jialiang Liang --- .../org/opensearch/sql/utils/DeserializationFilterUtil.java | 2 +- .../opensearch/sql/utils/DeserializationFilterUtilTest.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) 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 e5229d66c1d..fbe3095a470 100644 --- a/core/src/main/java/org/opensearch/sql/utils/DeserializationFilterUtil.java +++ b/core/src/main/java/org/opensearch/sql/utils/DeserializationFilterUtil.java @@ -47,7 +47,7 @@ public class DeserializationFilterUtil { + "com.google.common.collect.**;"; /** Structural limits on the deserialized object graph. */ - private static final String STRUCTURAL_LIMITS = "maxdepth=20;maxrefs=300;maxbytes=15000;"; + private static final String STRUCTURAL_LIMITS = "maxdepth=20;maxrefs=1000;maxbytes=15000;"; /** * Creates a logging filter that wraps the provided filter and logs rejected classes. diff --git a/core/src/test/java/org/opensearch/sql/utils/DeserializationFilterUtilTest.java b/core/src/test/java/org/opensearch/sql/utils/DeserializationFilterUtilTest.java index 69a627bb67a..a63a803635f 100644 --- a/core/src/test/java/org/opensearch/sql/utils/DeserializationFilterUtilTest.java +++ b/core/src/test/java/org/opensearch/sql/utils/DeserializationFilterUtilTest.java @@ -42,11 +42,11 @@ void depth_exceeding_limit_is_rejected() { @Test void refs_exceeding_limit_is_rejected() { - // Structural limit: maxrefs=300 → refs 301 must be rejected regardless of class. + // Structural limit: maxrefs=1000 → refs 1001 must be rejected regardless of class. ObjectInputFilter filter = DeserializationFilterUtil.createFilter(""); assertEquals( ObjectInputFilter.Status.REJECTED, - filter.checkInput(info(/*class*/ null, /*depth*/ 1, /*refs*/ 301, /*bytes*/ 100))); + filter.checkInput(info(/*class*/ null, /*depth*/ 1, /*refs*/ 1001, /*bytes*/ 100))); } @Test From 1d8907973db11db43a71e432aaad56793044015d Mon Sep 17 00:00:00 2001 From: Jialiang Liang Date: Wed, 26 Aug 2026 19:32:50 +0000 Subject: [PATCH 4/7] Make deserialization structural limits configurable via cluster settings Replace the hardcoded maxdepth/maxrefs/maxbytes with dynamic cluster settings so operators can tune them without a code change: plugins.query.deserialization.max_depth (default 20) plugins.query.deserialization.max_refs (default 1000) plugins.query.deserialization.max_bytes (default 15000) Values are injected via Settings (same pattern as AstBuildGuard): the pagination path receives Settings through Planner/DefaultImplementor, and the script-engine paths receive a Supplier because the script engine is constructed before plugin settings are initialized. createFilter falls back to the defaults when no Settings is available (serialize-only call sites and tests). Signed-off-by: Jialiang Liang --- .../sql/common/setting/Settings.java | 3 + .../executor/pagination/PlanSerializer.java | 16 ++++- .../sql/planner/DefaultImplementor.java | 14 ++++- .../org/opensearch/sql/planner/Planner.java | 17 ++++- .../sql/utils/DeserializationFilterUtil.java | 34 ++++++++-- .../utils/DeserializationFilterUtilTest.java | 63 ++++++++++++++----- .../setting/OpenSearchSettings.java | 46 ++++++++++++++ .../storage/script/CalciteScriptEngine.java | 8 ++- .../script/CompoundedScriptEngine.java | 15 +++-- .../serde/DefaultExpressionSerializer.java | 19 +++++- .../storage/serde/RelJsonSerializer.java | 17 ++++- .../org/opensearch/sql/plugin/SQLPlugin.java | 3 +- .../plugin/config/OpenSearchPluginModule.java | 6 +- 13 files changed, 222 insertions(+), 39 deletions(-) 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..16bd5521dd9 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 { @@ -91,6 +102,7 @@ protected Serializable deserialize(String code) { new CursorDeserializationStream(new ByteArrayInputStream(gzip.readAllBytes())); objectInput.setObjectInputFilter( DeserializationFilterUtil.createFilter( + settings, "org.opensearch.sql.planner.physical.*;" + "org.opensearch.sql.opensearch.storage.scan.*;" + "org.opensearch.sql.opensearch.data.type.*;" 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 fbe3095a470..7cd7ba3bd43 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,8 +47,14 @@ public class DeserializationFilterUtil { + "java.time.**;" + "com.google.common.collect.**;"; - /** Structural limits on the deserialized object graph. */ - private static final String STRUCTURAL_LIMITS = "maxdepth=20;maxrefs=1000;maxbytes=15000;"; + /** + * 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. @@ -74,13 +81,30 @@ public static ObjectInputFilter createLoggingFilter(ObjectInputFilter filter) { } /** - * Creates a filter with the base allowlist, structural limits, and additional patterns. + * Creates a filter with the base allowlist, structural limits, and additional patterns. The + * structural limits are read from the {@code plugins.query.deserialization.*} cluster settings, + * falling back to the defaults when {@code settings} is null (serialize-only call sites or + * tests). * + * @param settings cluster settings supplying the structural limits, or null for defaults * @param additionalPatterns Additional patterns to append to the base allowlist. * @return A logging filter with the combined allowlist and structural limits. */ - public static ObjectInputFilter createFilter(String additionalPatterns) { - String fullPattern = BASE_ALLOWLIST + additionalPatterns + STRUCTURAL_LIMITS + "!*"; + public static ObjectInputFilter createFilter(Settings settings, String additionalPatterns) { + int maxDepth = limit(settings, Settings.Key.DESERIALIZATION_MAX_DEPTH, DEFAULT_MAX_DEPTH); + int maxRefs = limit(settings, Settings.Key.DESERIALIZATION_MAX_REFS, DEFAULT_MAX_REFS); + int maxBytes = limit(settings, Settings.Key.DESERIALIZATION_MAX_BYTES, DEFAULT_MAX_BYTES); + 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)); } + + private static int limit(Settings settings, Settings.Key key, int defaultValue) { + if (settings == null) { + return defaultValue; + } + Integer value = settings.getSettingValue(key); + return value == null ? defaultValue : value; + } } diff --git a/core/src/test/java/org/opensearch/sql/utils/DeserializationFilterUtilTest.java b/core/src/test/java/org/opensearch/sql/utils/DeserializationFilterUtilTest.java index a63a803635f..a7fdf4f0848 100644 --- a/core/src/test/java/org/opensearch/sql/utils/DeserializationFilterUtilTest.java +++ b/core/src/test/java/org/opensearch/sql/utils/DeserializationFilterUtilTest.java @@ -8,16 +8,19 @@ 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(""); + ObjectInputFilter filter = DeserializationFilterUtil.createFilter(null, ""); assertEquals( ObjectInputFilter.Status.ALLOWED, filter.checkInput(info(String.class, /*depth*/ 1, /*refs*/ 1, /*bytes*/ 100))); @@ -25,48 +28,76 @@ void allowlisted_class_is_allowed() { @Test void disallowed_class_is_rejected() { - ObjectInputFilter filter = DeserializationFilterUtil.createFilter(""); + ObjectInputFilter filter = DeserializationFilterUtil.createFilter(null, ""); assertEquals( ObjectInputFilter.Status.REJECTED, filter.checkInput(info(java.net.URL.class, /*depth*/ 1, /*refs*/ 1, /*bytes*/ 100))); } @Test - void depth_exceeding_limit_is_rejected() { - // Structural limit: maxdepth=20 → depth 21 must be rejected regardless of class. - ObjectInputFilter filter = DeserializationFilterUtil.createFilter(""); + 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(null, ""); assertEquals( ObjectInputFilter.Status.REJECTED, filter.checkInput(info(/*class*/ null, /*depth*/ 21, /*refs*/ 1, /*bytes*/ 100))); - } - - @Test - void refs_exceeding_limit_is_rejected() { - // Structural limit: maxrefs=1000 → refs 1001 must be rejected regardless of class. - ObjectInputFilter filter = DeserializationFilterUtil.createFilter(""); 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 bytes_exceeding_limit_are_rejected() { - // Structural limit: maxbytes=15000 → 15001 bytes must be rejected regardless of class. - ObjectInputFilter filter = DeserializationFilterUtil.createFilter(""); + 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*/ 1, /*refs*/ 1, /*bytes*/ 15001))); + 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;"); + ObjectInputFilter filter = DeserializationFilterUtil.createFilter(null, "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() { 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..559176c4b33 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,8 @@ 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(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..5bf70d3337d 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,8 @@ 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(DeserializationFilterUtil.createFilter(settings, "")); exprStr = (String) objectInput.readObject(); // Deserialize RelDataType and RexNode by JSON 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 = From a70d0db36b97056f6511f139a138f079762b55ef Mon Sep 17 00:00:00 2001 From: Jialiang Liang Date: Wed, 26 Aug 2026 21:25:41 +0000 Subject: [PATCH 5/7] Add OpenSearchSettings tests for deserialization limit settings Verify the three plugins.query.deserialization.* settings have the expected defaults, are dynamically updatable, and are registered in pluginSettings(). Signed-off-by: Jialiang Liang --- .../setting/OpenSearchSettingsTest.java | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) 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 From 76943f376a07db39fdcd4924ab338923879ede3d Mon Sep 17 00:00:00 2001 From: Jialiang Liang Date: Wed, 26 Aug 2026 21:28:39 +0000 Subject: [PATCH 6/7] Add round-trip test for settings-driven deserialization limit Verifies a DefaultExpressionSerializer wired with a Supplier enforces the configured maxrefs during real deserialization, while the default serializer round-trips the same payload. Signed-off-by: Jialiang Liang --- .../DefaultExpressionSerializerTest.java | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) 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(); From bc9bc19c702a718b9a029d9200749cd526d15c23 Mon Sep 17 00:00:00 2001 From: Jialiang Liang Date: Wed, 26 Aug 2026 22:38:09 +0000 Subject: [PATCH 7/7] Address review: createFilter(Settings) no longer accepts null Per review feedback, the settings-based createFilter now requires a non-null Settings (the deserialize paths always provide one). Add an explicit createFilter(String) overload that uses the built-in defaults for serialize-only call sites and tests, and let each serializer pick the overload based on whether it has settings. Signed-off-by: Jialiang Liang --- .../executor/pagination/PlanSerializer.java | 18 ++++---- .../sql/utils/DeserializationFilterUtil.java | 41 +++++++++++-------- .../utils/DeserializationFilterUtilTest.java | 8 ++-- .../serde/DefaultExpressionSerializer.java | 5 ++- .../storage/serde/RelJsonSerializer.java | 5 ++- 5 files changed, 46 insertions(+), 31 deletions(-) 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 16bd5521dd9..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 @@ -100,15 +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( - settings, - "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/utils/DeserializationFilterUtil.java b/core/src/main/java/org/opensearch/sql/utils/DeserializationFilterUtil.java index 7cd7ba3bd43..d59ceb5bea2 100644 --- a/core/src/main/java/org/opensearch/sql/utils/DeserializationFilterUtil.java +++ b/core/src/main/java/org/opensearch/sql/utils/DeserializationFilterUtil.java @@ -81,30 +81,37 @@ public static ObjectInputFilter createLoggingFilter(ObjectInputFilter filter) { } /** - * Creates a filter with the base allowlist, structural limits, and additional patterns. The - * structural limits are read from the {@code plugins.query.deserialization.*} cluster settings, - * falling back to the defaults when {@code settings} is null (serialize-only call sites or - * tests). + * 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 settings cluster settings supplying the structural limits, or null for defaults * @param additionalPatterns Additional patterns to append to the base allowlist. - * @return A logging filter with the combined allowlist and structural limits. + * @return A logging filter with the combined allowlist and default structural limits. + */ + public static ObjectInputFilter createFilter(String 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) { - int maxDepth = limit(settings, Settings.Key.DESERIALIZATION_MAX_DEPTH, DEFAULT_MAX_DEPTH); - int maxRefs = limit(settings, Settings.Key.DESERIALIZATION_MAX_REFS, DEFAULT_MAX_REFS); - int maxBytes = limit(settings, Settings.Key.DESERIALIZATION_MAX_BYTES, DEFAULT_MAX_BYTES); + 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)); } - - private static int limit(Settings settings, Settings.Key key, int defaultValue) { - if (settings == null) { - return defaultValue; - } - Integer value = settings.getSettingValue(key); - return value == null ? defaultValue : value; - } } diff --git a/core/src/test/java/org/opensearch/sql/utils/DeserializationFilterUtilTest.java b/core/src/test/java/org/opensearch/sql/utils/DeserializationFilterUtilTest.java index a7fdf4f0848..4e5a868c556 100644 --- a/core/src/test/java/org/opensearch/sql/utils/DeserializationFilterUtilTest.java +++ b/core/src/test/java/org/opensearch/sql/utils/DeserializationFilterUtilTest.java @@ -20,7 +20,7 @@ public class DeserializationFilterUtilTest { @Test void allowlisted_class_is_allowed() { - ObjectInputFilter filter = DeserializationFilterUtil.createFilter(null, ""); + ObjectInputFilter filter = DeserializationFilterUtil.createFilter(""); assertEquals( ObjectInputFilter.Status.ALLOWED, filter.checkInput(info(String.class, /*depth*/ 1, /*refs*/ 1, /*bytes*/ 100))); @@ -28,7 +28,7 @@ void allowlisted_class_is_allowed() { @Test void disallowed_class_is_rejected() { - ObjectInputFilter filter = DeserializationFilterUtil.createFilter(null, ""); + ObjectInputFilter filter = DeserializationFilterUtil.createFilter(""); assertEquals( ObjectInputFilter.Status.REJECTED, filter.checkInput(info(java.net.URL.class, /*depth*/ 1, /*refs*/ 1, /*bytes*/ 100))); @@ -38,7 +38,7 @@ void disallowed_class_is_rejected() { 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(null, ""); + ObjectInputFilter filter = DeserializationFilterUtil.createFilter(""); assertEquals( ObjectInputFilter.Status.REJECTED, filter.checkInput(info(/*class*/ null, /*depth*/ 21, /*refs*/ 1, /*bytes*/ 100))); @@ -72,7 +72,7 @@ void limits_are_read_from_settings() { @Test void additional_pattern_is_honored() { // Patterns passed in should extend the base allowlist. - ObjectInputFilter filter = DeserializationFilterUtil.createFilter(null, "java.net.URI;"); + ObjectInputFilter filter = DeserializationFilterUtil.createFilter("java.net.URI;"); assertEquals( ObjectInputFilter.Status.ALLOWED, filter.checkInput(info(java.net.URI.class, /*depth*/ 1, /*refs*/ 1, /*bytes*/ 100))); 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 559176c4b33..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 @@ -52,7 +52,10 @@ public Expression deserialize(String code) { ByteArrayInputStream input = new ByteArrayInputStream(Base64.getDecoder().decode(code)); ObjectInputStream objectInput = new ObjectInputStream(input); Settings settings = settingsSupplier == null ? null : settingsSupplier.get(); - objectInput.setObjectInputFilter(DeserializationFilterUtil.createFilter(settings, "")); + 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 5bf70d3337d..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 @@ -136,7 +136,10 @@ public RexNode deserialize(String struct) { ByteArrayInputStream input = new ByteArrayInputStream(Base64.getDecoder().decode(struct)); ObjectInputStream objectInput = new ObjectInputStream(input); Settings settings = settingsSupplier == null ? null : settingsSupplier.get(); - objectInput.setObjectInputFilter(DeserializationFilterUtil.createFilter(settings, "")); + objectInput.setObjectInputFilter( + settings == null + ? DeserializationFilterUtil.createFilter("") + : DeserializationFilterUtil.createFilter(settings, "")); exprStr = (String) objectInput.readObject(); // Deserialize RelDataType and RexNode by JSON