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

[API Proposal]: IPAddress.IsValid #111282

Closed
MihaZupan opened this issue Jan 10, 2025 · 2 comments · Fixed by #111433
Closed

[API Proposal]: IPAddress.IsValid #111282

MihaZupan opened this issue Jan 10, 2025 · 2 comments · Fixed by #111433
Labels
api-approved API was approved in API review, it can be implemented area-System.Net in-pr There is an active PR which will close this issue when it is merged
Milestone

Comments

@MihaZupan
Copy link
Member

Background and motivation

Cheaper IPAddress.TryParse(span, out _) for when you don't need the result.

API Proposal

namespace System.Net;

public partial class IPAddress
{
    public static IPAddress Parse(string ipString);
    public static IPAddress Parse(ReadOnlySpan<char> ipSpan);
    public static IPAddress Parse(ReadOnlySpan<byte> utf8Text);
    public static bool TryParse(string? ipString, out IPAddress? address);
    public static bool TryParse(ReadOnlySpan<char> ipSpan, out IPAddress? address);
    public static bool TryParse(ReadOnlySpan<byte> utf8Text, out IPAddress? result);

+   public static bool IsValid(ReadOnlySpan<char> ipSpan);
+   public static bool IsValid(ReadOnlySpan<byte> utf8Text);
}

API Usage

-if (IPAddress.TryParse(text, out _))
+if (IPAddress.IsValid(text))

We have an internal helper TargetHostNameHelper.IsValidAddress that appears to be doing exactly what IPAddress.IsValid would be:

internal static unsafe bool IsValidAddress(string? hostname)

Alternative Designs

  • Allow specifying IPv4 or IPv6 only?

Risks

No response

@MihaZupan MihaZupan added api-suggestion Early API idea and discussion, it is NOT ready for implementation area-System.Net labels Jan 10, 2025
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Jan 10, 2025
Copy link
Contributor

Tagging subscribers to this area: @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

@MihaZupan MihaZupan added this to the Future milestone Jan 10, 2025
@MihaZupan MihaZupan added api-ready-for-review API is ready for review, it is NOT ready for implementation and removed untriaged New issue has not been triaged by the area owner api-suggestion Early API idea and discussion, it is NOT ready for implementation labels Jan 10, 2025
@terrajobst
Copy link
Contributor

terrajobst commented Jan 14, 2025

Video

  • We'd prefer to rename the second one because there is a constructor that takes a byte span which is expected to be the binary encoding
namespace System.Net;

public partial class IPAddress
{
    // Existing:
    // public static IPAddress Parse(string ipString);
    // public static IPAddress Parse(ReadOnlySpan<char> ipSpan);
    // public static IPAddress Parse(ReadOnlySpan<byte> utf8Text);
    // public static bool TryParse(string? ipString, out IPAddress? address);
    // public static bool TryParse(ReadOnlySpan<char> ipSpan, out IPAddress? address);
    // public static bool TryParse(ReadOnlySpan<byte> utf8Text, out IPAddress? result);

    public static bool IsValid(ReadOnlySpan<char> ipSpan);
    public static bool IsValidUtf8(ReadOnlySpan<byte> utf8Text);
}

@terrajobst terrajobst added api-approved API was approved in API review, it can be implemented and removed api-ready-for-review API is ready for review, it is NOT ready for implementation labels Jan 14, 2025
@dotnet-policy-service dotnet-policy-service bot added the in-pr There is an active PR which will close this issue when it is merged label Jan 14, 2025
@MihaZupan MihaZupan modified the milestones: Future, 10.0.0 Jan 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api-approved API was approved in API review, it can be implemented area-System.Net in-pr There is an active PR which will close this issue when it is merged
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants