diff --git a/GitBranchStateCache.Tests/Upstreams/RepositoryPatternTests.cs b/GitBranchStateCache.Tests/Upstreams/RepositoryPatternTests.cs
index 8a76fff..5299819 100644
--- a/GitBranchStateCache.Tests/Upstreams/RepositoryPatternTests.cs
+++ b/GitBranchStateCache.Tests/Upstreams/RepositoryPatternTests.cs
@@ -60,4 +60,54 @@ public void Matches_DotIsLiteral()
Assert.IsFalse(pattern!.Matches("studio/gameXgit"));
}
+
+ [TestMethod]
+ [DataRow("studio/*-*-*", "studio", 1000)]
+ [DataRow("studio/*-*", "studio", 4000)]
+ [DataRow("team/*-svc-*", "team", 4000)]
+ public void Matches_SeveralStarsInOneSegment_RejectsALongNonMatchingPathWithoutTimingOut(string text, string owner, int dashes)
+ {
+ // Two stars in one segment are adjacent [^/]* runs, which a backtracking engine takes
+ // polynomial time to reject. The allow-list runs before admission, so an unauthenticated
+ // caller could otherwise time the match out and get a 500 instead of a 404 (#111).
+ Assert.IsTrue(RepositoryPattern.TryParse(text, out RepositoryPattern? pattern, out _));
+ string path = $"{owner}/{new string('-', dashes)}/x";
+
+ // Warm the expression up on a short path first, so the timing below measures the match rather
+ // than the engine building its automaton.
+ _ = pattern!.Matches($"{owner}/a/x");
+
+ System.Diagnostics.Stopwatch stopwatch = System.Diagnostics.Stopwatch.StartNew();
+ bool matched = pattern.Matches(path);
+ stopwatch.Stop();
+
+ Assert.IsFalse(matched);
+ Assert.IsLessThan(50, stopwatch.ElapsedMilliseconds);
+ }
+
+ [TestMethod]
+ public void Matches_SeveralStarsInOneSegment_StillMatchesWhatItShould()
+ {
+ Assert.IsTrue(RepositoryPattern.TryParse("team/*-svc-*", out RepositoryPattern? pattern, out _));
+
+ Assert.IsTrue(pattern!.Matches("team/billing-svc-api"));
+ Assert.IsTrue(pattern.Matches("Team/Billing-SVC-Api"));
+ Assert.IsFalse(pattern.Matches("team/billing-api"));
+ Assert.IsFalse(pattern.Matches("team/billing-svc-api/extra"));
+ }
+
+ [TestMethod]
+ public void Matches_WhenTheMatchTimesOut_IsNotAllowedRatherThanThrowing()
+ {
+ // The allow-list runs before admission, so a timeout must read as "not allowed" (a 404) and
+ // never escape as an unhandled RegexMatchTimeoutException (a 500).
+ Assert.IsTrue(RepositoryPattern.TryParse(
+ "studio/**",
+ TimeSpan.FromTicks(1),
+ out RepositoryPattern? pattern,
+ out _));
+ string path = $"studio/{new string('a', 4_000_000)}";
+
+ Assert.IsFalse(pattern!.Matches(path));
+ }
}
diff --git a/GitBranchStateCache/Upstreams/RepositoryPattern.cs b/GitBranchStateCache/Upstreams/RepositoryPattern.cs
index 331aed8..59b8063 100644
--- a/GitBranchStateCache/Upstreams/RepositoryPattern.cs
+++ b/GitBranchStateCache/Upstreams/RepositoryPattern.cs
@@ -18,9 +18,10 @@ public sealed class RepositoryPattern
/// How long a single match may run before it is abandoned.
///
///
- /// The translated patterns have no backtracking construct, so this cannot fire in practice. It is
- /// set because a regular expression built from configuration and run against a request path is
- /// exactly the shape that should carry a timeout.
+ /// The patterns are matched with , which runs in time
+ /// linear in the path, so this should not fire. It is kept because a regular expression built from
+ /// configuration and run against a request path is exactly the shape that should carry a timeout,
+ /// and reads a timeout as no match.
///
private static readonly TimeSpan MatchTimeout = TimeSpan.FromMilliseconds(100);
@@ -48,7 +49,18 @@ private RepositoryPattern(string text, Regex expression)
/// The translated pattern, when it was acceptable.
/// Why the pattern was refused, when it was.
/// when the pattern was acceptable.
- public static bool TryParse(string? pattern, out RepositoryPattern? parsed, out string? failure)
+ public static bool TryParse(string? pattern, out RepositoryPattern? parsed, out string? failure) =>
+ TryParse(pattern, MatchTimeout, out parsed, out failure);
+
+ ///
+ /// Translates a configured pattern with a given match timeout, so a test can reach the timeout path.
+ ///
+ /// The pattern as configured.
+ /// How long a single match may run before it is abandoned.
+ /// The translated pattern, when it was acceptable.
+ /// Why the pattern was refused, when it was.
+ /// when the pattern was acceptable.
+ internal static bool TryParse(string? pattern, TimeSpan matchTimeout, out RepositoryPattern? parsed, out string? failure)
{
parsed = null;
failure = null;
@@ -76,7 +88,7 @@ public static bool TryParse(string? pattern, out RepositoryPattern? parsed, out
return false;
}
- parsed = new RepositoryPattern(pattern, Translate(trimmed));
+ parsed = new RepositoryPattern(pattern, Translate(trimmed, matchTimeout));
return true;
}
@@ -85,7 +97,21 @@ public static bool TryParse(string? pattern, out RepositoryPattern? parsed, out
///
/// The path following the upstream key, without surrounding slashes.
/// when it matches.
- public bool Matches(string repositoryPath) => _expression.IsMatch(repositoryPath);
+ ///
+ /// This runs before admission, on a path an unauthenticated caller chose, so a match that runs out
+ /// of time is refused as not allowed rather than surfacing as a server error.
+ ///
+ public bool Matches(string repositoryPath)
+ {
+ try
+ {
+ return _expression.IsMatch(repositoryPath);
+ }
+ catch (RegexMatchTimeoutException)
+ {
+ return false;
+ }
+ }
private static bool IsLiteral(string segment) =>
segment.Length > 0 && !segment.Contains('*', StringComparison.Ordinal);
@@ -100,11 +126,17 @@ private static bool IsLiteral(string segment) =>
/// replacement for the double contains no backslash, so the second replacement cannot reach inside
/// it.
///
+ /// Two single stars in one segment, as in team/*-svc-*, translate to adjacent
+ /// [^/]* runs, which a backtracking engine takes polynomial time to reject on a long
+ /// segment. The expression is therefore built with ,
+ /// whose matching time is linear in the path whatever the pattern.
+ ///
+ ///
/// Matching is case insensitive because forge repository names are, and a pattern that fails only
/// because someone typed Studio is a support ticket rather than a control.
///
///
- private static Regex Translate(string pattern)
+ private static Regex Translate(string pattern, TimeSpan matchTimeout)
{
string expression = Regex.Escape(pattern)
.Replace(@"\*\*", ".*", StringComparison.Ordinal)
@@ -112,7 +144,7 @@ private static Regex Translate(string pattern)
return new Regex(
$"^{expression}$",
- RegexOptions.IgnoreCase | RegexOptions.CultureInvariant,
- MatchTimeout);
+ RegexOptions.IgnoreCase | RegexOptions.CultureInvariant | RegexOptions.NonBacktracking,
+ matchTimeout);
}
}