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

[API Proposal]: UriCreationOptions #59099

Open
MihaZupan opened this issue Sep 14, 2021 · 8 comments
Open

[API Proposal]: UriCreationOptions #59099

MihaZupan opened this issue Sep 14, 2021 · 8 comments
Labels
api-approved API was approved in API review, it can be implemented area-System.Net
Milestone

Comments

@MihaZupan
Copy link
Member

MihaZupan commented Sep 14, 2021

Background and motivation

Uri validates the input and (if needed) converts it into a normalized (canonical) representation.
For example, it will:

  • Escape reserved characters (spaces turn into %20)
  • Unescape unreserved characters (%41 turns into A)
  • Compress dot segments (/foo/./bar/../ turns into /foo/)

It does this based on the recommendations of the Uri RFC.

Some services, however, rely on specific behavior that may be impossible to replicate using .NET's Uri (#52628, #58057).
For example, it is currently not possible to send /foo?A=%42 with HttpClient, as Uri will convert it to /foo?A=B.

I propose adding a UriCreationOptions type and a ctor/TryCreate overload to consume it, serving as the extension point for future Uri customizability.

Uri has support for implicitly converting a file path into a file:// uri.
/foo is a valid Unix absolute file path and support for Unix paths is limited to non-Windows platforms.
This can create confusion in cases like Uri.TryCreate("/foo", UriKind.Absolute) where the behavior depends on the platform.
Given an extension point like UriCreationOptions is available, I propose adding an AllowImplicitFilePaths flag to it.

API Proposal

namespace System
{
    // New type
    public readonly struct UriCreationOptions
    {
        public bool AllowImplicitFilePaths { get; init; }
        public bool DangerousUseRawPathAndQuery { get; init; }

        public UriKind UriKind { get; }

        public UriCreationOptions(UriKind uriKind);
    }

    public partial class Uri
    {
        // Existing
        public Uri(string uriString);
        public Uri(string uriString, UriKind uriKind);
        // New
        public Uri(string uriString, UriCreationOptions creationOptions);
        
        // Existing
        public static bool TryCreate(string? uriString, UriKind uriKind, out Uri? result);
        // New
        public static bool TryCreate(string? uriString, UriCreationOptions creationOptions, out Uri? result);
    }
}

Alternative names for DangerousUseRawPathAndQuery:

  • DangerousUseRawTarget
  • DangerousDisableCanonicalization

