Skip to content

Commit ef16be9

Browse files
authored
Catching StackOverflowErrors from bad regexes in GsubProcessor and rethrowing as an Exception (#106851)
1 parent 6ff3a26 commit ef16be9

File tree

3 files changed

+45
-1
lines changed

3 files changed

+45
-1
lines changed

docs/changelog/106851.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 106851
2+
summary: Catching `StackOverflowErrors` from bad regexes in `GsubProcessor`
3+
area: Ingest Node
4+
type: bug
5+
issues: []

modules/ingest-common/src/main/java/org/elasticsearch/ingest/common/GsubProcessor.java

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,9 @@
88

99
package org.elasticsearch.ingest.common;
1010

11+
import org.apache.logging.log4j.LogManager;
12+
import org.apache.logging.log4j.Logger;
13+
1114
import java.util.Map;
1215
import java.util.regex.Pattern;
1316

@@ -21,6 +24,7 @@
2124
public final class GsubProcessor extends AbstractStringProcessor<String> {
2225

2326
public static final String TYPE = "gsub";
27+
private static final Logger logger = LogManager.getLogger(GsubProcessor.class);
2428

2529
private final Pattern pattern;
2630
private final String replacement;
@@ -49,7 +53,19 @@ String getReplacement() {
4953

5054
@Override
5155
protected String process(String value) {
52-
return pattern.matcher(value).replaceAll(replacement);
56+
try {
57+
return pattern.matcher(value).replaceAll(replacement);
58+
} catch (StackOverflowError e) {
59+
/*
60+
* A bad regex on problematic data can trigger a StackOverflowError. In this case we can safely recover from the
61+
* StackOverflowError, so we rethrow it as an Exception instead. This way the document fails this processor, but processing
62+
* can carry on. The value would be useful to log here, but we do not do so for because we do not want to write potentially
63+
* sensitive data to the logs.
64+
*/
65+
String message = "Caught a StackOverflowError while processing gsub pattern: [" + pattern + "]";
66+
logger.trace(message, e);
67+
throw new IllegalArgumentException(message);
68+
}
5369
}
5470

5571
@Override

modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/GsubProcessorTests.java

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,10 @@
88

99
package org.elasticsearch.ingest.common;
1010

11+
import org.elasticsearch.ingest.IngestDocument;
12+
import org.elasticsearch.ingest.RandomDocumentPicks;
13+
14+
import java.util.Map;
1115
import java.util.regex.Pattern;
1216

1317
public class GsubProcessorTests extends AbstractStringProcessorTestCase<String> {
@@ -26,4 +30,23 @@ protected String modifyInput(String input) {
2630
protected String expectedResult(String input) {
2731
return "127-0-0-1";
2832
}
33+
34+
public void testStackOverflow() {
35+
// This tests that we rethrow StackOverflowErrors as ElasticsearchExceptions so that we don't take down the node
36+
String badRegex = "( (?=(?:[^'\"]|'[^']*'|\"[^\"]*\")*$))";
37+
GsubProcessor processor = new GsubProcessor(
38+
randomAlphaOfLength(10),
39+
null,
40+
"field",
41+
Pattern.compile(badRegex),
42+
"-",
43+
false,
44+
"targetField"
45+
);
46+
StringBuilder badSourceBuilder = new StringBuilder("key1=x key2=");
47+
badSourceBuilder.append("x".repeat(3000));
48+
Map<String, Object> source = Map.of("field", badSourceBuilder.toString());
49+
IngestDocument ingestDocument = RandomDocumentPicks.randomIngestDocument(random(), source);
50+
IllegalArgumentException exception = expectThrows(IllegalArgumentException.class, () -> processor.execute(ingestDocument));
51+
}
2952
}

0 commit comments

Comments
 (0)