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.

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

Jump to bottom

[Uri] TryCreate doubles file:/// Uris with certain escaped unreserved characters #36288

Closed
ScottLouvau opened this issue May 12, 2020 · 3 comments
Closed

[Uri] TryCreate doubles file:/// Uris with certain escaped unreserved characters #36288

ScottLouvau opened this issue May 12, 2020 · 3 comments
Labels
area-System.Net bug
Milestone
5.0.0

Comments

@ScottLouvau
Copy link

ScottLouvau commented May 12, 2020 •
edited by karelz
Loading

It looks like Uri.TryCreate() handles certain escaped characters incorrectly, doubling the Uri.

For example:

            string original = "file:///new%2Dhost/share/folder/file.cs";
            bool succeeded = Uri.TryCreate(original, UriKind.Absolute, out Uri result);
            string output = result.AbsoluteUri;
            // Succeeded: true

Output: file:///new%2Dhost/share/folder/file.cs/new-host/share/folder/file.cs

This reproduces for file:/// Uris with dash (%2D) and underscore (%5F), either in the first part of the Uri or later in the path. It does not reproduce for space (%20) or parens (%28, %29). I didn't try every escaping.
@Dotnet-GitSync-Bot Dotnet-GitSync-Bot added area-System.Net untriaged New issue has not been triaged by the area owner labels May 12, 2020
@ghost
Copy link

ghost commented May 12, 2020

Tagging subscribers to this area: @dotnet/ncl
Notify danmosemsft if you want to be subscribed.

@davidsh
Copy link
Contributor

davidsh commented May 12, 2020

@MihaZupan

ScottLouvau pushed a commit to microsoft/sarif-sdk that referenced this issue May 12, 2020
UriConverter fix to avoid doubling Uris with certain escaped characters.
2b51443 
  - Now serializing OriginalString as-is to ensure no doubling can occur.
  - This means we won't try to sanitize or canonicalize user-provided Uris anymore.
  - Consolidated test data into one of the two test classes.
  - Changed verification to check that the Uri reloads equal to the user-created original, not that the written SARIF has a canonicalized value.
  - Update an invalid Uri test expected file which expected somewhat-sanitized Uris.

  See dotnet/runtime#36288 for a .NET Framework bug I've reported on this issue.
@ScottLouvau ScottLouvau mentioned this issue May 12, 2020
Merged
@karelz karelz added the bug label May 12, 2020
@karelz karelz changed the title Uri.TryCreate doubles file:/// Uris with certain escaped unreserved characters [Uri] TryCreate doubles file:/// Uris with certain escaped unreserved characters May 12, 2020
@MihaZupan
Copy link
Member

Duplicate of #1061

@MihaZupan MihaZupan marked this as a duplicate of #1061 May 12, 2020
@davidsh davidsh removed the untriaged New issue has not been triaged by the area owner label May 12, 2020
ScottLouvau pushed a commit to microsoft/sarif-sdk that referenced this issue May 13, 2020
UriConverter fix to avoid doubling Uris with certain escaped characte…
b27027b 
…rs. (#1871)

* UriConverter fix to avoid doubling Uris with certain escaped characters.
  - Now serializing OriginalString as-is to ensure no doubling can occur.
  - This means we won't try to sanitize or canonicalize user-provided Uris anymore.
  - Consolidated test data into one of the two test classes.
  - Changed verification to check that the Uri reloads equal to the user-created original, not that the written SARIF has a canonicalized value.
  - Update an invalid Uri test expected file which expected somewhat-sanitized Uris.

  See dotnet/runtime#36288 for a .NET Framework bug I've reported on this issue.

* PR feedback, update version and release notes.
@karelz karelz added this to the 5.0.0 milestone Aug 18, 2020
@ghost ghost locked as resolved and limited conversation to collaborators Dec 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
area-System.Net bug
Projects
None yet
Milestone
5.0.0
Development

No branches or pull requests
5 participants
@karelz @davidsh @ScottLouvau @MihaZupan @Dotnet-GitSync-Bot