Runtime: [Uri] Uri.TryCreate(string, UriKind, out Uri) now considers /path as valid absolute paths on Linux/macOS

[kevinchalet]

When porting my OIDC server framework from ASP.NET Core 1.0 to 2.0, I discovered a very strange behavior change in Uri.TryCreate(string, UriKind, out Uri) that now considers /path as valid absolute paths on Linux and macOS (but not on Windows).

This can be reproduced using the latest CLI/runtime bits:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>netcoreapp2.0</TargetFramework>
  </PropertyGroup>

</Project>

using System;

namespace UriConsoleApp
{
    public static class Program
    {
        public static void Main(string[] args)
        {
            if (Uri.TryCreate("/path", UriKind.Absolute, out Uri uri))
            {
                Console.WriteLine("The '/path' string is considered as a valid absolute URI.");
            }
            else
            {
                Console.WriteLine("The '/path' string is not considered as a valid absolute URI.");
            }

            Console.ReadLine();
        }
    }
}

On Windows:

The '/path' string is not considered as a valid absolute URI.

On Ubuntu 14.04.3 LTS:

The '/path' string is considered as a valid absolute URI.

Is this inconsistency/behavior change expected?

/cc @karelz

[Tratcher]

Agreed this is terrible, but I didn't think it was new in 2.0. It's trying to mirror the windows behavior that says c:\ is a valid absolute Uri (also a terrible idea).

[kevinchalet]

@Tratcher not sure about .NET Core 1.1, but I'm 99,99% sure it used to work fine on .NET Core 1.0 (we have a unit test that ensures relative OIDC redirect_uris are correctly rejected for a long time now, and it only started to fail when moving to 2.0).

[akoeplinger]

sounds like https://github.com/dotnet/corefx/pull/15925

[stephentoub]

Is this inconsistency/behavior change expected?

cc: @tmds
This is by design. For better or worse, Uri supports paths, and on Unix, absolute paths begin with '/'.

[kevinchalet]

@akoeplinger @stephentoub erf, crap! I don't suppose there's a way to opt out of Unix paths support using the existing APIs? :sob:

[stephentoub]

I don't suppose there's a way to opt out of Unix paths support using the existing APIs?

I don't believe so, though you could check the IsFile property on the Uri. I believe the logic also shouldn't kick in if you use UriKind.RelativeOrAbsolute, only if you try creating it as Absolute.

@tmds, @akoeplinger, this is the same behavior Mono has, right? As in other places, I believe we used Mono as a guide here for how this should behave on Unix.

[akoeplinger]

this is the same behavior Mono has, right?

Correct.

[kevinchalet]

I'm all for offering a better cross-platform experience (even though I'm not sure Uri is the right class for file paths manipulations), but this change is particularly terrible as it breaks absolute/relative URLs recognition, which is kinda the essence of this class. Specially since the new behavior is not consistent across platforms.

[Tratcher]

Agreed, this is worse on linux due to the ambiguity. It makes several of the APIs unusable.

[Tratcher]

For a concrete example, this breaks HttpClient.

            var client = new HttpClient();
            client.BaseAddress = new Uri("http://example.com/base/");
            var result = await client.GetAsync("/path?query");

This works on Windows but throws on Mac/Linux because "/path?query" becomes an absolute file path instead of a uri relative to the http base.
https://github.com/dotnet/corefx/blob/0ec2a72a21eccba24764239e3ab985c9d2847969/src/System.Net.Http/src/System/Net/Http/HttpClient.cs#L678

ASP.NET Core tests have to work around it by leaving off the leading slash "path?query" which has subtly different semantics for uri processing, you end up with http://example.com/base/path?query rather than http://example.com/path?query.

[tmds]

This change makes it more obvious that implicit file paths get interpreted as file:// uris by the Uri class.

If you have some code like:

if (!Uri.TryCreate(request.RedirectUri, UriKind.Absolute, out Uri uri))

