Skip to content

Commit fe0d0f3

Browse files
authored
Fix OOM in SQL Parser ErrorHandler due to infinite loop in ATN traversal (#17023)
* Fix OOM in SQL Parser ErrorHandler due to infinite loop in ATN traversal - Fix infinite recursion in by tracking visited states during ATN traversal. - Add to verify the fix and prevent regression. - This resolves the OOM issue caused by invalid SQL queries like .
1 parent 3567c24 commit fe0d0f3

File tree

3 files changed

+101
-9
lines changed

3 files changed

+101
-9
lines changed

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,3 +124,6 @@ iotdb-core/tsfile/src/main/antlr4/org/apache/tsfile/parser/gen/
124124
.mvn/.gradle-enterprise/
125125
.mvn/.develocity/
126126
.run/
127+
128+
# Relational Grammar ANTLR
129+
iotdb-core/relational-grammar/src/main/antlr4/org/apache/iotdb/db/relational/grammar/sql/.antlr/

iotdb-core/datanode/src/main/java/org/apache/iotdb/db/queryengine/plan/relational/sql/parser/ErrorHandler.java

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,8 @@ public void syntaxError(
8484

8585
Result result = analyze(e, parser);
8686

87-
// pick the candidate tokens associated largest token index processed (i.e., the path that
87+
// pick the candidate tokens associated largest token index processed (i.e., the
88+
// path that
8889
// consumed the most input)
8990
String expected = result.getExpected().stream().sorted().collect(Collectors.joining(", "));
9091

@@ -209,10 +210,13 @@ public Result process(ATNState currentState, int tokenIndex, RuleContext context
209210
RuleStartState startState = atn.ruleToStartState[currentState.ruleIndex];
210211

211212
if (isReachable(currentState, startState)) {
212-
// We've been dropped inside a rule in a state that's reachable via epsilon transitions.
213+
// We've been dropped inside a rule in a state that's reachable via epsilon
214+
// transitions.
213215
// This is,
214-
// effectively, equivalent to starting at the beginning (or immediately outside) the rule.
215-
// In that case, backtrack to the beginning to be able to take advantage of logic that
216+
// effectively, equivalent to starting at the beginning (or immediately outside)
217+
// the rule.
218+
// In that case, backtrack to the beginning to be able to take advantage of
219+
// logic that
216220
// remaps
217221
// some rules to well-known names for reporting purposes
218222
currentState = startState;
@@ -238,6 +242,8 @@ public Result process(ATNState currentState, int tokenIndex, RuleContext context
238242
private boolean isReachable(ATNState target, RuleStartState from) {
239243
Deque<ATNState> activeStates = new ArrayDeque<>();
240244
activeStates.add(from);
245+
Set<Integer> visited = new HashSet<>();
246+
visited.add(from.stateNumber);
241247

242248
while (!activeStates.isEmpty()) {
243249
ATNState current = activeStates.pop();
@@ -250,7 +256,10 @@ private boolean isReachable(ATNState target, RuleStartState from) {
250256
Transition transition = current.transition(i);
251257

252258
if (transition.isEpsilon()) {
253-
activeStates.push(transition.target);
259+
if (!visited.contains(transition.target.stateNumber)) {
260+
activeStates.push(transition.target);
261+
visited.add(transition.target.stateNumber);
262+
}
254263
}
255264
}
256265
}
@@ -336,13 +345,17 @@ private Set<Integer> process(ParsingState start, int precedence) {
336345
labels.complement(IntervalSet.of(Token.MIN_USER_TOKEN_TYPE, atn.maxTokenType));
337346
}
338347

339-
// Surprisingly, TokenStream (i.e. BufferedTokenStream) may not have loaded all the
348+
// Surprisingly, TokenStream (i.e. BufferedTokenStream) may not have loaded all
349+
// the
340350
// tokens from the
341-
// underlying stream. TokenStream.get() does not force tokens to be buffered -- it just
351+
// underlying stream. TokenStream.get() does not force tokens to be buffered --
352+
// it just
342353
// returns what's
343-
// in the current buffer, or fail with an IndexOutOfBoundsError. Since Antlr decided the
354+
// in the current buffer, or fail with an IndexOutOfBoundsError. Since Antlr
355+
// decided the
344356
// error occurred
345-
// within the current set of buffered tokens, stop when we reach the end of the buffer.
357+
// within the current set of buffered tokens, stop when we reach the end of the
358+
// buffer.
346359
if (labels.contains(currentToken) && tokenIndex < stream.size() - 1) {
347360
activeStates.push(new ParsingState(transition.target, tokenIndex + 1, false, parser));
348361
} else {
Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
/*
2+
* Licensed to the Apache Software Foundation (ASF) under one
3+
* or more contributor license agreements. See the NOTICE file
4+
* distributed with this work for additional information
5+
* regarding copyright ownership. The ASF licenses this file
6+
* to you under the Apache License, Version 2.0 (the
7+
* "License"); you may not use this file except in compliance
8+
* with the License. You may obtain a copy of the License at
9+
*
10+
* http://www.apache.org/licenses/LICENSE-2.0
11+
*
12+
* Unless required by applicable law or agreed to in writing,
13+
* software distributed under the License is distributed on an
14+
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
15+
* KIND, either express or implied. See the License for the
16+
* specific language governing permissions and limitations
17+
* under the License.
18+
*/
19+
20+
package org.apache.iotdb.db.queryengine.plan.relational.sql.ast;
21+
22+
import org.apache.iotdb.db.queryengine.plan.relational.sql.parser.ParsingException;
23+
import org.apache.iotdb.db.queryengine.plan.relational.sql.parser.SqlParser;
24+
25+
import org.junit.Test;
26+
27+
import java.time.ZoneId;
28+
import java.util.concurrent.ExecutionException;
29+
import java.util.concurrent.ExecutorService;
30+
import java.util.concurrent.Executors;
31+
import java.util.concurrent.Future;
32+
import java.util.concurrent.TimeUnit;
33+
import java.util.concurrent.TimeoutException;
34+
35+
import static org.junit.Assert.assertTrue;
36+
import static org.junit.Assert.fail;
37+
38+
public class SqlParserErrorHandlerTest {
39+
40+
@Test
41+
public void testParseLimitOffsetError() {
42+
String sql = "select * from information_schema.queries offset 48 limit1";
43+
SqlParser sqlParser = new SqlParser();
44+
ExecutorService executor = Executors.newSingleThreadExecutor();
45+
Future<?> future =
46+
executor.submit(
47+
() -> {
48+
try {
49+
sqlParser.createStatement(sql, ZoneId.systemDefault(), null);
50+
fail("Expected ParsingException to be thrown");
51+
} catch (ParsingException e) {
52+
assertTrue(e.getMessage().contains("mismatched input 'limit1'"));
53+
}
54+
});
55+
56+
try {
57+
// The parsing should fail quickly. If it hangs (OOM), this timeout will
58+
// trigger.
59+
future.get(5, TimeUnit.SECONDS);
60+
} catch (InterruptedException e) {
61+
fail("Interrupted");
62+
} catch (ExecutionException e) {
63+
// If parsing exception propagates here (which it shouldn't as it's caught in
64+
// the task), handle it
65+
if (e.getCause() instanceof ParsingException) {
66+
assertTrue(e.getCause().getMessage().contains("mismatched input 'limit1'"));
67+
} else {
68+
fail("Unexpected exception: " + e.getCause());
69+
}
70+
} catch (TimeoutException e) {
71+
fail("Parsing timed out - potential endless loop/OOM detected");
72+
} finally {
73+
executor.shutdownNow();
74+
}
75+
}
76+
}

0 commit comments

Comments
 (0)