Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Proposed fix for #1183 - multiple regex within short time might give identical strings #1187

Merged
merged 5 commits into from
Jul 8, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
30 changes: 26 additions & 4 deletions Src/AutoFixture/RegularExpressionGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,18 @@ namespace AutoFixture
/// </summary>
public class RegularExpressionGenerator : ISpecimenBuilder
{
private readonly Random random;
private readonly object syncRoot;

/// <summary>
/// Initializes a new instance of the <see cref="RegularExpressionGenerator"/> class.
/// </summary>
public RegularExpressionGenerator()
{
this.random = new Random();
moodmosaic marked this conversation as resolved.
Show resolved Hide resolved
this.syncRoot = new object();
}

/// <summary>
/// Creates a string that is guaranteed to match a RegularExpressionRequest.
/// </summary>
Expand All @@ -28,16 +40,18 @@ public object Create(object request, ISpecimenContext context)
return new NoSpecimen();
}

return GenerateRegularExpression(regularExpressionRequest);
return this.GenerateRegularExpression(regularExpressionRequest);
}

private static object GenerateRegularExpression(RegularExpressionRequest request)
private object GenerateRegularExpression(RegularExpressionRequest request)
{
string pattern = request.Pattern;

try
{
string regex = new Xeger(pattern).Generate();
// Use the Xeger constructor overload that that takes an instance of Random.
// Otherwise identically strings can be generated, if regex are generated within short time.
string regex = new Xeger(pattern, new Random(this.GenerateSeed())).Generate();
if (Regex.IsMatch(regex, pattern))
{
return regex;
Expand All @@ -54,5 +68,13 @@ private static object GenerateRegularExpression(RegularExpressionRequest request

return new NoSpecimen();
}

private int GenerateSeed()
{
lock (this.syncRoot)
{
return this.random.Next();
}
}
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ namespace AutoFixtureUnitTest.DataAnnotations
{
public class RegularExpressionValidatedType
{
public const string Pattern = @"^[a-zA-Z''-'\s]{1,40}$";
public const string Pattern = @"^[a-zA-Z''-'\s]{20,40}$";

[RegularExpression(Pattern)]
public string Property { get; set; }
Expand Down
15 changes: 15 additions & 0 deletions Src/AutoFixtureUnitTest/FixtureTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1636,6 +1636,21 @@ public void CreateAnonymousWithRegularExpressionValidatedTypeReturnsCorrectResul
Assert.Matches(RegularExpressionValidatedType.Pattern, result.Property);
}

[Fact]
public void CreateManyAnonymousWithRegularExpressionValidatedTypeReturnsDifferentResults()
{
// This test exposes an issue with Xeger/Random.
// Xeger(pattern) internally creates an instance of Random with the default seed.
// This means that the RegularExpressionGenerator might create identical strings
// if called multiple times within a short time.

// Arrange
var fixture = new Fixture();
var result = fixture.CreateMany<RegularExpressionValidatedType>(10).Select(x => x.Property).ToArray();
// Assert
Assert.Equal(result.Distinct(), result);
}

[Fact]
public void CreateAnonymousWithStringLengthValidatedTypeReturnsCorrectResult()
{
Expand Down
24 changes: 23 additions & 1 deletion Src/AutoFixtureUnitTest/RegularExpressionGeneratorTest.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
using System.Text.RegularExpressions;
using System.Linq;
using System.Text.RegularExpressions;
using AutoFixture;
using AutoFixture.Kernel;
using AutoFixtureUnitTest.Kernel;
Expand Down Expand Up @@ -103,6 +104,27 @@ public void CreateWithRegularExpressionRequestReturnsCorrectResult(string patter
Assert.True(Regex.IsMatch(result.ToString(), pattern), string.Format("result: {0}", result));
}

[Theory]
[InlineData(@"^[A-Z]{27}$")]
public void CreateMultipleWithRegularExpressionRequestReturnsDifferentResults(string pattern)
{
// This test exposes an issue with Xeger/Random.
// Xeger(pattern) internally creates an instance of Random with the default seed.
// This means that the RegularExpressionGenerator might create identical strings
// if called multiple times within a short time.

// Arrange
var sut = new RegularExpressionGenerator();
var request = new RegularExpressionRequest(pattern);
var dummyContext = new DelegatingSpecimenContext();

// Repeat a few times to make the test more robust.
// Use ToArray to iterate the IEnumerable at this point.
var result = Enumerable.Range(0, 10).Select(_ => sut.Create(request, dummyContext)).ToArray();

Assert.Equal(result.Distinct(), result);
}

[Theory]
[InlineData("[")]
[InlineData(@"(?\[Test\]|\[Foo\]|\[Bar\])?(?:-)?(?\[[()a-zA-Z0-9_\s]+\])?(?:-)?(?\[[a-zA-Z0-9_\s]+\])?(?:-)?(?\[[a-zA-Z0-9_\s]+\])?(?:-)?(?\[[a-zA-Z0-9_\s]+\])?")]
Expand Down