Skip to content

Commit 8d58197

Browse files
jbachorikclaude
andcommitted
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 <noreply@anthropic.com>
1 parent 024f8c4 commit 8d58197

3 files changed

Lines changed: 100 additions & 6 deletions

File tree

‎reggie-codegen/src/main/java/com/datadoghq/reggie/codegen/analysis/PatternAnalyzer.java‎

Lines changed: 67 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2638,6 +2638,41 @@ private CharSet getFirstCharSet(RegexNode node) {
26382638
return null; // Unknown node type - be conservative
26392639
}
26402640

2641+
/**
2642+
* Fixed number of characters {@code node} always consumes, provided every character it can
2643+
* produce belongs to {@code charSet}. The pinned-backreference codegen's separator scan is a flat
2644+
* run of {@code charSet} characters with no notion of grouping, so a quantified atom (e.g. the
2645+
* {@code --} in {@code (?:--){2}}) can only be converted from a repetition count to a character
2646+
* count if its own characters are indistinguishable from that flat run. Returns -1 if the length
2647+
* isn't fixed or a character falls outside {@code charSet}.
2648+
*/
2649+
private int getFixedLengthInCharSet(RegexNode node, CharSet charSet) {
2650+
if (node instanceof LiteralNode) {
2651+
return charSet.contains(((LiteralNode) node).ch) ? 1 : -1;
2652+
}
2653+
if (node instanceof CharClassNode) {
2654+
CharClassNode charClass = (CharClassNode) node;
2655+
CharSet nodeSet = charClass.negated ? charClass.chars.complement() : charClass.chars;
2656+
return nodeSet.minus(charSet).isEmpty() ? 1 : -1;
2657+
}
2658+
if (node instanceof GroupNode) {
2659+
GroupNode group = (GroupNode) node;
2660+
return group.capturing ? -1 : getFixedLengthInCharSet(group.child, charSet);
2661+
}
2662+
if (node instanceof ConcatNode) {
2663+
int total = 0;
2664+
for (RegexNode child : ((ConcatNode) node).children) {
2665+
int len = getFixedLengthInCharSet(child, charSet);
2666+
if (len < 0) {
2667+
return -1;
2668+
}
2669+
total += len;
2670+
}
2671+
return total;
2672+
}
2673+
return -1;
2674+
}
2675+
26412676
private boolean isNullable(RegexNode node) {
26422677
if (node instanceof QuantifierNode) {
26432678
return ((QuantifierNode) node).min == 0;
@@ -3631,6 +3666,9 @@ private PinnedBackreferenceInfo detectPinnedBackreference(RegexNode ast) {
36313666
return null;
36323667
}
36333668
separatorCharSet = getFirstCharSet(separator);
3669+
if (separatorCharSet == null) {
3670+
return null;
3671+
}
36343672

36353673
// The scan can't backtrack, so it always consumes the longest run of separator-charset
36363674
// chars available; that's only correct if the separator's quantifier bounds either equal
@@ -3647,24 +3685,47 @@ private PinnedBackreferenceInfo detectPinnedBackreference(RegexNode ast) {
36473685
sepQuantCandidate = sepGroup.child;
36483686
}
36493687
}
3688+
RegexNode sepAtom;
3689+
int sepRepMin;
3690+
int sepRepMax;
36503691
if (sepQuantCandidate instanceof QuantifierNode) {
36513692
QuantifierNode sepQuant = (QuantifierNode) sepQuantCandidate;
36523693
if (!sepQuant.greedy) {
36533694
return null;
36543695
}
3655-
separatorMinLength = sepQuant.min;
3656-
separatorMaxLength = sepQuant.max;
3696+
sepAtom = sepQuant.child;
3697+
sepRepMin = sepQuant.min;
3698+
sepRepMax = sepQuant.max;
36573699
} else {
36583700
// No quantifier wrapping the separator node means exactly one occurrence.
3701+
sepAtom = sepQuantCandidate;
3702+
sepRepMin = 1;
3703+
sepRepMax = 1;
3704+
}
3705+
if (separatorCharSet.isEmpty()) {
3706+
// Zero-width separator (e.g. \b): the codegen has no code path to evaluate the anchor
3707+
// itself, and its scan of an empty charset always consumes 0 chars, so pin the bounds to
3708+
// something that scan can never satisfy - the pattern is unsatisfiable anyway, since the
3709+
// group and backreference charsets can't both be disjoint from and equal to each other.
36593710
separatorMinLength = 1;
36603711
separatorMaxLength = 1;
3661-
}
3662-
if (separatorMinLength < 1) {
3663-
return null;
3712+
} else {
3713+
// The codegen's separator scan reports a character count, not a repetition count, so the
3714+
// quantifier's bounds must be converted via the atom's own fixed character width (e.g. 2
3715+
// for the "--" in (?:--){2}) - reject if that width can't be established.
3716+
int sepAtomLength = getFixedLengthInCharSet(sepAtom, separatorCharSet);
3717+
if (sepAtomLength < 1) {
3718+
return null;
3719+
}
3720+
separatorMinLength = sepRepMin * sepAtomLength;
3721+
separatorMaxLength = sepRepMax == -1 ? -1 : sepRepMax * sepAtomLength;
3722+
if (separatorMinLength < 1) {
3723+
return null;
3724+
}
36643725
}
36653726

36663727
// Disjointness condition 2: separator vs. group content.
3667-
if (separatorCharSet == null || !groupCharSet.isDisjoint(separatorCharSet)) {
3728+
if (!groupCharSet.isDisjoint(separatorCharSet)) {
36683729
return null;
36693730
}
36703731
}

‎reggie-codegen/src/test/java/com/datadoghq/reggie/codegen/analysis/PinnedBackreferenceDetectionTest.java‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
*/
1616
package com.datadoghq.reggie.codegen.analysis;
1717

18+
import static org.junit.jupiter.api.Assertions.assertEquals;
1819
import static org.junit.jupiter.api.Assertions.assertNotNull;
1920
import static org.junit.jupiter.api.Assertions.assertNull;
2021

@@ -144,4 +145,15 @@ void rejectsNestedCapturingGroupInPinnedGroupBody() throws Exception {
144145
// silently unaccounted for by totalGroupCount(), so it must be rejected.
145146
assertNull(detect("((a)+)\\s+\\1"));
146147
}
148+
149+
@Test
150+
void detectsMultiCharLiteralSeparatorWithExplicitRepetitionCount() throws Exception {
151+
// (?:--){2} matches exactly 4 dashes; separatorMinLength/separatorMaxLength must be the
152+
// resulting character count (4), not the raw repetition count (2), since the codegen's
153+
// separator scan measures characters.
154+
PinnedBackreferenceInfo info = detect("(\\w+)(?:--){2}\\1");
155+
assertNotNull(info);
156+
assertEquals(4, info.separatorMinLength);
157+
assertEquals(4, info.separatorMaxLength);
158+
}
147159
}

