From b904a6027579c1cd0bccd5b93c4b24ecf477b6fa Mon Sep 17 00:00:00 2001 From: Andrei Stefan Date: Wed, 1 Jul 2020 13:51:45 +0300 Subject: [PATCH] EQL: Add case handling to stringContains (#58762) (#58813) Co-authored-by: Ross Wolf <31489089+rw-access@users.noreply.github.com> (cherry picked from commit 1a58776d3aa563beb364b067a1db46497122306f) --- .../scalar/string/StringContains.java | 39 ++++++++++++------- .../string/StringContainsFunctionPipe.java | 13 +++++-- .../StringContainsFunctionProcessor.java | 13 +++++-- .../function/scalar/string/StringUtils.java | 10 +++-- .../whitelist/InternalEqlScriptUtils.java | 4 +- .../xpack/eql/plugin/eql_whitelist.txt | 2 +- .../StringContainsFunctionPipeTests.java | 28 +++++++++---- .../StringContainsFunctionProcessorTests.java | 27 +++++++++---- .../scalar/string/StringUtilsTests.java | 36 ++++++++++++++--- .../src/test/resources/queryfolder_tests.txt | 15 +++++-- 10 files changed, 135 insertions(+), 52 deletions(-) diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/expression/function/scalar/string/StringContains.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/expression/function/scalar/string/StringContains.java index 48f5856c955..4728f6ad3e0 100644 --- a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/expression/function/scalar/string/StringContains.java +++ b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/expression/function/scalar/string/StringContains.java @@ -6,13 +6,15 @@ package org.elasticsearch.xpack.eql.expression.function.scalar.string; +import org.elasticsearch.xpack.eql.session.EqlConfiguration; import org.elasticsearch.xpack.ql.expression.Expression; import org.elasticsearch.xpack.ql.expression.Expressions; import org.elasticsearch.xpack.ql.expression.FieldAttribute; -import org.elasticsearch.xpack.ql.expression.function.scalar.ScalarFunction; +import org.elasticsearch.xpack.ql.expression.function.scalar.string.CaseSensitiveScalarFunction; import org.elasticsearch.xpack.ql.expression.gen.pipeline.Pipe; import org.elasticsearch.xpack.ql.expression.gen.script.ScriptTemplate; import org.elasticsearch.xpack.ql.expression.gen.script.Scripts; +import org.elasticsearch.xpack.ql.session.Configuration; import org.elasticsearch.xpack.ql.tree.NodeInfo; import org.elasticsearch.xpack.ql.tree.Source; import org.elasticsearch.xpack.ql.type.DataType; @@ -32,12 +34,12 @@ import static org.elasticsearch.xpack.ql.expression.gen.script.ParamsBuilder.par * stringContains(a, b) * Returns true if b is a substring of a */ -public class StringContains extends ScalarFunction { +public class StringContains extends CaseSensitiveScalarFunction { private final Expression string, substring; - public StringContains(Source source, Expression string, Expression substring) { - super(source, Arrays.asList(string, substring)); + public StringContains(Source source, Expression string, Expression substring, Configuration configuration) { + super(source, Arrays.asList(string, substring), configuration); this.string = string; this.substring = substring; } @@ -59,7 +61,7 @@ public class StringContains extends ScalarFunction { @Override protected Pipe makePipe() { return new StringContainsFunctionPipe(source(), this, - Expressions.pipe(string), Expressions.pipe(substring)); + Expressions.pipe(string), Expressions.pipe(substring), isCaseSensitive()); } @Override @@ -69,12 +71,12 @@ public class StringContains extends ScalarFunction { @Override public Object fold() { - return doProcess(string.fold(), substring.fold()); + return doProcess(string.fold(), substring.fold(), isCaseSensitive()); } @Override protected NodeInfo info() { - return NodeInfo.create(this, StringContains::new, string, substring); + return NodeInfo.create(this, StringContains::new, string, substring, eqlConfiguration()); } @Override @@ -83,14 +85,16 @@ public class StringContains extends ScalarFunction { } protected ScriptTemplate asScriptFrom(ScriptTemplate stringScript, ScriptTemplate substringScript) { - return new ScriptTemplate(format(Locale.ROOT, formatTemplate("{eql}.%s(%s,%s)"), + return new ScriptTemplate(format(Locale.ROOT, formatTemplate("{eql}.%s(%s,%s,%s)"), "stringContains", stringScript.template(), - substringScript.template()), + substringScript.template(), + "{}"), paramsBuilder() - .script(stringScript.params()) - .script(substringScript.params()) - .build(), dataType()); + .script(stringScript.params()) + .script(substringScript.params()) + .variable(isCaseSensitive()) + .build(), dataType()); } @Override @@ -111,6 +115,15 @@ public class StringContains extends ScalarFunction { throw new IllegalArgumentException("expected [2] children but received [" + newChildren.size() + "]"); } - return new StringContains(source(), newChildren.get(0), newChildren.get(1)); + return new StringContains(source(), newChildren.get(0), newChildren.get(1), eqlConfiguration()); + } + + public EqlConfiguration eqlConfiguration() { + return (EqlConfiguration) configuration(); + } + + @Override + public boolean isCaseSensitive() { + return eqlConfiguration().isCaseSensitive(); } } diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/expression/function/scalar/string/StringContainsFunctionPipe.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/expression/function/scalar/string/StringContainsFunctionPipe.java index 249f3d04069..09cf685bb16 100644 --- a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/expression/function/scalar/string/StringContainsFunctionPipe.java +++ b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/expression/function/scalar/string/StringContainsFunctionPipe.java @@ -19,11 +19,13 @@ import java.util.Objects; public class StringContainsFunctionPipe extends Pipe { private final Pipe string, substring; + private final boolean isCaseSensitive; - public StringContainsFunctionPipe(Source source, Expression expression, Pipe string, Pipe substring) { + public StringContainsFunctionPipe(Source source, Expression expression, Pipe string, Pipe substring, boolean isCaseSensitive) { super(source, expression, Arrays.asList(string, substring)); this.string = string; this.substring = substring; + this.isCaseSensitive = isCaseSensitive; } @Override @@ -55,7 +57,7 @@ public class StringContainsFunctionPipe extends Pipe { } protected StringContainsFunctionPipe replaceChildren(Pipe string, Pipe substring) { - return new StringContainsFunctionPipe(source(), expression(), string, substring); + return new StringContainsFunctionPipe(source(), expression(), string, substring, isCaseSensitive); } @Override @@ -66,12 +68,12 @@ public class StringContainsFunctionPipe extends Pipe { @Override protected NodeInfo info() { - return NodeInfo.create(this, StringContainsFunctionPipe::new, expression(), string, substring); + return NodeInfo.create(this, StringContainsFunctionPipe::new, expression(), string, substring, isCaseSensitive); } @Override public StringContainsFunctionProcessor asProcessor() { - return new StringContainsFunctionProcessor(string.asProcessor(), substring.asProcessor()); + return new StringContainsFunctionProcessor(string.asProcessor(), substring.asProcessor(), isCaseSensitive); } public Pipe string() { @@ -82,6 +84,9 @@ public class StringContainsFunctionPipe extends Pipe { return substring; } + protected boolean isCaseSensitive() { + return isCaseSensitive; + } @Override public int hashCode() { diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/expression/function/scalar/string/StringContainsFunctionProcessor.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/expression/function/scalar/string/StringContainsFunctionProcessor.java index c4b479d2811..5d8c79815be 100644 --- a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/expression/function/scalar/string/StringContainsFunctionProcessor.java +++ b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/expression/function/scalar/string/StringContainsFunctionProcessor.java @@ -19,29 +19,33 @@ public class StringContainsFunctionProcessor implements Processor { public static final String NAME = "sstc"; private final Processor string, substring; + private final boolean isCaseSensitive; - public StringContainsFunctionProcessor(Processor string, Processor substring) { + public StringContainsFunctionProcessor(Processor string, Processor substring, boolean isCaseSensitive) { this.string = string; this.substring = substring; + this.isCaseSensitive = isCaseSensitive; } public StringContainsFunctionProcessor(StreamInput in) throws IOException { string = in.readNamedWriteable(Processor.class); substring = in.readNamedWriteable(Processor.class); + isCaseSensitive = in.readBoolean(); } @Override public final void writeTo(StreamOutput out) throws IOException { out.writeNamedWriteable(string); out.writeNamedWriteable(substring); + out.writeBoolean(isCaseSensitive); } @Override public Object process(Object input) { - return doProcess(string.process(input), substring.process(input)); + return doProcess(string.process(input), substring.process(input), isCaseSensitive); } - public static Object doProcess(Object string, Object substring) { + public static Object doProcess(Object string, Object substring, boolean isCaseSensitive) { if (string == null) { return null; } @@ -51,7 +55,8 @@ public class StringContainsFunctionProcessor implements Processor { String strString = string.toString(); String strSubstring = substring.toString(); - return StringUtils.stringContains(strString, strSubstring); + + return StringUtils.stringContains(strString, strSubstring, isCaseSensitive); } private static void throwIfNotString(Object obj) { diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/expression/function/scalar/string/StringUtils.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/expression/function/scalar/string/StringUtils.java index fd334b3de09..734efd62099 100644 --- a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/expression/function/scalar/string/StringUtils.java +++ b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/expression/function/scalar/string/StringUtils.java @@ -65,15 +65,19 @@ final class StringUtils { * * @param string string to search through. * @param substring string to search for. + * @param isCaseSensitive toggle for case sensitivity. * @return {@code true} if {@code string} string contains {@code substring} string. */ - static boolean stringContains(String string, String substring) { + static boolean stringContains(String string, String substring, boolean isCaseSensitive) { if (hasLength(string) == false || hasLength(substring) == false) { return false; } - string = string.toLowerCase(Locale.ROOT); - substring = substring.toLowerCase(Locale.ROOT); + if (isCaseSensitive == false) { + string = string.toLowerCase(Locale.ROOT); + substring = substring.toLowerCase(Locale.ROOT); + } + return string.contains(substring); } diff --git a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/expression/function/scalar/whitelist/InternalEqlScriptUtils.java b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/expression/function/scalar/whitelist/InternalEqlScriptUtils.java index 692f4641671..9443fde3784 100644 --- a/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/expression/function/scalar/whitelist/InternalEqlScriptUtils.java +++ b/x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/expression/function/scalar/whitelist/InternalEqlScriptUtils.java @@ -57,8 +57,8 @@ public class InternalEqlScriptUtils extends InternalQlScriptUtils { return (String) ToStringFunctionProcessor.doProcess(s); } - public static Boolean stringContains(String string, String substring) { - return (Boolean) StringContainsFunctionProcessor.doProcess(string, substring); + public static Boolean stringContains(String string, String substring, Boolean isCaseSensitive) { + return (Boolean) StringContainsFunctionProcessor.doProcess(string, substring, isCaseSensitive); } public static Number number(String source, Number base) { diff --git a/x-pack/plugin/eql/src/main/resources/org/elasticsearch/xpack/eql/plugin/eql_whitelist.txt b/x-pack/plugin/eql/src/main/resources/org/elasticsearch/xpack/eql/plugin/eql_whitelist.txt index bab2baa8ca6..e490fef17c3 100644 --- a/x-pack/plugin/eql/src/main/resources/org/elasticsearch/xpack/eql/plugin/eql_whitelist.txt +++ b/x-pack/plugin/eql/src/main/resources/org/elasticsearch/xpack/eql/plugin/eql_whitelist.txt @@ -73,6 +73,6 @@ class org.elasticsearch.xpack.eql.expression.function.scalar.whitelist.InternalE Integer length(String) Number number(String, Number) String string(Object) - Boolean stringContains(String, String) + Boolean stringContains(String, String, Boolean) String substring(String, Number, Number) } diff --git a/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/expression/function/scalar/string/StringContainsFunctionPipeTests.java b/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/expression/function/scalar/string/StringContainsFunctionPipeTests.java index 54679158b6b..be37b348d3f 100644 --- a/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/expression/function/scalar/string/StringContainsFunctionPipeTests.java +++ b/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/expression/function/scalar/string/StringContainsFunctionPipeTests.java @@ -6,6 +6,7 @@ package org.elasticsearch.xpack.eql.expression.function.scalar.string; +import org.elasticsearch.xpack.eql.EqlTestUtils; import org.elasticsearch.xpack.ql.expression.Expression; import org.elasticsearch.xpack.ql.expression.gen.pipeline.Pipe; import org.elasticsearch.xpack.ql.tree.AbstractNodeTestCase; @@ -32,7 +33,11 @@ public class StringContainsFunctionPipeTests extends AbstractNodeTestCase Objects.equals(v, b1.expression()) ? newExpression : v, Expression.class)); StringContainsFunctionPipe b2 = randomInstance(); @@ -54,7 +60,8 @@ public class StringContainsFunctionPipeTests extends AbstractNodeTestCase Objects.equals(v, b2.source()) ? newLoc : v, Source.class)); } @@ -64,8 +71,9 @@ public class StringContainsFunctionPipeTests extends AbstractNodeTestCase randomStringLiteral()))); Pipe newSubstring = pipe(((Expression) randomValueOtherThan(b.substring(), () -> randomStringLiteral()))); + boolean newCaseSensitive = randomValueOtherThan(b.isCaseSensitive(), () -> randomBoolean()); StringContainsFunctionPipe newB = - new StringContainsFunctionPipe(b.source(), b.expression(), b.string(), b.substring()); + new StringContainsFunctionPipe(b.source(), b.expression(), b.string(), b.substring(), newCaseSensitive); StringContainsFunctionPipe transformed = newB.replaceChildren(newString, b.substring()); assertEquals(transformed.string(), newString); @@ -92,15 +100,18 @@ public class StringContainsFunctionPipeTests extends AbstractNodeTestCase new StringContainsFunctionPipe(f.source(), f.expression(), pipe(((Expression) randomValueOtherThan(f.string(), () -> randomStringLiteral()))), - f.substring())); + f.substring(), + randomValueOtherThan(f.isCaseSensitive(), () -> randomBoolean()))); randoms.add(f -> new StringContainsFunctionPipe(f.source(), f.expression(), f.string(), - pipe(((Expression) randomValueOtherThan(f.substring(), () -> randomStringLiteral()))))); + pipe(((Expression) randomValueOtherThan(f.substring(), () -> randomStringLiteral()))), + randomValueOtherThan(f.isCaseSensitive(), () -> randomBoolean()))); randoms.add(f -> new StringContainsFunctionPipe(f.source(), f.expression(), pipe(((Expression) randomValueOtherThan(f.string(), () -> randomStringLiteral()))), - pipe(((Expression) randomValueOtherThan(f.substring(), () -> randomStringLiteral()))))); + pipe(((Expression) randomValueOtherThan(f.substring(), () -> randomStringLiteral()))), + randomValueOtherThan(f.isCaseSensitive(), () -> randomBoolean()))); return randomFrom(randoms).apply(instance); } @@ -110,6 +121,7 @@ public class StringContainsFunctionPipeTests extends AbstractNodeTestCase { String substring = randomBoolean() ? null : randomAlphaOfLength(10); - String str = randomBoolean() ? null : randomAlphaOfLength(10); + String str = randomBoolean() ? null : randomValueOtherThan(substring, () -> randomAlphaOfLength(10)); + boolean caseSensitive = randomBoolean(); if (str != null && substring != null) { str += substring; - str += randomAlphaOfLength(10); + str += randomValueOtherThan(substring, () -> randomAlphaOfLength(10)); } final String string = str; // The string parameter can be null. Expect exception if any of other parameters is null. - if ((string != null) && (substring == null)) { + if (string != null && substring == null) { EqlIllegalArgumentException e = expectThrows(EqlIllegalArgumentException.class, - () -> StringContainsFunctionProcessor.doProcess(string, substring)); + () -> doProcess(string, substring, caseSensitive)); assertThat(e.getMessage(), equalTo("A string/char is required; received [null]")); } else { - assertThat(StringContainsFunctionProcessor.doProcess(string, substring), - equalTo(string == null ? null : true)); + assertThat(doProcess(string, substring, caseSensitive), equalTo(string == null ? null : true)); + + // deliberately make the test return "false" by lowercasing or uppercasing the substring in a case-sensitive scenario + if (caseSensitive && substring != null) { + String subsChanged = randomBoolean() ? substring.toLowerCase(Locale.ROOT) : substring.toUpperCase(Locale.ROOT); + if (substring.equals(subsChanged) == false) { + assertThat(doProcess(string, subsChanged, caseSensitive), equalTo(string == null ? null : false)); + } + } } + return null; }); } + } diff --git a/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/expression/function/scalar/string/StringUtilsTests.java b/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/expression/function/scalar/string/StringUtilsTests.java index 0d21282cb67..d00e5fb2b67 100644 --- a/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/expression/function/scalar/string/StringUtilsTests.java +++ b/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/expression/function/scalar/string/StringUtilsTests.java @@ -8,6 +8,8 @@ package org.elasticsearch.xpack.eql.expression.function.scalar.string; import org.elasticsearch.test.ESTestCase; +import java.util.Locale; + import static org.elasticsearch.xpack.eql.expression.function.scalar.string.StringUtils.stringContains; import static org.elasticsearch.xpack.eql.expression.function.scalar.string.StringUtils.substringSlice; import static org.elasticsearch.xpack.ql.util.StringUtils.EMPTY; @@ -140,14 +142,36 @@ public class StringUtilsTests extends ESTestCase { } public void testStringContainsWithNullOrEmpty() { - assertFalse(stringContains(null, null)); - assertFalse(stringContains(null, "")); - assertFalse(stringContains("", null)); + assertFalse(stringContains(null, null, true)); + assertFalse(stringContains(null, "", true)); + assertFalse(stringContains("", null, true)); + + assertFalse(stringContains(null, null, false)); + assertFalse(stringContains(null, "", false)); + assertFalse(stringContains("", null, false)); } - public void testStringContainsWithRandom() throws Exception { + public void testStringContainsWithRandomCaseSensitive() throws Exception { String substring = randomAlphaOfLength(10); - String string = randomAlphaOfLength(10) + substring + randomAlphaOfLength(10); - assertTrue(stringContains(string, substring)); + String string = randomValueOtherThan(substring, () -> randomAlphaOfLength(10)) + + substring + + randomValueOtherThan(substring, () -> randomAlphaOfLength(10)); + assertTrue(stringContains(string, substring, true)); + } + + public void testStringContainsWithRandomCaseInsensitive() throws Exception { + String substring = randomAlphaOfLength(10); + String subsChanged = substring.toUpperCase(Locale.ROOT); + String string = randomValueOtherThan(subsChanged, () -> randomAlphaOfLength(10)) + + subsChanged + + randomValueOtherThan(subsChanged, () -> randomAlphaOfLength(10)); + assertTrue(stringContains(string, substring, false)); + + substring = randomAlphaOfLength(10); + subsChanged = substring.toLowerCase(Locale.ROOT); + string = randomValueOtherThan(subsChanged, () -> randomAlphaOfLength(10)) + + subsChanged + + randomValueOtherThan(subsChanged, () -> randomAlphaOfLength(10)); + assertTrue(stringContains(string, substring, false)); } } diff --git a/x-pack/plugin/eql/src/test/resources/queryfolder_tests.txt b/x-pack/plugin/eql/src/test/resources/queryfolder_tests.txt index 1854d1961e2..aa5f5a6ef69 100644 --- a/x-pack/plugin/eql/src/test/resources/queryfolder_tests.txt +++ b/x-pack/plugin/eql/src/test/resources/queryfolder_tests.txt @@ -189,14 +189,23 @@ process where startsWith(user_name, 'A') or startsWith(user_name, 'B') {"prefix":{"user_name":{"value":"B","boost":1.0}}}],"adjust_pure_negative":true,"boost":1.0}}] ; -stringContains +stringContains-caseSensitive process where stringContains(process_name, "foo") ; "script":{"source":"InternalQlScriptUtils.nullSafeFilter(InternalEqlScriptUtils.stringContains( -InternalQlScriptUtils.docValue(doc,params.v0),params.v1))" -"params":{"v0":"process_name","v1":"foo"} +InternalQlScriptUtils.docValue(doc,params.v0),params.v1,params.v2))" +"params":{"v0":"process_name","v1":"foo","v2":true} ; +stringContains-caseInsensitive +process where stringContains(process_name, "foo") +; +"script":{"source":"InternalQlScriptUtils.nullSafeFilter(InternalEqlScriptUtils.stringContains( +InternalQlScriptUtils.docValue(doc,params.v0),params.v1,params.v2))" +"params":{"v0":"process_name","v1":"foo","v2":false} +; + + stringFunction process where string(pid) == "123" ;