Skip to content

[API Proposal]: Implement IUtf8SpanParsable on IPAddress & IPNetwork #103111

Closed

Description

@edwardneal

Background and motivation

This continues the work in #81500, expanding the existing UTF8 support to IPAddress and IPNetwork. I'm trying to tackle the low-hanging from from the list of expected types.

cc @tannergooding and linking this to the earlier PR #102144 - I can see the issue's gone to System.Net rather than System.Runtime.

API Proposal

The original issue expected IUtf8SpanParsable to be implemented implicitly, and I've done this on IPAddress. I've implemented it explicitly on IPNetwork though, adding an additional pair of Parse and TryParse overloads. This is because IPNetwork also implements ISpanParsable<IPAddress> explicitly, and I wanted to keep the API surface consistent between the two interface implementations. The original justification for it to implement ISpanParsable<IPAddress> explicitly was that it doesn't support custom formatting (API review results here), and I believe the same rationale applies here.

namespace System.Net;

public partial class IPAddress
    : ISpanFormattable, ISpanParsable<IPAddress>, IUtf8SpanFormattable
+   , IUtf8SpanParsable<IPAddress>
{
+   public static IPAddress Parse(ReadOnlySpan<byte> utf8Text, IFormatProvider? provider);

+   public static bool TryParse(ReadOnlySpan<byte> utf8Text, IFormatProvider? provider,
+       [NotNullWhen(true)] out IPAddress? result);
}

public readonly partial struct IPNetwork
    : ISpanFormattable, ISpanParsable<IPAddress>, IUtf8SpanFormattable
+   , IUtf8SpanParsable<IPNetwork>
{
    static IPNetwork ISpanParsable<IPNetwork>.Parse(ReadOnlySpan<char> s, IFormatProvider? provider);
    public static IPNetwork Parse(ReadOnlySpan<char> s);
+   static IPNetwork IUtf8SpanParsable<IPNetwork>.Parse(ReadOnlySpan<byte> utf8Text, IFormatProvider? provider);
+   public static IPNetwork Parse(ReadOnlySpan<byte> utf8Text);

    static bool ISpanParsable<IPNetwork>.TryParse(ReadOnlySpan<char> s, IFormatProvider? provider, out IPNetwork result);
    public static bool TryParse(ReadOnlySpan<char> s, out IPNetwork result);
+   static bool IUtf8SpanParsable<IPNetwork>.TryParse(ReadOnlySpan<byte> utf8Text, IFormatProvider? provider,
+       out IPNetwork result);
+   public static bool TryParse(ReadOnlySpan<byte> utf8Text,
+       [MaybeNullWhen(false)] out IPNetwork? result);
}

API Usage

ReadOnlySpan<byte> utf8Localhost = "127.0.0.1"u8;
ReadOnlySpan<byte> utf8LocalhostNetwork = "127.0.0.0/8"u8;

IPAddress parsedLocalhost = IPAddress.Parse(utf8Localhost, null);
IPNetwork parsedLocalhostNetwork = IPNetwork.Parse(utf8LocalhostNetwork);

bool successfullyParsed;
successfullyParsed = IPAddress.TryParse(utf8Localhost, null, out parsedLocalhost);
successfullyParsed = IPNetwork.TryParse(utf8LocalhostNetwork, null, out parsedLocalhostNetwork);

Alternative Designs

No response

Risks

At present, all parsing of IPAddresses from ReadOnlySpan<char> is handled by IPAddressParser, IPv4AddressHelper and IPv6AddressHelper. This works completely with chars and performs minimal allocations. I'd prefer to make these classes generic, but their codebases are also used privately in Uri parsing and by System.Net.Quic. The work required would also replace some of the of references to char* with ReadOnlySpan<char> in Uri (and might slightly improve GC performance by eliminating a few cases where we pin strings), but the changes cut across more projects.

I could alternatively implement the interface by just calling Encoding.UTF8.GetChars on the UTF8 input and passing through to ISpanParsable<IPAddress>.Parse, leaving the underlying IP address parsing as char-only. The longest possible IP address would fit in a 64-character stackalloc'd buffer, so I don't think this approach would necessarily need to allocate - it's just a tradeoff between the wider impact of changing IPAddressParser and any efficiency loss from a separate transcoding stage.