and request.RedirectUri is "C:\tmpfile.xml". This is considered a valid absolute Uri in .NET Framework and .NET Core 1.0.

Probably this wouldn't be a valid input either and a check for the Scheme (either excluding file, or only allowing specific schemes) is appropriate anyhow.

[tmds]

This works on Windows but throws on Mac/Linux because "/path?query" becomes an absolute file path instead of a uri relative to the http base.
https://github.com/dotnet/corefx/blob/0ec2a72a21eccba24764239e3ab985c9d2847969/src/System.Net.Http/src/System/Net/Http/HttpClient.cs#L678

@Tratcher Have you tried this? new Uri(uri, UriKind.RelativeOrAbsolute) is Relative for a uri like "/path?query".

[Tratcher]

Not recently, it caused a lot of test issues a while back and we changed everything. Are you saying relative is preferred over absolute now?

[kevinchalet]

This is considered a valid absolute Uri in .NET Framework and .NET Core 1.0.

That's right. And I'm not sure spreading this behavior was the right thing to do.

Probably this wouldn't be a valid input either and a check for the Scheme (either excluding file, or only allowing specific schemes) is appropriate anyhow.

The OAuth2 specification only forbids relative URLs and URLs that contain a fragment. Everything else is necessarily implementation-specific and thus not a good fit for a general check in a generic OIDC framework like the one I develop (end users are excepted to validate the redirect_uri using simple string comparison, anyway, so even if you specify a "file URI", it won't pass the validation stage unless the developer explicitly validates the redirect_uri).

Of course, it would make our lives easier if Uri really meant Uri and not UriOrFilePath. But given how fragile the distinction is, I'm not sure I want to add more checks in my code.

[Tratcher]

Interesting, the above HttpClient sample isn't broken anymore on Mac.

[tmds]

.NET Core 1.x:

new Uri("/tmp", UriKind.Absolute) -> throws
new Uri("/tmp", UriKind.RelativeOrAbsolute) -> Relative

.NET Core 2.0:

new Uri("/tmp", UriKind.Absolute) -> Absolute
new Uri("/tmp", UriKind.RelativeOrAbsolute) -> Relative

[tmds]

@PinpointTownes instead of excluding file, would it be appropriate for the implementation to check for http&https?

This specification is designed for use with HTTP ([RFC2616]). The
use of OAuth over any protocol other than HTTP is out of scope.

[kevinchalet]

@tmds nope, unfortunately. Using custom schemes is definitely allowed and even unavoidable in many cases (e.g in UWP, the redirect_uri is given by the WinRT APIs and starts with ms-app://[very long string]).

[tmds]

@PinpointTownes Then excluding file and blaming it on the Uri class is your best option.

[kevinchalet]

@tmds thing is there's nothing that prevents me from using file:///C:/callback.html as the redirect_uri. Adding an IsFile check would potentially reject redirect_uri that are compliant with the OAuth2 specification.

I guess I'll just disable the test on Linux/macOS... :trollface:

[tmds]

The rationale for "accepting unix paths" and "not accept them on Windows" is to "keep existing code working".
Trading off between backwards-compat and xplat...

[tmds]

@tmds thing is there's nothing that prevents me from using file:///C:/callback.html as the redirect_uri. Adding an IsFile check would potentially reject redirect_uri that are compliant with the OAuth2 specification.

Syntax would be compliant with the OAuth2 spec, but semantically file:// redirect_uris wouldn't make sense I guess.

[kevinchalet]

Well, accepting Unix paths using the file://... syntax is fine. What's way more annoying is the automatic conversion from /path to file:///path that can't even be disabled on Linux/macOS.

[kevinchalet]

Syntax would be compliant with the OAuth2 spec, but semantically file:// redirect_uris wouldn't make sense I guess.

file:// URLs are recognized by (most) browsers as valid navigation URLs. Even tho' I would definitely not do that myself, you could have a redirect_uri that points to a local file (or to a remote file in a shared folder) and that executes some JS logic.

While blocking file:// URIs would make sense in 99,99% cases, it's an assumption that goes beyond what the specification says.

[tmds]

I wonder if you could write your implementation using RelativeOrAbsolute and then check for Absolute.

[kevinchalet]

@tmds interesting, I'll give that a try tonight (I'm currently trying to determine why I'm getting a The type 'ECCurve' exists in both 'System.Core' and 'System.Security.Cryptography.Algorithms' error when targeting .NET 4.7).

[kevinchalet]

I suppose I could also use Uri.IsWellFormedUriString() instead of TryCreate()? According to the validation rules listed at https://msdn.microsoft.com/fr-fr/library/system.uri.iswellformeduristring(v=vs.110).aspx, /tmp shouldn't be considered as a valid absolute URI by this method, even on Linux/macOS.

[tmds]

it caused a lot of test issues a while back and we changed everything. Are you saying relative is preferred over absolute now?

@Tratcher I wonder if you saw this when using mono?

Mono: new Uri("/tmp", UriKind.RelativeOrAbsolute) -> Absolute

Since Mono 4.2 this behavior can be changed: http://www.mono-project.com/docs/faq/known-issues/urikind-relativeorabsolute/.

[kevinchalet]

@tmds FYI, I added a IsWellFormedOriginalString() check to ensure implicit paths are rejected and it seems to work a like charm.

I'll keep this ticket open in case you'd want to add a compat' switch to disable implicit paths support on Linux/macOS at the global level.

[tmds]

I think we shouldn't add a compat switch. Based on the discussion we had, these are the proper solutions for users that relied on Uri not accepting absolute paths starting with "/":

• check the Scheme, disallow File or only allow specific Schemes
• use IsWellFormedOriginalString to ensure syntax is a proper url

[karelz]

Next steps:

• Have a set of tests
• Create proposal for changes (behavioral)

Principles:

• Windows should not break
• File path auto-detection is lower priority than http Uri scheme correct behavior (we are not IO API)
• Exactly same behavior across OSs is nice-to-have but not mandotory

[kevinchalet]

@karelz hum, correct me if I'm wrong, but I thought the outcome of the discussion was that this change had been deliberately introduced in 2.0 to make Uri work with Unix file paths and that we'd basically have to live with that. Do you envision any change in 2.2?

[rmkerr]

@PinpointTownes I think the plan right now is just to write a set of tests to validate that our current behavior is consistent. I wouldn't anticipate a change unless we can come up with situations where things really are broken.

As mentioned by @karelz above we need to make sure that web scenarios (ie, http) work as expected. If they don't, that will be enough justification to change things. I will also be testing Unix style paths though, so we will at least know where we're at.

[tmds]

@karelz @rmkerr https://github.com/dotnet/corefx/pull/15925 included the appropriate tests. The support for unix paths is in the file:// scheme, so no effect on http. The unix paths are only supported on Unix, so no effect on Windows. Devs that rely on '/' not being a valid uri, should change to the options listed here. I think we can close this issue.

[tmds]

Note that this is a 2.0 change, afaik there were no other issues reported and @PinpointTownes (and others who ran into this) changed their code.

[rmkerr]

Thanks for pointing us in the right direction. I might add some tests before closing this issue, but it does sound like this has been addressed.

[andreujuanc]

Hi,
I just deployed a new service , and we chose .net core for it.

I'm using 2.1.401

I created a typed httpclient and added the base address like so: client.BaseAddress = new Uri(Configuration["SOMENEWSERVICE_URL"]);

Docker compose:
SOMENEWSERVICE_URL=http://somenewservice_url

Then in somewhere i do this: await Client.GetStringAsync("/api/values")
Works on windows, but fails on linux, it's trying to do this request: http://somenewservice_url//api/values
NOTE the double slash.

• Based on https://github.com/dotnet/corefx/blob/master/src/System.Net.Http/src/System/Net/Http/HttpClient.cs I assume that the string uri is being parsed like this new Uri(uri, UriKind.RelativeOrAbsolute);
• GetStringAsync(string requestUri) => GetStringAsync(CreateUri(requestUri));
• When I do this: https://github.com/dotnet/corefx/blob/master/src/System.Net.Http/src/System/Net/Http/HttpClient.cs#L666 with both values, BaseAddress and the relative parsed "/api/values", and it shows the correct result.

Is this something that was fixed and still not released? D: I dont want to validate input for every request. I might change the signature from a string to a Uri.

I was going to open a new issue about this, but this seems to be the same problem.

Edit, Maybe @karelz may know the status

UPDATE:
I just got http://somenewservice_url//api/values by using Uri only.

Say A = new Uri("http://domain") and B= new Uri("/api") then why the output in linux is http://domain//api when using HttpClient with base address.

This SO question : https://stackoverflow.com/questions/23438416/why-is-httpclient-baseaddress-not-working

Also RFC https://tools.ietf.org/html/rfc3986#section-5.2.3

UPDATE: Sorry for my rant but
this: new Uri(new Uri("http://domain"), new Uri("/api", UriKind.RelativeOrAbsolute));
returns http://domain/api in windows and Linux.

So there is a bug/undocumented feature in the HttpClient class or HttpMessageInvoker class.

[tmds]

@andreujuanc can you show a small, executable code snippet and describe how behavior differs between Windows and Linux?

[andreujuanc]

Sorry @tmds right now Im unable to do so. I'll set up a reminder .

[wdolek]

This thing just bit me! But if there's way how to check URI using .IsWellFormedOriginalString(), isn't it enough?

[tmds]

But if there's way how to check URI using .IsWellFormedOriginalString(), isn't it enough?

That should be enough. Either you use IsWellFormedOriginalString, or you check the Scheme.

[karelz]

Goal for 3.0: Understand how much we changed in 3.0 against 2.1/2.2.
Confirm current behavior is correct and we want to stick with it.

[tmds]

Goal for 3.0: Understand how much we changed in 3.0 against 2.1/2.2.

The change was made in 2.0. Nothing changed in 2.1/2.2.

Confirm current behavior is correct and we want to stick with it.

This is consistent with Windows where the Uri class also accepts an absolute file system path. Without that other things break.
To check the uri format use IsWellFormedOriginalString, and to disallow file paths, check the Scheme. This behaves consistent cross-platform.

[wtgodbe]

Given the following, I'm going to close this issue as resolved/intentional:

• The change was made in 2.0, so is not a 3.0 regression
• We intend to support absolute file system paths as URIs
• We have a solution for users that relied on Uri not accepting absolute paths starting with "/":
  > + check the Scheme, disallow File or only allow specific Schemes
  > + use IsWellFormedOriginalString to ensure syntax is a proper url
• We have a set of tests for this behavior, added by https://github.com/dotnet/corefx/pull/15925/files

[Maximys]

Is this bug fixed? Today I had spend whole day for understand what's going wrong with my microservice (.NET Core 2.2). On my developer PC with Windows 10 everything works fine, but on Docker with CentOS my microservice goes to invalid link ("file:////helloWorldService/getInfo").
I had try to reproduce this behavior on my PC (the same appconfig, the same environment variable), but no luck. Then, I had add logging to all lines of my code and only after that I found, that
c# Uri returnValue; if (!Uri.TryCreate(settingsFromConfig, UriKind.Absolute, out returnValue)) { returnValue = //other initialization } return returnValue;
works wrong. Can anybody tell me, what's wrong?

[karelz]

This was fixed in 3.0. Did you try one of the latest Previews?

[Maximys]

Thanks, @karelz . No, I did not try.
May be Microsoft can make hotfix for .NET Core 2.2?

[tmds]

@karelz @Maximys the behavior is by design and hasn't changed since 2.0. See https://github.com/dotnet/corefx/issues/22098#issuecomment-504500695.

[mlaily]

I wish Uri.TryCreate("/tmp", UriKind.Absolute, out var _) was true on all platforms... :/