diff --git a/spectator-api/src/main/java/com/netflix/spectator/impl/matcher/Optimizer.java b/spectator-api/src/main/java/com/netflix/spectator/impl/matcher/Optimizer.java
index 4b2742e2f..c758aeaa7 100644
--- a/spectator-api/src/main/java/com/netflix/spectator/impl/matcher/Optimizer.java
+++ b/spectator-api/src/main/java/com/netflix/spectator/impl/matcher/Optimizer.java
@@ -182,6 +182,7 @@ private static Matcher optimizeSinglePass(Matcher matcher) {
.rewrite(Optimizer::removeRepeatedStart)
.rewrite(Optimizer::combineAdjacentStart)
.rewrite(Optimizer::convertRepeatedAnyCharSeqToIndexOf)
+ .rewrite(Optimizer::flattenRepeatedUnbounded)
.rewriteEnd(Optimizer::removeTrailingMatchAny);
}
@@ -670,6 +671,95 @@ static Matcher convertRepeatedAnyCharSeqToIndexOf(Matcher matcher) {
return matcher;
}
+ /**
+ * Repetition of an unbounded repetition can be simplified to a single unbounded
+ * repetition. Since only the yes or no answer for a match is needed, this does not change
+ * the result, but allows the trailing zero or more to backtrack. Examples:
+ *
+ *
+ * (a{2,}){2} => a{4,}
+ * (a+){2,3} => a{2,}
+ * (a+)?b => a*b
+ * (a*)*b => a*b
+ *
+ */
+ static Matcher flattenRepeatedUnbounded(Matcher matcher) {
+ if (matcher instanceof RepeatMatcher) {
+ RepeatMatcher m = matcher.as();
+ Matcher x = unboundedRepeated(m.repeated());
+ if (x != null && m.min() > 0) {
+ long min = (long) unboundedMin(m.repeated()) * m.min();
+ return unbounded(x, (int) Math.min(min, Integer.MAX_VALUE), TrueMatcher.INSTANCE);
+ }
+ } else if (matcher instanceof ZeroOrOneMatcher) {
+ ZeroOrOneMatcher m = matcher.as();
+ Matcher x = unboundedRepeated(m.repeated());
+ if (x != null && unboundedMin(m.repeated()) <= 1) {
+ return new ZeroOrMoreMatcher(x, m.next());
+ }
+ } else if (matcher instanceof ZeroOrMoreMatcher) {
+ ZeroOrMoreMatcher m = matcher.as();
+ Matcher x = unboundedRepeated(m.repeated());
+ if (x != null && unboundedMin(m.repeated()) <= 1) {
+ return new ZeroOrMoreMatcher(x, m.next());
+ }
+ }
+ return matcher;
+ }
+
+ /**
+ * If the matcher is an unbounded repetition, {@code x{n,}}, with nothing following it,
+ * then return the repeated matcher {@code x}. Otherwise return null.
+ */
+ private static Matcher unboundedRepeated(Matcher matcher) {
+ if (matcher instanceof ZeroOrMoreMatcher) {
+ ZeroOrMoreMatcher zm = matcher.as();
+ return zm.next() instanceof TrueMatcher ? zm.repeated() : null;
+ } else if (matcher instanceof SeqMatcher) {
+ List ms = matcher.as().matchers();
+ if (ms.size() == 2) {
+ Matcher x = unboundedRepeated(ms.get(1));
+ if (x != null && ms.get(1) instanceof ZeroOrMoreMatcher && prefixCount(ms.get(0), x) > 0) {
+ return x;
+ }
+ }
+ }
+ return null;
+ }
+
+ /** Returns the minimum number of repetitions for an unbounded repetition. */
+ private static int unboundedMin(Matcher matcher) {
+ if (matcher instanceof SeqMatcher) {
+ List ms = matcher.as().matchers();
+ return prefixCount(ms.get(0), ms.get(1).as().repeated());
+ }
+ return 0;
+ }
+
+ /**
+ * Returns the number of times {@code x} is repeated by the prefix matcher, or -1 if it is
+ * not a fixed repetition of {@code x}.
+ */
+ private static int prefixCount(Matcher prefix, Matcher x) {
+ if (prefix.equals(x)) {
+ return 1;
+ } else if (prefix instanceof RepeatMatcher) {
+ RepeatMatcher r = prefix.as();
+ return r.min() == r.max() && r.repeated().equals(x) ? r.min() : -1;
+ }
+ return -1;
+ }
+
+ /** Create a matcher for an unbounded repetition, {@code x{min,}}. */
+ private static Matcher unbounded(Matcher x, int min, Matcher next) {
+ Matcher zeroOrMore = new ZeroOrMoreMatcher(x, next);
+ if (min == 0) {
+ return zeroOrMore;
+ }
+ Matcher prefix = (min == 1) ? x : new RepeatMatcher(x, min, min);
+ return SeqMatcher.create(prefix, zeroOrMore);
+ }
+
/**
* Zero or more start anchors is the same as not being anchored by the start.
*/
diff --git a/spectator-api/src/main/java/com/netflix/spectator/impl/matcher/Parser.java b/spectator-api/src/main/java/com/netflix/spectator/impl/matcher/Parser.java
index 44a6b3267..c6a5cf5da 100644
--- a/spectator-api/src/main/java/com/netflix/spectator/impl/matcher/Parser.java
+++ b/spectator-api/src/main/java/com/netflix/spectator/impl/matcher/Parser.java
@@ -87,6 +87,11 @@ private Matcher term() {
matchers.add(group());
break;
case '{':
+ // Can be empty if following another quantifier that consumed the rest of the
+ // term, e.g. a*{3} or a{2,}{3}.
+ if (matchers.isEmpty()) {
+ throw error("dangling modifier");
+ }
matchers.add(repeat(pop(matchers)));
break;
case '.':
@@ -313,18 +318,61 @@ private AsciiSet newNamedCharSet(String name, boolean invert) {
private Matcher repeat(Matcher matcher) {
int start = current;
advance(c -> c != '}');
- String[] numbers = tokens.subSequence(start, current - 1).toString().split(",");
+ if (previous() != '}') {
+ throw error("unclosed repetition");
+ }
+ // Use a negative limit so the trailing empty string for "{n,}" is preserved
+ String[] numbers = tokens.subSequence(start, current - 1).toString().split(",", -1);
+ if (numbers.length > 2) {
+ throw error("illegal repetition");
+ }
int min = Integer.parseInt(numbers[0]);
- int max = (numbers.length > 1) ? Integer.parseInt(numbers[1]) : min;
+ boolean unbounded = numbers.length == 2 && numbers[1].isEmpty();
+ int max;
+ if (numbers.length == 1) {
+ max = min;
+ } else if (unbounded) {
+ max = Integer.MAX_VALUE;
+ } else {
+ max = Integer.parseInt(numbers[1]);
+ }
if (min < 0) {
throw error("illegal repetition, min < 0");
}
if (min > max) {
throw error("illegal repetition, min > max");
}
+
+ boolean possessive = repeatModifier();
+ if (unbounded && !possessive) {
+ // No upper bound, treat as min repetitions followed by zero or more. This is
+ // consistent with how * and + are handled.
+ Matcher zeroOrMore = new ZeroOrMoreMatcher(matcher, term());
+ if (min == 0) {
+ return zeroOrMore;
+ }
+ Matcher prefix = (min == 1) ? matcher : new RepeatMatcher(matcher, min, min);
+ return SeqMatcher.create(prefix, zeroOrMore);
+ }
+
+ // RepeatMatcher is greedy without backtracking, so it also covers the possessive case
return new RepeatMatcher(matcher, min, max);
}
+ /**
+ * Consume the optional possessive (+) or reluctant (?) modifier for a repetition and
+ * return true if it is possessive. Reluctant matching behavior is the same as greedy,
+ * consistent with *? and +?. Modifiers are consumed here rather than in meta() because
+ * meta() cannot tell a modifier, a{2}?, apart from a quantifier on a group that ends
+ * with a repeat, (a{2})?.
+ */
+ private boolean repeatModifier() {
+ if (!isAtEnd() && (peek() == '+' || peek() == '?')) {
+ return advance() == '+';
+ }
+ return false;
+ }
+
private Matcher meta(Matcher matcher) {
int start = current - 1;
advance(c -> META.contains((char) c));
@@ -332,11 +380,6 @@ private Matcher meta(Matcher matcher) {
String quantifier = tokens.subSequence(start, current).toString();
switch (quantifier) {
case "?":
- // Makes repeat reluctant
- if (matcher instanceof RepeatMatcher) {
- return matcher;
- }
- // fall through
case "??":
case "?+":
return new ZeroOrOneMatcher(matcher, term());
@@ -349,7 +392,7 @@ private Matcher meta(Matcher matcher) {
case "+?":
return SeqMatcher.create(matcher, new ZeroOrMoreMatcher(matcher, term()));
case "++":
- return SeqMatcher.create(matcher, new RepeatMatcher(matcher, 1, Integer.MAX_VALUE));
+ return SeqMatcher.create(matcher, new RepeatMatcher(matcher, 0, Integer.MAX_VALUE));
default:
throw new IllegalArgumentException("unknown quantifier: " + quantifier);
}
diff --git a/spectator-api/src/main/java/com/netflix/spectator/impl/matcher/PatternUtils.java b/spectator-api/src/main/java/com/netflix/spectator/impl/matcher/PatternUtils.java
index 74dcf0133..e2cbfd2f4 100644
--- a/spectator-api/src/main/java/com/netflix/spectator/impl/matcher/PatternUtils.java
+++ b/spectator-api/src/main/java/com/netflix/spectator/impl/matcher/PatternUtils.java
@@ -418,7 +418,7 @@ private static void rewriteNextLookahead(Matcher matcher, Matcher[] results) {
}
} else if (matcher instanceof ZeroOrMoreMatcher) {
ZeroOrMoreMatcher m = matcher.as();
- if (containsLookahead(m.repeated())) {
+ if (containsUnsupported(m.repeated())) {
return;
}
removeNextLookahead(m.next(), results);
@@ -429,7 +429,7 @@ private static void rewriteNextLookahead(Matcher matcher, Matcher[] results) {
}
} else if (matcher instanceof ZeroOrOneMatcher) {
ZeroOrOneMatcher m = matcher.as();
- if (containsLookahead(m.repeated())) {
+ if (containsUnsupported(m.repeated())) {
return;
}
removeNextLookahead(m.next(), results);
@@ -440,28 +440,26 @@ private static void rewriteNextLookahead(Matcher matcher, Matcher[] results) {
}
} else if (matcher instanceof RepeatMatcher) {
RepeatMatcher m = matcher.as();
- if (m.max() > 1000 || containsLookahead(m.repeated())) {
- // Some engines like RE2 have limitations on the number of repetitions. Treat
- // those as failures to match to the expression.
+ if (containsUnsupported(m)) {
return;
} else {
results[0] = matcher;
}
} else if (matcher instanceof PositiveLookaheadMatcher) {
PositiveLookaheadMatcher m = matcher.as();
- if (containsLookahead(m.matcher())) {
+ if (containsUnsupported(m.matcher())) {
return;
}
results[0] = m.matcher();
results[2] = TrueMatcher.INSTANCE;
} else if (matcher instanceof NegativeLookaheadMatcher) {
NegativeLookaheadMatcher m = matcher.as();
- if (containsLookahead(m.matcher())) {
+ if (containsUnsupported(m.matcher())) {
return;
}
results[1] = m.matcher();
results[2] = TrueMatcher.INSTANCE;
- } else if (!containsLookahead(matcher)) {
+ } else if (!containsUnsupported(matcher)) {
results[0] = matcher;
}
@@ -472,37 +470,42 @@ private static void rewriteNextLookahead(Matcher matcher, Matcher[] results) {
}
}
- private static boolean containsLookahead(Matcher matcher) {
+ /**
+ * Returns true if the matcher contains a lookahead or a repetition with a large upper bound
+ * anywhere in the tree. Some engines like RE2 have limitations on the number of repetitions,
+ * treat those as failures to match to the expression.
+ */
+ private static boolean containsUnsupported(Matcher matcher) {
if (matcher instanceof NegativeLookaheadMatcher) {
return true;
} else if (matcher instanceof PositiveLookaheadMatcher) {
return true;
} else if (matcher instanceof IndexOfMatcher) {
IndexOfMatcher m = matcher.as();
- return containsLookahead(m.next());
+ return containsUnsupported(m.next());
} else if (matcher instanceof OrMatcher) {
for (Matcher m : matcher.as().matchers()) {
- if (containsLookahead(m)) {
+ if (containsUnsupported(m)) {
return true;
}
}
return false;
} else if (matcher instanceof RepeatMatcher) {
RepeatMatcher m = matcher.as();
- return containsLookahead(m.repeated());
+ return m.max() > 1000 || containsUnsupported(m.repeated());
} else if (matcher instanceof SeqMatcher) {
for (Matcher m : matcher.as().matchers()) {
- if (containsLookahead(m)) {
+ if (containsUnsupported(m)) {
return true;
}
}
return false;
} else if (matcher instanceof ZeroOrMoreMatcher) {
ZeroOrMoreMatcher m = matcher.as();
- return containsLookahead(m.repeated()) || containsLookahead(m.next());
+ return containsUnsupported(m.repeated()) || containsUnsupported(m.next());
} else if (matcher instanceof ZeroOrOneMatcher) {
ZeroOrOneMatcher m = matcher.as();
- return containsLookahead(m.repeated()) || containsLookahead(m.next());
+ return containsUnsupported(m.repeated()) || containsUnsupported(m.next());
} else {
return false;
}
diff --git a/spectator-api/src/main/java/com/netflix/spectator/impl/matcher/RepeatMatcher.java b/spectator-api/src/main/java/com/netflix/spectator/impl/matcher/RepeatMatcher.java
index 9a0d00f98..2e9787261 100644
--- a/spectator-api/src/main/java/com/netflix/spectator/impl/matcher/RepeatMatcher.java
+++ b/spectator-api/src/main/java/com/netflix/spectator/impl/matcher/RepeatMatcher.java
@@ -96,12 +96,14 @@ public int minLength() {
@Override
public Matcher rewrite(Function f) {
- return new RepeatMatcher(f.apply(repeated), min, max);
+ return f.apply(new RepeatMatcher(repeated.rewrite(f), min, max));
}
@Override
public Matcher rewriteEnd(Function f) {
- return new RepeatMatcher(f.apply(repeated), min, max);
+ // The repeated portion is not in the final position unless it is the last repetition,
+ // so only apply to this matcher.
+ return f.apply(this);
}
@Override
diff --git a/spectator-api/src/test/java/com/netflix/spectator/impl/matcher/AbstractPatternMatcherTest.java b/spectator-api/src/test/java/com/netflix/spectator/impl/matcher/AbstractPatternMatcherTest.java
index 556e264c4..3d813e79a 100644
--- a/spectator-api/src/test/java/com/netflix/spectator/impl/matcher/AbstractPatternMatcherTest.java
+++ b/spectator-api/src/test/java/com/netflix/spectator/impl/matcher/AbstractPatternMatcherTest.java
@@ -610,6 +610,97 @@ public void repeatZero() {
testRE("(abc){1,3}def", "abcdef");
}
+ @Test
+ public void repeatNoUpperBound() {
+ testRE("^a{2,}$", "a");
+ testRE("^a{2,}$", "aa");
+ testRE("^a{2,}$", "aaaaa");
+ testRE("xa{2,}b", "xab");
+ testRE("xa{2,}b", "xaaab");
+ testRE("^(ab){2,}$", "ababab");
+ testRE("^a{0,}$", "");
+ testRE("^a{0,}$", "aaa");
+ testRE("^a{2,}a$", "aa");
+ testRE("^a{2,}a$", "aaa");
+ testRE("(a*)(b{0,1})(b{1,})b{3}", "aaabbbbbbb");
+ testRE("x(a{2,})?y", "xy");
+ testRE("x(a{2,})?y", "xay");
+ testRE("x(a{2,})?y", "xaaay");
+ }
+
+ @Test
+ public void repeatNoUpperBoundModifiers() {
+ testRE("^a{2,}?$", "aaaa");
+ testRE("^a{2,}?a$", "aaa");
+ testRE("^a{2,}?b", "ab");
+ testRE("^\\d{2,}+$", "1111");
+ testRE("^a{2,}+$", "a");
+ testRE("^a{2,}+a$", "aaa");
+ testRE("x(a{2,}+)?y", "xy");
+ testRE("x(a{2,}+)?y", "xaay");
+ }
+
+ @Test
+ public void repeatOfUnbounded() {
+ // Repetition of an unbounded repetition is flattened so the result is consistent
+ testRE("^(a{2,}){2}$", "aaa");
+ testRE("^(a{2,}){2}$", "aaaa");
+ testRE("^(a{2,}){2}a$", "aaaaa");
+ testRE("^(a+){2}$", "a");
+ testRE("^(a+){2}$", "aa");
+ testRE("^(a+){2,3}$", "aaaa");
+ testRE("(a+){2}b", "xaab");
+ testRE("^(a+)?ab$", "aab");
+ testRE("^(?:a*)?ab$", "aab");
+ testRE("^(a+)*ab$", "aab");
+ testRE("^(a{2,}){0,2}$", "aaaa");
+ }
+
+ @Test
+ public void repeatIgnoreCase() {
+ testRE("(?i)(ab|cd){2,}", "ABCD");
+ testRE("(?i)(ab|cd){2}", "abCD");
+ testRE("(?i)(ab|cd){2}", "ABxCD");
+ }
+
+ @Test
+ public void repeatOneNoUpperBound() {
+ testRE("^ab{1,}c$", "ac");
+ testRE("^ab{1,}c$", "abc");
+ testRE("^ab{1,}c$", "abbbc");
+ Assertions.assertEquals("abc", PatternMatcher.compile("^abc{1,}").prefix());
+ }
+
+ @Test
+ public void optionalGroupWithRepeat() {
+ testRE("x(a{2})?y", "xy");
+ testRE("x(a{2})?y", "xaay");
+ testRE("x(?:a{2,3})?y", "xy");
+ testRE("x(?:a{2,3})?y", "xaaay");
+
+ // Large repeat count nested inside another matcher
+ testRE("x(a{0,2000}b)?y", "xaby");
+ testRE("x(a*+)?y", "xy");
+ }
+
+ @Test
+ public void oneOrMorePossessive() {
+ testRE("^a++$", "");
+ testRE("^a++$", "a");
+ testRE("^a++$", "aaa");
+ testRE("^a++b", "ab");
+ }
+
+ @Test
+ public void repeatMalformed() {
+ testBadExpression("a{2,3", "unclosed repetition");
+ testBadExpression("a{2,3,4}", "illegal repetition");
+ testBadExpression("a{2,,}", "illegal repetition");
+ testBadExpression("{2}", "dangling modifier");
+ testBadExpression("a*{3}", "dangling modifier");
+ testBadExpression("a{2,}{3}", "dangling modifier");
+ }
+
@Test
public void repeatEmptyClause() {
// Repetition whose body can match the empty string, via the {n,m} RepeatMatcher path
@@ -623,6 +714,27 @@ public void repeatEmptyClause() {
testRE("(abc|){2,4}def", "abcdef");
}
+ @Test
+ public void repeatInOptionalGroup() {
+ // The '?' applies to the group, it is not a reluctant modifier for the inner repetition
+ testRE("^v(\\d{2})?$", "v");
+ testRE("^v(\\d{2})?$", "v12");
+ testRE("^v(\\d{2})?$", "v1");
+ testRE("^ab(?:c{1,2})?d", "abd");
+ testRE("^ab(?:c{1,2})?d", "abccd");
+ }
+
+ @Test
+ public void repeatModifiers() {
+ testRE("^a{2}?$", "aa");
+ testRE("^a{1,2}?b", "aab");
+ testRE("^a{2}+$", "aa");
+ testRE("^a{2}+$", "aaaa");
+ testRE("^(?:ab){2}+$", "abababab");
+ testRE("^a{2,3}+a$", "aaaa");
+ testRE("^a{2,3}?$", "aaa");
+ }
+
@Test
public void repeatWithOr() {
testRE("(a|b|c){1,3}d", "aaad");
diff --git a/spectator-api/src/test/java/com/netflix/spectator/impl/matcher/ExpandOrTest.java b/spectator-api/src/test/java/com/netflix/spectator/impl/matcher/ExpandOrTest.java
index cee7f3d71..47306d3ec 100644
--- a/spectator-api/src/test/java/com/netflix/spectator/impl/matcher/ExpandOrTest.java
+++ b/spectator-api/src/test/java/com/netflix/spectator/impl/matcher/ExpandOrTest.java
@@ -58,7 +58,7 @@ public void expandPositiveLookaheadMatcher() {
@Test
public void expandRepeatMatcher() {
// OR within repeat cannot be expanded without changing the interpretation.
- Assertions.assertEquals(list("(.)*(?:((ABC)|(abc))){2,5}"), expand("(ABC|abc){2,5}"));
+ Assertions.assertEquals(list("(.)*(?:(ABC|abc)){2,5}"), expand("(ABC|abc){2,5}"));
}
@Test
diff --git a/spectator-api/src/test/java/com/netflix/spectator/impl/matcher/OptimizerTest.java b/spectator-api/src/test/java/com/netflix/spectator/impl/matcher/OptimizerTest.java
index 5cba64bb8..89b0eeca0 100644
--- a/spectator-api/src/test/java/com/netflix/spectator/impl/matcher/OptimizerTest.java
+++ b/spectator-api/src/test/java/com/netflix/spectator/impl/matcher/OptimizerTest.java
@@ -377,6 +377,56 @@ public void orMergeNext() {
Assertions.assertEquals(expected, Optimizer.mergeNext(input));
}
+ @Test
+ public void flattenRepeatedUnbounded() {
+ // (a{2,}){3} => a{6,}
+ Matcher a = new CharSeqMatcher("a");
+ Matcher input = new RepeatMatcher(
+ SeqMatcher.create(
+ new RepeatMatcher(a, 2, 2),
+ new ZeroOrMoreMatcher(a, TrueMatcher.INSTANCE)),
+ 3, 3);
+ Matcher expected = SeqMatcher.create(
+ new RepeatMatcher(a, 6, 6),
+ new ZeroOrMoreMatcher(a, TrueMatcher.INSTANCE));
+ Assertions.assertEquals(expected, Optimizer.flattenRepeatedUnbounded(input));
+ }
+
+ @Test
+ public void flattenRepeatedUnbounded_OneOrMore() {
+ // (a+){2} => a{2,}
+ Matcher a = new CharSeqMatcher("a");
+ Matcher input = new RepeatMatcher(
+ SeqMatcher.create(a, new ZeroOrMoreMatcher(a, TrueMatcher.INSTANCE)), 2, 2);
+ Matcher expected = SeqMatcher.create(
+ new RepeatMatcher(a, 2, 2),
+ new ZeroOrMoreMatcher(a, TrueMatcher.INSTANCE));
+ Assertions.assertEquals(expected, Optimizer.flattenRepeatedUnbounded(input));
+ }
+
+ @Test
+ public void flattenRepeatedUnbounded_Optional() {
+ // (a+)?b => a*b
+ Matcher a = new CharSeqMatcher("a");
+ Matcher b = new CharSeqMatcher("b");
+ Matcher input = new ZeroOrOneMatcher(
+ SeqMatcher.create(a, new ZeroOrMoreMatcher(a, TrueMatcher.INSTANCE)), b);
+ Matcher expected = new ZeroOrMoreMatcher(a, b);
+ Assertions.assertEquals(expected, Optimizer.flattenRepeatedUnbounded(input));
+ }
+
+ @Test
+ public void flattenRepeatedUnbounded_OptionalMinTwo() {
+ // (a{2,})?b cannot be simplified
+ Matcher a = new CharSeqMatcher("a");
+ Matcher input = new ZeroOrOneMatcher(
+ SeqMatcher.create(
+ new RepeatMatcher(a, 2, 2),
+ new ZeroOrMoreMatcher(a, TrueMatcher.INSTANCE)),
+ new CharSeqMatcher("b"));
+ Assertions.assertEquals(input, Optimizer.flattenRepeatedUnbounded(input));
+ }
+
@Test
public void removeRepeatedStart() {
Matcher input = new ZeroOrMoreMatcher(StartMatcher.INSTANCE, AnyMatcher.INSTANCE);