Skip to content

Add some validation to UriBuilder.Host setter #121083

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.

Sign up for GitHub

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
+71 −8
Draft

Add some validation to UriBuilder.Host setter #121083

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
33 changes: 29 additions & 4 deletions src/libraries/System.Private.Uri/src/System/UriBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,16 +158,41 @@ public string Password
}
}

/// <summary>
/// Problematic characters that could result in the Host component escaping into other components like the Path.</summary>
private static readonly SearchValues<char> s_hostReservedChars =
SearchValues.Create(@":/\?#@");

[AllowNull]
public string Host
{
get => _host;
set
{
if (!string.IsNullOrEmpty(value) && value.Contains(':') && value[0] != '[')
if (!string.IsNullOrEmpty(value) && value.AsSpan().ContainsAny(s_hostReservedChars))
{
//probable ipv6 address - Note: this is only supported for cases where the authority is inet-based.
value = "[" + value + "]";
if (value.Contains(':'))
{
if (!value.StartsWith('['))
{
// probable ipv6 address - Note: this is only supported for cases where the authority is inet-based.
value = "[" + value + "]";
}

if (value.AsSpan(0, value.Length - 1).Contains(']'))
{
// Reject inputs like "[::]/path" or "::]/path".
throw new ArgumentException(SR.net_uri_BadHostName, nameof(value));
}
}
else
{
// Reject inputs like "contoso.com/path" or "[email protected]".
// We don't take this branch if the input is an IPv6 address because those can contain '/' characters.
// If the input is an IPv6 address with invalid characters, Uri parsing will catch it later.
// Nonsensical inputs will only be allowed by a custom parser with GenericUriParserOptions.GenericAuthority set.
throw new ArgumentException(SR.net_uri_BadHostName, nameof(value));
}
}

_host = value ?? string.Empty;
Expand Down Expand Up @@ -365,7 +390,7 @@ public override string ToString()
}
}

var path = Path;
string path = Path;
if (path.Length != 0)
{
if (!path.StartsWith('/') && host.Length != 0)
Expand Down
46 changes: 42 additions & 4 deletions src/libraries/System.Private.Uri/tests/FunctionalTests/UriBuilderTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,6 @@ public void Ctor_Uri_Null()
[InlineData("http", "host", "http", "host")]
[InlineData("HTTP", "host", "http", "host")]
[InlineData("http", "[::1]", "http", "[::1]")]
[InlineData("https", "::1]", "https", "[::1]]")]
[InlineData("http", "::1", "http", "[::1]")]
[InlineData("http1:http2", "host", "http1", "host")]
[InlineData("http", "", "http", "")]
Expand All @@ -107,7 +106,6 @@ public void Ctor_String_String(string? schemeName, string? hostName, string expe
[InlineData("http", "host", 0, "http", "host")]
[InlineData("HTTP", "host", 20, "http", "host")]
[InlineData("http", "[::1]", 40, "http", "[::1]")]
[InlineData("https", "::1]", 60, "https", "[::1]]")]
[InlineData("http", "::1", 80, "http", "[::1]")]
[InlineData("http1:http2", "host", 100, "http1", "host")]
[InlineData("http", "", 120, "http", "")]
Expand All @@ -126,7 +124,6 @@ public void Ctor_String_String_Int(string? scheme, string? host, int port, strin
[InlineData("http", "host", 0, "/path", "http", "host", "/path")]
[InlineData("HTTP", "host", 20, "/path1/path2", "http", "host", "/path1/path2")]
[InlineData("http", "[::1]", 40, "/", "http", "[::1]", "/")]
[InlineData("https", "::1]", 60, "/path1/", "https", "[::1]]", "/path1/")]
[InlineData("http", "::1", 80, null, "http", "[::1]", "/")]
[InlineData("http1:http2", "host", 100, "path1", "http1", "host", "path1")]
[InlineData("http", "", 120, "path1/path2", "http", "", "path1/path2")]
Expand All @@ -145,7 +142,6 @@ public void Ctor_String_String_Int_String(string? schemeName, string? hostName,
[InlineData("http", "host", 0, "/path", "?query#fragment", "http", "host", "/path", "?query", "#fragment")]
[InlineData("HTTP", "host", 20, "/path1/path2", "?query&query2=value#fragment", "http", "host", "/path1/path2", "?query&query2=value", "#fragment")]
[InlineData("http", "[::1]", 40, "/", "#fragment?query", "http", "[::1]", "/", "", "#fragment?query")]
[InlineData("https", "::1]", 60, "/path1/", "?query", "https", "[::1]]", "/path1/", "?query", "")]
[InlineData("http", "::1", 80, null, "#fragment", "http", "[::1]", "/", "", "#fragment")]
[InlineData("http", "", 120, "path1/path2", "?#", "http", "", "path1/path2", "", "")]
[InlineData("", "host", 140, "path1/path2/path3/", "?", "", "host", "path1/path2/path3/", "", "")]
Expand Down Expand Up @@ -368,6 +364,7 @@ public static IEnumerable<object[]> ToString_TestData()
yield return new object[] { new UriBuilder() { Host = "host", Query = "query" }, "http://host/?query" };
yield return new object[] { new UriBuilder() { Host = "host", Fragment = "fragment" }, "http://host/#fragment" };
yield return new object[] { new UriBuilder() { Host = "host", Query = "query", Fragment = "fragment" }, "http://host/?query#fragment" };
yield return new object[] { new UriBuilder() { Host = "::%foo" }, "http://[::%foo]/" };
}

[Theory]
Expand Down Expand Up @@ -401,6 +398,47 @@ public void ToString_EncodingUserInfo(string username, string password, string e
Assert.Equal(password, uriBuilder.Password);
}

public static IEnumerable<object[]> InvalidHostStrings_TestData()
{
// Bool indicates whether the exception is expected to be thrown by the Host setter (true) or when creating the Uri (false)

// Presence of ':' is treated as a likely IPv6 address and the input is transformer into [host:80], which is invalid
yield return ["host:", false];
yield return ["host:80", false];

// Trailing space after IPv6 address
yield return ["[::] ", true];

// Invalid characters escape into other Uri components
yield return ["host/path", true];
yield return ["host?query", true];
yield return ["host#fragment", true];
yield return ["user@host", true];

yield return ["[::]/path", true];
yield return ["::]/path", true];
yield return ["::/path", false];
yield return ["::?query", false];
yield return ["::#fragment", false];
}

[Theory]
[MemberData(nameof(InvalidHostStrings_TestData))]
public void Host_Set_InvalidHostStrings_ThrowsArgumentException(string invalidHost, bool caughtByHostSetter)
{
var builder = new UriBuilder();

if (caughtByHostSetter)
{
AssertExtensions.Throws<ArgumentException>("value", () => builder.Host = invalidHost);
}
else
{
builder.Host = invalidHost;
Assert.Throws<UriFormatException>(() => builder.Uri);
}
}

private static void VerifyUriBuilder(UriBuilder uriBuilder, string scheme, string userName, string password, string host, int port, string path, string query, string fragment)
{
Assert.Equal(scheme, uriBuilder.Scheme);
Expand Down
Loading