From 9d93fb8ca0ba5df072ddee87c059e3623c650629 Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Tue, 7 Jul 2026 13:14:23 +0200 Subject: [PATCH 1/4] fix: PINNED_BACKREFERENCE bugs from PR #94 review Require group/backref pair to span the whole pattern, enforce group/separator length bounds in generated bytecode, and fix matchBounded to report absolute (not substring-relative) spans. Patterns with anchors outside the span now correctly fall through to VARIABLE_CAPTURE_BACKREF. --- .../codegen/analysis/PatternAnalyzer.java | 36 +++- .../analysis/PinnedBackreferenceInfo.java | 25 ++- .../PinnedBackreferenceBytecodeGenerator.java | 178 ++++++++++++++--- .../analysis/PatternRoutingPropertyTest.java | 13 +- .../PinnedBackreferenceDetectionTest.java | 10 +- .../PinnedBackreferenceEquivalenceTest.java | 189 +++--------------- .../analysis/PinnedBackreferenceInfoTest.java | 9 +- .../StrategySelectionExtendedTest.java | 25 ++- ...nedBackreferenceBytecodeGeneratorTest.java | 7 +- .../runtime/BackrefDigitAmbiguityTest.java | 27 ++- .../PinnedBackreferenceMatchResultTest.java | 11 +- 11 files changed, 318 insertions(+), 212 deletions(-) diff --git a/reggie-codegen/src/main/java/com/datadoghq/reggie/codegen/analysis/PatternAnalyzer.java b/reggie-codegen/src/main/java/com/datadoghq/reggie/codegen/analysis/PatternAnalyzer.java index 77bdfe7e..dca6d857 100644 --- a/reggie-codegen/src/main/java/com/datadoghq/reggie/codegen/analysis/PatternAnalyzer.java +++ b/reggie-codegen/src/main/java/com/datadoghq/reggie/codegen/analysis/PatternAnalyzer.java @@ -3575,6 +3575,14 @@ private PinnedBackreferenceInfo detectPinnedBackreference(RegexNode ast) { return null; } + // The generated matcher only ever scans the group, the optional separator, and the + // backreference echo - it has no code path for anything else in the concatenation. A + // non-empty prefix/suffix (e.g. the \b anchors in \b(\w+)\s+\1\b) would therefore be silently + // ignored rather than evaluated, so require the group/backref pair to span the whole pattern. + if (groupIndex != 0 || backrefIndex != children.size() - 1) { + return null; + } + // The codegen forward-scans the group's content as a single flat "one-or-more chars from // groupCharSet" loop with no notion of an upper bound, so only an unbounded (max == -1), // greedy, min >= 1 quantifier directly wrapping a charset-bearing child is safe - this @@ -3605,6 +3613,8 @@ private PinnedBackreferenceInfo detectPinnedBackreference(RegexNode ast) { // earlier occurrence of the same backreference) is rejected rather than approximated. RegexNode separator = null; CharSet separatorCharSet = null; + int separatorMinLength = 0; + int separatorMaxLength = -1; if (backrefIndex > groupIndex + 1) { List sepNodes = children.subList(groupIndex + 1, backrefIndex); if (sepNodes.size() != 1) { @@ -3613,6 +3623,27 @@ private PinnedBackreferenceInfo detectPinnedBackreference(RegexNode ast) { separator = sepNodes.get(0); separatorCharSet = getFirstCharSet(separator); + // The scan can't backtrack, so it always consumes the longest run of separator-charset + // chars available; that's only correct if the separator's quantifier bounds either equal + // that run (min <= run) or don't cap it below that run (max == -1 or max >= run). Track the + // bounds here so codegen can reject a candidate whose actual run falls outside them, instead + // of accepting whatever length the greedy scan happens to consume. + if (separator instanceof QuantifierNode) { + QuantifierNode sepQuant = (QuantifierNode) separator; + if (!sepQuant.greedy) { + return null; + } + separatorMinLength = sepQuant.min; + separatorMaxLength = sepQuant.max; + } else { + // No quantifier wrapping the separator node means exactly one occurrence. + separatorMinLength = 1; + separatorMaxLength = 1; + } + if (separatorMinLength < 1) { + return null; + } + // Disjointness condition 2: separator vs. group content. if (separatorCharSet == null || !groupCharSet.isDisjoint(separatorCharSet)) { return null; @@ -3623,8 +3654,11 @@ private PinnedBackreferenceInfo detectPinnedBackreference(RegexNode ast) { capturingGroup.groupNumber, capturingGroup.child, groupCharSet, + groupQuant.min, separator, - separatorCharSet); + separatorCharSet, + separatorMinLength, + separatorMaxLength); } /** Check if a node represents a single character or character class. */ diff --git a/reggie-codegen/src/main/java/com/datadoghq/reggie/codegen/analysis/PinnedBackreferenceInfo.java b/reggie-codegen/src/main/java/com/datadoghq/reggie/codegen/analysis/PinnedBackreferenceInfo.java index 12da5970..7c469d75 100644 --- a/reggie-codegen/src/main/java/com/datadoghq/reggie/codegen/analysis/PinnedBackreferenceInfo.java +++ b/reggie-codegen/src/main/java/com/datadoghq/reggie/codegen/analysis/PinnedBackreferenceInfo.java @@ -42,6 +42,9 @@ public class PinnedBackreferenceInfo implements PatternInfo { /** Character set the group's content matches. */ public final CharSet groupCharSet; + /** Minimum number of chars the group's quantifier requires (e.g. 2 for {@code \w{2,}}). */ + public final int groupMinLength; + /** * Node(s) between the group's close and the backreference site, or null if the backreference * directly follows the group. @@ -51,17 +54,34 @@ public class PinnedBackreferenceInfo implements PatternInfo { /** Character set of the separator, or null if there is no separator. */ public final CharSet separatorCharSet; + /** + * Minimum number of chars the separator's quantifier requires. Unused if there's no separator. + */ + public final int separatorMinLength; + + /** + * Maximum number of chars the separator's quantifier allows, or -1 if unbounded. Unused if + * there's no separator. + */ + public final int separatorMaxLength; + public PinnedBackreferenceInfo( int groupIndex, RegexNode groupBody, CharSet groupCharSet, + int groupMinLength, RegexNode separator, - CharSet separatorCharSet) { + CharSet separatorCharSet, + int separatorMinLength, + int separatorMaxLength) { this.groupIndex = groupIndex; this.groupBody = Objects.requireNonNull(groupBody); this.groupCharSet = Objects.requireNonNull(groupCharSet); + this.groupMinLength = groupMinLength; this.separator = separator; this.separatorCharSet = separatorCharSet; + this.separatorMinLength = separatorMinLength; + this.separatorMaxLength = separatorMaxLength; } /** Whether a separator exists between the group's close and the backreference site. */ @@ -75,8 +95,11 @@ public int structuralHashCode() { hash = 31 * hash + groupIndex; hash = 31 * hash + groupBody.getClass().getName().hashCode(); hash = 31 * hash + groupCharSet.hashCode(); + hash = 31 * hash + groupMinLength; hash = 31 * hash + (separator != null ? separator.getClass().getName().hashCode() : 0); hash = 31 * hash + (separatorCharSet != null ? separatorCharSet.hashCode() : 0); + hash = 31 * hash + separatorMinLength; + hash = 31 * hash + separatorMaxLength; return hash; } diff --git a/reggie-codegen/src/main/java/com/datadoghq/reggie/codegen/codegen/PinnedBackreferenceBytecodeGenerator.java b/reggie-codegen/src/main/java/com/datadoghq/reggie/codegen/codegen/PinnedBackreferenceBytecodeGenerator.java index 121b6bb9..e91c29e7 100644 --- a/reggie-codegen/src/main/java/com/datadoghq/reggie/codegen/codegen/PinnedBackreferenceBytecodeGenerator.java +++ b/reggie-codegen/src/main/java/com/datadoghq/reggie/codegen/codegen/PinnedBackreferenceBytecodeGenerator.java @@ -37,7 +37,7 @@ *

Supported Pattern Shape

* * * *

