Skip to content

Added null check to CorsPolicyBuilder #19831

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

Merged
5 commits merged into from
Mar 30, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions src/Middleware/CORS/src/Infrastructure/CorsPolicyBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,11 @@ public CorsPolicyBuilder(CorsPolicy policy)
/// </remarks>
public CorsPolicyBuilder WithOrigins(params string[] origins)
{
if (origins is null)
{
throw new ArgumentNullException(nameof(origins));
}

foreach (var origin in origins)
{
var normalizedOrigin = GetNormalizedOrigin(origin);
Expand All @@ -65,6 +70,11 @@ public CorsPolicyBuilder WithOrigins(params string[] origins)

internal static string GetNormalizedOrigin(string origin)
{
if (origin is null)
{
throw new ArgumentNullException(nameof(origin));
}

if (Uri.TryCreate(origin, UriKind.Absolute, out var uri) &&
(uri.Scheme == Uri.UriSchemeHttp || uri.Scheme == Uri.UriSchemeHttps) &&
!string.Equals(uri.IdnHost, uri.Host, StringComparison.Ordinal))
Expand All @@ -73,9 +83,9 @@ internal static string GetNormalizedOrigin(string origin)
if (!uri.IsDefaultPort)
{
// Uri does not have a way to differentiate between a port value inferred by default (e.g. Port = 80 for http://www.example.com) and
// a default port value that is specified (e.g. Port = 80 for http://www.example.com:80). Although the HTTP or FETCH spec does not say
// a default port value that is specified (e.g. Port = 80 for http://www.example.com:80). Although the HTTP or FETCH spec does not say
// anything about including the default port as part of the Origin header, at the time of writing, browsers drop "default" port when navigating
// and when sending the Origin header. All this goes to say, it appears OK to drop an explicitly specified port,
// and when sending the Origin header. All this goes to say, it appears OK to drop an explicitly specified port,
// if it is the default port when working with an IDN host.
builder.Port = uri.Port;
}
Expand Down Expand Up @@ -208,7 +218,7 @@ public CorsPolicyBuilder SetIsOriginAllowed(Func<string, bool> isOriginAllowed)

/// <summary>
/// Sets the <see cref="CorsPolicy.IsOriginAllowed"/> property of the policy to be a function
/// that allows origins to match a configured wildcarded domain when evaluating if the
/// that allows origins to match a configured wildcarded domain when evaluating if the
/// origin is allowed.
/// </summary>
/// <returns>The current policy builder.</returns>
Expand Down
22 changes: 22 additions & 0 deletions src/Middleware/CORS/test/UnitTests/CorsPolicyBuilderTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,28 @@ public void WithOrigins_NormalizesOrigins()
Assert.Equal(new List<string>() { "http://www.example.com", "https://example2.com" }, corsPolicy.Origins);
}

[Fact]
public void WithOrigins_ThrowsIfArgumentNull()
{
// Arrange
var builder = new CorsPolicyBuilder();
string[] args = null;

// Act / Assert
Assert.Throws<ArgumentNullException>(() => builder.WithOrigins(args));
}

[Fact]
public void WithOrigins_ThrowsIfArgumentArrayContainsNull()
{
// Arrange
var builder = new CorsPolicyBuilder();
string[] args = new string[] { null };

// Act / Assert
Assert.Throws<ArgumentNullException>(() => builder.WithOrigins(args));
}

[Fact]
public void AllowAnyOrigin_AllowsAny()
{
Expand Down