Skip to content

EQL: Replace ?"..." with """...""" for unescaped strings #62539

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Oct 2, 2020
Merged

EQL: Replace ?"..." with """...""" for unescaped strings #62539

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
32e85fd
EQL: Replace ?"..." with """...""" for unescaped strings
matriv Sep 17, 2020
480aac5
change error message
matriv Sep 17, 2020
356cabf
Merge remote-tracking branch 'upstream/master' into replace-unescaped…
matriv Sep 18, 2020
374bc92
fix escaped '
matriv Sep 18, 2020
ae7d4ed
add more tests
matriv Sep 28, 2020
b32a383
Merge remote-tracking branch 'upstream/master' into replace-unescaped…
matriv Sep 28, 2020
fec23ce
fix tests
matriv Sep 28, 2020
af8ca3d
Try to allow """ with one escapedwq
matriv Sep 29, 2020
3ada210
Merge remote-tracking branch 'upstream/master' into replace-unescaped…
matriv Oct 1, 2020
a8f4cbb
Use Python approach
matriv Oct 1, 2020
60e1b4e
Merge remote-tracking branch 'upstream/master' into replace-unescaped…
matriv Oct 1, 2020
822d494
Merge remote-tracking branch 'upstream/master' into replace-unescaped…
matriv Oct 1, 2020
23ffe1f
Merge remote-tracking branch 'upstream/master' into replace-unescaped…
matriv Oct 2, 2020
f838bc2
Disallow any triple double quote sequence and everything is raw
matriv Oct 2, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions x-pack/plugin/eql/qa/common/src/main/resources/additional_test_queries.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,24 +138,24 @@ expected_event_ids = [98]
notes = "regexp doesn't support character classes"
query = '''
//
// ?".*?net1\s+localgroup.*?")
process where match(command_line, ?".*?net1[ ]+localgroup.*?")
// """.*?net1\s+localgroup.*?""")
process where match(command_line, """.*?net1[ ]+localgroup.*?""")
'''

[[queries]]
name = "matchLiteAdditional"
expected_event_ids = [98]
query = '''
process where matchLite(command_line, ?".*?net1.*?")
process where matchLite(command_line, """.*?net1.*?""")
'''

[[queries]]
name = "matchWithCharacterClasses2"
expected_event_ids = [98]
notes = "regexp doesn't support predefined character classes (like \\s)"
query = '''
// ?".*?net1\s+\w{4,15}\s+.*?"
process where match(command_line, ?".*?net1[ ]+[a-z]{4,15}[ ]+.*?")
// """.*?net1\s+\w{4,15}\s+.*?"""
process where match(command_line, """.*?net1[ ]+[a-z]{4,15}[ ]+.*?""")
'''


Expand Down
10 changes: 5 additions & 5 deletions x-pack/plugin/eql/qa/common/src/main/resources/test_queries.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1399,35 +1399,35 @@ registry where bytes_written_string_list[1] : "en"
[[queries]]
name = "matchLite1"
query = '''
process where matchLite(command_line, ?".*?net1\s+localgroup\s+.*?")
process where matchLite(command_line, """.*?net1\s+localgroup\s+.*?""")
'''
expected_event_ids = [98]

[[queries]]
name = "matchLite2"
query = '''
process where matchLite(command_line, ?".*?net1\s+\w+\s+.*?")
process where matchLite(command_line, """.*?net1\s+\w+\s+.*?""")
'''
expected_event_ids = [98]

[[queries]]
name = "matchLite3"
query = '''
process where matchLite(command_line, ?".*?net1\s+\w{4,15}\s+.*?")
process where matchLite(command_line, """.*?net1\s+\w{4,15}\s+.*?""")
'''
expected_event_ids = [98]

[[queries]]
name = "match1"
expected_event_ids = [98]
query = '''
process where match(command_line, ?".*?net1\s+\w{4,15}\s+.*?")
process where match(command_line, """.*?net1\s+\w{4,15}\s+.*?""")
'''

[[queries]]
name = "matchLite4"
query = '''
process where matchLite(command_line, ?".*?net1\s+[localgrup]{4,15}\s+.*?")
process where matchLite(command_line, """.*?net1\s+[localgrup]{4,15}\s+.*?""")
'''
expected_event_ids = [98]

Expand Down
10 changes: 5 additions & 5 deletions x-pack/plugin/eql/qa/common/src/main/resources/test_queries_unsupported.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -797,35 +797,35 @@ registry where bytes_written_string_list[1] : "en"
[[queries]]
name = "matchLite1"
query = '''
process where matchLite(command_line, ?".*?net1\s+localgroup\s+.*?")
process where matchLite(command_line, """.*?net1\s+localgroup\s+.*?""")
'''
expected_event_ids = [98]

[[queries]]
name = "matchLite2"
query = '''
process where matchLite(command_line, ?".*?net1\s+\w+\s+.*?")
process where matchLite(command_line, """.*?net1\s+\w+\s+.*?""")
'''
expected_event_ids = [98]

[[queries]]
name = "matchLite3"
query = '''
process where matchLite(command_line, ?".*?net1\s+\w{4,15}\s+.*?")
process where matchLite(command_line, """.*?net1\s+\w{4,15}\s+.*?""")
'''
expected_event_ids = [98]

[[queries]]
name = "match1"
expected_event_ids = [98]
query = '''
process where match(command_line, ?".*?net1\s+\w{4,15}\s+.*?")
process where match(command_line, """.*?net1\s+\w{4,15}\s+.*?""")
'''

[[queries]]
name = "matchLite4"
query = '''
process where matchLite(command_line, ?".*?net1\s+[localgrup]{4,15}\s+.*?")
process where matchLite(command_line, """.*?net1\s+[localgrup]{4,15}\s+.*?""")
'''
expected_event_ids = [98]

Expand Down
1 change: 1 addition & 0 deletions x-pack/plugin/eql/src/main/antlr/EqlBase.g4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ STRING
| '"' ('\\' [btnfr"'\\] | ~[\r\n"\\])* '"'
| '?"' ('\\"' |~["\r\n])* '"'
| '?\'' ('\\\'' |~['\r\n])* '\''
| '"""' (~[\r\n])*? '"""' '"'? '"'?
;

INTEGER_VALUE
Expand Down
11 changes: 8 additions & 3 deletions x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/parser/AbstractBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -122,10 +122,15 @@ public static String unquoteString(Source source) {
return null;
}

// unescaped strings can be interpreted directly
// catch old method of ?" and ?' to define unescaped strings
if (text.startsWith("?")) {
checkForSingleQuotedString(source, text, 1);
return text.substring(2, text.length() - 1);
throw new ParsingException(source,
"Use triple double quotes [\"\"\"] to define unescaped string literals, not [?{}]", text.charAt(1));
}

// unescaped strings can be interpreted directly
if (text.startsWith("\"\"\"")) {
return text.substring(3, text.length() - 3);
}

checkForSingleQuotedString(source, text, 0);
Expand Down
247 changes: 128 additions & 119 deletions x-pack/plugin/eql/src/main/java/org/elasticsearch/xpack/eql/parser/EqlBaseLexer.java
View file
Open in desktop

Large diffs are not rendered by default.

104 changes: 88 additions & 16 deletions x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/parser/ExpressionTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,14 +66,24 @@ public void testStrings() {
assertEquals("hello\\\nworld", unquoteString(source("\"hello\\\\\\nworld\"")));
assertEquals("hello\\\"world", unquoteString(source("\"hello\\\\\\\"world\"")));

// test for unescaped strings: ?"...."
assertEquals("hello\"world", unquoteString(source("?\"hello\"world\"")));
assertEquals("hello\\\"world", unquoteString(source("?\"hello\\\"world\"")));
assertEquals("hello'world", unquoteString(source("?\"hello'world\"")));
assertEquals("hello\\nworld", unquoteString(source("?\"hello\\nworld\"")));
assertEquals("hello\\\\nworld", unquoteString(source("?\"hello\\\\nworld\"")));
assertEquals("hello\\\\\\nworld", unquoteString(source("?\"hello\\\\\\nworld\"")));
assertEquals("hello\\\\\\\"world", unquoteString(source("?\"hello\\\\\\\"world\"")));
// test for unescaped strings: """...."""
assertEquals("hello\"world", unquoteString(source("\"\"\"hello\"world\"\"\"")));
assertEquals("hello\\\"world", unquoteString(source("\"\"\"hello\\\"world\"\"\"")));
assertEquals("\"\"hello\"\\\"world\"\"\"", unquoteString(source("\"\"\"\"\"hello\"\\\"world\"\"\"\"\"\"")));
assertEquals("hello'world", unquoteString(source("\"\"\"hello'world\"\"\"")));
assertEquals("hello'world", unquoteString(source("\"\"\"hello\'world\"\"\"")));
assertEquals("hello\\'world", unquoteString(source("\"\"\"hello\\\'world\"\"\"")));
assertEquals("hello\\nworld", unquoteString(source("\"\"\"hello\\nworld\"\"\"")));
assertEquals("hello\\\\nworld", unquoteString(source("\"\"\"hello\\\\nworld\"\"\"")));
assertEquals("hello\\\\\\nworld", unquoteString(source("\"\"\"hello\\\\\\nworld\"\"\"")));
assertEquals("hello\\\\\\\"world", unquoteString(source("\"\"\"hello\\\\\\\"world\"\"\"")));
assertEquals("\"\\\"", unquoteString(source("\"\"\"\"\\\"\"\"\"")));
assertEquals("\\\"\"\"", unquoteString(source("\"\"\"\\\"\"\"\"\"\"")));
assertEquals("\"\\\"\"", unquoteString(source("\"\"\"\"\\\"\"\"\"\"")));
assertEquals("\"\"\\\"", unquoteString(source("\"\"\"\"\"\\\"\"\"\"")));
assertEquals("\"\"", unquoteString(source("\"\"\"\"\"\"\"\"")));
assertEquals("\"\" \"\"", unquoteString(source("\"\"\"\"\" \"\"\"\"\"")));
assertEquals("", unquoteString(source("\"\"\"\"\"\"")));
}

public void testLiterals() {
Expand All @@ -100,18 +110,80 @@ public void testDoubleQuotedString() {

public void testSingleQuotedUnescapedStringDisallowed() {
ParsingException e = expectThrows(ParsingException.class, () -> expr("?'hello world'"));
assertEquals("line 1:2: Use double quotes [\"] to define string literals, not single quotes [']",
assertEquals("line 1:2: Use triple double quotes [\"\"\"] to define unescaped string literals, not [?']",
e.getMessage());
e = expectThrows(ParsingException.class, () -> parser.createStatement("process where name==?'hello world'"));
assertEquals("line 1:22: Use double quotes [\"] to define string literals, not single quotes [']",
e = expectThrows(ParsingException.class, () -> parser.createStatement("process where name == ?'hello world'"));
assertEquals("line 1:24: Use triple double quotes [\"\"\"] to define unescaped string literals, not [?']",
e.getMessage());
}

public void testDoubleQuotedUnescapedString() {
// "hello \" world"
Expression parsed = expr("?\"hello \\\" world!\"");
Expression expected = new Literal(null, "hello \\\" world!", DataTypes.KEYWORD);
assertEquals(expected, parsed);
public void testDoubleQuotedUnescapedStringForbidden() {
ParsingException e = expectThrows(ParsingException.class, () -> expr("?\"hello world\""));
assertEquals("line 1:2: Use triple double quotes [\"\"\"] to define unescaped string literals, not [?\"]",
e.getMessage());
e = expectThrows(ParsingException.class, () -> parser.createStatement("process where name == ?\"hello world\""));
assertEquals("line 1:24: Use triple double quotes [\"\"\"] to define unescaped string literals, not [?\"]",
e.getMessage());
}

public void testTripleDoubleQuotedUnescapedString() {
Copy link
Contributor

@rw-access rw-access Sep 28, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how is """hello""" == """world""" interpreted?

it should be equivalent to this expression Equals("hello", "world):
"hello" == "world"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can add a test for that

// """hello world!"""" == """foobar""" => hello world! = foobar
String str = "\"\"\"hello world!\"\"\" == \"\"\"foobar\"\"\"";
String expectedStrLeft = "hello world!";
String expectedStrRight = "foobar";
Expression parsed = expr(str);
assertEquals(Equals.class, parsed.getClass());
Equals eq = (Equals) parsed;
assertEquals(Literal.class, eq.left().getClass());
assertEquals(expectedStrLeft, ((Literal) eq.left()).value());
assertEquals(Literal.class, eq.right().getClass());
assertEquals(expectedStrRight, ((Literal) eq.right()).value());

// """""hello""world!"""" == """"foo"bar""""" => ""hello""world!" = "foo""bar""
str = " \"\"\"\"\"hello\"\"world!\"\"\"\" == \"\"\"\"foo\"bar\"\"\"\"\" ";
expectedStrLeft = "\"\"hello\"\"world!\"";
expectedStrRight = "\"foo\"bar\"\"";
parsed = expr(str);
assertEquals(Equals.class, parsed.getClass());
eq = (Equals) parsed;
assertEquals(Literal.class, eq.left().getClass());
assertEquals(expectedStrLeft, ((Literal) eq.left()).value());
assertEquals(Literal.class, eq.right().getClass());
assertEquals(expectedStrRight, ((Literal) eq.right()).value());

// """""\""hello\\""\""world!\\""""" == """\\""\""foo""\\""\"bar""\\""\"""" =>
// ""\""hello\\""\""world!\\"" == \\""\""foo""\\""\"bar""\\""\"
str = " \"\"\"\"\"\\\"\"hello\\\\\"\"\\\"\"world!\\\\\"\"\"\"\" == " +
" \"\"\"\\\\\"\"\\\"\"foo\"\"\\\\\"\"\\\"bar\"\"\\\\\"\"\\\"\"\"\" ";
expectedStrLeft = "\"\"\\\"\"hello\\\\\"\"\\\"\"world!\\\\\"\"";
expectedStrRight = "\\\\\"\"\\\"\"foo\"\"\\\\\"\"\\\"bar\"\"\\\\\"\"\\\"";
parsed = expr(str);
assertEquals(Equals.class, parsed.getClass());
eq = (Equals) parsed;
assertEquals(Literal.class, eq.left().getClass());
assertEquals(expectedStrLeft, ((Literal) eq.left()).value());
assertEquals(Literal.class, eq.right().getClass());
assertEquals(expectedStrRight, ((Literal) eq.right()).value());