Behavior of DangerousUseRawPathAndQuery is such that we try not to be opinionated as much as possible:

  • Validation of the Path/Query is skipped, we assume they already are in the “perfect format”
  • We will not escape reserved characters (e.g. spaces won't be turned into %20)
  • We will not unescape unreserved characters (e.g. %41 won't be turned into A)
  • We will not compress dot segments (/../ or /./ will be kept)
  • Fragment is always empty – # and anything following it is attributed to the Path or Query
    • Not doing so would disallow sending the # literal as part of PathAndQuery
  • Properties like Path, Query, PathAndQuery will return the value as-is
  • We will not ensure Path/PathAndQuery start with /, that is left up to the user
  • Uris with DangerousUseRawPathAndQuery may only be equal to Uris with the same flag
  • The GetComponents API will ignore the UriFormat argument (values UriEscaped, Unescaped, SafeUnescaped) for Path & Query components

API Usage

string uriString = "/foo";

Uri.TryCreate(uriString, new UriCreationOptions { AllowImplicitFilePaths = false }, out var uri); // False
var options = new UriCreationOptions { DangerousUseRawPathAndQuery = true };
var uri = new Uri("http://foo/%41", options);

// Server receives /%41 instead of /A

Alternative API

  • Instead of a readonly struct, make UriCreationOptions an enum

    • Prevents us from easily exposing more complex options (e.g. ints, callbacks) in the future
    • Forces us into inverting names in some cases (e.g. AllowImplicitFilePaths would have to be DontAllowImplicitFilePaths) to match correctly defaults
    • Defers validation – with UriCreationOptions we can ensure it's always in a valid state (e.g. valid UriKind)
  • Instead of a DangerousUseRawPathAndQuery flag, expose granular options controlling specific transformations

    • Example: UnescapeUnreservedCharacters, CompressDotSegments
    • Any new scenarios that can't be addressed with existing options are blocked on the decision of the networking team, API review, implementation, and release of a major .NET version
    • Complicates the code base & testing - some options may conflict

Risks

Setting DangerousUseRawPathAndQuery means no validation/transformation of the input will take place past the authority.
If the input is not already in the correct format, HTTP requests using such a Uri may be malformed.

For example:

  • Reserved characters will not be escaped - space characters will not be changed to %20
@MihaZupan MihaZupan added the api-suggestion Early API idea and discussion, it is NOT ready for implementation label Sep 14, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added area-Extensions-Options untriaged New issue has not been triaged by the area owner labels Sep 14, 2021
@ghost
Copy link

ghost commented Sep 14, 2021

Tagging subscribers to this area: @maryamariyan
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

Uri validates the input and (if needed) converts it into a normalized (canonical) representation.
For example, it will:

  • Escape reserved characters (spaces turn into %20)
  • Unescape unreserved characters (%41 turns into A)
  • Compress dot segments (/foo/./bar/../ turns into /foo/)

It does this based on the recommendations of the Uri RFC.

Some services, however, rely on specific behavior that may be impossible to replicate using .NET's Uri (#52628, #58057).
For example, it is currently not possible to send /foo?A=%42 with HttpClient, as Uri will convert it to /foo?A=B.

I propose adding a UriCreationOptions type and a ctor/TryCreate overload to consume it, serving as the extension point for future Uri customizability.

Uri has support for implicitly converting a file path into a file:// uri.
/foo is a valid Unix absolute file path and support for Unix paths is limited to non-Windows platforms.
This can create confusion in cases like Uri.TryCreate("/foo", UriKind.Absolute) where the behavior depends on the platform.
Given an extension point like UriCreationOptions is available, I propose adding an AllowImplicitFilePaths flag to it.

API Proposal

namespace System
{
    // New type
    public readonly struct UriCreationOptions
    {
        public bool AllowImplicitFilePaths { get; init; }
        public bool DangerousUseRawPathAndQuery { get; init; }

        public UriKind UriKind { get; }

        public UriCreationOptions(UriKind uriKind);
    }

    public partial class Uri
    {
        // Existing
        public Uri(string uriString);
        public Uri(string uriString, UriKind uriKind);
        // New
        public Uri(string uriString, UriCreationOptions creationOptions);
        
        // Existing
        public static bool TryCreate(string? uriString, UriKind uriKind, out Uri? result);
        // New
        public static bool TryCreate(string? uriString, UriCreationOptions creationOptions, out Uri? result);
    }
}

Alternative names for DangerousUseRawPathAndQuery:

  • DangerousUseRawTarget
  • DangerousDisableCanonicalization

Behavior of DangerousUseRawPathAndQuery is such that we try not to be opinionated as much as possible:

  • Validation of the Path/Query is skipped, we assume they already are in the “perfect format”
  • We will not escape reserved characters (e.g. spaces won't be turned into %20)
  • We will not unescape unreserved characters (e.g. %41 won't be turned into A)
  • We will not compress dot segments (/../ or /./ will be kept)
  • Fragment is always empty – # and anything following it is attributed to the Path or Query
    • Not doing so would disallow sending the # literal as part of PathAndQuery
  • Properties like Path, Query, PathAndQuery will return the value as-is
  • We will not ensure Path/PathAndQuery start with /, that is left up to the user
  • Uris with “UseRawPathAndQuery” may only be equal to Uris with the same flag
  • The GetComponents API will ignore the UriFormat argument for Path & Query components

API Usage

string uriString = "/foo";

Uri.TryCreate(uriString, new UriCreationOptions { AllowImplicitFilePaths = false }, out var uri); // False
var options = new UriCreationOptions { DangerousUseRawPathAndQuery = true };
var uri = new Uri("http://foo/%41", options);

// Server receives /%41 instead of /A

Alternative API

  • Instead of a readonly struct, make UriCreationOptions an enum

    • Prevents us from easily exposing more complex options (e.g. ints, callbacks) in the future
    • Forces us into inverting names in some cases (e.g. AllowImplicitFilePaths would have to be DontAllowImplicitFilePaths)
    • Defers validation – with UriCreationOptions we can ensure it's always in a valid state (e.g. valid UriKind)
  • Instead of a DangerousUseRawPathAndQuery flag, expose granular options controlling specific transformations

    • Example: UnescapeUnreservedCharacters, CompressDotSegments
    • Any new scenarios that can't be addressed with existing options are blocked on the decision of the networking team, API review, implementation, and release of a major .NET version
    • Complicates the code base & testing - some options may conflict

Risks

Setting DangerousUseRawPathAndQuery means no validation/transformation of the input will take place past the authority.
If the input is not already in the correct format, HTTP requests using such a Uri may be malformed.

For example:

  • Reserved characters will not be escaped - space characters will not be changed to %20
Author: MihaZupan
Assignees: -
Labels:

api-suggestion, untriaged, area-Extensions-Options

Milestone: -

@ghost ghost added this to Untriaged in ML, Extensions, Globalization, etc, POD. Sep 14, 2021
@karelz karelz added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed api-suggestion Early API idea and discussion, it is NOT ready for implementation untriaged New issue has not been triaged by the area owner labels Sep 14, 2021
@karelz karelz added this to the 7.0.0 milestone Sep 14, 2021
@ghost ghost moved this from Untriaged to 7.0.0 in ML, Extensions, Globalization, etc, POD. Sep 14, 2021
@ghost
Copy link

ghost commented Sep 14, 2021

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

Issue Details

Background and motivation

Uri validates the input and (if needed) converts it into a normalized (canonical) representation.
For example, it will:

  • Escape reserved characters (spaces turn into %20)
  • Unescape unreserved characters (%41 turns into A)
  • Compress dot segments (/foo/./bar/../ turns into /foo/)

It does this based on the recommendations of the Uri RFC.

Some services, however, rely on specific behavior that may be impossible to replicate using .NET's Uri (#52628, #58057).
For example, it is currently not possible to send /foo?A=%42 with HttpClient, as Uri will convert it to /foo?A=B.

I propose adding a UriCreationOptions type and a ctor/TryCreate overload to consume it, serving as the extension point for future Uri customizability.

Uri has support for implicitly converting a file path into a file:// uri.
/foo is a valid Unix absolute file path and support for Unix paths is limited to non-Windows platforms.
This can create confusion in cases like Uri.TryCreate("/foo", UriKind.Absolute) where the behavior depends on the platform.
Given an extension point like UriCreationOptions is available, I propose adding an AllowImplicitFilePaths flag to it.

API Proposal

namespace System
{
    // New type
    public readonly struct UriCreationOptions
    {
        public bool AllowImplicitFilePaths { get; init; }
        public bool DangerousUseRawPathAndQuery { get; init; }

        public UriKind UriKind { get; }

        public UriCreationOptions(UriKind uriKind);
    }

    public partial class Uri
    {
        // Existing
        public Uri(string uriString);
        public Uri(string uriString, UriKind uriKind);
        // New
        public Uri(string uriString, UriCreationOptions creationOptions);
        
        // Existing
        public static bool TryCreate(string? uriString, UriKind uriKind, out Uri? result);
        // New
        public static bool TryCreate(string? uriString, UriCreationOptions creationOptions, out Uri? result);
    }
}

Alternative names for DangerousUseRawPathAndQuery:

  • DangerousUseRawTarget
  • DangerousDisableCanonicalization

Behavior of DangerousUseRawPathAndQuery is such that we try not to be opinionated as much as possible:

  • Validation of the Path/Query is skipped, we assume they already are in the “perfect format”
  • We will not escape reserved characters (e.g. spaces won't be turned into %20)
  • We will not unescape unreserved characters (e.g. %41 won't be turned into A)
  • We will not compress dot segments (/../ or /./ will be kept)
  • Fragment is always empty – # and anything following it is attributed to the Path or Query
    • Not doing so would disallow sending the # literal as part of PathAndQuery
  • Properties like Path, Query, PathAndQuery will return the value as-is
  • We will not ensure Path/PathAndQuery start with /, that is left up to the user
  • Uris with “UseRawPathAndQuery” may only be equal to Uris with the same flag
  • The GetComponents API will ignore the UriFormat argument for Path & Query components

API Usage

string uriString = "/foo";

Uri.TryCreate(uriString, new UriCreationOptions { AllowImplicitFilePaths = false }, out var uri); // False
var options = new UriCreationOptions { DangerousUseRawPathAndQuery = true };
var uri = new Uri("http://foo/%41", options);

// Server receives /%41 instead of /A

Alternative API

  • Instead of a readonly struct, make UriCreationOptions an enum

    • Prevents us from easily exposing more complex options (e.g. ints, callbacks) in the future
    • Forces us into inverting names in some cases (e.g. AllowImplicitFilePaths would have to be DontAllowImplicitFilePaths)
    • Defers validation – with UriCreationOptions we can ensure it's always in a valid state (e.g. valid UriKind)
  • Instead of a DangerousUseRawPathAndQuery flag, expose granular options controlling specific transformations

    • Example: UnescapeUnreservedCharacters, CompressDotSegments
    • Any new scenarios that can't be addressed with existing options are blocked on the decision of the networking team, API review, implementation, and release of a major .NET version
    • Complicates the code base & testing - some options may conflict

Risks

Setting DangerousUseRawPathAndQuery means no validation/transformation of the input will take place past the authority.
If the input is not already in the correct format, HTTP requests using such a Uri may be malformed.

For example:

  • Reserved characters will not be escaped - space characters will not be changed to %20
Author: MihaZupan
Assignees: -
Labels:

area-System.Net, api-ready-for-review

Milestone: 7.0.0

@ghost ghost removed this from 7.0.0 in ML, Extensions, Globalization, etc, POD. Sep 14, 2021
@terrajobst terrajobst added the blocking Marks issues that we want to fast track in order to unblock other important work label Sep 14, 2021
@terrajobst
Copy link
Member

terrajobst commented Sep 14, 2021

Video

  • The struct should be a mutable struct
    • We should pass it in via in, which means we should mark the getter as readonly
  • GetComponents() should throw InvaliOperationException
    • Note: even UriFormat.Unescaped does some transformations, so this also doesn't make sense to allow
namespace System
{
    public struct UriCreationOptions
    {
        public UriKind UriKind { readonly get; set; }
        public bool AllowImplicitFilePaths { readonly get; set; }
        public bool DangerousDisablePathAndQueryCanonicalization { readonly get; set; }
    }

    public partial class Uri
    {
        // Existing
        // public Uri(string uriString);
        // public Uri(string uriString, UriKind uriKind);
        public Uri(string uriString, in UriCreationOptions creationOptions);
        
        // Existing
        // public static bool TryCreate(string? uriString, UriKind uriKind, out Uri? result);
        public static bool TryCreate(string? uriString, in UriCreationOptions creationOptions, out Uri? result);
    }
}

@terrajobst terrajobst added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Sep 14, 2021
@MihaZupan MihaZupan removed the blocking Marks issues that we want to fast track in order to unblock other important work label Sep 14, 2021
@karelz
Copy link
Member

karelz commented Sep 20, 2021

Part of the API surface was added in 6.0 RC2 to address #52628 and #58057 customer reprots.

Added in 6.0 RC2 and 7.0 PRs:

public struct UriCreationOptions
{
    public bool DangerousDisablePathAndQueryCanonicalization { readonly get; set; }
}

public partial class Uri
{
    // Existing
    // public Uri(string uriString);
    // public Uri(string uriString, UriKind uriKind);
    public Uri(string uriString, in UriCreationOptions creationOptions);

    // Existing
    // public static bool TryCreate(string? uriString, UriKind uriKind, out Uri? result);
    public static bool TryCreate(string? uriString, in UriCreationOptions creationOptions, out Uri? result);
}

Remaining parts, not yet implemented:

public struct UriCreationOptions
{
    public UriKind UriKind { readonly get; set; }
    public bool AllowImplicitFilePaths { readonly get; set; }
}

@am11
Copy link
Member

am11 commented Sep 25, 2021

Uri has support for implicitly converting a file path into a file:// uri.

There is one case in RFC 8089, which Uri.TryCreate fails to accept: https://sharplab.io/#gist:7680fb1f8c7f5c0c0a41334e71f3f137. file:c:/foo.txt is allowed, file:/tmp/foo.txt isn't. Would it be allowed implicitly with AllowImplicitFilePaths?

@MihaZupan
Copy link
Member Author

AllowImplicitFilePaths would remain true by default.
If there are other formats we should support, we can discuss it in a separate issue (it would impact the default Uri behavior).

@am11
Copy link
Member

am11 commented Sep 25, 2021

I read it as if we are adding a new mode/flag for canonical form to support formats which weren't possible to support before (due to Uri's compatibility concerns). If that's not the case, nvm.

@MihaZupan
Copy link
Member Author

Note to self: most of the missing implementation work is done in MihaZupan@fef4435 (not updated after #59173).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-approved API was approved in API review, it can be implemented area-System.Net
Projects
None yet
Development

No branches or pull requests

4 participants