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

New Url.TryParse method, and possibly breaking change for Url.Parse #696

Open
gunr2171 opened this issue May 21, 2022 · 3 comments
Open

Comments

@gunr2171
Copy link

gunr2171 commented May 21, 2022

This feature request has two parts:

  1. New TryParse static method
  2. *Parse methods actually parse

Part 1 - TryParse

I think it would be beneficial to add a TryParse static method to the Url class. This would act similar to int.TryParse. An example of it's usage would be:

string userInputUrl = "http://exam!ple.com/abc"; // intentional error
if (!Url.TryParse(userInputUrl, out Url url))
{
    Console.WriteLine("That's not a valid url");
}

// continue to use "url" variable as normal.

The TryParse would remove the need to have a method such as this (using 3.0.6)

bool TryParseUrl(string rawUrl, [NotNullWhen(true)] out Url? parsedUrl)
{
    parsedUrl = null;
    try
    {
        parsedUrl = Url.Parse(rawUrl);
        _ = parsedUrl.Host; // trigger parsing in Url class ("EnsureParsed").
        return true;
    }
    catch // specific type would be good
    {
        return false;
    }
}

Currently, Url.Parse() and new Url(string) don't actually do any parsing; they're lazy. EnsureParsed (the private instance method) only gets called when accessing one of the Url's properties. This means that a Url object can be successfully constructed from an improperly formatted URL string, and an exception will only be thrown when a property is used.

While TryParse could be implemented with my example implementation above, I think it would be better for the Url class to call EnsuredParse itself after the constructor has been run because it has access to it.

Part 2 - Parse should parse

This will cause a breaking change, and I 100% understand if it's not feasible.

int has Parse and TryParse. Both methods attempt to convert a string into an integer, but Parse will let exceptions bubble up while TryParse will capture the exception and instead return a boolean based on if one was thrown.

As a developer, I would expect the Parse method to throw an exception if the value I pass it can't be parsed. That's true with int.Parse, that's not true with Url.Parse.

@tmenier
Copy link
Owner

tmenier commented Dec 16, 2022

Thank you for the well-thought-out idea and I apologize that it slipped through the cracks. Given that Url.Parse supports relative URLs and (generally) encodes illegal characters, I do think the scenarios where it will fail are pretty limited. You provided one (illegal character in the hostname), but quite honestly that's an error bubbled up from Uri that wasn't even necessarily intended (although I think it's reasonable to leave it be).

All of that said, I will leave this open for future consideration.

@tmenier
Copy link
Owner

tmenier commented Dec 20, 2022

I'm starting to warm up to this idea. At the very least, I think Url.Parse should parse eagerly, and I've implemented that. Before adding a TryParse however, I'm thinking an optional parameter to specify whether it's relative or absolute might be a good idea. As a method used to validate user input, it seems like it would be much more valuable in cases where an absolute URL is required.

@jovball
Copy link

jovball commented Jul 18, 2023

I'll add another vote for TryParse. As of now, it's easier to use Uri.TryCreate and then pass the Uri output variable to create a Flurl Url (shown below). I'd rather not have to use Uri at all.

if (Uri.TryCreate(uri, UriKind.Absolute, out var redirectUri)) { var url = new Flurl.Url(redirectUri); }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants