Include timeout value in Regex cache key (#49352)

Static Regex methods use a cache of recently used regexes.  We currently include in the key for that cache whether a timeout value was specified, since whether there's a timeout can impact how the regex is built, but we don't currently include the actual value of the timeout.  This leads to oddities where if the same pattern, culture, and options are used with a static regex method and one non-infinite timeout, and then those exact same values are used with a different non-infinite timeout, and then original data was still in the cache, we'll end up using the previous timeout value rather than the new timeout value.  The fix is just to include the timeout itself in the key.

diff --git a/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/Regex.Cache.cs b/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/Regex.Cache.cs
index af0251c..1b24c2f 100644 (file)
--- a/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/Regex.Cache.cs
+++ b/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/Regex.Cache.cs
@@ -110,7 +110,7 @@ namespace System.Text.RegularExpressions
             Regex.ValidatePattern(pattern);
 
             CultureInfo culture = CultureInfo.CurrentCulture;
-            Key key = new Key(pattern, culture.ToString(), RegexOptions.None, hasTimeout: false);
+            Key key = new Key(pattern, culture.ToString(), RegexOptions.None, Regex.s_defaultMatchTimeout);
 
             if (!TryGet(key, out Regex? regex))
             {
@@ -128,7 +128,7 @@ namespace System.Text.RegularExpressions
             Regex.ValidateMatchTimeout(matchTimeout);
 
             CultureInfo culture = (options & RegexOptions.CultureInvariant) != 0 ? CultureInfo.InvariantCulture : CultureInfo.CurrentCulture;
-            Key key = new Key(pattern, culture.ToString(), options, matchTimeout != Regex.InfiniteMatchTimeout);
+            Key key = new Key(pattern, culture.ToString(), options, matchTimeout);
 
             if (!TryGet(key, out Regex? regex))
             {
@@ -265,9 +265,9 @@ namespace System.Text.RegularExpressions
             private readonly string _pattern;
             private readonly string _culture;
             private readonly RegexOptions _options;
-            private readonly bool _hasTimeout;
+            private readonly TimeSpan _matchTimeout;
 
-            public Key(string pattern, string culture, RegexOptions options, bool hasTimeout)
+            public Key(string pattern, string culture, RegexOptions options, TimeSpan matchTimeout)
             {
                 Debug.Assert(pattern != null, "Pattern must be provided");
                 Debug.Assert(culture != null, "Culture must be provided");
@@ -275,7 +275,7 @@ namespace System.Text.RegularExpressions
                 _pattern = pattern;
                 _culture = culture;
                 _options = options;
-                _hasTimeout = hasTimeout;
+                _matchTimeout = matchTimeout;
             }
 
             public override bool Equals([NotNullWhen(true)] object? obj) =>
@@ -285,7 +285,7 @@ namespace System.Text.RegularExpressions
                 _pattern.Equals(other._pattern) &&
                 _culture.Equals(other._culture) &&
                 _options == other._options &&
-                _hasTimeout == other._hasTimeout;
+                _matchTimeout == other._matchTimeout;
 
             public static bool operator ==(Key left, Key right) =>
                 left.Equals(right);

diff --git a/src/libraries/System.Text.RegularExpressions/tests/Regex.Match.Tests.cs b/src/libraries/System.Text.RegularExpressions/tests/Regex.Match.Tests.cs
index 27acb93..d2cd6b4 100644 (file)
--- a/src/libraries/System.Text.RegularExpressions/tests/Regex.Match.Tests.cs
+++ b/src/libraries/System.Text.RegularExpressions/tests/Regex.Match.Tests.cs
@@ -2,6 +2,7 @@
 // The .NET Foundation licenses this file to you under the MIT license.
 
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.Globalization;
 using System.Linq;
 using System.Tests;
@@ -552,10 +553,42 @@ namespace System.Text.RegularExpressions.Tests
                 string input = new string('a', 50) + "@a.a";
 
                 AppDomain.CurrentDomain.SetData(RegexHelpers.DefaultMatchTimeout_ConfigKeyName, TimeSpan.FromMilliseconds(100));
+
+                if ((RegexOptions)int.Parse(optionsString, CultureInfo.InvariantCulture) == RegexOptions.None)
+                {
+                    Assert.Throws<RegexMatchTimeoutException>(() => new Regex(Pattern).Match(input));
+                    Assert.Throws<RegexMatchTimeoutException>(() => new Regex(Pattern).IsMatch(input));
+                    Assert.Throws<RegexMatchTimeoutException>(() => new Regex(Pattern).Matches(input).Count);
+
+                    Assert.Throws<RegexMatchTimeoutException>(() => Regex.Match(input, Pattern));
+                    Assert.Throws<RegexMatchTimeoutException>(() => Regex.IsMatch(input, Pattern));
+                    Assert.Throws<RegexMatchTimeoutException>(() => Regex.Matches(input, Pattern).Count);
+                }
+
                 Assert.Throws<RegexMatchTimeoutException>(() => new Regex(Pattern, (RegexOptions)int.Parse(optionsString, CultureInfo.InvariantCulture)).Match(input));
+                Assert.Throws<RegexMatchTimeoutException>(() => new Regex(Pattern, (RegexOptions)int.Parse(optionsString, CultureInfo.InvariantCulture)).IsMatch(input));
+                Assert.Throws<RegexMatchTimeoutException>(() => new Regex(Pattern, (RegexOptions)int.Parse(optionsString, CultureInfo.InvariantCulture)).Matches(input).Count);
+
+                Assert.Throws<RegexMatchTimeoutException>(() => Regex.Match(input, Pattern, (RegexOptions)int.Parse(optionsString, CultureInfo.InvariantCulture)));
+                Assert.Throws<RegexMatchTimeoutException>(() => Regex.IsMatch(input, Pattern, (RegexOptions)int.Parse(optionsString, CultureInfo.InvariantCulture)));
+                Assert.Throws<RegexMatchTimeoutException>(() => Regex.Matches(input, Pattern, (RegexOptions)int.Parse(optionsString, CultureInfo.InvariantCulture)).Count);
             }, ((int)options).ToString(CultureInfo.InvariantCulture)).Dispose();
         }
 
+        [Theory]
+        [InlineData(RegexOptions.None)]
+        [InlineData(RegexOptions.None | (RegexOptions)0x80 /* Debug */)]
+        [InlineData(RegexOptions.Compiled)]
+        [InlineData(RegexOptions.Compiled | (RegexOptions)0x80 /* Debug */)]
+        public void Match_CachedPattern_NewTimeoutApplies(RegexOptions options)
+        {
+            const string PatternLeadingToLotsOfBacktracking = @"^(\w+\s?)*$";
+            Assert.True(Regex.IsMatch("", PatternLeadingToLotsOfBacktracking, options, TimeSpan.FromDays(1)));
+            var sw = Stopwatch.StartNew();
+            Assert.Throws<RegexMatchTimeoutException>(() => Regex.IsMatch("An input string that takes a very very very very very very very very very very very long time!", PatternLeadingToLotsOfBacktracking, options, TimeSpan.FromMilliseconds(1)));
+            Assert.InRange(sw.Elapsed.TotalSeconds, 0, 10); // arbitrary upper bound that should be well above what's needed with a 1ms timeout
+        }
+
         // On 32-bit we can't test these high inputs as they cause OutOfMemoryExceptions.
         // On Linux, we may get killed by the OOM Killer; on Windows, it will swap instead
         [OuterLoop("Can take several seconds")]