From 10e292fc762c64b1a0fcdebe0a93db9fcfa2ea71 Mon Sep 17 00:00:00 2001 From: Eric Erhardt Date: Wed, 22 Feb 2023 12:34:01 -0600 Subject: [PATCH 1/3] Allow Regex engine to be trimmed Move the lazy Regex creation code to a delegate that is only set with the RegexRouteConstraint constructor that takes a string regexPattern. This allows for the Regex engine to be trimmed when the regexPattern constructor is trimmed. Fix #46142 --- .../src/Constraints/RegexRouteConstraint.cs | 25 +++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/src/Http/Routing/src/Constraints/RegexRouteConstraint.cs b/src/Http/Routing/src/Constraints/RegexRouteConstraint.cs index bc3ee556e974..561f13d75255 100644 --- a/src/Http/Routing/src/Constraints/RegexRouteConstraint.cs +++ b/src/Http/Routing/src/Constraints/RegexRouteConstraint.cs @@ -1,6 +1,7 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.Text.RegularExpressions; @@ -15,7 +16,7 @@ namespace Microsoft.AspNetCore.Routing.Constraints; public class RegexRouteConstraint : IRouteConstraint, IParameterLiteralNodeMatchingPolicy { private static readonly TimeSpan RegexMatchTimeout = TimeSpan.FromSeconds(10); - private readonly string _regexPattern; + private readonly Func? _regexFactory; private Regex? _constraint; /// @@ -27,7 +28,6 @@ public RegexRouteConstraint(Regex regex) ArgumentNullException.ThrowIfNull(regex); _constraint = regex; - _regexPattern = regex.ToString(); } /// @@ -35,12 +35,17 @@ public RegexRouteConstraint(Regex regex) /// /// A string containing the regex pattern. public RegexRouteConstraint( - [StringSyntax(StringSyntaxAttribute.Regex, RegexOptions.CultureInvariant | RegexOptions.IgnoreCase)] + [StringSyntax(StringSyntaxAttribute.Regex, RegexOptions.CultureInvariant | RegexOptions.Compiled | RegexOptions.IgnoreCase)] string regexPattern) { ArgumentNullException.ThrowIfNull(regexPattern); - _regexPattern = regexPattern; + // Create regex instance lazily to avoid compiling regexes at app startup. Delay creation until Constraint is first evaluated. + // This is not thread safe. No side effect but multiple instances of a regex instance could be created from a burst of requests. + _regexFactory = () => new Regex( + regexPattern, + RegexOptions.CultureInvariant | RegexOptions.Compiled | RegexOptions.IgnoreCase, + RegexMatchTimeout); } /// @@ -50,12 +55,12 @@ public Regex Constraint { get { - // Create regex instance lazily to avoid compiling regexes at app startup. Delay creation until constraint is first evaluated. - // This is not thread safe. No side effect but multiple instances of a regex instance could be created from a burst of requests. - _constraint ??= new Regex( - _regexPattern, - RegexOptions.CultureInvariant | RegexOptions.Compiled | RegexOptions.IgnoreCase, - RegexMatchTimeout); + if (_constraint is null) + { + Debug.Assert(_regexFactory is not null); + + _constraint = _regexFactory(); + } return _constraint; } From bc7fc50c500d010c970a3f875defeb70cd8eab0f Mon Sep 17 00:00:00 2001 From: Eric Erhardt Date: Wed, 22 Feb 2023 16:44:14 -0600 Subject: [PATCH 2/3] PR feedback --- src/Http/Routing/src/Constraints/RegexRouteConstraint.cs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Http/Routing/src/Constraints/RegexRouteConstraint.cs b/src/Http/Routing/src/Constraints/RegexRouteConstraint.cs index 561f13d75255..2ee3adf08ebe 100644 --- a/src/Http/Routing/src/Constraints/RegexRouteConstraint.cs +++ b/src/Http/Routing/src/Constraints/RegexRouteConstraint.cs @@ -41,7 +41,8 @@ public RegexRouteConstraint( ArgumentNullException.ThrowIfNull(regexPattern); // Create regex instance lazily to avoid compiling regexes at app startup. Delay creation until Constraint is first evaluated. - // This is not thread safe. No side effect but multiple instances of a regex instance could be created from a burst of requests. + // This is not thread-safe. No side effect, but multiple instances of a regex instance could be created from a burst of requests. + // The regex instance is created by a delegate here to allow the regex engine to be trimmed when this constructor is trimmed. _regexFactory = () => new Regex( regexPattern, RegexOptions.CultureInvariant | RegexOptions.Compiled | RegexOptions.IgnoreCase, From 130c16b44550cec37f24107f67104af2b6cfc336 Mon Sep 17 00:00:00 2001 From: Eric Erhardt Date: Wed, 22 Feb 2023 21:54:30 -0600 Subject: [PATCH 3/3] Apply suggestions from code review --- src/Http/Routing/src/Constraints/RegexRouteConstraint.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Http/Routing/src/Constraints/RegexRouteConstraint.cs b/src/Http/Routing/src/Constraints/RegexRouteConstraint.cs index 2ee3adf08ebe..a136ac70c15a 100644 --- a/src/Http/Routing/src/Constraints/RegexRouteConstraint.cs +++ b/src/Http/Routing/src/Constraints/RegexRouteConstraint.cs @@ -41,7 +41,6 @@ public RegexRouteConstraint( ArgumentNullException.ThrowIfNull(regexPattern); // Create regex instance lazily to avoid compiling regexes at app startup. Delay creation until Constraint is first evaluated. - // This is not thread-safe. No side effect, but multiple instances of a regex instance could be created from a burst of requests. // The regex instance is created by a delegate here to allow the regex engine to be trimmed when this constructor is trimmed. _regexFactory = () => new Regex( regexPattern, @@ -60,6 +59,7 @@ public Regex Constraint { Debug.Assert(_regexFactory is not null); + // This is not thread-safe. No side effect, but multiple instances of a regex instance could be created from a burst of requests. _constraint = _regexFactory(); }