Skip to content

Commit 4c46a60

Browse files
aksOpsclaude
andauthored
fix(detector): eliminate regex backtracking in Liquibase YAML patterns (#61)
SonarCloud S5998 (reliability: stack overflow on large input) + S5852 (security hotspot: ReDoS) both flagged the reluctant-outer quantifier `(?:\s++[^\n]*+\n)*?` in LQ_CREATE_TABLE_YAML and LQ_ADD_FK_YAML. The engine was free to backtrack through the reluctant `*?`. Rewrite with negative-lookahead + possessive `*+`: intermediate lines that would otherwise match the target key (tableName / baseTableName / referencedTableName) are excluded up front, so the outer match terminates deterministically exactly where the reluctant version did. No semantic change for valid Liquibase YAML. Adds a regression test exercising a 500-line pathological createTable block — completes in <1s on the rewritten patterns; the pre-fix reluctant walk would have scaled quadratically. Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent bfab2e7 commit 4c46a60

2 files changed

Lines changed: 47 additions & 3 deletions

File tree

src/main/java/io/github/randomcodespace/iq/detector/sql/SqlMigrationDetector.java

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -142,12 +142,20 @@ public class SqlMigrationDetector extends AbstractRegexDetector {
142142
+ "[^>]*?\\breferencedTableName\\s*+=\\s*+\"(\\w++)\"");
143143

144144
// -- Liquibase YAML patterns (regex-based; avoids pulling in SnakeYAML here) --
145+
// Intermediate lines use a negative-lookahead + possessive `*+` instead of reluctant `*?`:
146+
// no backtracking (prevents SonarCloud S5998 stack-overflow / S5852 ReDoS), same
147+
// semantics — lines that *would* match the target key are excluded from the skip group,
148+
// so the outer match terminates exactly where the reluctant version would have.
145149
private static final Pattern LQ_CREATE_TABLE_YAML = Pattern.compile(
146-
"createTable\\s*+:[^\\n]*+\\n(?:\\s++[^\\n]*+\\n)*?\\s++tableName\\s*+:\\s*+([\\w\"']++)");
150+
"createTable\\s*+:[^\\n]*+\\n"
151+
+ "(?:(?!\\s++tableName\\s*+:)\\s++[^\\n]*+\\n)*+"
152+
+ "\\s++tableName\\s*+:\\s*+([\\w\"']++)");
147153
private static final Pattern LQ_ADD_FK_YAML = Pattern.compile(
148154
"addForeignKeyConstraint\\s*+:[^\\n]*+\\n"
149-
+ "(?:\\s++[^\\n]*+\\n)*?\\s++baseTableName\\s*+:\\s*+([\\w\"']++)[^\\n]*+\\n"
150-
+ "(?:\\s++[^\\n]*+\\n)*?\\s++referencedTableName\\s*+:\\s*+([\\w\"']++)");
155+
+ "(?:(?!\\s++baseTableName\\s*+:)\\s++[^\\n]*+\\n)*+"
156+
+ "\\s++baseTableName\\s*+:\\s*+([\\w\"']++)[^\\n]*+\\n"
157+
+ "(?:(?!\\s++referencedTableName\\s*+:)\\s++[^\\n]*+\\n)*+"
158+
+ "\\s++referencedTableName\\s*+:\\s*+([\\w\"']++)");
151159

152160
// -- Flyway version parsing --
153161
private static final Pattern FLYWAY_VERSION = Pattern.compile(

src/test/java/io/github/randomcodespace/iq/detector/sql/SqlMigrationDetectorTest.java

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -209,6 +209,42 @@ void liquibaseYamlChangeSetDetected() {
209209
&& e.getTarget().getId().endsWith(":invoices")));
210210
}
211211

212+
@Test
213+
void liquibaseYamlChangeSet_largeIntermediateContent_completesQuickly() {
214+
// Regression guard for SonarCloud S5998 / S5852 on LQ_*_YAML: the reluctant-outer
215+
// `(?:\s++[^\n]*+\n)*?` patterns were rewritten with a negative-lookahead +
216+
// possessive outer so stack/runtime cannot blow up on many indented lines.
217+
// Pathological input: a createTable with 500 indented column lines preceding
218+
// tableName — the pre-fix reluctant walk would scale quadratically.
219+
StringBuilder columns = new StringBuilder();
220+
for (int i = 0; i < 500; i++) {
221+
columns.append(" - column_").append(i).append(": stub_value\n");
222+
}
223+
String yaml = """
224+
databaseChangeLog:
225+
- changeSet:
226+
id: 1
227+
author: bob
228+
changes:
229+
- createTable:
230+
"""
231+
+ columns
232+
+ " tableName: wide_table\n";
233+
DetectorContext ctx = new DetectorContext(
234+
"src/main/resources/db/db.changelog-wide.yml", "yaml", yaml);
235+
236+
long start = System.nanoTime();
237+
DetectorResult result = detector.detect(ctx);
238+
long elapsedMs = (System.nanoTime() - start) / 1_000_000;
239+
240+
assertTrue(hasEntity(sqlEntitiesOf(result), "wide_table", "table"),
241+
"wide_table must still be detected after the anti-backtracking rewrite");
242+
// 2s is 100x the observed fast-path time; we only care that it doesn't blow up
243+
// exponentially — the exact threshold isn't load-bearing.
244+
assertTrue(elapsedMs < 2000,
245+
"detection of wide_table should complete in under 2s, was " + elapsedMs + "ms");
246+
}
247+
212248
// -- Positive: Rails --
213249

214250
@Test

0 commit comments

Comments
 (0)