diff --git a/docs/changelog/107333.yaml b/docs/changelog/107333.yaml new file mode 100644 index 0000000000000..0762e9a19795c --- /dev/null +++ b/docs/changelog/107333.yaml @@ -0,0 +1,18 @@ +pr: 107333 +summary: Limit how much space some string functions can use +area: SQL +type: breaking +issues: [] +breaking: + title: Limit how much space some string functions can use + area: REST API + details: "Before this change, some of the string functions could return a result\ + \ of any arbitrary length, which could force the VM to allocate large chunks of\ + \ memory or even make it exit. Any user with access to the SQL API can invoke\ + \ these functions. This change introduces a limitation of how much memory the\ + \ result returned by a function call can consume. The functions affected by this\ + \ change are: CONCAT, INSERT, REPEAT, REPLACE and SPACE." + impact: "The affected functions used to return a result of any length. After this\ + \ change, a result can no longer exceed 1MB in length. Note that this is a bytes\ + \ length, the character count may be lower." + notable: false diff --git a/docs/reference/sql/functions/string.asciidoc b/docs/reference/sql/functions/string.asciidoc index 318b8d6996415..2535ccbe0556d 100644 --- a/docs/reference/sql/functions/string.asciidoc +++ b/docs/reference/sql/functions/string.asciidoc @@ -109,6 +109,8 @@ CONCAT( *Description*: Returns a character string that is the result of concatenating `string_exp1` to `string_exp2`. +The resulting string cannot exceed a byte length of 1 MB. + [source, sql] -------------------------------------------------- include-tagged::{sql-specs}/docs/docs.csv-spec[stringConcat] @@ -137,6 +139,8 @@ INSERT( *Description*: Returns a string where `length` characters have been deleted from `source`, beginning at `start`, and where `replacement` has been inserted into `source`, beginning at `start`. +The resulting string cannot exceed a byte length of 1 MB. + [source, sql] -------------------------------------------------- include-tagged::{sql-specs}/docs/docs.csv-spec[stringInsert] @@ -330,6 +334,8 @@ REPEAT( *Description*: Returns a character string composed of `string_exp` repeated `count` times. +The resulting string cannot exceed a byte length of 1 MB. + [source, sql] -------------------------------------------------- include-tagged::{sql-specs}/docs/docs.csv-spec[stringRepeat] @@ -356,6 +362,8 @@ REPLACE( *Description*: Search `source` for occurrences of `pattern`, and replace with `replacement`. +The resulting string cannot exceed a byte length of 1 MB. + [source, sql] -------------------------------------------------- include-tagged::{sql-specs}/docs/docs.csv-spec[stringReplace] @@ -423,6 +431,8 @@ SPACE(count) <1> *Description*: Returns a character string consisting of `count` spaces. +The resulting string cannot exceed a byte length of 1 MB. + [source, sql] -------------------------------------------------- include-tagged::{sql-specs}/docs/docs.csv-spec[stringSpace] diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/BinaryStringNumericProcessor.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/BinaryStringNumericProcessor.java index 434c5a66cd06e..6cab7c5d3f0b0 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/BinaryStringNumericProcessor.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/BinaryStringNumericProcessor.java @@ -16,6 +16,8 @@ import java.io.IOException; import java.util.function.BiFunction; +import static org.elasticsearch.xpack.sql.expression.function.scalar.string.StringProcessor.checkResultLength; + /** * Processor class covering string manipulating functions that have the first parameter as string, * second parameter as numeric and a string result. @@ -42,12 +44,8 @@ public enum BinaryStringNumericOperation implements BiFunction op) { diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/ConcatFunctionProcessor.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/ConcatFunctionProcessor.java index 3d7e403468b7d..2eb36f35240d1 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/ConcatFunctionProcessor.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/ConcatFunctionProcessor.java @@ -16,6 +16,8 @@ import java.io.IOException; import java.util.Objects; +import static org.elasticsearch.xpack.sql.expression.function.scalar.string.StringProcessor.checkResultLength; + public class ConcatFunctionProcessor extends BinaryProcessor { public static final String NAME = "scon"; @@ -62,7 +64,10 @@ public static Object process(Object source1, Object source2) { throw new SqlIllegalArgumentException("A string/char is required; received [{}]", source2); } - return source1.toString().concat(source2.toString()); + String str1 = source1.toString(); + String str2 = source2.toString(); + checkResultLength(str1.length() + str2.length()); + return str1.concat(str2); } @Override diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/InsertFunctionProcessor.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/InsertFunctionProcessor.java index ebd3dec176258..98a06ea58ea72 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/InsertFunctionProcessor.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/InsertFunctionProcessor.java @@ -15,6 +15,8 @@ import java.io.IOException; import java.util.Objects; +import static org.elasticsearch.xpack.sql.expression.function.scalar.string.StringProcessor.checkResultLength; + public class InsertFunctionProcessor implements Processor { private final Processor input, start, length, replacement; @@ -71,7 +73,9 @@ public static Object doProcess(Object input, Object start, Object length, Object StringBuilder sb = new StringBuilder(input.toString()); String replString = (replacement.toString()); - return sb.replace(realStart, realStart + ((Number) length).intValue(), replString).toString(); + int cutLength = ((Number) length).intValue(); + checkResultLength(sb.length() - cutLength + replString.length()); + return sb.replace(realStart, realStart + cutLength, replString).toString(); } @Override diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/ReplaceFunctionProcessor.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/ReplaceFunctionProcessor.java index 98c35036c272c..9c08dc17269e1 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/ReplaceFunctionProcessor.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/ReplaceFunctionProcessor.java @@ -58,11 +58,20 @@ public static Object doProcess(Object input, Object pattern, Object replacement) throw new SqlIllegalArgumentException("A string/char is required; received [{}]", replacement); } - return Strings.replace( - input instanceof Character ? input.toString() : (String) input, - pattern instanceof Character ? pattern.toString() : (String) pattern, - replacement instanceof Character ? replacement.toString() : (String) replacement - ); + String inputStr = input instanceof Character ? input.toString() : (String) input; + String patternStr = pattern instanceof Character ? pattern.toString() : (String) pattern; + String replacementStr = replacement instanceof Character ? replacement.toString() : (String) replacement; + checkResultLength(inputStr, patternStr, replacementStr); + return Strings.replace(inputStr, patternStr, replacementStr); + } + + private static void checkResultLength(String input, String pattern, String replacement) { + int patternLen = pattern.length(); + long matches = 0; + for (int i = input.indexOf(pattern); i >= 0; i = input.indexOf(pattern, i + patternLen)) { + matches++; + } + StringProcessor.checkResultLength(input.length() + matches * (replacement.length() - patternLen)); } @Override diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/StringProcessor.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/StringProcessor.java index 4d922b6c10ea1..f90b625997216 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/StringProcessor.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/StringProcessor.java @@ -17,8 +17,12 @@ import java.util.Locale; import java.util.function.Function; +import static org.elasticsearch.common.unit.ByteSizeUnit.MB; + public class StringProcessor implements Processor { + static final long MAX_RESULT_LENGTH = MB.toBytes(1); + private interface StringFunction { default R apply(Object o) { if ((o instanceof String || o instanceof Character) == false) { @@ -60,6 +64,7 @@ public enum StringOperation { if (i < 0) { return null; } + checkResultLength(n.longValue()); char[] spaces = new char[i]; char whitespace = ' '; Arrays.fill(spaces, whitespace); @@ -125,6 +130,17 @@ StringOperation processor() { return processor; } + static void checkResultLength(long needed) { + if (needed > MAX_RESULT_LENGTH) { + throw new SqlIllegalArgumentException( + "Required result length [{}] exceeds implementation limit [{}] bytes", + needed, + MAX_RESULT_LENGTH + ); + } + + } + @Override public boolean equals(Object obj) { if (obj == null || obj.getClass() != getClass()) { diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/BinaryStringNumericProcessorTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/BinaryStringNumericProcessorTests.java index e4fa7dab1db9c..4758feef1e8b9 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/BinaryStringNumericProcessorTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/BinaryStringNumericProcessorTests.java @@ -18,6 +18,8 @@ import static org.elasticsearch.xpack.ql.expression.function.scalar.FunctionTestUtils.l; import static org.elasticsearch.xpack.ql.tree.Source.EMPTY; +import static org.elasticsearch.xpack.sql.expression.function.scalar.string.StringFunctionProcessorTests.maxResultLengthTest; +import static org.elasticsearch.xpack.sql.expression.function.scalar.string.StringProcessor.MAX_RESULT_LENGTH; public class BinaryStringNumericProcessorTests extends AbstractWireSerializingTestCase { @@ -150,6 +152,10 @@ public void testRepeatFunctionWithEdgeCases() { assertNull(new Repeat(EMPTY, l("foo"), l(-1)).makePipe().asProcessor().process(null)); assertNull(new Repeat(EMPTY, l("foo"), l(0)).makePipe().asProcessor().process(null)); assertNull(new Repeat(EMPTY, l('f'), l(Integer.MIN_VALUE)).makePipe().asProcessor().process(null)); + assertEquals( + MAX_RESULT_LENGTH, + new Repeat(EMPTY, l('f'), l(MAX_RESULT_LENGTH)).makePipe().asProcessor().process(null).toString().length() + ); } public void testRepeatFunctionInputsValidation() { @@ -179,5 +185,14 @@ public void testRepeatFunctionInputsValidation() { e = expectThrows(InvalidArgumentException.class, () -> new Repeat(EMPTY, l("foo"), l(1.0)).makePipe().asProcessor().process(null)); assertEquals("A fixed point number is required for [count]; received [java.lang.Double]", e.getMessage()); + + maxResultLengthTest( + MAX_RESULT_LENGTH + 1, + () -> new Repeat(EMPTY, l("f"), l(MAX_RESULT_LENGTH + 1)).makePipe().asProcessor().process(null) + ); + + String str = "foo"; + long count = (MAX_RESULT_LENGTH / str.length()) + 1; + maxResultLengthTest(count * str.length(), () -> new Repeat(EMPTY, l(str), l(count)).makePipe().asProcessor().process(null)); } } diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/ConcatProcessorTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/ConcatProcessorTests.java index 3f5699dab7163..2662e9d7cd991 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/ConcatProcessorTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/ConcatProcessorTests.java @@ -16,6 +16,8 @@ import static org.elasticsearch.xpack.ql.expression.function.scalar.FunctionTestUtils.l; import static org.elasticsearch.xpack.ql.tree.Source.EMPTY; +import static org.elasticsearch.xpack.sql.expression.function.scalar.string.StringFunctionProcessorTests.maxResultLengthTest; +import static org.elasticsearch.xpack.sql.expression.function.scalar.string.StringProcessor.MAX_RESULT_LENGTH; public class ConcatProcessorTests extends AbstractWireSerializingTestCase { @@ -65,4 +67,11 @@ public void testConcatFunctionInputsValidation() { ); assertEquals("A string/char is required; received [3]", siae.getMessage()); } + + public void testMaxResultLength() { + String str = "a".repeat((int) MAX_RESULT_LENGTH - 1); + assertEquals(MAX_RESULT_LENGTH, new Concat(EMPTY, l(str), l("b")).makePipe().asProcessor().process(null).toString().length()); + + maxResultLengthTest(MAX_RESULT_LENGTH + 1, () -> new Concat(EMPTY, l(str), l("bb")).makePipe().asProcessor().process(null)); + } } diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/InsertProcessorTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/InsertProcessorTests.java index 46beb99eb9a70..b93bb25fc6727 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/InsertProcessorTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/InsertProcessorTests.java @@ -17,6 +17,8 @@ import static org.elasticsearch.xpack.ql.expression.function.scalar.FunctionTestUtils.l; import static org.elasticsearch.xpack.ql.tree.Source.EMPTY; +import static org.elasticsearch.xpack.sql.expression.function.scalar.string.StringFunctionProcessorTests.maxResultLengthTest; +import static org.elasticsearch.xpack.sql.expression.function.scalar.string.StringProcessor.MAX_RESULT_LENGTH; public class InsertProcessorTests extends AbstractWireSerializingTestCase { @@ -123,5 +125,21 @@ public void testInsertInputsValidation() { () -> new Insert(EMPTY, l("foobar"), l(1), l((long) Integer.MAX_VALUE + 1), l("bar")).makePipe().asProcessor().process(null) ); assertEquals("[length] out of the allowed range [0, 2147483647], received [2147483648]", e.getMessage()); + + String str = "a".repeat((int) MAX_RESULT_LENGTH); + String replaceWith = "bar"; + assertEquals( + MAX_RESULT_LENGTH, + new Insert(EMPTY, l(str), l(1), l(replaceWith.length()), l(replaceWith)).makePipe() + .asProcessor() + .process(null) + .toString() + .length() + ); + + maxResultLengthTest( + MAX_RESULT_LENGTH + 1, + () -> new Insert(EMPTY, l(str), l(1), l(replaceWith.length() - 1), l(replaceWith)).makePipe().asProcessor().process(null) + ); } } diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/ReplaceProcessorTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/ReplaceProcessorTests.java index 83f20cfd77ee0..237933a354ce8 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/ReplaceProcessorTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/ReplaceProcessorTests.java @@ -16,6 +16,8 @@ import static org.elasticsearch.xpack.ql.expression.function.scalar.FunctionTestUtils.l; import static org.elasticsearch.xpack.ql.tree.Source.EMPTY; +import static org.elasticsearch.xpack.sql.expression.function.scalar.string.StringFunctionProcessorTests.maxResultLengthTest; +import static org.elasticsearch.xpack.sql.expression.function.scalar.string.StringProcessor.MAX_RESULT_LENGTH; public class ReplaceProcessorTests extends AbstractWireSerializingTestCase { @@ -72,5 +74,16 @@ public void testReplaceFunctionInputsValidation() { () -> new Replace(EMPTY, l("foobarbar"), l("bar"), l(3)).makePipe().asProcessor().process(null) ); assertEquals("A string/char is required; received [3]", siae.getMessage()); + + String str = "b" + "a".repeat((int) MAX_RESULT_LENGTH - 2) + "b"; + assertEquals( + MAX_RESULT_LENGTH, + new Replace(EMPTY, l(str), l("b"), l("c")).makePipe().asProcessor().process(null).toString().length() + ); + + maxResultLengthTest( + MAX_RESULT_LENGTH + 2, + () -> new Replace(EMPTY, l(str), l("b"), l("cc")).makePipe().asProcessor().process(null) + ); } } diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/StringFunctionProcessorTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/StringFunctionProcessorTests.java index c1dc98571999f..1b3b103f575a0 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/StringFunctionProcessorTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/function/scalar/string/StringFunctionProcessorTests.java @@ -13,6 +13,8 @@ import java.util.Locale; +import static org.elasticsearch.xpack.sql.expression.function.scalar.string.StringProcessor.MAX_RESULT_LENGTH; + public class StringFunctionProcessorTests extends AbstractWireSerializingTestCase { public static StringProcessor randomStringFunctionProcessor() { return new StringProcessor(randomFrom(StringOperation.values())); @@ -196,9 +198,20 @@ public void testSpace() { assertEquals("", proc.process(0)); assertNull(proc.process(-1)); + assertEquals(MAX_RESULT_LENGTH, proc.process(MAX_RESULT_LENGTH).toString().length()); + maxResultLengthTest(MAX_RESULT_LENGTH + 1, () -> proc.process(MAX_RESULT_LENGTH + 1)); + numericInputValidation(proc); } + static void maxResultLengthTest(long required, ThrowingRunnable runnable) { + Exception e = expectThrows(SqlIllegalArgumentException.class, runnable); + assertEquals( + "Required result length [" + required + "] exceeds implementation limit [" + MAX_RESULT_LENGTH + "] bytes", + e.getMessage() + ); + } + public void testBitLength() { StringProcessor proc = new StringProcessor(StringOperation.BIT_LENGTH); assertNull(proc.process(null));