Activity

  1. added
    api-suggestionEarly API idea and discussion, it is NOT ready for implementation
    on Jun 6, 2024
  2. dotnet-policy-service commented on Jun 6, 2024

    @dotnet-policy-service
    Contributor

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

  3. added this to the Future milestone on Jun 6, 2024
  4. added
    api-ready-for-reviewAPI is ready for review, it is NOT ready for implementation
    and removed
    untriagedNew issue has not been triaged by the area owner
    on Jun 6, 2024
  5. antonfirsov commented on Jun 6, 2024

    @antonfirsov
    Contributor

    Looks good as proposed, so marked api-ready-for-review. Triaging to Future since it's not critical for 9.0. Actually, since there is a PR open let's mark it in 9.0 in order to get the API reviewed faster.

    @dotnet/ncl please scream if you disagree with either of these decisions.

  6. modified the milestones: Future, 9.0.0 on Jun 6, 2024
  7. removed
    api-suggestionEarly API idea and discussion, it is NOT ready for implementation
    on Jun 6, 2024
  8. bartonjs commented on Jun 13, 2024

    @bartonjs
    Member

    Video

    • IPAddress doesn't respect the IFormatProvider, so should use the explicit implementation pattern.
    • Otherwise, looks good as proposed.
    namespace System.Net;
    
    public partial class IPAddress
        : ISpanFormattable, ISpanParsable<IPAddress>, IUtf8SpanFormattable
    +   , IUtf8SpanParsable<IPAddress>
    {
    +   static IPAddress IUtf8SpanParsable<IPAddress>.Parse(ReadOnlySpan<byte> utf8Text, IFormatProvider? provider);
    +   public static IPAddress Parse(ReadOnlySpan<byte> utf8Text);
    
    +   static bool IUtf8SpanParsable<IPAddress>.TryParse(ReadOnlySpan<byte> utf8Text, IFormatProvider? provider,
    +       [NotNullWhen(true)] out IPAddress? result);
    +   public static bool TryParse(ReadOnlySpan<byte> utf8Text, [NotNullWhen(true)] out IPAddress? result);
    
    }
    
    public readonly partial struct IPNetwork
        : ISpanFormattable, ISpanParsable<IPAddress>, IUtf8SpanFormattable
    +   , IUtf8SpanParsable<IPNetwork>
    {
        static IPNetwork ISpanParsable<IPNetwork>.Parse(ReadOnlySpan<char> s, IFormatProvider? provider);
        public static IPNetwork Parse(ReadOnlySpan<char> s);
    +   static IPNetwork IUtf8SpanParsable<IPNetwork>.Parse(ReadOnlySpan<byte> utf8Text, IFormatProvider? provider);
    +   public static IPNetwork Parse(ReadOnlySpan<byte> utf8Text);
    
        static bool ISpanParsable<IPNetwork>.TryParse(ReadOnlySpan<char> s, IFormatProvider? provider, out IPNetwork result);
        public static bool TryParse(ReadOnlySpan<char> s, out IPNetwork result);
    +   static bool IUtf8SpanParsable<IPNetwork>.TryParse(ReadOnlySpan<byte> utf8Text, IFormatProvider? provider,
    +       out IPNetwork result);
    +   public static bool TryParse(ReadOnlySpan<byte> utf8Text,
    +       [MaybeNullWhen(false)] out IPNetwork? result);
    }
  9. added
    api-approvedAPI was approved in API review, it can be implemented
    and removed
    api-ready-for-reviewAPI is ready for review, it is NOT ready for implementation
    on Jun 13, 2024
  10. karelz commented on Jun 25, 2024

    @karelz
    Member

    @MihaZupan is it fully covered in PR #102144?

  11. MihaZupan commented on Jul 2, 2024

    @MihaZupan
    Member

    Yes

  12. karelz commented on Jul 23, 2024

    @karelz
    Member

    Looks like this feature missed the Platform feature complete deadline yesterday. Moving it to .NET 10.

  13. modified the milestones: 9.0.0, 10.0.0 on Jul 23, 2024
  14. added
    in-prThere is an active PR which will close this issue when it is merged
    on Aug 26, 2024
  15. locked and limited conversation to collaborators on Nov 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    api-approvedAPI was approved in API review, it can be implementedarea-System.Netin-prThere is an active PR which will close this issue when it is merged

    Type

    No type

    Projects

    No projects

      Milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions