tmenier / Flurl

Fluent URL builder and testable HTTP client for .NET
https://flurl.dev
MIT License
4.22k stars 387 forks source link

SetQueryParam strips leading slash in file URI with empty hostname #751

Open markmccaigue opened 1 year ago

markmccaigue commented 1 year ago

Hi,

I have written the below test to illustrate what I suspect is a problem with the handling of slashes with file URIs.

[Test]
public void FlurlUrl_PreservesLeadingSlash()
{
    var urlString = "file:///document";

    var flurl = new Url(urlString);

    Assert.AreEqual(urlString, flurl.ToString());

    var queryKey = "queryKey";
    var queryValue = "queryValue";
    flurl = flurl.SetQueryParam(queryKey, queryValue);

    Assert.AreEqual($"{urlString}?{queryKey}={queryValue}", flurl.ToString());
}

I would expect this test to pass, but instead it fails with the following message:

String lengths are both 36. Strings differ at index 7.
Expected: "file:///document?queryKey=queryValue"
But was:  "file://document/?queryKey=queryValue"

In summary, I believe Flurl is removing that third slash and inadvertently generating an invalid URI.

Many thanks for your help.

tmenier commented 1 year ago

Interesting. I'll be honest, I didn't know the triple-slash notation was a thing when I wrote the normalization logic. But it looks legitimate and I'll make sure it's not tampered with when specified.

In summary, I believe Flurl is removing that third slash and inadvertently generating an invalid URI.

Technically not correct to say it's an invalid URI, but it's certainly the URI you expected, so point taken. 😉

tmenier commented 1 year ago

note to self: https://superuser.com/a/352134

markmccaigue commented 1 year ago

Interesting. I'll be honest, I didn't know the triple-slash notation was a thing when I wrote the normalization logic. But it looks legitimate and I'll make sure it's not tampered with when specified.

In summary, I believe Flurl is removing that third slash and inadvertently generating an invalid URI.

Technically not correct to say it's an invalid URI, but it's certainly the URI you expected, so point taken. 😉

Haha fair enough 😁 Thanks a million!