Skip to content

Commit

Permalink
SQL: Prevent StackOverflowError when parsing large statements (#33902)
Browse files Browse the repository at this point in the history 
Implement circuit breaker logic in the parser which catches expressions
that can blow up the tree and result in StackOverflowError being thrown.

Co-authored-by: Costin Leau <[email protected]>
  • Loading branch information
@costin @matriv
2 people authored and matriv committed Sep 25, 2018
1 parent 2b43c8a commit 6a4e841
Show file tree
Hide file tree
Showing 3 changed files with 125 additions and 7 deletions.
44 changes: 39 additions & 5 deletions x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/SqlParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/
package org.elasticsearch.xpack.sql.parser;

import com.carrotsearch.hppc.ObjectShortHashMap;
import org.antlr.v4.runtime.BaseErrorListener;
import org.antlr.v4.runtime.CharStream;
import org.antlr.v4.runtime.CommonToken;
Expand All @@ -22,8 +23,8 @@
import org.antlr.v4.runtime.dfa.DFA;
import org.antlr.v4.runtime.misc.Pair;
import org.antlr.v4.runtime.tree.TerminalNode;
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
import org.elasticsearch.common.logging.Loggers;
import org.elasticsearch.xpack.sql.expression.Expression;
import org.elasticsearch.xpack.sql.plan.logical.LogicalPlan;
import org.elasticsearch.xpack.sql.proto.SqlTypedParamValue;
Expand All @@ -41,7 +42,8 @@
import static java.lang.String.format;

public class SqlParser {
private static final Logger log = Loggers.getLogger(SqlParser.class);

private static final Logger log = LogManager.getLogger();

private final boolean DEBUG = false;

Expand Down Expand Up @@ -83,7 +85,9 @@ public Expression createExpression(String expression, List<SqlTypedParamValue> p
return invokeParser(expression, params, SqlBaseParser::singleExpression, AstBuilder::expression);
}

private <T> T invokeParser(String sql, List<SqlTypedParamValue> params, Function<SqlBaseParser, ParserRuleContext> parseFunction,
private <T> T invokeParser(String sql,
List<SqlTypedParamValue> params, Function<SqlBaseParser,
ParserRuleContext> parseFunction,
BiFunction<AstBuilder, ParserRuleContext, T> visitor) {
SqlBaseLexer lexer = new SqlBaseLexer(new CaseInsensitiveStream(sql));

Expand All @@ -96,6 +100,7 @@ private <T> T invokeParser(String sql, List<SqlTypedParamValue> params, Function
CommonTokenStream tokenStream = new CommonTokenStream(tokenSource);
SqlBaseParser parser = new SqlBaseParser(tokenStream);

parser.addParseListener(new CircuitBreakerListener());
parser.addParseListener(new PostProcessor(Arrays.asList(parser.getRuleNames())));

parser.removeErrorListeners();
Expand Down Expand Up @@ -125,7 +130,7 @@ private <T> T invokeParser(String sql, List<SqlTypedParamValue> params, Function
return visitor.apply(new AstBuilder(paramTokens), tree);
}

private void debug(SqlBaseParser parser) {
private static void debug(SqlBaseParser parser) {

// when debugging, use the exact prediction mode (needed for diagnostics as well)
parser.getInterpreter().setPredictionMode(PredictionMode.LL_EXACT_AMBIG_DETECTION);
Expand Down Expand Up @@ -154,7 +159,7 @@ private class PostProcessor extends SqlBaseBaseListener {
public void exitBackQuotedIdentifier(SqlBaseParser.BackQuotedIdentifierContext context) {
Token token = context.BACKQUOTED_IDENTIFIER().getSymbol();
throw new ParsingException(
"backquoted indetifiers not supported; please use double quotes instead",
"backquoted identifiers not supported; please use double quotes instead",
null,
token.getLine(),
token.getCharPositionInLine());
Expand Down Expand Up @@ -205,6 +210,35 @@ public void exitNonReserved(SqlBaseParser.NonReservedContext context) {
}
}

/**
* Used to catch large expressions that can lead to stack overflows
*/
private class CircuitBreakerListener extends SqlBaseBaseListener {

private static final short MAX_RULE_DEPTH = 100;

// Keep current depth for every rule visited.
// The totalDepth alone cannot be used as expressions like: e1 OR e2 OR e3 OR ...
// are processed as e1 OR (e2 OR (e3 OR (... and this results in the totalDepth not growing
// while the stack call depth is, leading to a StackOverflowError.
private ObjectShortHashMap<String> depthCounts = new ObjectShortHashMap<>();

@Override
public void enterEveryRule(ParserRuleContext ctx) {
short currentDepth = depthCounts.putOrAdd(ctx.getClass().getSimpleName(), (short) 1, (short) 1);
if (currentDepth > MAX_RULE_DEPTH) {
throw new ParsingException("expression is too large to parse, (tree's depth exceeds {})", MAX_RULE_DEPTH);
}
super.enterEveryRule(ctx);
}

@Override
public void exitEveryRule(ParserRuleContext ctx) {
depthCounts.putOrAdd(ctx.getClass().getSimpleName(), (short) 0, (short) -1);
super.exitEveryRule(ctx);
}
}

private static final BaseErrorListener ERROR_LISTENER = new BaseErrorListener() {
@Override
public void syntaxError(Recognizer<?, ?> recognizer, Object offendingSymbol, int line,
Expand Down
4 changes: 2 additions & 2 deletions x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/expression/QuotingTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public void testBackQuotedAttribute() {
String name = "@timestamp";
ParsingException ex = expectThrows(ParsingException.class, () ->
new SqlParser().createExpression(quote + name + quote));
assertThat(ex.getMessage(), equalTo("line 1:1: backquoted indetifiers not supported; please use double quotes instead"));
assertThat(ex.getMessage(), equalTo("line 1:1: backquoted identifiers not supported; please use double quotes instead"));
}

public void testQuotedAttributeAndQualifier() {
Expand All @@ -92,7 +92,7 @@ public void testBackQuotedAttributeAndQualifier() {
String name = "@timestamp";
ParsingException ex = expectThrows(ParsingException.class, () ->
new SqlParser().createExpression(quote + qualifier + quote + "." + quote + name + quote));
assertThat(ex.getMessage(), equalTo("line 1:1: backquoted indetifiers not supported; please use double quotes instead"));
assertThat(ex.getMessage(), equalTo("line 1:1: backquoted identifiers not supported; please use double quotes instead"));
}

public void testGreedyQuoting() {
Expand Down
84 changes: 84 additions & 0 deletions x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/SqlParserTests.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/
package org.elasticsearch.xpack.sql.parser;

import com.google.common.base.Joiner;
import org.elasticsearch.test.ESTestCase;
import org.elasticsearch.xpack.sql.expression.NamedExpression;
import org.elasticsearch.xpack.sql.expression.Order;
Expand All @@ -22,6 +23,7 @@
import java.util.ArrayList;
import java.util.List;

import static java.util.Collections.nCopies;
import static java.util.stream.Collectors.toList;
import static org.hamcrest.Matchers.hasEntry;
import static org.hamcrest.Matchers.hasSize;
Expand Down Expand Up @@ -136,6 +138,88 @@ public void testMultiMatchQuery() {
assertThat(mmqp.optionMap(), hasEntry("fuzzy_rewrite", "scoring_boolean"));
}

public void testLimitToPreventStackOverflowFromLargeUnaryBooleanExpression() {
// Create expression in the form of NOT(NOT(NOT ... (b) ...)

// 40 elements is ok
new SqlParser().createExpression(
Joiner.on("").join(nCopies(40, "NOT(")).concat("b").concat(Joiner.on("").join(nCopies(40, ")"))));

// 100 elements parser's "circuit breaker" is triggered
ParsingException e = expectThrows(ParsingException.class, () -> new SqlParser().createExpression(
Joiner.on("").join(nCopies(100, "NOT(")).concat("b").concat(Joiner.on("").join(nCopies(100, ")")))));
assertEquals("expression is too large to parse, (tree's depth exceeds 100)", e.getErrorMessage());
}

public void testLimitToPreventStackOverflowFromLargeBinaryBooleanExpression() {
// Create expression in the form of a = b OR a = b OR ... a = b

// 50 elements is ok
new SqlParser().createExpression(Joiner.on(" OR ").join(nCopies(50, "a = b")));

// 100 elements parser's "circuit breaker" is triggered
ParsingException e = expectThrows(ParsingException.class, () ->
new SqlParser().createExpression(Joiner.on(" OR ").join(nCopies(100, "a = b"))));
assertEquals("expression is too large to parse, (tree's depth exceeds 100)", e.getErrorMessage());
}

public void testLimitToPreventStackOverflowFromLargeUnaryArithmeticExpression() {
// Create expression in the form of abs(abs(abs ... (i) ...)

// 50 elements is ok
new SqlParser().createExpression(
Joiner.on("").join(nCopies(50, "abs(")).concat("i").concat(Joiner.on("").join(nCopies(50, ")"))));

// 101 elements parser's "circuit breaker" is triggered
ParsingException e = expectThrows(ParsingException.class, () -> new SqlParser().createExpression(
Joiner.on("").join(nCopies(101, "abs(")).concat("i").concat(Joiner.on("").join(nCopies(101, ")")))));
assertEquals("expression is too large to parse, (tree's depth exceeds 100)", e.getErrorMessage());
}

public void testLimitToPreventStackOverflowFromLargeBinaryArithmeticExpression() {
// Create expression in the form of a + a + a + ... + a

// 100 elements is ok
new SqlParser().createExpression(Joiner.on(" + ").join(nCopies(100, "a")));

// 101 elements parser's "circuit breaker" is triggered
ParsingException e = expectThrows(ParsingException.class, () ->
new SqlParser().createExpression(Joiner.on(" + ").join(nCopies(101, "a"))));
assertEquals("expression is too large to parse, (tree's depth exceeds 100)", e.getErrorMessage());
}

public void testLimitToPreventStackOverflowFromLargeSubselectTree() {
// Test with queries in the form of `SELECT * FROM (SELECT * FROM (... t) ...)

// 100 elements is ok
new SqlParser().createStatement(
Joiner.on(" (").join(nCopies(100, "SELECT * FROM"))
.concat("t")
.concat(Joiner.on("").join(nCopies(99, ")"))));

// 101 elements parser's "circuit breaker" is triggered
ParsingException e = expectThrows(ParsingException.class, () -> new SqlParser().createStatement(
Joiner.on(" (").join(nCopies(101, "SELECT * FROM"))
.concat("t")
.concat(Joiner.on("").join(nCopies(100, ")")))));
assertEquals("expression is too large to parse, (tree's depth exceeds 100)", e.getErrorMessage());
}

public void testLimitToPreventStackOverflowFromLargeComplexSubselectTree() {
// Test with queries in the form of `SELECT true OR true OR .. FROM (SELECT true OR true OR... FROM (... t) ...)

new SqlParser().createStatement(
Joiner.on(" (").join(nCopies(20, "SELECT ")).
concat(Joiner.on(" OR ").join(nCopies(50, "true"))).concat(" FROM")
.concat("t").concat(Joiner.on("").join(nCopies(19, ")"))));

ParsingException e = expectThrows(ParsingException.class, () -> new SqlParser().createStatement(
Joiner.on(" (").join(nCopies(20, "SELECT ")).
concat(Joiner.on(" OR ").join(nCopies(100, "true"))).concat(" FROM")
.concat("t").concat(Joiner.on("").join(nCopies(19, ")")))));
assertEquals("expression is too large to parse, (tree's depth exceeds 100)", e.getErrorMessage());
}

private LogicalPlan parseStatement(String sql) {
return new SqlParser().createStatement(sql);
}
Expand Down

0 comments on commit 6a4e841

Please sign in to comment.