Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Wrap "Pattern too complex" exception into an IllegalArgumentException #109173

Merged
merged 3 commits into from
May 31, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions docs/changelog/109173.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 109173
summary: Wrap "Pattern too complex" exception into an `IllegalArgumentException`
kderusso marked this conversation as resolved.
Show resolved Hide resolved
area: Mapping
type: bug
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,32 @@
- match: { tokens.0.token: sha }
- match: { tokens.1.token: hay }

---
"Too complex regex pattern":
- do:
catch: bad_request
indices.create:
index: test_too_complex_regex_pattern
body:
settings:
index:
analysis:
analyzer:
Copy link
Contributor

@saikatsarkar056 saikatsarkar056 May 29, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is the rectangle shaped char at the end of each line? Can we remove them?

Screenshot 2024-05-29 at 9 57 38 AM

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GH does not render it well - but this is an actual case where we know Pattern::compile throws an OutOfMemoryError so I think it is fine to keep it. Might be worth adding a comment here for folks that might read this code in the future? @afoucret

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I moved this test to a more appropriate place and added a comment to it

my_analyzer:
tokenizer: standard
char_filter:
- my_char_filter
char_filter:
my_char_filter:
type: "pattern_replace"
pattern: "(\\d+)-(?=\\d\nͭͭͭͭͭͭͭͭͭͭͭͭͭͭͭ"
flags: CASE_INSENSITIVE|MULTILINE|DOTALL|UNICODE_CASE|CANON_EQ
replacement: "_$1"
- match: { status: 400 }
- match: { error.type: illegal_argument_exception }
- match: { error.reason: "Too complex regex pattern" }


---
"Custom normalizer in request":
- do:
Expand Down
22 changes: 20 additions & 2 deletions server/src/main/java/org/elasticsearch/common/regex/Regex.java
Original file line number Diff line number Diff line change
Expand Up @@ -230,8 +230,26 @@ public static boolean simpleMatch(final List<String> patterns, final String str)
}

public static Pattern compile(String regex, String flags) {
int pFlags = flags == null ? 0 : flagsFromString(flags);
return Pattern.compile(regex, pFlags);
try {
int pFlags = flags == null ? 0 : flagsFromString(flags);
return Pattern.compile(regex, pFlags);
} catch (OutOfMemoryError e) {
if (e.getMessage().equals("Pattern too complex")) {
// Normally, we do try to handle OutOfMemoryError errors, as they typically indicate the JVM is not healthy.
//
// In the context of Pattern::compile, an OutOfMemoryError can occur if the pattern is too complex.
// In this case, the OutOfMemoryError is thrown by a pre-check rather than actual memory exhaustion.
//
// Because the JVM has not encountered a real memory issue, we can treat this as a recoverable exception by wrapping
// the original OutOfMemoryError in an IllegalArgumentException.
//
// For additional details, see:
// - https://bugs.openjdk.org/browse/JDK-8300207
// - https://github.com/openjdk/jdk/commit/030b071db1fb6197a2633a04b20aa95432a903bc
throw new IllegalArgumentException("Too complex regex pattern", e);
}
throw e;
}
}

public static int flagsFromString(String flags) {
Expand Down