Skip to content

Commit d08019d

Browse files
jbachorikjbachorik
committed
fix: restore CharSet.WHITESPACE, ] scanner, executor lifecycle, tests
- Restore \u000B in CharSet.WHITESPACE - Restore hasSourceInlineModifier ] literal handling - Remove redundant .*}/.{ early check - Restore try-with-resources executor lifecycle in concurrency tests - Restore coverageRecursesThroughMultipleNestedOptionalLevels and quotedDelimiterCaptureFailsWhenClosingDelimiterIsMissing tests
1 parent 71fed0d commit d08019d

5 files changed

Lines changed: 84 additions & 62 deletions

File tree

‎reggie-codegen/src/main/java/com/datadoghq/reggie/codegen/automaton/CharSet.java‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,7 @@ public static CharSet fromRanges(List<Range> ranges) {
169169
new Range(' ', ' '),
170170
new Range('\t', '\t'),
171171
new Range('\n', '\n'),
172+
new Range('\u000B', '\u000B'),
172173
new Range('\r', '\r'),
173174
new Range('\f', '\f')));
174175

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

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,14 @@ void coverageIncludesCapturesNestedInOptionalSequences() throws Exception {
7474
assertFalse(plan.coversCaptureIndexes(List.of(2)));
7575
}
7676

