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

Implement UriCreationOptions #59173

Merged
merged 2 commits into from
Sep 16, 2021
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
3 changes: 3 additions & 0 deletions src/libraries/System.Private.Uri/src/Resources/Strings.resx
Original file line number Diff line number Diff line change
Expand Up @@ -198,4 +198,7 @@
<data name="net_uri_InitializeCalledAlreadyOrTooLate" xml:space="preserve">
<value>UriParser's base InitializeAndValidate may only be called once on a single Uri instance and only from an override of InitializeAndValidate.</value>
</data>
<data name="net_uri_GetComponentsCalledWhenCanonicalizationDisabled" xml:space="preserve">
<value>GetComponents() may not be used for Path/Query on a Uri instance created with UriCreationOptions.DangerousDisablePathAndQueryCanonicalization.</value>
</data>
</root>
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
<Compile Include="System\UncNameHelper.cs" />
<Compile Include="System\Uri.cs" />
<Compile Include="System\UriBuilder.cs" />
<Compile Include="System\UriCreationOptions.cs" />
<Compile Include="System\UriEnumTypes.cs" />
<Compile Include="System\UriExt.cs" />
<Compile Include="System\UriFormatException.cs" />
Expand Down
71 changes: 67 additions & 4 deletions src/libraries/System.Private.Uri/src/System/Uri.cs
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,11 @@ internal enum Flags : ulong
IriCanonical = 0x78000000000,
UnixPath = 0x100000000000,

/// <summary>
/// Disables any validation/normalization past the authority. Fragments will always be empty. GetComponents will throw for Path/Query.
/// </summary>
DisablePathAndQueryCanonicalization = 0x200000000000,

/// <summary>
/// Used to ensure that InitializeAndValidate is only called once per Uri instance and only from an override of InitializeAndValidate
/// </summary>
Expand Down Expand Up @@ -267,6 +272,8 @@ internal static bool IriParsingStatic(UriParser? syntax)
return syntax is null || syntax.InFact(UriSyntaxFlags.AllowIriParsing);
}

internal bool DisablePathAndQueryCanonicalization => (_flags & Flags.DisablePathAndQueryCanonicalization) != 0;

internal bool UserDrivenParsing
{
get
Expand Down Expand Up @@ -410,6 +417,20 @@ public Uri(string uriString, UriKind uriKind)
DebugSetLeftCtor();
}

/// <summary>
/// Initializes a new instance of the <see cref="Uri"/> class with the specified URI and additional <see cref="UriCreationOptions"/>.
/// </summary>
/// <param name="uriString">A string that identifies the resource to be represented by the <see cref="Uri"/> instance.</param>
/// <param name="creationOptions">Options that control how the <seealso cref="Uri"/> is created and behaves.</param>
public Uri(string uriString, in UriCreationOptions creationOptions)
{
if (uriString is null)
throw new ArgumentNullException(nameof(uriString));

CreateThis(uriString, false, UriKind.Absolute, in creationOptions);
DebugSetLeftCtor();
}

//
// Uri(Uri, string)
//
Expand Down Expand Up @@ -1639,6 +1660,9 @@ public override bool Equals([NotNullWhen(true)] object? comparand)
// canonicalize the comparand, making comparison possible
if (obj is null)
{
if (DisablePathAndQueryCanonicalization)
return false;

if (!(comparand is string s))
return false;

Expand All @@ -1649,6 +1673,9 @@ public override bool Equals([NotNullWhen(true)] object? comparand)
return false;
}

if (DisablePathAndQueryCanonicalization != obj.DisablePathAndQueryCanonicalization)
return false;