Modeled directly on {@link FixedRepetitionBackrefBytecodeGenerator} (single forward @@ -75,6 +75,7 @@ public void generateMatchesMethod(ClassWriter cw) { int groupLenVar = alloc.allocate(); int scanCharVar = alloc.allocate(); int sepStartVar = info.hasSeparator() ? alloc.allocate() : -1; + int sepLenVar = info.hasSeparator() ? alloc.allocate() : -1; Label returnFalse = new Label(); @@ -101,16 +102,13 @@ public void generateMatchesMethod(ClassWriter cw) { mv.visitVarInsn(ILOAD, groupStartVar); mv.visitInsn(ISUB); mv.visitVarInsn(ISTORE, groupLenVar); - mv.visitVarInsn(ILOAD, groupLenVar); - mv.visitJumpInsn(IFLE, returnFalse); + generateGroupLengthCheck(mv, groupLenVar, returnFalse); if (info.hasSeparator()) { mv.visitVarInsn(ILOAD, posVar); mv.visitVarInsn(ISTORE, sepStartVar); generateCharSetScan(mv, posVar, lenVar, scanCharVar, info.separatorCharSet); - mv.visitVarInsn(ILOAD, posVar); - mv.visitVarInsn(ILOAD, sepStartVar); - mv.visitJumpInsn(IF_ICMPEQ, returnFalse); + generateSeparatorLengthCheck(mv, posVar, sepStartVar, sepLenVar, returnFalse); } generateBackrefCheck(mv, posVar, lenVar, groupStartVar, groupLenVar, returnFalse); @@ -175,6 +173,7 @@ public void generateFindFromMethod(ClassWriter cw) { int groupLenVar = alloc.allocate(); int scanCharVar = alloc.allocate(); int sepStartVar = info.hasSeparator() ? alloc.allocate() : -1; + int sepLenVar = info.hasSeparator() ? alloc.allocate() : -1; Label notNull = new Label(); mv.visitVarInsn(ALOAD, 1); @@ -187,8 +186,15 @@ public void generateFindFromMethod(ClassWriter cw) { mv.visitMethodInsn(INVOKEVIRTUAL, "java/lang/String", "length", "()I", false); mv.visitVarInsn(ISTORE, lenVar); + // int start = Math.max(0, startPos); mv.visitVarInsn(ILOAD, 2); mv.visitVarInsn(ISTORE, startVar); + Label startNotNeg = new Label(); + mv.visitVarInsn(ILOAD, startVar); + mv.visitJumpInsn(IFGE, startNotNeg); + mv.visitInsn(ICONST_0); + mv.visitVarInsn(ISTORE, startVar); + mv.visitLabel(startNotNeg); Label outerLoop = new Label(); Label outerEnd = new Label(); @@ -214,16 +220,13 @@ public void generateFindFromMethod(ClassWriter cw) { mv.visitVarInsn(ILOAD, groupStartVar); mv.visitInsn(ISUB); mv.visitVarInsn(ISTORE, groupLenVar); - mv.visitVarInsn(ILOAD, groupLenVar); - mv.visitJumpInsn(IFLE, tryNextStart); + generateGroupLengthCheck(mv, groupLenVar, tryNextStart); if (info.hasSeparator()) { mv.visitVarInsn(ILOAD, posVar); mv.visitVarInsn(ISTORE, sepStartVar); generateCharSetScan(mv, posVar, lenVar, scanCharVar, info.separatorCharSet); - mv.visitVarInsn(ILOAD, posVar); - mv.visitVarInsn(ILOAD, sepStartVar); - mv.visitJumpInsn(IF_ICMPEQ, tryNextStart); + generateSeparatorLengthCheck(mv, posVar, sepStartVar, sepLenVar, tryNextStart); } generateBackrefCheck(mv, posVar, lenVar, groupStartVar, groupLenVar, tryNextStart); @@ -262,6 +265,7 @@ public void generateMatchMethod(ClassWriter cw) { int groupLenVar = alloc.allocate(); int scanCharVar = alloc.allocate(); int sepStartVar = info.hasSeparator() ? alloc.allocate() : -1; + int sepLenVar = info.hasSeparator() ? alloc.allocate() : -1; int startsArrayVar = alloc.allocate(); int endsArrayVar = alloc.allocate(); @@ -306,13 +310,12 @@ public void generateMatchMethod(ClassWriter cw) { generateCharSetScan(mv, posVar, lenVar, scanCharVar, info.groupCharSet); - // groupLen = pos - groupStart; require non-empty. + // groupLen = pos - groupStart; require it meets the quantifier's minimum. mv.visitVarInsn(ILOAD, posVar); mv.visitVarInsn(ILOAD, groupStartVar); mv.visitInsn(ISUB); mv.visitVarInsn(ISTORE, groupLenVar); - mv.visitVarInsn(ILOAD, groupLenVar); - mv.visitJumpInsn(IFLE, returnNull); + generateGroupLengthCheck(mv, groupLenVar, returnNull); // ends[groupIndex] = pos mv.visitVarInsn(ALOAD, endsArrayVar); @@ -324,9 +327,7 @@ public void generateMatchMethod(ClassWriter cw) { mv.visitVarInsn(ILOAD, posVar); mv.visitVarInsn(ISTORE, sepStartVar); generateCharSetScan(mv, posVar, lenVar, scanCharVar, info.separatorCharSet); - mv.visitVarInsn(ILOAD, posVar); - mv.visitVarInsn(ILOAD, sepStartVar); - mv.visitJumpInsn(IF_ICMPEQ, returnNull); + generateSeparatorLengthCheck(mv, posVar, sepStartVar, sepLenVar, returnNull); } generateBackrefCheck(mv, posVar, lenVar, groupStartVar, groupLenVar, returnNull); @@ -399,8 +400,10 @@ public void generateMatchesBoundedMethod(ClassWriter cw) { } /** - * Generate matchBounded(CharSequence, int, int) - extracts the bounded region, delegates to - * match(String). + * Generate matchBounded(CharSequence, int, int) - matches the bounded region, then shifts the + * result's spans by {@code start} so they're relative to the original CharSequence rather than + * the region substring, matching the contract established by {@code ReggieMatcher}'s default + * {@code matchBounded}/{@code shiftSpans}. */ public void generateMatchBoundedMethod(ClassWriter cw) { MethodVisitor mv = @@ -412,9 +415,15 @@ public void generateMatchBoundedMethod(ClassWriter cw) { null); mv.visitCode(); + // Local vars: 0=this, 1=input, 2=start, 3=end LocalVarAllocator alloc = new LocalVarAllocator(4); int subStringVar = alloc.allocate(); + int resultVar = alloc.allocate(); + int startsVar = alloc.allocate(); + int endsVar = alloc.allocate(); + int iVar = alloc.allocate(); + // String sub = input.subSequence(start, end).toString(); mv.visitVarInsn(ALOAD, 1); mv.visitVarInsn(ILOAD, 2); mv.visitVarInsn(ILOAD, 3); @@ -428,6 +437,7 @@ public void generateMatchBoundedMethod(ClassWriter cw) { INVOKEINTERFACE, "java/lang/CharSequence", "toString", "()Ljava/lang/String;", true); mv.visitVarInsn(ASTORE, subStringVar); + // MatchResult inner = this.match(sub); mv.visitVarInsn(ALOAD, 0); mv.visitVarInsn(ALOAD, subStringVar); mv.visitMethodInsn( @@ -436,9 +446,85 @@ public void generateMatchBoundedMethod(ClassWriter cw) { "match", "(Ljava/lang/String;)Lcom/datadoghq/reggie/runtime/MatchResult;", false); + mv.visitVarInsn(ASTORE, resultVar); + + // if (inner == null) return null; + Label notNull = new Label(); + mv.visitVarInsn(ALOAD, resultVar); + mv.visitJumpInsn(IFNONNULL, notNull); + mv.visitInsn(ACONST_NULL); mv.visitInsn(ARETURN); + mv.visitLabel(notNull); - mv.visitMaxs(3, alloc.peek()); + int totalSlots = totalGroupCount() + 1; + BytecodeUtil.pushInt(mv, totalSlots); + mv.visitIntInsn(NEWARRAY, T_INT); + mv.visitVarInsn(ASTORE, startsVar); + + BytecodeUtil.pushInt(mv, totalSlots); + mv.visitIntInsn(NEWARRAY, T_INT); + mv.visitVarInsn(ASTORE, endsVar); + + // for (int i = 0; i <= totalGroupCount; i++) { starts[i] = inner.start(i) + start; ... } + mv.visitInsn(ICONST_0); + mv.visitVarInsn(ISTORE, iVar); + + Label loopCond = new Label(); + Label loopBody = new Label(); + mv.visitJumpInsn(GOTO, loopCond); + mv.visitLabel(loopBody); + + // starts[i] = inner.start(i) + start + mv.visitVarInsn(ALOAD, startsVar); + mv.visitVarInsn(ILOAD, iVar); + mv.visitVarInsn(ALOAD, resultVar); + mv.visitVarInsn(ILOAD, iVar); + mv.visitMethodInsn( + INVOKEINTERFACE, "com/datadoghq/reggie/runtime/MatchResult", "start", "(I)I", true); + mv.visitVarInsn(ILOAD, 2); + mv.visitInsn(IADD); + mv.visitInsn(IASTORE); + + // ends[i] = inner.end(i) + start + mv.visitVarInsn(ALOAD, endsVar); + mv.visitVarInsn(ILOAD, iVar); + mv.visitVarInsn(ALOAD, resultVar); + mv.visitVarInsn(ILOAD, iVar); + mv.visitMethodInsn( + INVOKEINTERFACE, "com/datadoghq/reggie/runtime/MatchResult", "end", "(I)I", true); + mv.visitVarInsn(ILOAD, 2); + mv.visitInsn(IADD); + mv.visitInsn(IASTORE); + + mv.visitIincInsn(iVar, 1); + + mv.visitLabel(loopCond); + mv.visitVarInsn(ILOAD, iVar); + BytecodeUtil.pushInt(mv, totalGroupCount()); + mv.visitJumpInsn(IF_ICMPLE, loopBody); + + // return new MatchResultImpl(input.toString(), starts, ends, totalGroupCount) + int fullInputVar = alloc.allocate(); + mv.visitVarInsn(ALOAD, 1); + mv.visitMethodInsn( + INVOKEINTERFACE, "java/lang/CharSequence", "toString", "()Ljava/lang/String;", true); + mv.visitVarInsn(ASTORE, fullInputVar); + + mv.visitTypeInsn(NEW, "com/datadoghq/reggie/runtime/MatchResultImpl"); + mv.visitInsn(DUP); + mv.visitVarInsn(ALOAD, fullInputVar); + mv.visitVarInsn(ALOAD, startsVar); + mv.visitVarInsn(ALOAD, endsVar); + BytecodeUtil.pushInt(mv, totalGroupCount()); + mv.visitMethodInsn( + INVOKESPECIAL, + "com/datadoghq/reggie/runtime/MatchResultImpl", + "", + "(Ljava/lang/String;[I[II)V", + false); + mv.visitInsn(ARETURN); + + mv.visitMaxs(6, alloc.peek()); mv.visitEnd(); } @@ -492,6 +578,7 @@ public void generateFindMatchFromMethod(ClassWriter cw) { int groupLenVar = alloc.allocate(); int scanCharVar = alloc.allocate(); int sepStartVar = info.hasSeparator() ? alloc.allocate() : -1; + int sepLenVar = info.hasSeparator() ? alloc.allocate() : -1; int startsArrayVar = alloc.allocate(); int endsArrayVar = alloc.allocate(); @@ -508,8 +595,15 @@ public void generateFindMatchFromMethod(ClassWriter cw) { mv.visitMethodInsn(INVOKEVIRTUAL, "java/lang/String", "length", "()I", false); mv.visitVarInsn(ISTORE, lenVar); + // int start = Math.max(0, startPos); mv.visitVarInsn(ILOAD, 2); mv.visitVarInsn(ISTORE, startVar); + Label startNotNeg = new Label(); + mv.visitVarInsn(ILOAD, startVar); + mv.visitJumpInsn(IFGE, startNotNeg); + mv.visitInsn(ICONST_0); + mv.visitVarInsn(ISTORE, startVar); + mv.visitLabel(startNotNeg); BytecodeUtil.pushInt(mv, totalGroupCount() + 1); mv.visitIntInsn(NEWARRAY, T_INT); @@ -555,8 +649,7 @@ public void generateFindMatchFromMethod(ClassWriter cw) { mv.visitVarInsn(ILOAD, groupStartVar); mv.visitInsn(ISUB); mv.visitVarInsn(ISTORE, groupLenVar); - mv.visitVarInsn(ILOAD, groupLenVar); - mv.visitJumpInsn(IFLE, tryNextStart); + generateGroupLengthCheck(mv, groupLenVar, tryNextStart); // ends[groupIndex] = pos mv.visitVarInsn(ALOAD, endsArrayVar); @@ -568,9 +661,7 @@ public void generateFindMatchFromMethod(ClassWriter cw) { mv.visitVarInsn(ILOAD, posVar); mv.visitVarInsn(ISTORE, sepStartVar); generateCharSetScan(mv, posVar, lenVar, scanCharVar, info.separatorCharSet); - mv.visitVarInsn(ILOAD, posVar); - mv.visitVarInsn(ILOAD, sepStartVar); - mv.visitJumpInsn(IF_ICMPEQ, tryNextStart); + generateSeparatorLengthCheck(mv, posVar, sepStartVar, sepLenVar, tryNextStart); } generateBackrefCheck(mv, posVar, lenVar, groupStartVar, groupLenVar, tryNextStart); @@ -713,4 +804,41 @@ private void generateCharSetCheck( mv.visitLabel(matches); } } + + /** + * Jumps to {@code failLabel} unless the scanned group run meets the quantifier's minimum length + * ({@code info.groupMinLength}). + */ + private void generateGroupLengthCheck(MethodVisitor mv, int groupLenVar, Label failLabel) { + mv.visitVarInsn(ILOAD, groupLenVar); + BytecodeUtil.pushInt(mv, info.groupMinLength); + mv.visitJumpInsn(IF_ICMPLT, failLabel); + } + + /** + * Computes the scanned separator run length ({@code posVar - sepStartVar}), stores it in {@code + * sepLenVar}, and jumps to {@code failLabel} unless that length falls within the separator + * quantifier's bounds ({@code info.separatorMinLength}..{@code info.separatorMaxLength}, where -1 + * means unbounded). + */ + private void generateSeparatorLengthCheck( + MethodVisitor mv, int posVar, int sepStartVar, int sepLenVar, Label failLabel) { + // sepLen = pos - sepStart + mv.visitVarInsn(ILOAD, posVar); + mv.visitVarInsn(ILOAD, sepStartVar); + mv.visitInsn(ISUB); + mv.visitVarInsn(ISTORE, sepLenVar); + + // if (sepLen < info.separatorMinLength) goto failLabel; + mv.visitVarInsn(ILOAD, sepLenVar); + BytecodeUtil.pushInt(mv, info.separatorMinLength); + mv.visitJumpInsn(IF_ICMPLT, failLabel); + + if (info.separatorMaxLength != -1) { + // if (sepLen > info.separatorMaxLength) goto failLabel; + mv.visitVarInsn(ILOAD, sepLenVar); + BytecodeUtil.pushInt(mv, info.separatorMaxLength); + mv.visitJumpInsn(IF_ICMPGT, failLabel); + } + } } diff --git a/reggie-codegen/src/test/java/com/datadoghq/reggie/codegen/analysis/PatternRoutingPropertyTest.java b/reggie-codegen/src/test/java/com/datadoghq/reggie/codegen/analysis/PatternRoutingPropertyTest.java index cfabefd8..a15add92 100644 --- a/reggie-codegen/src/test/java/com/datadoghq/reggie/codegen/analysis/PatternRoutingPropertyTest.java +++ b/reggie-codegen/src/test/java/com/datadoghq/reggie/codegen/analysis/PatternRoutingPropertyTest.java @@ -269,9 +269,10 @@ void pinnedBackrefPositive(PatternRoutingTestCase testCase) throws Exception { static Stream providePinnedBackrefPositiveExamples() { return Stream.of( - // Repeated-word shape: \w+ content is disjoint from the \s+ separator. + // Repeated-word shape: \w+ content is disjoint from the \s+ separator. No anchors, so + // the group/backref pair spans the whole pattern. new PatternRoutingTestCase( - "\\b(\\w+)\\s+\\1\\b", + "(\\w+)\\s+\\1", PINNED_BACKREFERENCE, "repeated-word shape: word charset disjoint from whitespace separator")); } @@ -309,6 +310,14 @@ static Stream providePinnedBackrefNegativeExamples() { SPECIALIZED_BACKREFERENCE, "tag-close shape: multi-node separator, must not be PINNED_BACKREFERENCE"), + // Repeated-word shape with \b anchors outside the group/backref span: the generated + // matcher has no code path to evaluate them, so the detector rejects this and it falls + // through to VARIABLE_CAPTURE_BACKREF. + new PatternRoutingTestCase( + "\\b(\\w+)\\s+\\1\\b", + VARIABLE_CAPTURE_BACKREF, + "repeated-word shape with anchors outside the group/backref span"), + // Overlapping-charset shape (backreferenced variant): [bc] overlaps with 'c' in // (c+d), so the group's own closing boundary is genuinely ambiguous. new PatternRoutingTestCase( diff --git a/reggie-codegen/src/test/java/com/datadoghq/reggie/codegen/analysis/PinnedBackreferenceDetectionTest.java b/reggie-codegen/src/test/java/com/datadoghq/reggie/codegen/analysis/PinnedBackreferenceDetectionTest.java index e7a31477..17a3cd66 100644 --- a/reggie-codegen/src/test/java/com/datadoghq/reggie/codegen/analysis/PinnedBackreferenceDetectionTest.java +++ b/reggie-codegen/src/test/java/com/datadoghq/reggie/codegen/analysis/PinnedBackreferenceDetectionTest.java @@ -79,7 +79,15 @@ void rejectsHtmlTagCloseShapeWithMultiNodeSeparator() throws Exception { @Test void detectsRepeatedWordShape() throws Exception { - assertNotNull(detect("\\b(\\w+)\\s+\\1\\b")); + assertNotNull(detect("(\\w+)\\s+\\1")); + } + + @Test + void rejectsPatternWithAnchorsOutsideGroupAndBackrefSpan() throws Exception { + // The generated matcher only scans the group, separator, and backreference echo - it has + // no code path for the \b anchors here, so the whole pattern must be rejected rather than + // silently ignoring them. + assertNull(detect("\\b(\\w+)\\s+\\1\\b")); } @Test diff --git a/reggie-codegen/src/test/java/com/datadoghq/reggie/codegen/analysis/PinnedBackreferenceEquivalenceTest.java b/reggie-codegen/src/test/java/com/datadoghq/reggie/codegen/analysis/PinnedBackreferenceEquivalenceTest.java index e7c3c73f..fb1f1d74 100644 --- a/reggie-codegen/src/test/java/com/datadoghq/reggie/codegen/analysis/PinnedBackreferenceEquivalenceTest.java +++ b/reggie-codegen/src/test/java/com/datadoghq/reggie/codegen/analysis/PinnedBackreferenceEquivalenceTest.java @@ -22,16 +22,11 @@ import com.datadoghq.reggie.codegen.ast.RegexNode; import com.datadoghq.reggie.codegen.automaton.NFA; import com.datadoghq.reggie.codegen.automaton.ThompsonBuilder; -import com.datadoghq.reggie.codegen.codegen.BackreferenceBytecodeGenerator; -import com.datadoghq.reggie.codegen.codegen.PinnedBackreferenceBytecodeGenerator; import com.datadoghq.reggie.codegen.parsing.RegexParser; import java.lang.reflect.Method; import java.util.List; -import java.util.concurrent.atomic.AtomicInteger; import org.junit.jupiter.api.DynamicTest; import org.junit.jupiter.api.TestFactory; -import org.objectweb.asm.ClassWriter; -import org.objectweb.asm.Opcodes; /** * Evidence-gathering test for a FUTURE decision about retiring {@code detectHTMLTagPattern} / @@ -39,12 +34,9 @@ * predicate (design doc {@code doc/2026-07-06-backreference-pinned-boundary-design.md}, §5/§6; impl * plan Task 8.4). * - *

This test does NOT change routing or retire either hardcoded detector - it only (a) records, - * for a corpus of tag-close-shaped and repeated-word-shaped pattern variants, whether the general - * detector and the corresponding hardcoded detector agree on eligibility, and (b) where both are - * eligible for the same pattern, confirms the two independently-generated matchers (({@code - * PinnedBackreferenceBytecodeGenerator} vs. {@code BackreferenceBytecodeGenerator}) produce - * byte-identical {@code matches}/{@code find}/{@code findFrom} results on a shared input corpus. + *

This test does NOT change routing or retire either hardcoded detector - it only records, for a + * corpus of tag-close-shaped and repeated-word-shaped pattern variants, whether the general + * detector and the corresponding hardcoded detector agree on eligibility. * *

Disagreement between the two detectors for a given variant is an expected, documented outcome * for this test - not necessarily a bug (see per-case comments below for why each disagreement is @@ -52,32 +44,6 @@ */ class PinnedBackreferenceEquivalenceTest { - private static final AtomicInteger CLASS_COUNTER = new AtomicInteger(); - - /** - * Shared differential input corpus exercised against any repeated-word pattern eligible for both - * detectors (no tag-close-shaped variant in this corpus is eligible for both - see the - * eligibility tests below - so no tag-input corpus is needed). - */ - private static final String[] WORD_INPUTS = { - "hello hello", - "the the cat", - "hello world", - "word word", - "foo\tfoo", - "", - "single", - "hello hello hello", - "Hello hello", - " hello hello ", - }; - - private static final class TestClassLoader extends ClassLoader { - Class defineClass(String name, byte[] bytecode) { - return defineClass(name, bytecode, 0, bytecode.length); - } - } - private static RegexNode parse(String pattern) throws Exception { return new RegexParser().parse(pattern); } @@ -231,30 +197,31 @@ DynamicTest htmlNoMiddleBothReject() { // ── Eligibility agreement: repeated-word-shaped corpus ─────────────────── /** - * {@code \b(\w+)\s+\1\b} - the canonical repeated-word shape. Both detectors accept: the - * separator here is a single node ({@code \s+}) whose charset (whitespace) is disjoint from the - * group's charset (word chars), satisfying the general detector's single-node-separator - * requirement, and matching the hardcoded shape exactly. + * {@code \b(\w+)\s+\1\b} - the canonical repeated-word shape. The general detector now rejects + * this: it requires the group/backref pair to span the whole pattern, and the leading/trailing + * {@code \b} anchors here violate that (the generated matcher has no code path to evaluate them). + * The hardcoded detector still accepts it (its shape explicitly requires those anchors), so this + * is now a documented disagreement rather than an agreement. */ @TestFactory - DynamicTest repeatedWordCanonicalShapeAgreement() { + DynamicTest repeatedWordCanonicalShapeDivergence() { return DynamicTest.dynamicTest( - "\\b(\\w+)\\s+\\1\\b : both accept", + "\\b(\\w+)\\s+\\1\\b : pinned rejects (anchors outside group/backref span), hardcoded accepts", () -> { Eligibility e = eligibilityForWord("\\b(\\w+)\\s+\\1\\b"); - assertEquals(true, e.pinned); + assertEquals(false, e.pinned); assertEquals(true, e.hardcoded); }); } - /** Restricted word-charset variant - same agreement. */ + /** Restricted word-charset variant - same divergence, for the same reason. */ @TestFactory - DynamicTest repeatedWordRestrictedCharsetAgreement() { + DynamicTest repeatedWordRestrictedCharsetDivergence() { return DynamicTest.dynamicTest( - "\\b([a-z]+)\\s+\\1\\b : both accept", + "\\b([a-z]+)\\s+\\1\\b : pinned rejects (anchors outside group/backref span), hardcoded accepts", () -> { Eligibility e = eligibilityForWord("\\b([a-z]+)\\s+\\1\\b"); - assertEquals(true, e.pinned); + assertEquals(false, e.pinned); assertEquals(true, e.hardcoded); }); } @@ -296,136 +263,44 @@ DynamicTest repeatedWordNoBoundariesDivergence() { } /** - * Literal separator instead of {@code \s+} - both reject (hardcoded needs a {@code \s+} - * QuantifierNode; general detector's own disjointness proof would actually hold for a literal - * separator too, but the shape doesn't match {@code detectRepeatedWordPattern} at all here since - * we're only invoking it directly, not the pinned detector's separator-node acceptance path - - * recorded for completeness). + * Literal separator instead of {@code \s+}, with {@code \b} anchors - both reject: the hardcoded + * detector needs a {@code \s+} QuantifierNode separator, and the general detector rejects the + * {@code \b} anchors outside the group/backref span regardless of the separator shape. */ @TestFactory - DynamicTest repeatedWordLiteralSeparatorHardcodedRejects() { + DynamicTest repeatedWordLiteralSeparatorBothReject() { return DynamicTest.dynamicTest( - "\\b(\\w+),\\1\\b : pinned accepts (literal separator disjoint from \\w), hardcoded rejects", + "\\b(\\w+),\\1\\b : both reject (pinned: anchors outside span; hardcoded: wrong separator)", () -> { Eligibility e = eligibilityForWord("\\b(\\w+),\\1\\b"); - assertEquals(true, e.pinned); + assertEquals(false, e.pinned); assertEquals(false, e.hardcoded); }); } - // ── Byte-identical behavior where BOTH detectors agree the pattern is eligible ── - - private Object compilePinnedBooleanApi(PinnedBackreferenceInfo info) throws Exception { - String className = "PinnedEquiv" + CLASS_COUNTER.incrementAndGet(); - ClassWriter cw = new ClassWriter(ClassWriter.COMPUTE_FRAMES | ClassWriter.COMPUTE_MAXS); - cw.visit( - Opcodes.V21, - Opcodes.ACC_PUBLIC | Opcodes.ACC_FINAL, - className, - null, - "java/lang/Object", - null); - org.objectweb.asm.MethodVisitor ctor = - cw.visitMethod(Opcodes.ACC_PUBLIC, "", "()V", null, null); - ctor.visitCode(); - ctor.visitVarInsn(Opcodes.ALOAD, 0); - ctor.visitMethodInsn(Opcodes.INVOKESPECIAL, "java/lang/Object", "", "()V", false); - ctor.visitInsn(Opcodes.RETURN); - ctor.visitMaxs(1, 1); - ctor.visitEnd(); - - PinnedBackreferenceBytecodeGenerator gen = - new PinnedBackreferenceBytecodeGenerator(info, className); - gen.generateMatchesMethod(cw); - gen.generateFindMethod(cw); - gen.generateFindFromMethod(cw); - cw.visitEnd(); - - byte[] bytecode = cw.toByteArray(); - Class cls = new TestClassLoader().defineClass(className, bytecode); - return cls.getDeclaredConstructor().newInstance(); - } - - private Object compileHardcodedBooleanApi(BackreferencePatternInfo info) throws Exception { - String className = "HardcodedEquiv" + CLASS_COUNTER.incrementAndGet(); - ClassWriter cw = new ClassWriter(ClassWriter.COMPUTE_FRAMES | ClassWriter.COMPUTE_MAXS); - cw.visit( - Opcodes.V21, - Opcodes.ACC_PUBLIC | Opcodes.ACC_FINAL, - className, - null, - "java/lang/Object", - null); - org.objectweb.asm.MethodVisitor ctor = - cw.visitMethod(Opcodes.ACC_PUBLIC, "", "()V", null, null); - ctor.visitCode(); - ctor.visitVarInsn(Opcodes.ALOAD, 0); - ctor.visitMethodInsn(Opcodes.INVOKESPECIAL, "java/lang/Object", "", "()V", false); - ctor.visitInsn(Opcodes.RETURN); - ctor.visitMaxs(1, 1); - ctor.visitEnd(); - - BackreferenceBytecodeGenerator gen = new BackreferenceBytecodeGenerator(info); - gen.generateMatchesMethod(cw, className); - gen.generateFindMethod(cw, className); - gen.generateFindFromMethod(cw, className); - cw.visitEnd(); - - byte[] bytecode = cw.toByteArray(); - Class cls = new TestClassLoader().defineClass(className, bytecode); - return cls.getDeclaredConstructor().newInstance(); - } - - private static boolean invokeMatches(Object matcher, String input) throws Exception { - Method m = matcher.getClass().getMethod("matches", String.class); - return (Boolean) m.invoke(matcher, input); - } - - private static boolean invokeFind(Object matcher, String input) throws Exception { - Method m = matcher.getClass().getMethod("find", String.class); - return (Boolean) m.invoke(matcher, input); - } - - private static int invokeFindFrom(Object matcher, String input, int from) throws Exception { - Method m = matcher.getClass().getMethod("findFrom", String.class, int.class); - return (Integer) m.invoke(matcher, input, from); - } - /** - * Only the repeated-word canonical shape has both detectors eligible in this corpus (see - * eligibility tests above), so it is the only case exercised for byte-identical behavior. + * The general detector now requires the group/backref pair to span the whole pattern, so no + * pattern in the repeated-word corpus is eligible for both detectors simultaneously anymore (the + * hardcoded shape requires {@code \b} anchors, which the general detector always rejects as + * prefix/suffix content). This test therefore just confirms that absence of overlap rather than + * exercising byte-identical behavior, which no longer applies. */ @TestFactory - DynamicTest repeatedWordCanonicalShapeProducesIdenticalResults() throws Exception { + DynamicTest noSharedEligiblePatternRemainsInWordCorpus() throws Exception { String pattern = "\\b(\\w+)\\s+\\1\\b"; RegexNode ast = parse(pattern); PatternAnalyzer analyzer = newAnalyzer(pattern, ast); PinnedBackreferenceInfo pinnedInfo = detectPinned(analyzer, ast); BackreferencePatternInfo hardcodedInfo = detectRepeatedWord(analyzer, ((ConcatNode) ast).children); - assertNotNull(pinnedInfo, "expected pinned detector to accept the canonical shape"); - assertNotNull(hardcodedInfo, "expected hardcoded detector to accept the canonical shape"); - - Object pinnedMatcher = compilePinnedBooleanApi(pinnedInfo); - Object hardcodedMatcher = compileHardcodedBooleanApi(hardcodedInfo); return DynamicTest.dynamicTest( - "matches/find/findFrom identical across WORD_INPUTS for " + pattern, + "pinned rejects, hardcoded accepts " + pattern, () -> { - for (String input : WORD_INPUTS) { - assertEquals( - invokeMatches(hardcodedMatcher, input), - invokeMatches(pinnedMatcher, input), - "matches() diverged for input: " + input); - assertEquals( - invokeFind(hardcodedMatcher, input), - invokeFind(pinnedMatcher, input), - "find() diverged for input: " + input); - assertEquals( - invokeFindFrom(hardcodedMatcher, input, 0), - invokeFindFrom(pinnedMatcher, input, 0), - "findFrom(input, 0) diverged for input: " + input); - } + assertEquals( + null, pinnedInfo, "pinned detector should reject anchors outside group/backref span"); + assertNotNull( + hardcodedInfo, "hardcoded detector should still accept the canonical shape"); }); } } diff --git a/reggie-codegen/src/test/java/com/datadoghq/reggie/codegen/analysis/PinnedBackreferenceInfoTest.java b/reggie-codegen/src/test/java/com/datadoghq/reggie/codegen/analysis/PinnedBackreferenceInfoTest.java index 33a7afb8..f00c4284 100644 --- a/reggie-codegen/src/test/java/com/datadoghq/reggie/codegen/analysis/PinnedBackreferenceInfoTest.java +++ b/reggie-codegen/src/test/java/com/datadoghq/reggie/codegen/analysis/PinnedBackreferenceInfoTest.java @@ -32,9 +32,10 @@ private static RegexNode bodyNode(CharSet charSet) { @Test void structuralHashCodeDiffersOnGroupIndexAndCharSet() { PinnedBackreferenceInfo a = - new PinnedBackreferenceInfo(1, bodyNode(CharSet.WORD), CharSet.WORD, null, null); + new PinnedBackreferenceInfo(1, bodyNode(CharSet.WORD), CharSet.WORD, 1, null, null, 0, -1); PinnedBackreferenceInfo b = - new PinnedBackreferenceInfo(2, bodyNode(CharSet.DIGIT), CharSet.DIGIT, null, null); + new PinnedBackreferenceInfo( + 2, bodyNode(CharSet.DIGIT), CharSet.DIGIT, 1, null, null, 0, -1); assertNotEquals(a.structuralHashCode(), b.structuralHashCode()); } @@ -42,13 +43,13 @@ void structuralHashCodeDiffersOnGroupIndexAndCharSet() { @Test void hasSeparatorReflectsConstructorArgument() { PinnedBackreferenceInfo noSeparator = - new PinnedBackreferenceInfo(1, bodyNode(CharSet.WORD), CharSet.WORD, null, null); + new PinnedBackreferenceInfo(1, bodyNode(CharSet.WORD), CharSet.WORD, 1, null, null, 0, -1); assertFalse(noSeparator.hasSeparator()); RegexNode separatorNode = bodyNode(CharSet.WHITESPACE); PinnedBackreferenceInfo withSeparator = new PinnedBackreferenceInfo( - 1, bodyNode(CharSet.WORD), CharSet.WORD, separatorNode, CharSet.WHITESPACE); + 1, bodyNode(CharSet.WORD), CharSet.WORD, 1, separatorNode, CharSet.WHITESPACE, 1, -1); assertTrue(withSeparator.hasSeparator()); } } diff --git a/reggie-codegen/src/test/java/com/datadoghq/reggie/codegen/analysis/StrategySelectionExtendedTest.java b/reggie-codegen/src/test/java/com/datadoghq/reggie/codegen/analysis/StrategySelectionExtendedTest.java index 0cba5e95..5dec645d 100644 --- a/reggie-codegen/src/test/java/com/datadoghq/reggie/codegen/analysis/StrategySelectionExtendedTest.java +++ b/reggie-codegen/src/test/java/com/datadoghq/reggie/codegen/analysis/StrategySelectionExtendedTest.java @@ -243,10 +243,11 @@ void testLinearBackreferenceSingleChar() throws Exception { @Test void testSpecializedBackrefRepeatedWord() throws Exception { - // Word-charset content is disjoint from the whitespace separator, so this now routes to - // PINNED_BACKREFERENCE (single forward scan, no retry) instead of VARIABLE_CAPTURE_BACKREF. + // The \b anchors sit outside the group/backref span, which PINNED_BACKREFERENCE now rejects + // (its generated matcher has no code path to evaluate anything outside that span), so this + // falls through to VARIABLE_CAPTURE_BACKREF instead. PatternAnalyzer.MatchingStrategyResult result = analyze("\\b(\\w+)\\s+\\1\\b"); - assertEquals(PatternAnalyzer.MatchingStrategy.PINNED_BACKREFERENCE, result.strategy); + assertEquals(PatternAnalyzer.MatchingStrategy.VARIABLE_CAPTURE_BACKREF, result.strategy); } // ── VARIABLE_CAPTURE_BACKREF ───────────────────────────────────────────── @@ -262,9 +263,11 @@ void testVariableCaptureBackrefWhitespace() throws Exception { @Test void testVariableCaptureBackrefAnchored() throws Exception { - // Anchors don't change the disjointness proof - still routes to PINNED_BACKREFERENCE. + // The ^ and $ anchors sit outside the group/backref span, which PINNED_BACKREFERENCE now + // rejects (its generated matcher has no code path to evaluate them), so this routes to + // VARIABLE_CAPTURE_BACKREF instead. PatternAnalyzer.MatchingStrategyResult result = analyze("^(\\w+)\\s+\\1$"); - assertEquals(PatternAnalyzer.MatchingStrategy.PINNED_BACKREFERENCE, result.strategy); + assertEquals(PatternAnalyzer.MatchingStrategy.VARIABLE_CAPTURE_BACKREF, result.strategy); if (result.patternInfo != null) assertNotNull(result.patternInfo.toString()); } @@ -452,8 +455,18 @@ void testNestedQuantifiedGroupsWordChar() throws Exception { @Test void testPinnedBackrefRepeatedWordShape() throws Exception { - // \w+ content disjoint from \s+ separator - proven single forward-scan boundary. + // \w+ content disjoint from \s+ separator - proven single forward-scan boundary - but the + // \b anchors sit outside the group/backref span, which PINNED_BACKREFERENCE now rejects, so + // it falls through to VARIABLE_CAPTURE_BACKREF instead. PatternAnalyzer.MatchingStrategyResult result = analyze("\\b(\\w+)\\s+\\1\\b"); + assertEquals(PatternAnalyzer.MatchingStrategy.VARIABLE_CAPTURE_BACKREF, result.strategy); + } + + @Test + void testPinnedBackrefRepeatedWordShapeWithoutAnchors() throws Exception { + // Same shape without the \b anchors - the group/backref pair now spans the whole pattern, + // so this is PINNED_BACKREFERENCE-eligible. + PatternAnalyzer.MatchingStrategyResult result = analyze("(\\w+)\\s+\\1"); assertEquals(PatternAnalyzer.MatchingStrategy.PINNED_BACKREFERENCE, result.strategy); } diff --git a/reggie-codegen/src/test/java/com/datadoghq/reggie/codegen/codegen/PinnedBackreferenceBytecodeGeneratorTest.java b/reggie-codegen/src/test/java/com/datadoghq/reggie/codegen/codegen/PinnedBackreferenceBytecodeGeneratorTest.java index 687ce535..e90fcbaa 100644 --- a/reggie-codegen/src/test/java/com/datadoghq/reggie/codegen/codegen/PinnedBackreferenceBytecodeGeneratorTest.java +++ b/reggie-codegen/src/test/java/com/datadoghq/reggie/codegen/codegen/PinnedBackreferenceBytecodeGeneratorTest.java @@ -54,14 +54,17 @@ private PinnedBackreferenceInfo repeatedWordInfo() { 1, new CharClassNode(CharSet.WORD, false), CharSet.WORD, + 1, new CharClassNode(CharSet.WHITESPACE, false), - CharSet.WHITESPACE); + CharSet.WHITESPACE, + 1, + -1); } /** (\w+)\1 with no separator between the group's close and the backreference site. */ private PinnedBackreferenceInfo noSeparatorInfo() { return new PinnedBackreferenceInfo( - 1, new CharClassNode(CharSet.WORD, false), CharSet.WORD, null, null); + 1, new CharClassNode(CharSet.WORD, false), CharSet.WORD, 1, null, null, 0, -1); } private ClassWriter newClassWriter(String className) { diff --git a/reggie-runtime/src/test/java/com/datadoghq/reggie/runtime/BackrefDigitAmbiguityTest.java b/reggie-runtime/src/test/java/com/datadoghq/reggie/runtime/BackrefDigitAmbiguityTest.java index f1c18144..f65fca67 100644 --- a/reggie-runtime/src/test/java/com/datadoghq/reggie/runtime/BackrefDigitAmbiguityTest.java +++ b/reggie-runtime/src/test/java/com/datadoghq/reggie/runtime/BackrefDigitAmbiguityTest.java @@ -105,20 +105,27 @@ void pcreCanonicalBackrefCase() { } /** - * {@code (\w+)#\1(a?)(\2)} is pinned-eligible ({@code \w+}'s charset is disjoint from the literal - * {@code #} separator between group 1 and backref {@code \1}), so it would route to {@code - * PINNED_BACKREFERENCE} if the B9 danger-condition guard did not also cover that strategy. Group - * 3 {@code (\2)} is a capturing group whose body is a backref to the nullable group 2 ({@code - * a?}), so {@code hasNullableBackrefInsideCapturingGroup} must still exclude it, routing to JDK - * fallback instead. + * {@code (\w+)#\1(a?)(\2)} has trailing content ({@code (a?)(\2)}) after the group 1 / backref 1 + * pair, so PINNED_BACKREFERENCE now requires the group and its backreference to span the whole + * pattern and rejects it. It routes to VARIABLE_CAPTURE_BACKREF instead, which is unaffected by + * the B9 danger condition (backref to nullable group 2 inside capturing group 3) and matches + * correctly without needing the JDK fallback. */ @Test - void pinnedEligibleNullableBackrefInsideCapturingGroup_routesToFallback_notPinnedBackreference() { + void pinnedIneligibleNullableBackrefInsideCapturingGroup_matchesNatively() { String pat = "(\\w+)#\\1(a?)(\\2)"; + String input = "hello#hello"; + Matcher jdk = Pattern.compile(pat).matcher(input); + assertTrue(jdk.matches(), "JDK should match"); ReggieMatcher reg = Reggie.compile(pat, WITH_FALLBACK); - assertTrue( + assertFalse( reg instanceof JavaRegexFallbackMatcher, - "(\\w+)#\\1(a?)(\\2) must fall back to JDK: pinned-eligible boundary but nullable " - + "backref inside capturing group (B9 guard) must still apply"); + "(\\w+)#\\1(a?)(\\2) is not PINNED_BACKREFERENCE-eligible (trailing groups after the " + + "backref), so it should match natively rather than falling back to JDK"); + MatchResult r = reg.match(input); + assertNotNull(r, "Reggie should match"); + assertEquals(jdk.group(1), r.group(1)); + assertEquals(jdk.group(2), r.group(2)); + assertEquals(jdk.group(3), r.group(3)); } } diff --git a/reggie-runtime/src/test/java/com/datadoghq/reggie/runtime/PinnedBackreferenceMatchResultTest.java b/reggie-runtime/src/test/java/com/datadoghq/reggie/runtime/PinnedBackreferenceMatchResultTest.java index dbcd040a..74cc559a 100644 --- a/reggie-runtime/src/test/java/com/datadoghq/reggie/runtime/PinnedBackreferenceMatchResultTest.java +++ b/reggie-runtime/src/test/java/com/datadoghq/reggie/runtime/PinnedBackreferenceMatchResultTest.java @@ -41,11 +41,16 @@ private static PatternAnalyzer.MatchingStrategy routeOf(String pattern) throws E } @Test - public void testRepeatedWordRoutesToPinnedBackreference() throws Exception { - assertEquals( + public void testRepeatedWordWithBoundaryAnchorsDoesNotRouteToPinnedBackreference() + throws Exception { + // \b(\w+)\s+\1\b has \b anchors outside the group/backref span, which the generated + // matcher has no code path to evaluate - PINNED_BACKREFERENCE requires the group and + // backreference to span the whole pattern, so this falls through to another strategy. + assertNotEquals( PatternAnalyzer.MatchingStrategy.PINNED_BACKREFERENCE, routeOf("\\b(\\w+)\\s+\\1\\b"), - "Pattern \\b(\\w+)\\s+\\1\\b should route to PINNED_BACKREFERENCE"); + "Pattern \\b(\\w+)\\s+\\1\\b has anchors outside the group/backref span and must not " + + "route to PINNED_BACKREFERENCE"); } @Test From 8c29f21dcecc826bd62c496be8f82288b6570040 Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Tue, 7 Jul 2026 19:19:30 +0200 Subject: [PATCH 2/4] fix: address remaining sphinx review findings on PINNED_BACKREFERENCE Tighten minCandidateLength to real group/separator bounds, fix stale javadoc examples and test labels, add mutation-killing detector tests. Co-Authored-By: Claude Sonnet 5 --- .../codegen/analysis/PatternAnalyzer.java | 23 ++- .../analysis/PinnedBackreferenceInfo.java | 7 +- .../PinnedBackreferenceBytecodeGenerator.java | 138 +----------------- .../PinnedBackreferenceDetectionTest.java | 16 ++ ...nedBackreferenceBytecodeGeneratorTest.java | 2 - .../ReggieMatcherBytecodeGenerator.java | 1 - .../reggie/runtime/RuntimeCompiler.java | 1 - .../PinnedBackreferenceMatchResultTest.java | 13 +- 8 files changed, 55 insertions(+), 146 deletions(-) diff --git a/reggie-codegen/src/main/java/com/datadoghq/reggie/codegen/analysis/PatternAnalyzer.java b/reggie-codegen/src/main/java/com/datadoghq/reggie/codegen/analysis/PatternAnalyzer.java index dca6d857..7df32777 100644 --- a/reggie-codegen/src/main/java/com/datadoghq/reggie/codegen/analysis/PatternAnalyzer.java +++ b/reggie-codegen/src/main/java/com/datadoghq/reggie/codegen/analysis/PatternAnalyzer.java @@ -3596,6 +3596,12 @@ private PinnedBackreferenceInfo detectPinnedBackreference(RegexNode ast) { if (!groupQuant.greedy || groupQuant.min < 1 || groupQuant.max != -1) { return null; } + // The generated matcher's group count is derived solely from the pinned group's own number; + // a capturing group nested inside its quantified body would be silently unaccounted for, + // making group(n) inaccessible or throwing for n beyond the pinned group's index. + if (hasCapturingGroup(groupQuant.child)) { + return null; + } CharSet groupCharSet = getNodeCharSet(groupQuant.child); // Charset of whatever immediately follows the group in the concat. @@ -3621,6 +3627,9 @@ private PinnedBackreferenceInfo detectPinnedBackreference(RegexNode ast) { return null; } separator = sepNodes.get(0); + if (hasCapturingGroup(separator)) { + return null; + } separatorCharSet = getFirstCharSet(separator); // The scan can't backtrack, so it always consumes the longest run of separator-charset @@ -3628,8 +3637,18 @@ private PinnedBackreferenceInfo detectPinnedBackreference(RegexNode ast) { // that run (min <= run) or don't cap it below that run (max == -1 or max >= run). Track the // bounds here so codegen can reject a candidate whose actual run falls outside them, instead // of accepting whatever length the greedy scan happens to consume. - if (separator instanceof QuantifierNode) { - QuantifierNode sepQuant = (QuantifierNode) separator; + // A non-capturing group (e.g. (?:\s+)) wraps its child without changing what's matched, so + // unwrap it before checking for a quantifier - otherwise a wrapped quantifier's bounds are + // silently replaced with "exactly one occurrence" below. + RegexNode sepQuantCandidate = separator; + if (sepQuantCandidate instanceof GroupNode) { + GroupNode sepGroup = (GroupNode) sepQuantCandidate; + if (!sepGroup.capturing && !sepGroup.atomic) { + sepQuantCandidate = sepGroup.child; + } + } + if (sepQuantCandidate instanceof QuantifierNode) { + QuantifierNode sepQuant = (QuantifierNode) sepQuantCandidate; if (!sepQuant.greedy) { return null; } diff --git a/reggie-codegen/src/main/java/com/datadoghq/reggie/codegen/analysis/PinnedBackreferenceInfo.java b/reggie-codegen/src/main/java/com/datadoghq/reggie/codegen/analysis/PinnedBackreferenceInfo.java index 7c469d75..26d58a1d 100644 --- a/reggie-codegen/src/main/java/com/datadoghq/reggie/codegen/analysis/PinnedBackreferenceInfo.java +++ b/reggie-codegen/src/main/java/com/datadoghq/reggie/codegen/analysis/PinnedBackreferenceInfo.java @@ -28,8 +28,11 @@ * reduces to a single forward scan to that boundary, an optional separator scan, and one equality * check against the backreference site - no retry or backtracking is needed. * - *

Examples: - {@code <(\w+)>.*} - tag body charset disjoint from the {@code The group/backref pair must span the entire pattern (no prefix or suffix), since the generated + * matcher has no code path for anything outside that span. + * + *

Examples: - {@code (\w+)\s+\1} - word charset disjoint from the whitespace separator - {@code + * (\w+)(?:\s+)\1} - same, with the separator wrapped in a non-capturing group */ public class PinnedBackreferenceInfo implements PatternInfo { diff --git a/reggie-codegen/src/main/java/com/datadoghq/reggie/codegen/codegen/PinnedBackreferenceBytecodeGenerator.java b/reggie-codegen/src/main/java/com/datadoghq/reggie/codegen/codegen/PinnedBackreferenceBytecodeGenerator.java index e91c29e7..dd25fb0d 100644 --- a/reggie-codegen/src/main/java/com/datadoghq/reggie/codegen/codegen/PinnedBackreferenceBytecodeGenerator.java +++ b/reggie-codegen/src/main/java/com/datadoghq/reggie/codegen/codegen/PinnedBackreferenceBytecodeGenerator.java @@ -151,13 +151,12 @@ public void generateFindMethod(ClassWriter cw) { } /** - * Minimum number of chars a match needs from a given start: 1+ for the group, 1+ for the - * separator (if present), plus groupLen more for the backreference - the last term is unknown - * until the group is scanned, so the static lower bound only covers the two known-nonempty pieces - * plus a single char for the (at-least-1-char) backreference echo. + * Minimum number of chars a match needs from a given start: {@code groupMinLength} for the group, + * {@code separatorMinLength} for the separator (if present), plus {@code groupMinLength} again + * for the backreference echo, which always matches the group's length. */ private int minCandidateLength() { - return 2 + (info.hasSeparator() ? 1 : 0); + return 2 * info.groupMinLength + (info.hasSeparator() ? info.separatorMinLength : 0); } /** Generate the findFrom() method. */ @@ -399,135 +398,6 @@ public void generateMatchesBoundedMethod(ClassWriter cw) { mv.visitEnd(); } - /** - * Generate matchBounded(CharSequence, int, int) - matches the bounded region, then shifts the - * result's spans by {@code start} so they're relative to the original CharSequence rather than - * the region substring, matching the contract established by {@code ReggieMatcher}'s default - * {@code matchBounded}/{@code shiftSpans}. - */ - public void generateMatchBoundedMethod(ClassWriter cw) { - MethodVisitor mv = - cw.visitMethod( - ACC_PUBLIC, - "matchBounded", - "(Ljava/lang/CharSequence;II)Lcom/datadoghq/reggie/runtime/MatchResult;", - null, - null); - mv.visitCode(); - - // Local vars: 0=this, 1=input, 2=start, 3=end - LocalVarAllocator alloc = new LocalVarAllocator(4); - int subStringVar = alloc.allocate(); - int resultVar = alloc.allocate(); - int startsVar = alloc.allocate(); - int endsVar = alloc.allocate(); - int iVar = alloc.allocate(); - - // String sub = input.subSequence(start, end).toString(); - mv.visitVarInsn(ALOAD, 1); - mv.visitVarInsn(ILOAD, 2); - mv.visitVarInsn(ILOAD, 3); - mv.visitMethodInsn( - INVOKEINTERFACE, - "java/lang/CharSequence", - "subSequence", - "(II)Ljava/lang/CharSequence;", - true); - mv.visitMethodInsn( - INVOKEINTERFACE, "java/lang/CharSequence", "toString", "()Ljava/lang/String;", true); - mv.visitVarInsn(ASTORE, subStringVar); - - // MatchResult inner = this.match(sub); - mv.visitVarInsn(ALOAD, 0); - mv.visitVarInsn(ALOAD, subStringVar); - mv.visitMethodInsn( - INVOKEVIRTUAL, - className, - "match", - "(Ljava/lang/String;)Lcom/datadoghq/reggie/runtime/MatchResult;", - false); - mv.visitVarInsn(ASTORE, resultVar); - - // if (inner == null) return null; - Label notNull = new Label(); - mv.visitVarInsn(ALOAD, resultVar); - mv.visitJumpInsn(IFNONNULL, notNull); - mv.visitInsn(ACONST_NULL); - mv.visitInsn(ARETURN); - mv.visitLabel(notNull); - - int totalSlots = totalGroupCount() + 1; - BytecodeUtil.pushInt(mv, totalSlots); - mv.visitIntInsn(NEWARRAY, T_INT); - mv.visitVarInsn(ASTORE, startsVar); - - BytecodeUtil.pushInt(mv, totalSlots); - mv.visitIntInsn(NEWARRAY, T_INT); - mv.visitVarInsn(ASTORE, endsVar); - - // for (int i = 0; i <= totalGroupCount; i++) { starts[i] = inner.start(i) + start; ... } - mv.visitInsn(ICONST_0); - mv.visitVarInsn(ISTORE, iVar); - - Label loopCond = new Label(); - Label loopBody = new Label(); - mv.visitJumpInsn(GOTO, loopCond); - mv.visitLabel(loopBody); - - // starts[i] = inner.start(i) + start - mv.visitVarInsn(ALOAD, startsVar); - mv.visitVarInsn(ILOAD, iVar); - mv.visitVarInsn(ALOAD, resultVar); - mv.visitVarInsn(ILOAD, iVar); - mv.visitMethodInsn( - INVOKEINTERFACE, "com/datadoghq/reggie/runtime/MatchResult", "start", "(I)I", true); - mv.visitVarInsn(ILOAD, 2); - mv.visitInsn(IADD); - mv.visitInsn(IASTORE); - - // ends[i] = inner.end(i) + start - mv.visitVarInsn(ALOAD, endsVar); - mv.visitVarInsn(ILOAD, iVar); - mv.visitVarInsn(ALOAD, resultVar); - mv.visitVarInsn(ILOAD, iVar); - mv.visitMethodInsn( - INVOKEINTERFACE, "com/datadoghq/reggie/runtime/MatchResult", "end", "(I)I", true); - mv.visitVarInsn(ILOAD, 2); - mv.visitInsn(IADD); - mv.visitInsn(IASTORE); - - mv.visitIincInsn(iVar, 1); - - mv.visitLabel(loopCond); - mv.visitVarInsn(ILOAD, iVar); - BytecodeUtil.pushInt(mv, totalGroupCount()); - mv.visitJumpInsn(IF_ICMPLE, loopBody); - - // return new MatchResultImpl(input.toString(), starts, ends, totalGroupCount) - int fullInputVar = alloc.allocate(); - mv.visitVarInsn(ALOAD, 1); - mv.visitMethodInsn( - INVOKEINTERFACE, "java/lang/CharSequence", "toString", "()Ljava/lang/String;", true); - mv.visitVarInsn(ASTORE, fullInputVar); - - mv.visitTypeInsn(NEW, "com/datadoghq/reggie/runtime/MatchResultImpl"); - mv.visitInsn(DUP); - mv.visitVarInsn(ALOAD, fullInputVar); - mv.visitVarInsn(ALOAD, startsVar); - mv.visitVarInsn(ALOAD, endsVar); - BytecodeUtil.pushInt(mv, totalGroupCount()); - mv.visitMethodInsn( - INVOKESPECIAL, - "com/datadoghq/reggie/runtime/MatchResultImpl", - "", - "(Ljava/lang/String;[I[II)V", - false); - mv.visitInsn(ARETURN); - - mv.visitMaxs(6, alloc.peek()); - mv.visitEnd(); - } - /** Generate findMatch(String) - delegates to findMatchFrom(input, 0). */ public void generateFindMatchMethod(ClassWriter cw) { MethodVisitor mv = diff --git a/reggie-codegen/src/test/java/com/datadoghq/reggie/codegen/analysis/PinnedBackreferenceDetectionTest.java b/reggie-codegen/src/test/java/com/datadoghq/reggie/codegen/analysis/PinnedBackreferenceDetectionTest.java index 17a3cd66..0e4535b6 100644 --- a/reggie-codegen/src/test/java/com/datadoghq/reggie/codegen/analysis/PinnedBackreferenceDetectionTest.java +++ b/reggie-codegen/src/test/java/com/datadoghq/reggie/codegen/analysis/PinnedBackreferenceDetectionTest.java @@ -99,4 +99,20 @@ void rejectsOverlappingGroupAndSuffixCharsets() throws Exception { void rejectsAmbiguousQuotedStringWithEscape() throws Exception { assertNull(detect("([\"'])(?:\\\\\\1|.)*?\\1")); } + + @Test + void rejectsPatternWithPrefixEvenWhenBackrefIsLast() throws Exception { + // Exercises the groupIndex/backrefIndex span check with only one side violated: a prefix + // before the group, but the backreference is still the last child. Both conditions must + // independently be able to trigger the rejection, not just their conjunction. + assertNull(detect("x(\\w+)\\s+\\1")); + } + + @Test + void rejectsZeroMinLengthSeparator() throws Exception { + // A separator quantifier with min == 0 (e.g. \s*) can match empty, making the group's + // closing boundary ambiguous - the scan can't tell where the group ends and the separator + // begins, so this must be rejected rather than accepted with a bogus zero-length separator. + assertNull(detect("(\\w+)\\s*\\1")); + } } diff --git a/reggie-codegen/src/test/java/com/datadoghq/reggie/codegen/codegen/PinnedBackreferenceBytecodeGeneratorTest.java b/reggie-codegen/src/test/java/com/datadoghq/reggie/codegen/codegen/PinnedBackreferenceBytecodeGeneratorTest.java index e90fcbaa..2e6290e9 100644 --- a/reggie-codegen/src/test/java/com/datadoghq/reggie/codegen/codegen/PinnedBackreferenceBytecodeGeneratorTest.java +++ b/reggie-codegen/src/test/java/com/datadoghq/reggie/codegen/codegen/PinnedBackreferenceBytecodeGeneratorTest.java @@ -105,7 +105,6 @@ void generatesFullRichApiMethodSetWithoutCrashing() { gen.generateFindFromMethod(cw); gen.generateMatchMethod(cw); gen.generateMatchesBoundedMethod(cw); - gen.generateMatchBoundedMethod(cw); gen.generateFindMatchMethod(cw); gen.generateFindMatchFromMethod(cw); cw.visitEnd(); @@ -136,7 +135,6 @@ void generatesFullRichApiMethodSetWithoutSeparatorWithoutCrashing() { gen.generateFindFromMethod(cw); gen.generateMatchMethod(cw); gen.generateMatchesBoundedMethod(cw); - gen.generateMatchBoundedMethod(cw); gen.generateFindMatchMethod(cw); gen.generateFindMatchFromMethod(cw); cw.visitEnd(); diff --git a/reggie-processor/src/main/java/com/datadoghq/reggie/processor/ReggieMatcherBytecodeGenerator.java b/reggie-processor/src/main/java/com/datadoghq/reggie/processor/ReggieMatcherBytecodeGenerator.java index 9a35d39f..7cd4e574 100644 --- a/reggie-processor/src/main/java/com/datadoghq/reggie/processor/ReggieMatcherBytecodeGenerator.java +++ b/reggie-processor/src/main/java/com/datadoghq/reggie/processor/ReggieMatcherBytecodeGenerator.java @@ -671,7 +671,6 @@ public byte[] generate() throws Exception { pinnedBackrefGen.generateFindMatchMethod(cw); pinnedBackrefGen.generateFindMatchFromMethod(cw); pinnedBackrefGen.generateMatchesBoundedMethod(cw); - pinnedBackrefGen.generateMatchBoundedMethod(cw); break; case NESTED_QUANTIFIED_GROUPS: diff --git a/reggie-runtime/src/main/java/com/datadoghq/reggie/runtime/RuntimeCompiler.java b/reggie-runtime/src/main/java/com/datadoghq/reggie/runtime/RuntimeCompiler.java index 0572ba29..968086b0 100644 --- a/reggie-runtime/src/main/java/com/datadoghq/reggie/runtime/RuntimeCompiler.java +++ b/reggie-runtime/src/main/java/com/datadoghq/reggie/runtime/RuntimeCompiler.java @@ -1212,7 +1212,6 @@ private static byte[] generateBytecode( pinnedBackrefGen.generateFindMatchMethod(cw); pinnedBackrefGen.generateFindMatchFromMethod(cw); pinnedBackrefGen.generateMatchesBoundedMethod(cw); - pinnedBackrefGen.generateMatchBoundedMethod(cw); break; case VARIABLE_CAPTURE_BACKREF: diff --git a/reggie-runtime/src/test/java/com/datadoghq/reggie/runtime/PinnedBackreferenceMatchResultTest.java b/reggie-runtime/src/test/java/com/datadoghq/reggie/runtime/PinnedBackreferenceMatchResultTest.java index 74cc559a..aa6d1e3a 100644 --- a/reggie-runtime/src/test/java/com/datadoghq/reggie/runtime/PinnedBackreferenceMatchResultTest.java +++ b/reggie-runtime/src/test/java/com/datadoghq/reggie/runtime/PinnedBackreferenceMatchResultTest.java @@ -203,11 +203,16 @@ public void testMatchBounded_failure() { "matchBounded with a mismatched prefix in the bound should fail"); } - // ---- \b(\w+)\s+\1\b : whitespace separator ---- + // ---- (\w+)\s+\1 : whitespace separator ---- + // Unlike \b(\w+)\s+\1\b (covered by + // testRepeatedWordWithBoundaryAnchorsDoesNotRouteToPinnedBackreference + // above), this pattern has no anchors outside the group/backref span, so it routes to + // PINNED_BACKREFERENCE - the \b anchors are zero-width, so removing them doesn't change the + // expected match boundaries below. @Test public void testRepeatedWord_match() { - ReggieMatcher matcher = RuntimeCompiler.compile("\\b(\\w+)\\s+\\1\\b"); + ReggieMatcher matcher = RuntimeCompiler.compile("(\\w+)\\s+\\1"); MatchResult result = matcher.match("hello hello"); assertNotNull(result, "match(\"hello hello\") should succeed"); @@ -217,7 +222,7 @@ public void testRepeatedWord_match() { @Test public void testRepeatedWord_findMatch() { - ReggieMatcher matcher = RuntimeCompiler.compile("\\b(\\w+)\\s+\\1\\b"); + ReggieMatcher matcher = RuntimeCompiler.compile("(\\w+)\\s+\\1"); MatchResult result = matcher.findMatch("say hello hello now"); assertNotNull(result, "findMatch should locate the repeated word"); @@ -228,7 +233,7 @@ public void testRepeatedWord_findMatch() { @Test public void testRepeatedWord_notFound() { - ReggieMatcher matcher = RuntimeCompiler.compile("\\b(\\w+)\\s+\\1\\b"); + ReggieMatcher matcher = RuntimeCompiler.compile("(\\w+)\\s+\\1"); assertNull(matcher.findMatch("hello world foo"), "no repeated word should return null"); } From 024f8c48b64cc1e3d98702e132d12e74ca8a65f7 Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Tue, 7 Jul 2026 19:38:36 +0200 Subject: [PATCH 3/4] test: cover remaining PINNED_BACKREFERENCE detector branches Adds coverage for the non-capturing-group separator unwrap, lazy separator rejection, and nested-capturing-group rejections flagged as uncovered by codecov. Co-Authored-By: Claude Sonnet 5 --- .../PinnedBackreferenceDetectionTest.java | 29 +++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/reggie-codegen/src/test/java/com/datadoghq/reggie/codegen/analysis/PinnedBackreferenceDetectionTest.java b/reggie-codegen/src/test/java/com/datadoghq/reggie/codegen/analysis/PinnedBackreferenceDetectionTest.java index 0e4535b6..9bf65a6a 100644 --- a/reggie-codegen/src/test/java/com/datadoghq/reggie/codegen/analysis/PinnedBackreferenceDetectionTest.java +++ b/reggie-codegen/src/test/java/com/datadoghq/reggie/codegen/analysis/PinnedBackreferenceDetectionTest.java @@ -115,4 +115,33 @@ void rejectsZeroMinLengthSeparator() throws Exception { // begins, so this must be rejected rather than accepted with a bogus zero-length separator. assertNull(detect("(\\w+)\\s*\\1")); } + + @Test + void detectsSeparatorWrappedInNonCapturingGroup() throws Exception { + // (?:\s+) wraps the separator's quantifier without changing what's matched, so it must be + // unwrapped to read the real bounds instead of being treated as a single fixed occurrence. + PinnedBackreferenceInfo info = detect("(\\w+)(?:\\s+)\\1"); + assertNotNull(info); + } + + @Test + void rejectsLazySeparatorQuantifier() throws Exception { + // The codegen's separator scan always consumes the longest available run; a lazy quantifier + // (\s+?) has the opposite semantics, so it must be rejected rather than approximated. + assertNull(detect("(\\w+)\\s+?\\1")); + } + + @Test + void rejectsNestedCapturingGroupInSeparator() throws Exception { + // A capturing group nested inside the separator (even wrapped in a non-capturing group) + // would be silently unaccounted for by totalGroupCount(), so it must be rejected. + assertNull(detect("(\\w+)(?:(x)+)\\1")); + } + + @Test + void rejectsNestedCapturingGroupInPinnedGroupBody() throws Exception { + // A capturing group nested inside the pinned group's quantified body would likewise be + // silently unaccounted for by totalGroupCount(), so it must be rejected. + assertNull(detect("((a)+)\\s+\\1")); + } } From 8d581979ab4ecb92ff199bbd0403f0a90eb02f4f Mon Sep 17 00:00:00 2001 From: Jaroslav Bachorik Date: Tue, 7 Jul 2026 20:22:32 +0200 Subject: [PATCH 4/4] fix: separator length bounds must be char count, not repetition count generateSeparatorLengthCheck compares a scanned character count against separatorMinLength/separatorMaxLength, but detectPinnedBackreference set those from the raw quantifier repetition count (e.g. 2 for (?:--){2} instead of 4 chars), rejecting valid matches like ab----ab. Co-Authored-By: Claude Sonnet 5 --- .../codegen/analysis/PatternAnalyzer.java | 73 +++++++++++++++++-- .../PinnedBackreferenceDetectionTest.java | 12 +++ .../PinnedBackreferenceMatchResultTest.java | 21 ++++++ 3 files changed, 100 insertions(+), 6 deletions(-) diff --git a/reggie-codegen/src/main/java/com/datadoghq/reggie/codegen/analysis/PatternAnalyzer.java b/reggie-codegen/src/main/java/com/datadoghq/reggie/codegen/analysis/PatternAnalyzer.java index 7df32777..a8e40b43 100644 --- a/reggie-codegen/src/main/java/com/datadoghq/reggie/codegen/analysis/PatternAnalyzer.java +++ b/reggie-codegen/src/main/java/com/datadoghq/reggie/codegen/analysis/PatternAnalyzer.java @@ -2638,6 +2638,41 @@ private CharSet getFirstCharSet(RegexNode node) { return null; // Unknown node type - be conservative } + /** + * Fixed number of characters {@code node} always consumes, provided every character it can + * produce belongs to {@code charSet}. The pinned-backreference codegen's separator scan is a flat + * run of {@code charSet} characters with no notion of grouping, so a quantified atom (e.g. the + * {@code --} in {@code (?:--){2}}) can only be converted from a repetition count to a character + * count if its own characters are indistinguishable from that flat run. Returns -1 if the length + * isn't fixed or a character falls outside {@code charSet}. + */ + private int getFixedLengthInCharSet(RegexNode node, CharSet charSet) { + if (node instanceof LiteralNode) { + return charSet.contains(((LiteralNode) node).ch) ? 1 : -1; + } + if (node instanceof CharClassNode) { + CharClassNode charClass = (CharClassNode) node; + CharSet nodeSet = charClass.negated ? charClass.chars.complement() : charClass.chars; + return nodeSet.minus(charSet).isEmpty() ? 1 : -1; + } + if (node instanceof GroupNode) { + GroupNode group = (GroupNode) node; + return group.capturing ? -1 : getFixedLengthInCharSet(group.child, charSet); + } + if (node instanceof ConcatNode) { + int total = 0; + for (RegexNode child : ((ConcatNode) node).children) { + int len = getFixedLengthInCharSet(child, charSet); + if (len < 0) { + return -1; + } + total += len; + } + return total; + } + return -1; + } + private boolean isNullable(RegexNode node) { if (node instanceof QuantifierNode) { return ((QuantifierNode) node).min == 0; @@ -3631,6 +3666,9 @@ private PinnedBackreferenceInfo detectPinnedBackreference(RegexNode ast) { return null; } separatorCharSet = getFirstCharSet(separator); + if (separatorCharSet == null) { + return null; + } // The scan can't backtrack, so it always consumes the longest run of separator-charset // chars available; that's only correct if the separator's quantifier bounds either equal @@ -3647,24 +3685,47 @@ private PinnedBackreferenceInfo detectPinnedBackreference(RegexNode ast) { sepQuantCandidate = sepGroup.child; } } + RegexNode sepAtom; + int sepRepMin; + int sepRepMax; if (sepQuantCandidate instanceof QuantifierNode) { QuantifierNode sepQuant = (QuantifierNode) sepQuantCandidate; if (!sepQuant.greedy) { return null; } - separatorMinLength = sepQuant.min; - separatorMaxLength = sepQuant.max; + sepAtom = sepQuant.child; + sepRepMin = sepQuant.min; + sepRepMax = sepQuant.max; } else { // No quantifier wrapping the separator node means exactly one occurrence. + sepAtom = sepQuantCandidate; + sepRepMin = 1; + sepRepMax = 1; + } + if (separatorCharSet.isEmpty()) { + // Zero-width separator (e.g. \b): the codegen has no code path to evaluate the anchor + // itself, and its scan of an empty charset always consumes 0 chars, so pin the bounds to + // something that scan can never satisfy - the pattern is unsatisfiable anyway, since the + // group and backreference charsets can't both be disjoint from and equal to each other. separatorMinLength = 1; separatorMaxLength = 1; - } - if (separatorMinLength < 1) { - return null; + } else { + // The codegen's separator scan reports a character count, not a repetition count, so the + // quantifier's bounds must be converted via the atom's own fixed character width (e.g. 2 + // for the "--" in (?:--){2}) - reject if that width can't be established. + int sepAtomLength = getFixedLengthInCharSet(sepAtom, separatorCharSet); + if (sepAtomLength < 1) { + return null; + } + separatorMinLength = sepRepMin * sepAtomLength; + separatorMaxLength = sepRepMax == -1 ? -1 : sepRepMax * sepAtomLength; + if (separatorMinLength < 1) { + return null; + } } // Disjointness condition 2: separator vs. group content. - if (separatorCharSet == null || !groupCharSet.isDisjoint(separatorCharSet)) { + if (!groupCharSet.isDisjoint(separatorCharSet)) { return null; } } diff --git a/reggie-codegen/src/test/java/com/datadoghq/reggie/codegen/analysis/PinnedBackreferenceDetectionTest.java b/reggie-codegen/src/test/java/com/datadoghq/reggie/codegen/analysis/PinnedBackreferenceDetectionTest.java index 9bf65a6a..d8747e2f 100644 --- a/reggie-codegen/src/test/java/com/datadoghq/reggie/codegen/analysis/PinnedBackreferenceDetectionTest.java +++ b/reggie-codegen/src/test/java/com/datadoghq/reggie/codegen/analysis/PinnedBackreferenceDetectionTest.java @@ -15,6 +15,7 @@ */ package com.datadoghq.reggie.codegen.analysis; +import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNull; @@ -144,4 +145,15 @@ void rejectsNestedCapturingGroupInPinnedGroupBody() throws Exception { // silently unaccounted for by totalGroupCount(), so it must be rejected. assertNull(detect("((a)+)\\s+\\1")); } + + @Test + void detectsMultiCharLiteralSeparatorWithExplicitRepetitionCount() throws Exception { + // (?:--){2} matches exactly 4 dashes; separatorMinLength/separatorMaxLength must be the + // resulting character count (4), not the raw repetition count (2), since the codegen's + // separator scan measures characters. + PinnedBackreferenceInfo info = detect("(\\w+)(?:--){2}\\1"); + assertNotNull(info); + assertEquals(4, info.separatorMinLength); + assertEquals(4, info.separatorMaxLength); + } } diff --git a/reggie-runtime/src/test/java/com/datadoghq/reggie/runtime/PinnedBackreferenceMatchResultTest.java b/reggie-runtime/src/test/java/com/datadoghq/reggie/runtime/PinnedBackreferenceMatchResultTest.java index aa6d1e3a..efbb4a11 100644 --- a/reggie-runtime/src/test/java/com/datadoghq/reggie/runtime/PinnedBackreferenceMatchResultTest.java +++ b/reggie-runtime/src/test/java/com/datadoghq/reggie/runtime/PinnedBackreferenceMatchResultTest.java @@ -282,4 +282,25 @@ public void testCharsetSeparator_mismatch() { ReggieMatcher matcher = RuntimeCompiler.compile("([a-z]+)\\d+\\1"); assertNull(matcher.match("ab12cd"), "match(\"ab12cd\") should return null"); } + + // ---- (\w+)(?:--){2}\1 : multi-char literal separator with explicit repetition count ---- + // (?:--){2} matches exactly 4 dashes; separator length bounds must be tracked in characters, + // not in repetitions of the "--" atom, or a valid 4-dash separator is wrongly rejected. + + @Test + public void testMultiCharSeparatorWithRepetitionCount_match() { + ReggieMatcher matcher = RuntimeCompiler.compile("(\\w+)(?:--){2}\\1"); + MatchResult result = matcher.match("ab----ab"); + + assertNotNull(result, "match(\"ab----ab\") should succeed (4 dashes = (?:--){2})"); + assertEquals("ab----ab", result.group(0)); + assertEquals("ab", result.group(1)); + } + + @Test + public void testMultiCharSeparatorWithRepetitionCount_mismatchWrongDashCount() { + ReggieMatcher matcher = RuntimeCompiler.compile("(\\w+)(?:--){2}\\1"); + assertNull(matcher.match("ab--ab"), "2 dashes should not satisfy (?:--){2} (needs 4)"); + assertNull(matcher.match("ab------ab"), "6 dashes should not satisfy (?:--){2} (needs 4)"); + } }