// """"""hello world!""" == """foobar"""
ParsingException e = expectThrows(ParsingException.class, "Expected syntax error",
() -> expr("\"\"\"\"\"\"hello world!\"\"\" == \"\"\"foobar\"\"\""));
assertThat(e.getMessage(), startsWith("line 1:7: mismatched input 'hello' expecting {<EOF>,"));

// """""\"hello world!"""""" == """foobar"""
e = expectThrows(ParsingException.class, "Expected syntax error",
() -> expr("\"\"\"\"\"\\\"hello world!\"\"\"\"\"\" == \"\"\"foobar\"\"\""));
assertThat(e.getMessage(), startsWith("line 1:25: mismatched input '\" == \"' expecting {<EOF>,"));

// """""\"hello world!""\"""" == """"""foobar"""
e = expectThrows(ParsingException.class, "Expected syntax error",
() -> expr("\"\"\"\"\"\\\"hello world!\"\"\\\"\"\"\" == \"\"\"\"\"\"foobar\"\"\""));
assertThat(e.getMessage(), startsWith("line 1:37: mismatched input 'foobar' expecting {<EOF>,"));

// """""\"hello world!""\"""" == """""\"foobar\"\""""""
e = expectThrows(ParsingException.class, "Expected syntax error",
() -> expr("\"\"\"\"\"\\\"hello world!\"\"\\\"\"\"\" == \"\"\"\"\"\\\"foobar\\\"\\\"\"\"\"\"\""));
assertEquals("line 1:52: token recognition error at: '\"'", e.getMessage());
}

public void testNumbers() {
Expand Down
10 changes: 5 additions & 5 deletions x-pack/plugin/eql/src/test/resources/queries-supported.eql
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,19 +178,19 @@ process where command_line : "*%*%*" ;
process where command_line : "%*%*" ;


process where match(?".*?net1\s+localgroup\s+.*?", command_line)
process where match(""".*?net1\s+localgroup\s+.*?""", command_line)
;

process where match(?".*?net1\s+\w+\s+.*?", command_line)
process where match(""".*?net1\s+\w+\s+.*?""", command_line)
;

process where match(?".*?net1\s+\w{4,15}\s+.*?", command_line)
process where match(""".*?net1\s+\w{4,15}\s+.*?""", command_line)
;

process where match(?".*?net1\s+\w{4,15}\s+.*?", command_line)
process where match(""".*?net1\s+\w{4,15}\s+.*?""", command_line)
;

process where match(?".*?net1\s+[localgrup]{4,15}\s+.*?", command_line)
process where match(""".*?net1\s+[localgrup]{4,15}\s+.*?""", command_line)
;

file where opcode:0 and startsWith(file_name, "exploRER.")
Expand Down