if (ReferenceEquals(OriginalString, obj.OriginalString))
{
return true;
Expand Down Expand Up @@ -2553,7 +2580,7 @@ private unsafe void GetHostViaCustomSyntax()
//
internal string GetParts(UriComponents uriParts, UriFormat formatAs)
{
return GetComponents(uriParts, formatAs);
return InternalGetComponents(uriParts, formatAs);
}

private string GetEscapedParts(UriComponents uriParts)
Expand Down Expand Up @@ -3158,9 +3185,6 @@ private unsafe void ParseRemaining()
idx = _info.Offset.Path;
origIdx = _info.Offset.Path;

//Some uris do not have a query
// When '?' is passed as delimiter, then it's special case
// so both '?' and '#' will work as delimiters
if (buildIriStringFromPath)
{
DebugAssertInCtor();
Expand All @@ -3180,6 +3204,45 @@ private unsafe void ParseRemaining()

_info.Offset.Path = (ushort)_string.Length;
idx = _info.Offset.Path;
}

// If the user explicitly disabled canonicalization, only figure out the offsets
if (DisablePathAndQueryCanonicalization)
{
if (buildIriStringFromPath)
{
DebugAssertInCtor();
_string += _originalUnicodeString.Substring(origIdx);
}

string str = _string;

if (IsImplicitFile || (syntaxFlags & UriSyntaxFlags.MayHaveQuery) == 0)
{
idx = str.Length;
}
else
{
idx = str.IndexOf('?');
if (idx == -1)
{
idx = str.Length;
}
}

_info.Offset.Query = (ushort)idx;
_info.Offset.Fragment = (ushort)str.Length; // There is no fragment in UseRawTarget mode
_info.Offset.End = (ushort)str.Length;

goto Done;
}

//Some uris do not have a query
// When '?' is passed as delimiter, then it's special case
// so both '?' and '#' will work as delimiters
if (buildIriStringFromPath)
{
DebugAssertInCtor();

int offset = origIdx;
if (IsImplicitFile || ((syntaxFlags & (UriSyntaxFlags.MayHaveQuery | UriSyntaxFlags.MayHaveFragment)) == 0))
Expand Down
28 changes: 28 additions & 0 deletions src/libraries/System.Private.Uri/src/System/UriCreationOptions.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

namespace System
{
/// <summary>
/// Options that control how a <seealso cref="Uri"/> is created and behaves.
/// </summary>
public struct UriCreationOptions
{
private bool _disablePathAndQueryCanonicalization;

/// <summary>
/// Disables validation and normalization of the Path and Query.
/// No transformations of the URI past the Authority will take place.
/// <see cref="Uri"/> instances created with this option do not support <see cref="Uri.Fragment"/>s.
/// <see cref="Uri.GetComponents(UriComponents, UriFormat)"/> may not be used for <see cref="UriComponents.Path"/> or <see cref="UriComponents.Query"/>.
/// Be aware that disabling canonicalization also means that reserved characters will not be escaped,
/// which may corrupt the HTTP request and makes the application subject to request smuggling.
/// Only set this option if you have ensured that the URI string is already sanitized.
/// </summary>
public bool DangerousDisablePathAndQueryCanonicalization
{
readonly get => _disablePathAndQueryCanonicalization;
set => _disablePathAndQueryCanonicalization = value;
}
Comment on lines +22 to +26
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you please explain what readonly get does, and link me to associated docs? Thank you

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

}
}
40 changes: 38 additions & 2 deletions src/libraries/System.Private.Uri/src/System/UriExt.cs
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ public partial class Uri
//
// All public ctors go through here
//
private void CreateThis(string? uri, bool dontEscape, UriKind uriKind)
private void CreateThis(string? uri, bool dontEscape, UriKind uriKind, in UriCreationOptions creationOptions = default)
{
DebugAssertInCtor();

Expand All @@ -31,6 +31,9 @@ private void CreateThis(string? uri, bool dontEscape, UriKind uriKind)
if (dontEscape)
_flags |= Flags.UserEscaped;

if (creationOptions.DangerousDisablePathAndQueryCanonicalization)
_flags |= Flags.DisablePathAndQueryCanonicalization;

ParsingError err = ParseScheme(_string, ref _flags, ref _syntax!);

InitializeUri(err, uriKind, out UriFormatException? e);
Expand Down Expand Up @@ -259,6 +262,26 @@ public static bool TryCreate([NotNullWhen(true)] string? uriString, UriKind uriK
return e is null && result != null;
}

/// <summary>
/// Creates a new <see cref="Uri"/> using the specified <see cref="string"/> instance and <see cref="UriCreationOptions"/>.
/// </summary>
/// <param name="uriString">The string representation of the <see cref="Uri"/>.</param>
/// <param name="creationOptions">Options that control how the <seealso cref="Uri"/> is created and behaves.</param>
/// <param name="result">The constructed <see cref="Uri"/>.</param>
/// <returns><see langword="true"/> if the <see cref="Uri"/> was successfully created; otherwise, <see langword="false"/>.</returns>
public static bool TryCreate([NotNullWhen(true)] string? uriString, in UriCreationOptions creationOptions, [NotNullWhen(true)] out Uri? result)
{
if (uriString is null)
{
result = null;
return false;
}
UriFormatException? e = null;
result = CreateHelper(uriString, false, UriKind.Absolute, ref e, in creationOptions);
result?.DebugSetLeftCtor();
return e is null && result != null;
}

public static bool TryCreate(Uri? baseUri, string? relativeUri, [NotNullWhen(true)] out Uri? result)
{
if (TryCreate(relativeUri, UriKind.RelativeOrAbsolute, out Uri? relativeLink))
Expand Down Expand Up @@ -309,6 +332,16 @@ public static bool TryCreate(Uri? baseUri, Uri? relativeUri, [NotNullWhen(true)]
}

public string GetComponents(UriComponents components, UriFormat format)
{
if (DisablePathAndQueryCanonicalization && (components & (UriComponents.Path | UriComponents.Query)) != 0)
{
throw new InvalidOperationException(SR.net_uri_GetComponentsCalledWhenCanonicalizationDisabled);
}
Comment on lines +336 to +339
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Offline discussion: I'm concerned that there's no way for the consumer to know if DisablePathAndQueryCanonicalization is set and avoid this exception. This method can also throw IOE for relative Uris, but there you can check IsAbsoluteUri first.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method can also throw IOE for relative Uris, but there you can check IsAbsoluteUri first.

In the current version this is unreachable for relative Uris (since UriKind hasn't been exposed yet), but I will keep it in mind when adding the rest of the API. It's not critical as it would always throw anyway, but the change wasn't intentional.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should consider adding a way to expose this, something like IsPathAndQueryCanonicalizationDisabled.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to be super-clear: We can track that for 7.0, we do not consider it blocking attempt for 6.0 backport.
@geoffkizer please let us know if you disagree.


return InternalGetComponents(components, format);
}

private string InternalGetComponents(UriComponents components, UriFormat format)
{
if (((components & UriComponents.SerializationInfoString) != 0) && components != UriComponents.SerializationInfoString)
throw new ArgumentOutOfRangeException(nameof(components), components, SR.net_uri_NotJustSerialization);
Expand Down Expand Up @@ -590,7 +623,7 @@ private Uri(Flags flags, UriParser? uriParser, string uri)
//
// a Uri.TryCreate() method goes through here.
//
internal static Uri? CreateHelper(string uriString, bool dontEscape, UriKind uriKind, ref UriFormatException? e)
internal static Uri? CreateHelper(string uriString, bool dontEscape, UriKind uriKind, ref UriFormatException? e, in UriCreationOptions creationOptions = default)
{
// if (!Enum.IsDefined(typeof(UriKind), uriKind)) -- We currently believe that Enum.IsDefined() is too slow
// to be used here.
Expand All @@ -606,6 +639,9 @@ private Uri(Flags flags, UriParser? uriParser, string uri)
if (dontEscape)
flags |= Flags.UserEscaped;

if (creationOptions.DangerousDisablePathAndQueryCanonicalization)
flags |= Flags.DisablePathAndQueryCanonicalization;

// We won't use User factory for these errors
if (err != ParsingError.None)
{
Expand Down
3 changes: 3 additions & 0 deletions src/libraries/System.Private.Uri/src/System/UriScheme.cs
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,9 @@ protected virtual string GetComponents(Uri uri, UriComponents components, UriFor
if (!uri.IsAbsoluteUri)
throw new InvalidOperationException(SR.net_uri_NotAbsolute);

if (uri.DisablePathAndQueryCanonicalization && (components & (UriComponents.Path | UriComponents.Query)) != 0)
throw new InvalidOperationException(SR.net_uri_GetComponentsCalledWhenCanonicalizationDisabled);

return uri.GetComponentsHelper(components, format);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
<Compile Include="UriBuilderParameterTest.cs" />
<Compile Include="UriBuilderRefreshTest.cs" />
<Compile Include="UriBuilderTests.cs" />
<Compile Include="UriCreationOptionsTest.cs" />
<Compile Include="UriEscapingTest.cs" />
<Compile Include="UriGetComponentsTest.cs" />
<Compile Include="UriIpHostTest.cs" />
Expand Down
Loading