Skip to content

Commit 6030d4b

Browse files
authored
[INGEST] Interrupt the current thread if evaluation grok expressions take too long (#31024)
This adds a thread interrupter that allows us to encapsulate calls to org.joni.Matcher#search() This method can hang forever if the regex expression is too complex. The thread interrupter in the background checks every 3 seconds whether there are threads execution the org.joni.Matcher#search() method for longer than 5 seconds and if so interrupts these threads. Joni has checks that that for every 30k iterations it checks if the current thread is interrupted and if so returns org.joni.Matcher#INTERRUPTED Closes #28731
1 parent 1dbe554 commit 6030d4b

File tree

11 files changed

+385
-53
lines changed

11 files changed

+385
-53
lines changed

docs/reference/ingest/ingest-node.asciidoc

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1512,6 +1512,24 @@ The above request will return a response body containing a key-value representat
15121512

15131513
This can be useful to reference as the built-in patterns change across versions.
15141514

1515+
[[grok-watchdog]]
1516+
==== Grok watchdog
1517+
1518+
Grok expressions that take too long to execute are interrupted and
1519+
the grok processor then fails with an exception. The grok
1520+
processor has a watchdog thread that determines when evaluation of
1521+
a grok expression takes too long and is controlled by the following
1522+
settings:
1523+
1524+
[[grok-watchdog-options]]
1525+
.Grok watchdog settings
1526+
[options="header"]
1527+
|======
1528+
| Name | Default | Description
1529+
| `ingest.grok.watchdog.interval` | 1s | How often to check whether there are grok evaluations that take longer than the maximum allowed execution time.
1530+
| `ingest.grok.watchdog.max_execution_time` | 1s | The maximum allowed execution of a grok expression evaluation.
1531+
|======
1532+
15151533
[[gsub-processor]]
15161534
=== Gsub Processor
15171535
Converts a string field by applying a regular expression and a replacement.

libs/grok/src/main/java/org/elasticsearch/grok/Grok.java

Lines changed: 41 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -76,15 +76,24 @@ public final class Grok {
7676
private final Map<String, String> patternBank;
7777
private final boolean namedCaptures;
7878
private final Regex compiledExpression;
79+
private final ThreadWatchdog threadWatchdog;
7980

8081
public Grok(Map<String, String> patternBank, String grokPattern) {
81-
this(patternBank, grokPattern, true);
82+
this(patternBank, grokPattern, true, ThreadWatchdog.noop());
8283
}
83-
84-
@SuppressWarnings("unchecked")
84+
85+
public Grok(Map<String, String> patternBank, String grokPattern, ThreadWatchdog threadWatchdog) {
86+
this(patternBank, grokPattern, true, threadWatchdog);
87+
}
88+
8589
Grok(Map<String, String> patternBank, String grokPattern, boolean namedCaptures) {
90+
this(patternBank, grokPattern, namedCaptures, ThreadWatchdog.noop());
91+
}
92+
93+
private Grok(Map<String, String> patternBank, String grokPattern, boolean namedCaptures, ThreadWatchdog threadWatchdog) {
8694
this.patternBank = patternBank;
8795
this.namedCaptures = namedCaptures;
96+
this.threadWatchdog = threadWatchdog;
8897

8998
for (Map.Entry<String, String> entry : patternBank.entrySet()) {
9099
String name = entry.getKey();
@@ -163,7 +172,13 @@ public String toRegex(String grokPattern) {
163172
byte[] grokPatternBytes = grokPattern.getBytes(StandardCharsets.UTF_8);
164173
Matcher matcher = GROK_PATTERN_REGEX.matcher(grokPatternBytes);
165174

166-
int result = matcher.search(0, grokPatternBytes.length, Option.NONE);
175+
int result;
176+
try {
177+
threadWatchdog.register();
178+
result = matcher.search(0, grokPatternBytes.length, Option.NONE);
179+
} finally {
180+
threadWatchdog.unregister();
181+
}
167182
if (result != -1) {
168183
Region region = matcher.getEagerRegion();
169184
String namedPatternRef = groupMatch(NAME_GROUP, region, grokPattern);
@@ -205,7 +220,13 @@ public String toRegex(String grokPattern) {
205220
*/
206221
public boolean match(String text) {
207222
Matcher matcher = compiledExpression.matcher(text.getBytes(StandardCharsets.UTF_8));
208-
int result = matcher.search(0, text.length(), Option.DEFAULT);
223+
int result;
224+
try {
225+
threadWatchdog.register();
226+
result = matcher.search(0, text.length(), Option.DEFAULT);
227+
} finally {
228+
threadWatchdog.unregister();
229+
}
209230
return (result != -1);
210231
}
211232

@@ -220,8 +241,20 @@ public Map<String, Object> captures(String text) {
220241
byte[] textAsBytes = text.getBytes(StandardCharsets.UTF_8);
221242
Map<String, Object> fields = new HashMap<>();
222243
Matcher matcher = compiledExpression.matcher(textAsBytes);
223-
int result = matcher.search(0, textAsBytes.length, Option.DEFAULT);
224-
if (result != -1 && compiledExpression.numberOfNames() > 0) {
244+
int result;
245+
try {
246+
threadWatchdog.register();
247+
result = matcher.search(0, textAsBytes.length, Option.DEFAULT);
248+
} finally {
249+
threadWatchdog.unregister();
250+
}
251+
if (result == Matcher.INTERRUPTED) {
252+
throw new RuntimeException("grok pattern matching was interrupted after [" +
253+
threadWatchdog.maxExecutionTimeInMillis() + "] ms");
254+
} else if (result == Matcher.FAILED) {
255+
// TODO: I think we should throw an error here?
256+
return null;
257+
} else if (compiledExpression.numberOfNames() > 0) {
225258
Region region = matcher.getEagerRegion();
226259
for (Iterator<NameEntry> entry = compiledExpression.namedBackrefIterator(); entry.hasNext();) {
227260
NameEntry e = entry.next();
@@ -235,13 +268,9 @@ public Map<String, Object> captures(String text) {
235268
break;
236269
}
237270
}
238-
239271
}
240-
return fields;
241-
} else if (result != -1) {
242-
return fields;
243272
}
244-
return null;
273+
return fields;
245274
}
246275

247276
public static Map<String, String> getBuiltinPatterns() {
Lines changed: 148 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,148 @@
1+
/*
2+
* Licensed to Elasticsearch under one or more contributor
3+
* license agreements. See the NOTICE file distributed with
4+
* this work for additional information regarding copyright
5+
* ownership. Elasticsearch licenses this file to you under
6+
* the Apache License, Version 2.0 (the "License"); you may
7+
* not use this file except in compliance with the License.
8+
* 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+
package org.elasticsearch.grok;
20+
21+
import java.util.Map;
22+
import java.util.concurrent.ConcurrentHashMap;
23+
import java.util.concurrent.ScheduledFuture;
24+
import java.util.function.BiFunction;
25+
import java.util.function.LongSupplier;
26+
27+
/**
28+
* Protects against long running operations that happen between the register and unregister invocations.
29+
* Threads that invoke {@link #register()}, but take too long to invoke the {@link #unregister()} method
30+
* will be interrupted.
31+
*
32+
* This is needed for Joni's {@link org.joni.Matcher#search(int, int, int)} method, because
33+
* it can end up spinning endlessly if the regular expression is too complex. Joni has checks
34+
* that for every 30k iterations it checks if the current thread is interrupted and if so
35+
* returns {@link org.joni.Matcher#INTERRUPTED}.
36+
*/
37+
public interface ThreadWatchdog {
38+
39+
/**
40+
* Registers the current thread and interrupts the current thread
41+
* if the takes too long for this thread to invoke {@link #unregister()}.
42+
*/
43+
void register();
44+
45+
/**
46+
* @return The maximum allowed time in milliseconds for a thread to invoke {@link #unregister()}
47+
* after {@link #register()} has been invoked before this ThreadWatchDog starts to interrupting that thread.
48+
*/
49+
long maxExecutionTimeInMillis();
50+
51+
/**
52+
* Unregisters the current thread and prevents it from being interrupted.
53+
*/
54+
void unregister();
55+
56+
/**
57+
* Returns an implementation that checks for each fixed interval if there are threads that have invoked {@link #register()}
58+
* and not {@link #unregister()} and have been in this state for longer than the specified max execution interval and
59+
* then interrupts these threads.
60+
*
61+
* @param interval The fixed interval to check if there are threads to interrupt
62+
* @param maxExecutionTime The time a thread has the execute an operation.
63+
* @param relativeTimeSupplier A supplier that returns relative time
64+
* @param scheduler A scheduler that is able to execute a command for each fixed interval
65+
*/
66+
static ThreadWatchdog newInstance(long interval,
67+
long maxExecutionTime,
68+
LongSupplier relativeTimeSupplier,
69+
BiFunction<Long, Runnable, ScheduledFuture<?>> scheduler) {
70+
return new Default(interval, maxExecutionTime, relativeTimeSupplier, scheduler);
71+
}
72+
73+
/**
74+
* @return A noop implementation that does not interrupt threads and is useful for testing and pre-defined grok expressions.
75+
*/
76+
static ThreadWatchdog noop() {
77+
return Noop.INSTANCE;
78+
}
79+
80+
class Noop implements ThreadWatchdog {
81+
82+
private static final Noop INSTANCE = new Noop();
83+
84+
private Noop() {
85+
}
86+
87+
@Override
88+
public void register() {
89+
}
90+
91+
@Override
92+
public long maxExecutionTimeInMillis() {
93+
return Long.MAX_VALUE;
94+
}
95+
96+
@Override
97+
public void unregister() {
98+
}
99+
}
100+
101+
class Default implements ThreadWatchdog {
102+
103+
private final long interval;
104+
private final long maxExecutionTime;
105+
private final LongSupplier relativeTimeSupplier;
106+
private final BiFunction<Long, Runnable, ScheduledFuture<?>> scheduler;
107+
final ConcurrentHashMap<Thread, Long> registry = new ConcurrentHashMap<>();
108+
109+
private Default(long interval,
110+
long maxExecutionTime,
111+
LongSupplier relativeTimeSupplier,
112+
BiFunction<Long, Runnable, ScheduledFuture<?>> scheduler) {
113+
this.interval = interval;
114+
this.maxExecutionTime = maxExecutionTime;
115+
this.relativeTimeSupplier = relativeTimeSupplier;
116+
this.scheduler = scheduler;
117+
scheduler.apply(interval, this::interruptLongRunningExecutions);
118+
}
119+
120+
public void register() {
121+
Long previousValue = registry.put(Thread.currentThread(), relativeTimeSupplier.getAsLong());
122+
assert previousValue == null;
123+
}
124+
125+
@Override
126+
public long maxExecutionTimeInMillis() {
127+
return maxExecutionTime;
128+
}
129+
130+
public void unregister() {
131+
Long previousValue = registry.remove(Thread.currentThread());
132+
assert previousValue != null;
133+
}
134+
135+
private void interruptLongRunningExecutions() {
136+
final long currentRelativeTime = relativeTimeSupplier.getAsLong();
137+
for (Map.Entry<Thread, Long> entry : registry.entrySet()) {
138+
if ((currentRelativeTime - entry.getValue()) > maxExecutionTime) {
139+
entry.getKey().interrupt();
140+
// not removing the entry here, this happens in the unregister() method.
141+
}
142+
}
143+
scheduler.apply(interval, this::interruptLongRunningExecutions);
144+
}
145+
146+
}
147+
148+
}

libs/grok/src/test/java/org/elasticsearch/grok/GrokTests.java

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -20,27 +20,24 @@
2020
package org.elasticsearch.grok;
2121

2222
import org.elasticsearch.test.ESTestCase;
23-
import org.junit.Before;
2423

2524
import java.util.Arrays;
2625
import java.util.Collections;
2726
import java.util.HashMap;
2827
import java.util.List;
2928
import java.util.Map;
3029
import java.util.TreeMap;
30+
import java.util.concurrent.ScheduledFuture;
31+
import java.util.concurrent.atomic.AtomicBoolean;
32+
import java.util.function.BiFunction;
3133

3234
import static org.hamcrest.Matchers.equalTo;
3335
import static org.hamcrest.Matchers.is;
3436
import static org.hamcrest.Matchers.nullValue;
3537

3638

3739
public class GrokTests extends ESTestCase {
38-
private Map<String, String> basePatterns;
39-
40-
@Before
41-
public void setup() {
42-
basePatterns = Grok.getBuiltinPatterns();
43-
}
40+
private static final Map<String, String> basePatterns = Grok.getBuiltinPatterns();
4441

4542
public void testMatchWithoutCaptures() {
4643
String line = "value";
@@ -415,4 +412,31 @@ public void testMultipleNamedCapturesWithSameName() {
415412
expected.put("num", "1");
416413
assertThat(grok.captures("12"), equalTo(expected));
417414
}
415+
416+
public void testExponentialExpressions() {
417+
AtomicBoolean run = new AtomicBoolean(true); // to avoid a lingering thread when test has completed
418+
419+
String grokPattern = "Bonsuche mit folgender Anfrage: Belegart->\\[%{WORD:param2},(?<param5>(\\s*%{NOTSPACE})*)\\] " +
420+
"Zustand->ABGESCHLOSSEN Kassennummer->%{WORD:param9} Bonnummer->%{WORD:param10} Datum->%{DATESTAMP_OTHER:param11}";
421+
String logLine = "Bonsuche mit folgender Anfrage: Belegart->[EINGESCHRAENKTER_VERKAUF, VERKAUF, NACHERFASSUNG] " +
422+
"Zustand->ABGESCHLOSSEN Kassennummer->2 Bonnummer->6362 Datum->Mon Jan 08 00:00:00 UTC 2018";
423+
BiFunction<Long, Runnable, ScheduledFuture<?>> scheduler = (delay, command) -> {
424+
try {
425+
Thread.sleep(delay);
426+
} catch (InterruptedException e) {
427+
throw new AssertionError(e);
428+
}
429+
Thread t = new Thread(() -> {
430+
if (run.get()) {
431+
command.run();
432+
}
433+
});
434+
t.start();
435+
return null;
436+
};
437+
Grok grok = new Grok(basePatterns, grokPattern, ThreadWatchdog.newInstance(10, 200, System::currentTimeMillis, scheduler));
438+
Exception e = expectThrows(RuntimeException.class, () -> grok.captures(logLine));
439+
run.set(false);
440+
assertThat(e.getMessage(), equalTo("grok pattern matching was interrupted after [200] ms"));
441+
}
418442
}

0 commit comments

Comments
 (0)