False negatives with Url.IsValid

[tomasr78]

The SetQueryParam method produces "Cannot create a Request. Neither BaseUrl nor the first segment passed is a valid URL. " exception on some strings like

var uri = "http://myhost.com/upload".SetQueryParam("name", "Dedant-Simon - ELR & Hollange - IPESS Ougrée pg.2.jpg")

The exception is throw on POST

        using (var client = new FlurlClient(uri))
        {
            fileUploadResultDto = client.Request()
                .WithHeader("accept", "application/json")
                .PostMultipartAsync(p =>
                {
                    var fileStream = file.OpenRead();
                    p.AddFile(fileName, fileStream, fileName, GetMimeType(Path.GetExtension(fileName)));
                })
                .ReceiveJson<FileUploadResultDto>().Result;
        }

If I replace the string "Dedant-Simon - ELR & Hollange - IPESS Ougrée pg.2.jpg" with simple one everything works fine.

[tmenier]

Sorry for the delay. This appears to be legit. The error is thrown from FlurlClient.Request when it does not have a valid absolute URI to use as a starting point. Under the hood it uses Uri.IsWellFormedUriString to make that determination. I took your example and reduced it as much as possible to come up with these tests:

Assert.IsTrue(Uri.IsWellFormedUriString("http://myhost.com/%26", UriKind.Absolute)); // pass
Assert.IsTrue(Uri.IsWellFormedUriString("http://myhost.com/%C3%A9", UriKind.Absolute)); //pass
Assert.IsTrue(Uri.IsWellFormedUriString("http://myhost.com/%26%C3%A9", UriKind.Absolute)); //fail

WTF.

After doing some searching I found this:

https://github.com/dotnet/corefx/issues/19630

This comment seems to nail it exactly - it's the combination of different types of encoded characters that seems to trigger the false negative.

I'll keep this open but I'm not quite sure what to do about it yet. Hopefully you have a work-around for now.

[tmenier]

This might be the answer, i.e. testing it in its unescaped form.

[julian94]

We had this same issue, and what we did as a workaround as to explicitly make a FlurlRequest instead of just making the request from the string.
I.e:
new FlurlRequest(url).WithClient(new FlurlClient(Http)).AllowAnyHttpStatus().WithBasicAuth(username, password).GetAsync()
instead of:
url.WithClient(new FlurlClient(Http)).AllowAnyHttpStatus().WithBasicAuth(username, password).GetAsync()

[bluewalk]

This one seems to be back in 3.x? Haven't had any problems with the 2.x builds but since 3.x this error is thrown, using the same code @julian94 mentions (second line)

[tmenier]

@bluewalk This is an open issue that hasn't been fixed. If you have a specific example of a scenario that works in 2.x and not 3.x, please provide it.

[bluewalk]

@tmenier Of course, you can actually the diff from a project below to see which changes I had to make to get it to work with 3.x

bluewalk/lidlplus-dotnet-client@786f218

I've had to make the same type of changes with other projects as well, remove the baseUrl for the client and prepend it to the string used for IFlurlRequest.

[kemsky]

This is definetely a problem:

        [Test]
        public void TwoParamsEncoded()
        {
            var url = Url.Parse("https://www.google.com");

            url.SetQueryParam("name", Url.Encode("á"), isEncoded: true);

            url.SetQueryParam("email", Url.Encode("@"), isEncoded: true);

            Assert.That(Url.IsValid(url), Is.True); // fails
        }

Literally out of nowhere...

[tmenier]

Bummer, I was hoping MS would fix this in .NET 6 but apparently that won't happen.

If you've run into this issue, please upvote this one.

[tmenier]

More examples reported in #717:

var x1 = Url.IsValid("http://www.example.com?q=® ts"); // false
var x2 = Url.IsValid("http://www.example.com?q=®ts"); // true
var x3 = Url.IsValid("http://www.example.com?q=%C2%AE%20ts"); //false
var x4 = Url.IsValid("http://www.example.com?q=%C2%AEts"); // true

[tmenier]

.NET team reported this won't get fixed in .NET 8 either. A work-around may be something along the lines of what was done in #761 but we're not quite there.