[API Proposal]: Add a type to represent IP networks based on CIDR notation

[geeknoid]

Background and motivation

We've been using a custom IPAddressRange in our project, but this seems like a general-purpose concern. Looking across internal code bases at Microsoft, we've found dozens of similar implementations of this type, reinforcing the notion that this is general-purpose and should be in the framework.

API Proposal

Edit by @antonfirsov: Added implementations for ISpanFormattable and IParsable<IPSubnet>, harmonized naming of method arguments to match current BCL standards.

namespace System.Net;

// Alternative name ideas: IPSubnet, IPAddressSpace
public readonly struct IPNetwork : IEquatable<IPSubnet>, ISpanFormattable, ISpanParsable<IPSubnet>
{
    public IPAddress BaseAddress { get; }
    public byte MaskLength { get; }

    public IPNetwork(IPAddress baseAddress, byte maskLength);

    public bool Contains(IPAddress address);

    // ISpanParsable
    public static IPNetwork Parse(string s, IFormatProvider? provider = null);
    public static bool TryParse(string s, IFormatProvider? provider, out IPNetwork result);
    public static IPNetwork Parse(ReadOnlySpan<char> s, IFormatProvider? provider = null);
    public static bool TryParse(ReadOnlySpan<char> s, IFormatProvider? provider, out IPNetwork result);

    // Parsing - Convenience methods
    public static bool TryParse(string s, out IPNetwork result);
    public static bool TryParse(ReadOnlySpan<char> s, out IPNetwork result);

    // ISpanFormattable
    public string ToString(string? format, IFormatProvider? provider = null);
    public bool TryFormat(Span<char> destination, out int charsWritten, ReadOnlySpan<char> format = default, IFormatProvider? provider = null);

    // Equality
    public bool Equals(IPNetwork value);
    public static bool operator ==(IPNetwork left, IPNetwork right);
    public static bool operator !=(IPNetwork left, IPNetwork right);
}

API Usage

We're using this for parsing and logging, so the parsing and tostring functionality are particularly interesting.

Alternative Designs

This type could potentially implement ISpanFormattable<T> and ISpanParsable<T>, but those interfaces require an IFormatProvider instance which isn't relevant in this context. Still, those interfaces might provide better interop elsewhere in the framework. Note that IPAddress doesn't implement these interfaces either.

Risks

No response

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[ghost]

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

Issue Details

---

Background and motivation

We've been using a custom IPAddressRange in our project, but this seems like a general-purpose concern. Looking across internal code bases at Microsoft, we've found dozens of similar implementations of this type, reinforcing the notion that this is general-purpose and should be in the framework.

API Proposal

namespace System.Net;

/// <summary>
/// Represents an IP address range using CIDR notation.
/// </summary>
/// <remarks>The range contains addresses starting with <see cref="BaseAddress"/>,
/// and including every combination of bit values for the rightmost variable part after the subnet mask.
/// I.e. "13.73.248.16/29" will contain 2^3 addresses. 32(IPv4 bits) - 29(subnet mask length) = 3(remaining variable bits).
/// </remarks>
public readonly struct IPAddressRange : IEquatable<IPAddressRange>
{
    /// <summary>
    /// Gets the network prefix in CIDR notation.
    /// </summary>
    /// <remarks><see href="https://en.wikipedia.org/wiki/Classless_Inter-Domain_Routing#CIDR_notation"/>.</remarks>
    public IPAddress BaseAddress { get; }

    /// <summary>
    /// Gets the subnet mask length in bits.
    /// </summary>
    /// <remarks><see href="https://en.wikipedia.org/wiki/Classless_Inter-Domain_Routing#CIDR_notation"/>.</remarks>
    public byte MaskLength { get; }

    /// <summary>
    /// Initializes a new instance of the <see cref="IPAddressRange"/> struct.
    /// </summary>
    /// <param name="baseAddress">Network prefix in CIDR notation.</param>
    /// <param name="maskLength">Subnet mask length in bits.</param>
    public IPAddressRange(IPAddress baseAddress, byte maskLength);

    /// <summary>
    /// Parses string IP range to <see cref="IPAddressRange"/> object.
    /// </summary>
    /// <param name="rangeString">IP range in CIDR notation.</param>
    /// <returns>Parsed IP range.</returns>
    /// <exception cref="FormatException">Thrown if passed value is not in correct format.</exception>
    public static IPAddressRange Parse(string rangeString);

    /// <summary>
    /// Parses string IP range to <see cref="IPAddressRange"/> object.
    /// </summary>
    /// <param name="rangeSpan">IP range in CIDR notation.</param>
    /// <returns>Parsed IP range.</returns>
    /// <exception cref="FormatException">Thrown if passed value is not in correct format.</exception>
    public static IPAddressRange Parse(ReadOnlySpan<char> rangeSpan, IFormatProvider? provider);

    /// <summary>
    /// Parses string IP range to <see cref="IPAddressRange"/> object.
    /// </summary>
    /// <param name="rangeString">IP range in CIDR notation.</param>
    /// <param name="range">Parsed IP range.</param>
    /// <returns><see langword="true"/> if parsing was successful, otherwise <see langword="false"/>.</returns>
    public static bool TryParse(string rangeString, IFormatProvider? provider, out IPAddressRange range);

    /// <summary>
    /// Parses string IP range to <see cref="IPAddressRange"/> object.
    /// </summary>
    /// <param name="rangeSpan">IP range in CIDR notation.</param>
    /// <param name="range">Parsed IP range.</param>
    /// <returns><see langword="true"/> if parsing was successful, otherwise <see langword="false"/>.</returns>
    public static bool TryParse(ReadOnlySpan<char> rangeSpan, out IPAddressRange range);

    /// <summary>
    /// Tests whether a particular address falls within the current range.
    /// </summary>
    /// <param name="ip">The IP address to test.</param>
    /// <returns><see langword="true"> if the given address is covered by the range; otherwise <see langword="false" />.</returns>
    public bool Contains(IPAddress ip);

    /// <summary>
    /// Checks if objects are equal.
    /// </summary>
    /// <param name="obj">Object to check.</param>
    /// <returns>True if equal, false otherwise.</returns>
    public override bool Equals(object? obj);

    /// <summary>
    /// Checks if objects are equal.
    /// </summary>
    /// <param name="other">Object to check.</param>
    /// <returns>True if equal, false otherwise.</returns>
    public bool Equals(IPAddressRange other);

    /// <summary>
    /// Gets hash code.
    /// </summary>
    /// <returns>Hash code.</returns>
    public override int GetHashCode();

    /// <summary>
    /// Produces a CIDR representation of this range.
    /// </summary>
    public override string ToString();

    /// <summary>
    /// Compares two ranges.
    /// </summary>
    /// <param name="left">Left element to compare.</param>
    /// <param name="right">Right element to compare.</param>
    /// <returns><see langword="true" /> when equal, <see langword="false" /> otherwise.</returns>
    public static bool operator ==(IPAddressRange left, IPAddressRange right);

    /// <summary>
    /// Compares two ranges.
    /// </summary>
    /// <param name="left">Left argument of the comparison.</param>
    /// <param name="right">Right argument of the comparison.</param>
    /// <returns><see langword="true" /> when not equal, <see langword="false" /> otherwise.</returns>
    public static bool operator !=(IPAddressRange left, IPAddressRange right);
}

API Usage

We're using this for parsing and logging, so the parsing and tostring functionality are particularly interesting.

Alternative Designs

This type could potentially implement ISpanFormattable<T> and ISpanParsable<T>, but those interfaces require an IFormatProvider instance which isn't relevant in this context. Still, those interfaces might provide better interop elsewhere in the framework. Note that IPAddress doesn't implement these interfaces either.

Risks

No response

Author:	geeknoid
Assignees:	-
Labels:	api-suggestion, area-System.Net, untriaged
Milestone:	-

[teo-tsirpanis]

Should it have a bool TryFormat(Span<char>, out int)?

[NN---]

When I saw IPAddress range, I assumed a range of address not only by a bitmask.
E.g.
AddRange(IPAddress start, IPAddress end)

[svick]

This looks like a duplicate of #64033 and #36428, which were closed as being too niche.

Note that ASP.NET Core does have the IPNetwork class, though it's quite limited.

[sakno]

IPNetwork2 perfectly serves to that purpose.

[geeknoid]

This looks like a duplicate of #64033 and #36428, which were closed as being too niche.

Definitely in the eye of the beholder :-) As I said, we've found dozens of implementations of this within Microsoft's various code bases.

Note that ASP.NET Core does have the IPNetwork class, though it's quite limited.

Missed that one. From a composition perspective, it's a bit too high in the stack for what we'd like I think. But it does show that the concept is a useful one.

[geeknoid]

Should it have a bool TryFormat(Span<char>, out int)?

Definitely could, and it could also implement ISpanFormattable/ISpanParsable. Note that IPAddress could also use these.

[teo-tsirpanis]

And how about we call it IPAddressSubnet? It more closely matches what it is.

[geeknoid]

@mddddb @rymeskar

[mddddb]

And how about we call it IPAddressSubnet? It more closely matches what it is.

