Skip to content

Commit bc842da

Browse files
committed
SQL: Handle StackOverflowError when parsing large statements
Catch StackOverflowError exception and return a descriptive message to the client. This prevents large statement from killing the cluster. Fixes: #32942
1 parent b9e2b5c commit bc842da

File tree

2 files changed

+31
-7
lines changed

2 files changed

+31
-7
lines changed

x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/parser/SqlParser.java

Lines changed: 15 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -22,8 +22,8 @@
2222
import org.antlr.v4.runtime.dfa.DFA;
2323
import org.antlr.v4.runtime.misc.Pair;
2424
import org.antlr.v4.runtime.tree.TerminalNode;
25+
import org.apache.logging.log4j.LogManager;
2526
import org.apache.logging.log4j.Logger;
26-
import org.elasticsearch.common.logging.Loggers;
2727
import org.elasticsearch.xpack.sql.expression.Expression;
2828
import org.elasticsearch.xpack.sql.plan.logical.LogicalPlan;
2929
import org.elasticsearch.xpack.sql.proto.SqlTypedParamValue;
@@ -41,7 +41,8 @@
4141
import static java.lang.String.format;
4242

4343
public class SqlParser {
44-
private static final Logger log = Loggers.getLogger(SqlParser.class);
44+
45+
private static final Logger log = LogManager.getLogger();
4546

4647
private final boolean DEBUG = false;
4748

@@ -62,7 +63,7 @@ public LogicalPlan createStatement(String sql, List<SqlTypedParamValue> params)
6263
if (log.isDebugEnabled()) {
6364
log.debug("Parsing as statement: {}", sql);
6465
}
65-
return invokeParser(sql, params, SqlBaseParser::singleStatement, AstBuilder::plan);
66+
return invokeParser("statement", sql, params, SqlBaseParser::singleStatement, AstBuilder::plan);
6667
}
6768

6869
/**
@@ -80,10 +81,13 @@ public Expression createExpression(String expression, List<SqlTypedParamValue> p
8081
log.debug("Parsing as expression: {}", expression);
8182
}
8283

83-
return invokeParser(expression, params, SqlBaseParser::singleExpression, AstBuilder::expression);
84+
return invokeParser("expression", expression, params, SqlBaseParser::singleExpression, AstBuilder::expression);
8485
}
8586

86-
private <T> T invokeParser(String sql, List<SqlTypedParamValue> params, Function<SqlBaseParser, ParserRuleContext> parseFunction,
87+
private <T> T invokeParser(String name,
88+
String sql,
89+
List<SqlTypedParamValue> params, Function<SqlBaseParser,
90+
ParserRuleContext> parseFunction,
8791
BiFunction<AstBuilder, ParserRuleContext, T> visitor) {
8892
SqlBaseLexer lexer = new SqlBaseLexer(new CaseInsensitiveStream(sql));
8993

@@ -122,10 +126,14 @@ private <T> T invokeParser(String sql, List<SqlTypedParamValue> params, Function
122126
log.info("Parse tree {} " + tree.toStringTree());
123127
}
124128

125-
return visitor.apply(new AstBuilder(paramTokens), tree);
129+
try {
130+
return visitor.apply(new AstBuilder(paramTokens), tree);
131+
} catch (StackOverflowError e) {
132+
throw new ParsingException("{} is too large to parse (causes stack overflow)", name);
133+
}
126134
}
127135

128-
private void debug(SqlBaseParser parser) {
136+
private static void debug(SqlBaseParser parser) {
129137

130138
// when debugging, use the exact prediction mode (needed for diagnostics as well)
131139
parser.getInterpreter().setPredictionMode(PredictionMode.LL_EXACT_AMBIG_DETECTION);

x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/parser/SqlParserTests.java

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55
*/
66
package org.elasticsearch.xpack.sql.parser;
77

8+
import com.google.common.base.Joiner;
9+
import org.elasticsearch.ElasticsearchParseException;
810
import org.elasticsearch.test.ESTestCase;
911
import org.elasticsearch.xpack.sql.expression.NamedExpression;
1012
import org.elasticsearch.xpack.sql.expression.Order;
@@ -22,7 +24,9 @@
2224
import java.util.ArrayList;
2325
import java.util.List;
2426

27+
import static java.util.Collections.nCopies;
2528
import static java.util.stream.Collectors.toList;
29+
import static org.hamcrest.Matchers.equalTo;
2630
import static org.hamcrest.Matchers.hasEntry;
2731
import static org.hamcrest.Matchers.hasSize;
2832
import static org.hamcrest.Matchers.instanceOf;
@@ -136,6 +140,18 @@ public void testMultiMatchQuery() {
136140
assertThat(mmqp.optionMap(), hasEntry("fuzzy_rewrite", "scoring_boolean"));
137141
}
138142

143+
public void testStackOverflowStatement() {
144+
ParsingException e = expectThrows(ParsingException.class, () ->
145+
parseStatement("SELECT " + Joiner.on(" OR ").join(nCopies(2000, "a = b"))));
146+
assertEquals("statement is too large to parse (causes stack overflow)", e.getErrorMessage());
147+
}
148+
149+
public void testStackOverflowExpression() {
150+
ParsingException e = expectThrows(ParsingException.class, () ->
151+
new SqlParser().createExpression(Joiner.on(" OR ").join(nCopies(2000, "a = b"))));
152+
assertEquals("expression is too large to parse (causes stack overflow)", e.getErrorMessage());
153+
}
154+
139155
private LogicalPlan parseStatement(String sql) {
140156
return new SqlParser().createStatement(sql);
141157
}

0 commit comments

Comments
 (0)