‎reggie-runtime/src/test/java/com/datadoghq/reggie/runtime/PinnedBackreferenceMatchResultTest.java‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,4 +282,25 @@ public void testCharsetSeparator_mismatch() {
282282
ReggieMatcher matcher = RuntimeCompiler.compile("([a-z]+)\\d+\\1");
283283
assertNull(matcher.match("ab12cd"), "match(\"ab12cd\") should return null");
284284
}
285+
286+
// ---- (\w+)(?:--){2}\1 : multi-char literal separator with explicit repetition count ----
287+
// (?:--){2} matches exactly 4 dashes; separator length bounds must be tracked in characters,
288+
// not in repetitions of the "--" atom, or a valid 4-dash separator is wrongly rejected.
289+
290+
@Test
291+
public void testMultiCharSeparatorWithRepetitionCount_match() {
292+
ReggieMatcher matcher = RuntimeCompiler.compile("(\\w+)(?:--){2}\\1");
293+
MatchResult result = matcher.match("ab----ab");
294+
295+
assertNotNull(result, "match(\"ab----ab\") should succeed (4 dashes = (?:--){2})");
296+
assertEquals("ab----ab", result.group(0));
297+
assertEquals("ab", result.group(1));
298+
}
299+
300+
@Test
301+
public void testMultiCharSeparatorWithRepetitionCount_mismatchWrongDashCount() {
302+
ReggieMatcher matcher = RuntimeCompiler.compile("(\\w+)(?:--){2}\\1");
303+
assertNull(matcher.match("ab--ab"), "2 dashes should not satisfy (?:--){2} (needs 4)");
304+
assertNull(matcher.match("ab------ab"), "6 dashes should not satisfy (?:--){2} (needs 4)");
305+
}
285306
}

0 commit comments

Comments
 (0)