IMO in order to make this an actual IP address range maybe we should make this more flexible in terms of the First address and the Last address in the range, i.e. cover the cases of custom address ranges that cannot be covered by CIDR notation (subset of addresses in a subnet).

So we can have both
(IPAddress begin, IPAddress end) and (IPAddress baseAddress, byte maskLength) constructors, and more if needed.
Although I am not sure if we need to have some kind of a property specifying whether the range can be considered valid in CIDR.

[geeknoid]

@mddddb Then the MaskLength property would need to be removed or made optional or something, since a range might not in fact be just a mask thing.

[mddddb]

@geeknoid Yep, I am thinking either a nullable prop or a method with nullable return type

[karelz]

Triage: Prioritizing it for 8.0 as the number of ad-hoc implementations clearly shows demand.
During design we should take into account previous asks pointed out by @svick (thanks!) above: #79946 (comment)

[antonfirsov]

IMO in order to make this an actual IP address range maybe we should make this more flexible in terms of the First address and the Last address in the range, i.e. cover the cases of custom address ranges that cannot be covered by CIDR notation

This doesn't sound like a feature that would be highly used. Instead I would just go with a class name that is less confusing, eg. IPNetwork sounds good enough. Edit: IPAddressSubnet from #79946 (comment) also works.

[antonfirsov]

Triage:

• To avoid duplications, we should clarify with the aspnet team if they would be able to adapt this new type. Ideal scenario: add new API-s with the BCL type and obsolete old ones using IPNetwork. Less ideal: adapt the implementation at least. We want to see how far this can be pushed.
• The name should be IPSubnet or IPNetwork. IPSubnet is slightly preferred, because it doesn't collide with aspnet's IPNetwork.
• We should consider adding ReadOnlySpan<char> formatting overloads without implementing ISpanFormattable<T>, similarly to IPAddress.

[geeknoid]

@antonfirsov What's the motivation for not implementing ISpanFormattable?

[antonfirsov]

@geeknoid we would also need to implement it on IPAddress then, otherwise the API would look inconsistent. I'm in favor keeping this proposal as lean as possible in order to get it approved and done quickly. Implementing ISpanFormattable on both IPAddress and IPSubnet/Network can be a follow-up enhancement.

[antonfirsov]

I realized now that since IPAddress is mutable, readonly struct would not guarantee actual immutability of entire instances. Shall we turn the type into a class instead to avoid the false illusion of an immutable type? Are there any performance considerations behind the proposed type being a struct? (= do we expect many allocations of instances?)

[geeknoid]

readonly struct does not guarantee deep immutability.

We generally prefer structs for things that are small and not extensible. We do have a couple scenarios which maintain a bunch of these things in collections, so having those as structs is helpful there. Not a big deal either way though.

[stephentoub]

we would also need to implement it on IPAddress then, otherwise the API would look inconsistent

IPAddress already exposes both TryFormat and TryParse. Whether or not it also inherits the interfaces doesn't impact whether IPSubnet can implement TryFormat/TryParse nor whether it implements the interfaces. We should design the type in this issue the way we think it should be, inclusive of the interfaces if we think it should implement them; it'd be fine to file a separate issue to follow-up on adding the interfaces to IPAddress if we think that's desirable.

[antonfirsov]

@geeknoid are you interested to port your implementation to dotnet/runtime? If yes, any ETA?

[mddddb]

@antonfirsov @karelz @geeknoid A draft PR has been created, there are couple of questions I left there. There is still some polishing and documentation left to be done, although I am not sure what the preference is on that part - should I continue with the changes? Or will someone take it from here? Anyway, some feedback is appreciated before the PR gets published

[antonfirsov]

@mddddb thanks! I will take a look tomorrow. It would be nice if you can react to basic feedback, but I'm happy to take over more complicated stuff, eg. the changes needed if we decide to ship it as a NuGet package.

[mddddb]

@antonfirsov I am happy to make the necessary changes, as long as we don't have burning deadlines. This is an interesting exercise and a new experience to me, I just have to juggle this together with other tasks that I have🙂

[antonfirsov]

We should consider producing a netstandard2.0-consumable OOB package for this type similarly to #65577 / #79120.
Following that precedent, I propose the package name Microsoft.Bcl.Net.Primtives, that should target netstandard2.0, net8.0 (with typeoforward), and probably .NET Framework if that's our preferred practice.

However, we would need to omit ISpanFormattable and ISpanParsable<IPNetwork> for the old targets. Those API-s should be conditional to net8.0+.

namespace System.Net;

public readonly struct IPNetwork : IEquatable<IPNetwork>
#if NET8_0_OR_GREATER
  ISpanFormattable, ISpanParsable<IPNetwork>
#else
  IFormattable
