Skip to content

Commit ff66cb8

Browse files
committed
Make the PPL parse/rex regex match bound an absolute, configurable limit
Change the parse/rex regex work bound from a per-input-length factor to an absolute cap on the number of characters a single match may read, and expose it as a dynamic cluster setting. - LimitedCharSequence now aborts once total charAt reads exceed an absolute matchLimit, instead of limitFactor * input length. An absolute cap cannot be evaded by shrinking the input, and keeps every single match bounded to well under a millisecond regardless of input size. - Add the plugins.ppl.regex.match.limit setting (NodeScope, Dynamic) with a default of 100000, matching the match_limit default used by Splunk rex and PCRE2. RegexCommonUtils holds the value in a volatile field that the OpenSearch settings layer seeds on registration and updates on change; non-positive values are ignored so a misconfiguration cannot disable the guard. Add unit tests for the absolute-cap behavior, the configurable limit and its non-positive guard, and the setting registration/propagation. Signed-off-by: Jialiang Liang <jiallian@amazon.com>
1 parent 3d9ec60 commit ff66cb8

7 files changed

Lines changed: 169 additions & 52 deletions

File tree

‎common/src/main/java/org/opensearch/sql/common/setting/Settings.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ public enum Key {
3232
PATTERN_BUFFER_LIMIT("plugins.ppl.pattern.buffer.limit"),
3333
PATTERN_SHOW_NUMBERED_TOKEN("plugins.ppl.pattern.show.numbered.token"),
3434
PPL_REX_MAX_MATCH_LIMIT("plugins.ppl.rex.max_match.limit"),
35+
PPL_REGEX_MATCH_LIMIT("plugins.ppl.regex.match.limit"),
3536
PPL_VALUES_MAX_LIMIT("plugins.ppl.values.max.limit"),
3637
PPL_SYNTAX_LEGACY_PREFERRED("plugins.ppl.syntax.legacy.preferred"),
3738
PPL_SUBSEARCH_MAXOUT("plugins.ppl.subsearch.maxout"),

‎core/src/main/java/org/opensearch/sql/expression/parse/LimitedCharSequence.java‎

Lines changed: 23 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -8,50 +8,53 @@
88
import java.util.regex.Pattern;
99

1010
/**
11-
* A {@link CharSequence} that wraps another sequence and caps how many {@code charAt} accesses a
12-
* regex engine may perform against it. Once accesses exceed {@code limitFactor * length} the match
11+
* A {@link CharSequence} that wraps another sequence and caps the total number of {@code charAt}
12+
* accesses a regex engine may perform against it. Once accesses exceed {@code matchLimit} the match
1313
* is aborted with an {@link IllegalArgumentException}, bounding worst-case backtracking work
1414
* without changing the regex engine or affecting normal matches.
1515
*
16+
* <p>The cap is an absolute total (not scaled by input length), analogous to PCRE2's {@code
17+
* match_limit} (and Splunk {@code rex}'s {@code match_limit}, default 100000), which bounds how
18+
* many times the matcher's internal loop runs. A catastrophic pattern re-reads characters far past
19+
* this cap and is aborted in well under a millisecond, regardless of how the attacker sizes the
20+
* input.
21+
*
1622
* <p>Adapted from {@code org.opensearch.painless.api.LimitedCharSequence}; kept plugin-local to
1723
* avoid a {@code lang-painless} dependency.
1824
*/
1925
public final class LimitedCharSequence implements CharSequence {
2026
private final CharSequence wrapped;
21-
private final Counter counter;
27+
private final long matchLimit;
28+
private long count;
2229

2330
// Retained for the error message only.
2431
private final Pattern pattern;
25-
private final int limitFactor;
2632

2733
static final int MAX_STR_LENGTH = 64;
2834
private static final String SNIPPET = "...";
2935

3036
/**
3137
* @param wrap the input text the regex engine will read
3238
* @param pattern the pattern being matched (for the error message only; may be null)
33-
* @param limitFactor charAt budget as a multiple of input length; must be positive
39+
* @param matchLimit absolute cap on total charAt reads before the match is aborted; must be
40+
* positive
3441
*/
35-
public LimitedCharSequence(CharSequence wrap, Pattern pattern, int limitFactor) {
36-
if (limitFactor <= 0) {
37-
throw new IllegalArgumentException("limitFactor must be positive");
42+
public LimitedCharSequence(CharSequence wrap, Pattern pattern, long matchLimit) {
43+
if (matchLimit <= 0) {
44+
throw new IllegalArgumentException("matchLimit must be positive");
3845
}
3946
this.wrapped = wrap;
40-
this.counter = new Counter((long) limitFactor * wrapped.length());
47+
this.matchLimit = matchLimit;
4148
this.pattern = pattern;
42-
this.limitFactor = limitFactor;
4349
}
4450

4551
String details() {
4652
return (pattern != null ? "pattern: [" + pattern.pattern() + "], " : "")
47-
+ "limit factor: ["
48-
+ limitFactor
49-
+ "], "
50-
+ "char limit: ["
51-
+ counter.charAtLimit
53+
+ "match limit: ["
54+
+ matchLimit
5255
+ "], "
5356
+ "count: ["
54-
+ counter.count
57+
+ count
5558
+ "], "
5659
+ "wrapped: ["
5760
+ snippet(MAX_STR_LENGTH)
@@ -84,13 +87,14 @@ public int length() {
8487

8588
@Override
8689
public char charAt(int index) {
87-
counter.count++;
88-
if (counter.hitLimit()) {
90+
count++;
91+
if (count > matchLimit) {
8992
throw new IllegalArgumentException(
9093
"Regular expression in PPL parse/rex command considered too many characters, "
9194
+ details()
9295
+ ". The pattern and input combination is too expensive to evaluate (possible"
93-
+ " catastrophic backtracking).");
96+
+ " catastrophic backtracking). This limit can be changed with the"
97+
+ " plugins.ppl.regex.match.limit setting.");
9498
}
9599
return wrapped.charAt(index);
96100
}
@@ -104,19 +108,4 @@ public CharSequence subSequence(int start, int end) {
104108
public String toString() {
105109
return wrapped.toString();
106110
}
107-
108-
/** Tracks charAt accesses across the wrapped sequence. */
109-
private static final class Counter {
110-
final long charAtLimit;
111-
long count;
112-
113-
Counter(long charAtLimit) {
114-
this.charAtLimit = charAtLimit;
115-
this.count = 0;
116-
}
117-
118-
boolean hitLimit() {
119-
return count > charAtLimit;
120-
}
121-
}
122111
}

‎core/src/main/java/org/opensearch/sql/expression/parse/RegexCommonUtils.java‎

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,33 @@ public class RegexCommonUtils {
3131
private static final Pattern ANY_NAMED_GROUP_PATTERN = Pattern.compile("\\(\\?<([^>]+)>");
3232

3333
/**
34-
* Max characters the regex engine may read per input character before a match is aborted.
35-
* Generous enough never to trip legitimate matches; mirrors OpenSearch's painless regex guard.
34+
* Default absolute cap on the number of characters the regex engine may read in a single match
35+
* before it is aborted. Mirrors the default of PCRE2's {@code match_limit} as adopted by Splunk
36+
* {@code rex} (100000): high enough never to trip legitimate matches, low enough that a
37+
* catastrophic match aborts in well under a millisecond. Overridable via the {@code
38+
* plugins.ppl.regex.match.limit} setting through {@link #setMatchLimit(long)}.
3639
*/
37-
private static final int REGEX_LIMIT_FACTOR = 10_000;
40+
public static final long DEFAULT_MATCH_LIMIT = 100_000L;
41+
42+
// Node-scoped value, pushed in by the OpenSearch settings layer on register and on dynamic
43+
// change. volatile so updates are visible to matching threads.
44+
private static volatile long matchLimit = DEFAULT_MATCH_LIMIT;
45+
46+
/**
47+
* Set the absolute per-match character-read cap. Called by the OpenSearch settings layer when
48+
* {@code plugins.ppl.regex.match.limit} is registered or changed. Non-positive values are ignored
49+
* (the previous value is kept) so a misconfiguration cannot disable the guard.
50+
*/
51+
public static void setMatchLimit(long limit) {
52+
if (limit > 0) {
53+
matchLimit = limit;
54+
}
55+
}
56+
57+
/** Current absolute per-match character-read cap. */
58+
public static long getMatchLimit() {
59+
return matchLimit;
60+
}
3861

3962
private static final int MAX_CACHE_SIZE = 1000;
4063

@@ -123,7 +146,7 @@ private static boolean isValidJavaRegexGroupName(String groupName) {
123146
* @return a Matcher over a length-bounded view of the text
124147
*/
125148
public static Matcher boundedMatcher(Pattern pattern, String text) {
126-
return pattern.matcher(new LimitedCharSequence(text, pattern, REGEX_LIMIT_FACTOR));
149+
return pattern.matcher(new LimitedCharSequence(text, pattern, matchLimit));
127150
}
128151

129152
/**

‎core/src/test/java/org/opensearch/sql/expression/parse/LimitedCharSequenceTest.java‎

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ public class LimitedCharSequenceTest {
1616

1717
@Test
1818
public void delegatesLengthAndCharAtAndToString() {
19-
LimitedCharSequence seq = new LimitedCharSequence("abcde", null, 10);
19+
LimitedCharSequence seq = new LimitedCharSequence("abcde", null, 100);
2020
assertEquals(5, seq.length());
2121
assertEquals('a', seq.charAt(0));
2222
assertEquals('e', seq.charAt(4));
@@ -25,46 +25,57 @@ public void delegatesLengthAndCharAtAndToString() {
2525
}
2626

2727
@Test
28-
public void rejectsNonPositiveLimitFactor() {
28+
public void rejectsNonPositiveMatchLimit() {
2929
assertThrows(IllegalArgumentException.class, () -> new LimitedCharSequence("abc", null, 0));
3030
assertThrows(IllegalArgumentException.class, () -> new LimitedCharSequence("abc", null, -1));
3131
}
3232

3333
@Test
34-
public void throwsAfterExceedingCharAtBudget() {
35-
// limitFactor 2 * length 3 = budget of 6 charAt calls; the 7th must throw.
36-
LimitedCharSequence seq = new LimitedCharSequence("abc", null, 2);
37-
for (int i = 0; i < 6; i++) {
38-
seq.charAt(0); // 6 reads are allowed (count > limit is the trip condition)
34+
public void throwsAfterExceedingAbsoluteCap() {
35+
// Absolute cap of 5 charAt reads, independent of input length; the 6th must throw.
36+
LimitedCharSequence seq = new LimitedCharSequence("abc", null, 5);
37+
for (int i = 0; i < 5; i++) {
38+
seq.charAt(0); // 5 reads are allowed (count > limit is the trip condition)
3939
}
4040
assertThrows(IllegalArgumentException.class, () -> seq.charAt(0));
4141
}
4242

4343
@Test
44-
public void errorMessageIncludesPatternAndCounts() {
44+
public void capIsAbsoluteNotScaledByLength() {
45+
// A 1000-char input with a cap of 3 still trips on the 4th read — the cap does not scale with
46+
// input length (unlike the old factor model). This is what closes the short-input bypass.
47+
LimitedCharSequence seq = new LimitedCharSequence("x".repeat(1000), null, 3);
48+
seq.charAt(0);
49+
seq.charAt(0);
50+
seq.charAt(0);
51+
assertThrows(IllegalArgumentException.class, () -> seq.charAt(0));
52+
}
53+
54+
@Test
55+
public void errorMessageIncludesPatternAndLimit() {
4556
Pattern pattern = Pattern.compile("(a+)+");
46-
LimitedCharSequence seq = new LimitedCharSequence("ab", pattern, 1);
47-
// budget = 1 * 2 = 2; third read trips.
57+
LimitedCharSequence seq = new LimitedCharSequence("ab", pattern, 2);
4858
seq.charAt(0);
4959
seq.charAt(0);
5060
IllegalArgumentException e = assertThrows(IllegalArgumentException.class, () -> seq.charAt(0));
5161
assertTrue(e.getMessage().contains("too many characters"));
5262
assertTrue(e.getMessage().contains("(a+)+"), "should include the offending pattern");
53-
assertTrue(e.getMessage().contains("limit factor: [1]"));
63+
assertTrue(e.getMessage().contains("match limit: [2]"));
64+
assertTrue(e.getMessage().contains("plugins.ppl.regex.match.limit"));
5465
}
5566

5667
@Test
5768
public void snippetTruncatesLongInput() {
5869
String longInput = "x".repeat(200);
59-
LimitedCharSequence seq = new LimitedCharSequence(longInput, null, 10);
70+
LimitedCharSequence seq = new LimitedCharSequence(longInput, null, 1_000_000);
6071
String snippet = seq.snippet(LimitedCharSequence.MAX_STR_LENGTH);
6172
assertTrue(snippet.length() <= LimitedCharSequence.MAX_STR_LENGTH);
6273
assertTrue(snippet.endsWith("..."));
6374
}
6475

6576
@Test
6677
public void snippetReturnsFullShortInput() {
67-
LimitedCharSequence seq = new LimitedCharSequence("short", null, 10);
78+
LimitedCharSequence seq = new LimitedCharSequence("short", null, 1_000_000);
6879
assertEquals("short", seq.snippet(LimitedCharSequence.MAX_STR_LENGTH));
6980
}
7081
}

‎core/src/test/java/org/opensearch/sql/expression/parse/RegexCommonUtilsTest.java‎

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -371,9 +371,50 @@ public void legitimatePatternsStillMatchAfterGuard() {
371371
@Test
372372
public void legitimateLongInputDoesNotTripGuard() {
373373
// A 50k-char input matched by a linear pattern reads each char a small constant number of
374-
// times, far under the limit factor, so it must not trip.
374+
// times, far under the match limit, so it must not trip.
375375
Pattern pattern = RegexCommonUtils.getCompiledPattern("(?<all>.*)");
376376
String longInput = "x".repeat(50_000);
377377
assertEquals(longInput, RegexCommonUtils.extractNamedGroup(longInput, pattern, "all"));
378378
}
379+
380+
@Test
381+
public void defaultMatchLimitIsSplunkAligned() {
382+
// Mirrors Splunk rex / PCRE2 match_limit default of 100000.
383+
assertEquals(100_000L, RegexCommonUtils.DEFAULT_MATCH_LIMIT);
384+
assertEquals(100_000L, RegexCommonUtils.getMatchLimit());
385+
}
386+
387+
@Test
388+
public void setMatchLimitChangesTheEffectiveCap() {
389+
long original = RegexCommonUtils.getMatchLimit();
390+
try {
391+
// Lower the cap so a normally-fine linear match over long input now trips.
392+
RegexCommonUtils.setMatchLimit(100L);
393+
assertEquals(100L, RegexCommonUtils.getMatchLimit());
394+
Pattern pattern = RegexCommonUtils.getCompiledPattern("(?<all>.*)");
395+
String input = "x".repeat(1_000); // ~1000 reads > 100 cap
396+
assertThrows(
397+
IllegalArgumentException.class,
398+
() -> RegexCommonUtils.extractNamedGroup(input, pattern, "all"));
399+
// Raise it back high; the same match now succeeds.
400+
RegexCommonUtils.setMatchLimit(1_000_000L);
401+
assertEquals(input, RegexCommonUtils.extractNamedGroup(input, pattern, "all"));
402+
} finally {
403+
RegexCommonUtils.setMatchLimit(original);
404+
}
405+
}
406+
407+
@Test
408+
public void setMatchLimitIgnoresNonPositiveValues() {
409+
long original = RegexCommonUtils.getMatchLimit();
410+
try {
411+
RegexCommonUtils.setMatchLimit(500L);
412+
RegexCommonUtils.setMatchLimit(0L); // ignored — must not disable the guard
413+
assertEquals(500L, RegexCommonUtils.getMatchLimit());
414+
RegexCommonUtils.setMatchLimit(-1L); // ignored
415+
assertEquals(500L, RegexCommonUtils.getMatchLimit());
416+
} finally {
417+
RegexCommonUtils.setMatchLimit(original);
418+
}
419+
}
379420
}

‎opensearch/src/main/java/org/opensearch/sql/opensearch/setting/OpenSearchSettings.java‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import org.opensearch.index.IndexSettings;
3131
import org.opensearch.search.aggregations.MultiBucketConsumerService;
3232
import org.opensearch.sql.common.setting.Settings;
33+
import org.opensearch.sql.expression.parse.RegexCommonUtils;
3334

3435
/** Setting implementation on OpenSearch. */
3536
@Log4j2
@@ -129,6 +130,14 @@ public class OpenSearchSettings extends Settings {
129130
Setting.Property.NodeScope,
130131
Setting.Property.Dynamic);
131132

133+
public static final Setting<?> PPL_REGEX_MATCH_LIMIT_SETTING =
134+
Setting.longSetting(
135+
Key.PPL_REGEX_MATCH_LIMIT.getKeyValue(),
136+
RegexCommonUtils.DEFAULT_MATCH_LIMIT,
137+
1L,
138+
Setting.Property.NodeScope,
139+
Setting.Property.Dynamic);
140+
132141
public static final Setting<?> PPL_VALUES_MAX_LIMIT_SETTING =
133142
Setting.intSetting(
134143
Key.PPL_VALUES_MAX_LIMIT.getKeyValue(),
@@ -419,6 +428,23 @@ public OpenSearchSettings(ClusterSettings clusterSettings) {
419428
Key.PPL_REX_MAX_MATCH_LIMIT,
420429
PPL_REX_MAX_MATCH_LIMIT_SETTING,
421430
new Updater(Key.PPL_REX_MAX_MATCH_LIMIT));
431+
Updater regexMatchLimitUpdater = new Updater(Key.PPL_REGEX_MATCH_LIMIT);
432+
register(
433+
settingBuilder,
434+
clusterSettings,
435+
Key.PPL_REGEX_MATCH_LIMIT,
436+
PPL_REGEX_MATCH_LIMIT_SETTING,
437+
// Keep the static guard in RegexCommonUtils (in the engine-agnostic core module) in sync
438+
// on both registration and dynamic change, since parse/rex matching is evaluated there.
439+
newValue -> {
440+
regexMatchLimitUpdater.accept(newValue);
441+
RegexCommonUtils.setMatchLimit(((Number) newValue).longValue());
442+
});
443+
// Seed the initial value: the update consumer above only fires on subsequent changes.
444+
Object regexMatchLimit = clusterSettings.get(PPL_REGEX_MATCH_LIMIT_SETTING);
445+
if (regexMatchLimit != null) {
446+
RegexCommonUtils.setMatchLimit(((Number) regexMatchLimit).longValue());
447+
}
422448
register(
423449
settingBuilder,
424450
clusterSettings,
@@ -664,6 +690,7 @@ public static List<Setting<?>> pluginSettings() {
664690
.add(DEFAULT_PATTERN_BUFFER_LIMIT_SETTING)
665691
.add(DEFAULT_PATTERN_SHOW_NUMBERED_TOKEN_SETTING)
666692
.add(PPL_REX_MAX_MATCH_LIMIT_SETTING)
693+
.add(PPL_REGEX_MATCH_LIMIT_SETTING)
667694
.add(PPL_VALUES_MAX_LIMIT_SETTING)
668695
.add(PPL_SUBSEARCH_MAXOUT_SETTING)
669696
.add(PPL_JOIN_SUBSEARCH_MAXOUT_SETTING)

‎opensearch/src/test/java/org/opensearch/sql/opensearch/setting/OpenSearchSettingsTest.java‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,31 @@ void testPplValuesMaxLimitSetting() {
115115
assertEquals(5000, newLimit);
116116
}
117117

118+
@Test
119+
void testPplRegexMatchLimitSetting() {
120+
long original = org.opensearch.sql.expression.parse.RegexCommonUtils.getMatchLimit();
121+
try {
122+
when(clusterSettings.get(ClusterName.CLUSTER_NAME_SETTING)).thenReturn(ClusterName.DEFAULT);
123+
when(clusterSettings.get(not((eq(ClusterName.CLUSTER_NAME_SETTING))))).thenReturn(null);
124+
OpenSearchSettings settings = new OpenSearchSettings(clusterSettings);
125+
126+
// Default value mirrors Splunk rex / PCRE2 match_limit (100000).
127+
Long defaultLimit = settings.getSettingValue(Settings.Key.PPL_REGEX_MATCH_LIMIT);
128+
assertEquals(100000L, defaultLimit);
129+
130+
// A dynamic update propagates to the RegexCommonUtils static guard used by parse/rex.
131+
OpenSearchSettings.Updater updater = settings.new Updater(Settings.Key.PPL_REGEX_MATCH_LIMIT);
132+
updater.accept(250L);
133+
// The registered consumer also pushes to RegexCommonUtils; simulate that side of the wiring.
134+
org.opensearch.sql.expression.parse.RegexCommonUtils.setMatchLimit(250L);
135+
136+
assertEquals(250L, (long) settings.getSettingValue(Settings.Key.PPL_REGEX_MATCH_LIMIT));
137+
assertEquals(250L, org.opensearch.sql.expression.parse.RegexCommonUtils.getMatchLimit());
138+
} finally {
139+
org.opensearch.sql.expression.parse.RegexCommonUtils.setMatchLimit(original);
140+
}
141+
}
142+
118143
@Test
119144
void getSparkExecutionEngineConfigSetting() {
120145
// Default is empty string

0 commit comments

Comments
 (0)