matcher: fix quantifiers that follow a {n,m} repeat

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:
+   *
+   * <pre>
+   * (a{2,}){2}  => a{4,}
+   * (a+){2,3}   => a{2,}
+   * (a+)?b      => a*b
+   * (a*)*b      => a*b
+   * </pre>
+   */
+  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<Matcher> ms = matcher.<SeqMatcher>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<Matcher> ms = matcher.<SeqMatcher>as().matchers();
+      return prefixCount(ms.get(0), ms.get(1).<ZeroOrMoreMatcher>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.
    */

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,30 +318,68 @@ 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));
     --current;
     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);
     }

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.<OrMatcher>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.<SeqMatcher>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;
     }

spectator-api/src/main/java/com/netflix/spectator/impl/matcher/RepeatMatcher.java

@@ -96,12 +96,14 @@ public int minLength() {
 
   @Override
   public Matcher rewrite(Function<Matcher, Matcher> f) {
-    return new RepeatMatcher(f.apply(repeated), min, max);
+    return f.apply(new RepeatMatcher(repeated.rewrite(f), min, max));
   }
 
   @Override
   public Matcher rewriteEnd(Function<Matcher, Matcher> 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