#endif
{
    public IPAddress BaseAddress { get; }
    public int PrefixLength { get; }

    public IPNetwork(IPAddress baseAddress, int prefixLength);

    public bool Contains(IPAddress address);

#if NET8_0_OR_GREATER
    // ISpanParsable (explicit)
    static IPNetwork ISpanParsable<IPNetwork>.Parse(string s, IFormatProvider? provider = null);
    static bool ISpanParsable<IPNetwork>.TryParse(string s, IFormatProvider? provider, out IPNetwork result);
    static IPNetwork ISpanParsable<IPNetwork>.Parse(ReadOnlySpan<char> s, IFormatProvider? provider = null);
    static bool ISpanParsable<IPNetwork>.TryParse(ReadOnlySpan<char> s, IFormatProvider? provider, out IPNetwork result);
#endif

    // Parsing - Public methods
    public static IPNetwork Parse(string s);
    public static IPNetwork Parse(ReadOnlySpan<char> s);
    public static bool TryParse(string s, out IPNetwork result);
    public static bool TryParse(ReadOnlySpan<char> s, out IPNetwork result);

    // IFormattable (explicit)
    string IFormattable<IPNetwork>.ToString(string? format, IFormatProvider? provider = null);

#if NET8_0_OR_GREATER 
    // ISpanFormattable (explicit)
    bool ISpanFormattable<IPNetwork>.TryFormat(Span<char> destination, out int charsWritten, ReadOnlySpan<char> format = default, IFormatProvider? provider = null);
    // IPAddress has on TryFormat(Span,int) under netstandard2.0, so we are excluding this overload too
    public bool TryFormat(Span<char> destination, out int charsWritten);
#endif

    // Formatting (public)
    public override string ToString();

    // Equality
    public bool Equals(IPNetwork value);
    public static bool operator ==(IPNetwork left, IPNetwork right);
    public static bool operator !=(IPNetwork left, IPNetwork right);
}

[scottleyg]

I think that my initial recommendation includes the concepts discussed here such as where to use struct versus class, what are the "core" behaviors of the class and what are extensions. The framework has changed much since my initial reference; am happy to extend/change the initial reference to meet the ISpanParseable and other shapes.

some comments:

1. the name of this class must not be IPNetwork - an IP Network considers many other concepts that would lead this class to be misunderstood as to its purpose (e.g. IPNetwork considers routing, nat'ing. The scope of this class is to encapsulate logic related to CIDR notation). It should be CidrBlock to match the RFC Specification for such an object which also describes the use cases for such an object.
2. I have implemented a class derived from IPAddress to share parsing behavior, but use IPv4Numeric and IPv6Numeric structs that implement an IPNumeric interface for usage when performing extensions, like Contains, IsAdjacentTo, and Overlaps.

[antonfirsov]

@scottleyg this is a lightweight API focusing on core functionality - parsing and Contains. The API in #36428 seems to be really heavyweight, useful advanced for tools dealing with CIDR.

It should be CidrBlock

I actually considered that name, but "IP Network" seemed to be the a more widespread name for types dealing with this concern, eg. go has IPNet, python has ip_network, aspnet has (a now to be obsoleted) IPNetwork. The only example I was able to find that uses the word CIDR in their type names is rust (trait Cidr), but then the docs immediately say "Types implementing Cidr represent IP networks". I don't think there is a name that is ideal from all perspectives, and I think IPNetwork should be recognizable for most users, thus I find it to be an ok compromise. We are past API-review now anyways, and I'm about to merge the implementing PR.

If you think have some valuable additions to the approved API, which are important enough for common scenarios to be included in the BCL, feel free to open a new API proposal, but bare in mind that we will likely not commit to advanced/rare stuff.

[antonfirsov]

We are keeping this open to track downlevel packaging.

[danmoseley]

Microsoft.Bcl.Net.Primtives

Let's please avoid using the term BCL any more publicly. It's another thing that folks new to .NET have to learn.

[antonfirsov]

We have a bunch of packages prefixed Microsoft.Bcl and recently #79120 created the package Microsoft.Bcl.Cryptography. We can introduce a new naming scheme for new packages going forward, but IMO it will create more friction. /cc @terrajobst who's working on a design doc for OOB-ing.

[danmoseley]

We can introduce a new naming scheme for new packages going forward, but IMO it will create more friction.

yeah fair enough I got that feedback offline.

[karelz]

Triage: Remaining work is to decide on downlevel packaging - waiting for API Review Board's decision.

[tarekgh]

Is this still not decided yet? just trying to track the progress, no pressure.

[karelz]

I created separate issue to track the netstandard2.0 decision - #88965

[karelz]

Fixed in PR #82779