77+
@Test
78+
void coverageRecursesThroughMultipleNestedOptionalLevels() throws Exception {
79+
LinearTokenSequencePlan plan = planFor("(?:(?:(?:(?<deep>\\d+))|)|)");
80+
81+
assertTrue(plan.coversCaptureIndexes(List.of(1)));
82+
assertFalse(plan.coversCaptureIndexes(List.of(2)));
83+
}
84+
7785
@Test
7886
void coverageRequiresEveryNamedCaptureNotOnlyTheLargestGroupNumber() throws Exception {
7987
RegexParser parser = new RegexParser();

‎reggie-runtime/src/main/java/com/datadoghq/reggie/runtime/RuntimeCompiler.java‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -603,6 +603,15 @@ static boolean hasSourceInlineModifier(String source) {
603603
if (ch == '[') {
604604
inCharacterClass = true;
605605
index++;
606+
// In Java regex, ']' as the first character of a class (or first after '^')
607+
// is a literal, not the class terminator. Skip it so the scanner does not
608+
// exit the class prematurely.
609+
if (index < source.length() && source.charAt(index) == '^') {
610+
index++;
611+
}
612+
if (index < source.length() && source.charAt(index) == ']') {
613+
index++;
614+
}
606615
continue;
607616
}
608617
if (ch == '(' && index + 1 < source.length() && source.charAt(index + 1) == '?') {
@@ -985,9 +994,6 @@ private static NamedOnlyLtsCompilation admitNamedOnlyLinearTokenSequence(
985994
RegexNode ast,
986995
Map<String, Integer> nameMap,
987996
LinearTokenSequenceAdmission admission) {
988-
if ((pattern.contains(".*") || pattern.contains(".{")) && !admission.isDotAll()) {
989-
return NamedOnlyLtsCompilation.rejected(NamedOnlyLtsRejection.PROFILE_INELIGIBLE);
990-
}
991997
LinearTokenSequencePlan plan =
992998
LinearTokenSequencePlan.from(PatternCategorizer.categorize(ast)).orElse(null);
993999
if (plan == null) {

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

Lines changed: 59 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -67,37 +67,37 @@ void cachedNamedOnlyLinearTokenSequenceMatcherIsSafeForConcurrentUse() throws Ex
6767

6868
int threads = 16;
6969
int iterations = 1_000;
70-
ExecutorService executor = Executors.newFixedThreadPool(threads);
7170
CountDownLatch ready = new CountDownLatch(threads);
7271
CountDownLatch start = new CountDownLatch(1);
7372
CountDownLatch done = new CountDownLatch(threads);
7473
ConcurrentLinkedQueue<Throwable> failures = new ConcurrentLinkedQueue<>();
7574

76-
for (int thread = 0; thread < threads; thread++) {
77-
executor.execute(
78-
() -> {
79-
ready.countDown();
80-
try {
81-
start.await();
82-
for (int iteration = 0; iteration < iterations; iteration++) {
83-
assertMatchWithOptionalCaptures(shared, groupNumbers);
84-
assertMatchWithoutOptionalCaptures(shared, groupNumbers);
85-
assertFailedMatchLeavesArraysUntouched(shared, groupCount);
86-
assertTrue(shared.find("noise " + WITH_OPTIONAL_CAPTURES));
87-
assertFalse(shared.find("noise malformed access log"));
75+
try (ExecutorService executor = Executors.newFixedThreadPool(threads)) {
76+
for (int thread = 0; thread < threads; thread++) {
77+
executor.execute(
78+
() -> {
79+
ready.countDown();
80+
try {
81+
start.await();
82+
for (int iteration = 0; iteration < iterations; iteration++) {
83+
assertMatchWithOptionalCaptures(shared, groupNumbers);
84+
assertMatchWithoutOptionalCaptures(shared, groupNumbers);
85+
assertFailedMatchLeavesArraysUntouched(shared, groupCount);
86+
assertTrue(shared.find("noise " + WITH_OPTIONAL_CAPTURES));
87+
assertFalse(shared.find("noise malformed access log"));
88+
}
89+
} catch (Throwable failure) {
90+
failures.add(failure);
91+
} finally {
92+
done.countDown();
8893
}
89-
} catch (Throwable failure) {
90-
failures.add(failure);
91-
} finally {
92-
done.countDown();
93-
}
94-
});
95-
}
94+
});
95+
}
9696

97-
assertTrue(ready.await(10, TimeUnit.SECONDS), "workers did not become ready");
98-
start.countDown();
99-
assertTrue(done.await(30, TimeUnit.SECONDS), "workers did not finish");
100-
executor.shutdownNow();
97+
assertTrue(ready.await(10, TimeUnit.SECONDS), "workers did not become ready");
98+
start.countDown();
99+
assertTrue(done.await(30, TimeUnit.SECONDS), "workers did not finish");
100+
}
101101
assertTrue(failures.isEmpty(), () -> "concurrent LTS failure: " + failures.peek());
102102
}
103103

@@ -112,48 +112,48 @@ void cachedLinearTokenSequenceMatcherSafelyRollsBackNestedOptionalSequences() th
112112

113113
int threads = 16;
114114
int iterations = 1_000;
115-
ExecutorService executor = Executors.newFixedThreadPool(threads);
116115
CountDownLatch ready = new CountDownLatch(threads);
117116
CountDownLatch start = new CountDownLatch(1);
118117
CountDownLatch done = new CountDownLatch(threads);
119118
ConcurrentLinkedQueue<Throwable> failures = new ConcurrentLinkedQueue<>();
120119

121-
for (int thread = 0; thread < threads; thread++) {
122-
executor.execute(
123-
() -> {
124-
ready.countDown();
125-
try {
126-
start.await();
127-
for (int iteration = 0; iteration < iterations; iteration++) {
128-
assertTrue(shared.matches("abc"));
129-
assertTrue(shared.matches("ac"));
130-
assertTrue(shared.matches("c"));
131-
assertFalse(shared.matches("ab"));
132-
assertNotNull(shared.match("abc"));
133-
assertNotNull(shared.match("ac"));
134-
assertNotNull(shared.match("c"));
135-
assertNull(shared.match("ab"));
136-
assertTrue(shared.find("noise abc"));
137-
assertFalse(shared.find("noise ab"));
138-
139-
int[] starts = {17};
140-
int[] ends = {19};
141-
assertFalse(shared.matchInto("ab", starts, ends));
142-
assertEquals(17, starts[0]);
143-
assertEquals(19, ends[0]);
120+
try (ExecutorService executor = Executors.newFixedThreadPool(threads)) {
121+
for (int thread = 0; thread < threads; thread++) {
122+
executor.execute(
123+
() -> {
124+
ready.countDown();
125+
try {
126+
start.await();
127+
for (int iteration = 0; iteration < iterations; iteration++) {
128+
assertTrue(shared.matches("abc"));
129+
assertTrue(shared.matches("ac"));
130+
assertTrue(shared.matches("c"));
131+
assertFalse(shared.matches("ab"));
132+
assertNotNull(shared.match("abc"));
133+
assertNotNull(shared.match("ac"));
134+
assertNotNull(shared.match("c"));
135+
assertNull(shared.match("ab"));
136+
assertTrue(shared.find("noise abc"));
137+
assertFalse(shared.find("noise ab"));
138+
139+
int[] starts = {17};
140+
int[] ends = {19};
141+
assertFalse(shared.matchInto("ab", starts, ends));
142+
assertEquals(17, starts[0]);
143+
assertEquals(19, ends[0]);
144+
}
145+
} catch (Throwable failure) {
146+
failures.add(failure);
147+
} finally {
148+
done.countDown();
144149
}
145-
} catch (Throwable failure) {
146-
failures.add(failure);
147-
} finally {
148-
done.countDown();
149-
}
150-
});
151-
}
150+
});
151+
}
152152

153-
assertTrue(ready.await(10, TimeUnit.SECONDS), "workers did not become ready");
154-
start.countDown();
155-
assertTrue(done.await(30, TimeUnit.SECONDS), "workers did not finish");
156-
executor.shutdownNow();
153+
assertTrue(ready.await(10, TimeUnit.SECONDS), "workers did not become ready");
154+
start.countDown();
155+
assertTrue(done.await(30, TimeUnit.SECONDS), "workers did not finish");
156+
}
157157
assertTrue(
158158
failures.isEmpty(), () -> "concurrent nested-optional LTS failure: " + failures.peek());
159159
}

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,13 @@ void handlesQuotedDelimiterCaptures() throws Exception {
6262
assertEquals("https://example.com/index.html", result.group("referer"));
6363
}
6464

65+
@Test
66+
void quotedDelimiterCaptureFailsWhenClosingDelimiterIsMissing() throws Exception {
67+
ReggieMatcher matcher = matcherFor("referer=\"(?<referer>[^\"]*)\"");
68+
69+
assertNull(matcher.match("referer=\"https://example.com/index.html"));
70+
}
71+
6572
@Test
6673
void matchReturnsNullWhenSequenceDoesNotMatch() throws Exception {
6774
ReggieMatcher matcher = matcherFor("host=(?<host>\\S+) status=(?<status>[+-]?\\d+)");

0 commit comments

Comments
 (0)