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);