From e86db2fd69d46477a1aef8ba217bec5f0ce7b374 Mon Sep 17 00:00:00 2001 From: Murat Duisenbayev Date: Mon, 27 Feb 2023 19:27:59 +0100 Subject: [PATCH 01/13] Add draft IPNetwork type --- .../src/Resources/Strings.resx | 3 + .../src/System.Net.Primitives.csproj | 1 + .../src/System/Net/IPNetwork.cs | 324 ++++++++++++++++++ .../tests/FunctionalTests/IPNetworkTest.cs | 20 ++ ...tem.Net.Primitives.Functional.Tests.csproj | 1 + 5 files changed, 349 insertions(+) create mode 100644 src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs create mode 100644 src/libraries/System.Net.Primitives/tests/FunctionalTests/IPNetworkTest.cs diff --git a/src/libraries/System.Net.Primitives/src/Resources/Strings.resx b/src/libraries/System.Net.Primitives/src/Resources/Strings.resx index 5a998979f8548e..97a9b96ca04c5f 100644 --- a/src/libraries/System.Net.Primitives/src/Resources/Strings.resx +++ b/src/libraries/System.Net.Primitives/src/Resources/Strings.resx @@ -78,6 +78,9 @@ An invalid IP address was specified. + + An invalid IPNetwork was specified. + An error occurred when adding a cookie to the container. diff --git a/src/libraries/System.Net.Primitives/src/System.Net.Primitives.csproj b/src/libraries/System.Net.Primitives/src/System.Net.Primitives.csproj index 46fd9cc48db7fb..47385e4c229894 100644 --- a/src/libraries/System.Net.Primitives/src/System.Net.Primitives.csproj +++ b/src/libraries/System.Net.Primitives/src/System.Net.Primitives.csproj @@ -28,6 +28,7 @@ + + /// + /// Provides an Internet Protocol (IP) subnet/range in CIDR notation. + /// + /// + public readonly struct IPNetwork : IEquatable, ISpanFormattable, ISpanParsable + { + public IPAddress BaseAddress { get; private init; } + public int PrefixLength { get; private init; } + + private const int bitsPerByte = 8; + private const string + baseAddressParamName = "baseAddress", + prefixLengthParamName = "prefixLength"; + public IPNetwork(IPAddress baseAddress, int prefixLength) + { + BaseAddress = baseAddress; + PrefixLength = prefixLength; + + var validationError = Validate(); + if (validationError.HasValue) + { + throw validationError.Value.CtorExceptionFactoryMethod(); + } + } + + public bool Contains(IPAddress address) + { + ArgumentNullException.ThrowIfNull(address); + + if (address.AddressFamily != BaseAddress.AddressFamily) + { + return false; + } + + var expectedBytesCount = GetAddressFamilyByteLength(BaseAddress.AddressFamily); + if (expectedBytesCount * bitsPerByte == PrefixLength) + { + return BaseAddress.Equals(address); + } + + Span baseAddressBytes = stackalloc byte[expectedBytesCount]; + if (!BaseAddress.TryWriteBytes(baseAddressBytes, out _)) + { + throw new UnreachableException(); + } + + Span otherAddressBytes = stackalloc byte[expectedBytesCount]; + if (!address.TryWriteBytes(otherAddressBytes, out _)) + { + throw new UnreachableException(); + } + + for (int processedBitsCount = 0, i = 0; processedBitsCount < PrefixLength; processedBitsCount += bitsPerByte, i++) + { + var baseAddressByte = baseAddressBytes[i]; + var otherAddressByte = otherAddressBytes[i]; + + if (processedBitsCount + bitsPerByte <= PrefixLength) + { + if (baseAddressByte == otherAddressByte) + { + continue; + } + else + { + return false; + } + } + else + { + var shiftAmount = bitsPerByte - PrefixLength + processedBitsCount; + baseAddressByte >>= shiftAmount; + otherAddressByte >>= shiftAmount; + if (baseAddressByte == otherAddressByte) + { + return true; + } + } + } + + return true; + } + + #region Parsing (public) + public static IPNetwork Parse(string s) + { + ArgumentNullException.ThrowIfNull(s); + return Parse(s.AsSpan()); + } + public static IPNetwork Parse(ReadOnlySpan s) + { + if(TryParseInternal(s, out var result, out var error)) + { + return result; + } + + throw new FormatException(error); + } + + public static bool TryParse(string? s, out IPNetwork result) + { + if (s == null) + { + result = default; + return false; + } + + return TryParseInternal(s.AsSpan(), out result, out _); + } + + public static bool TryParse(ReadOnlySpan s, out IPNetwork result) + { + return TryParseInternal(s, out result, out _); + } + #endregion + + #region Private and Internal methods + internal static bool TryParseInternal(ReadOnlySpan s, [MaybeNullWhen(false)] out IPNetwork result, [NotNullWhen(false)] out string? error) + { + int slashExpectedStartingIndex = s.Length - 4; + if (s.Slice(slashExpectedStartingIndex).IndexOf('/') != -1) + { + var ipAddressSpan = s.Slice(0, slashExpectedStartingIndex); + var prefixLengthSpan = s.Slice(slashExpectedStartingIndex + 1); + + if (IPAddress.TryParse(ipAddressSpan, out var ipAddress) && int.TryParse(prefixLengthSpan, out var prefixLength)) + { + result = new IPNetwork + { + BaseAddress = ipAddress, + PrefixLength = prefixLength + }; + + error = result.Validate()?.ParseExceptionMessage; + return error == null; + } + else + { + error = SR.dns_bad_ip_network; + result = default; + return false; + } + } + else + { + error = SR.dns_bad_ip_network; + result = default; + return false; + } + } + + private readonly record struct ValidationError( + Func CtorExceptionFactoryMethod, + string ParseExceptionMessage); + + private static readonly ValidationError + _baseAddressIsNullError = new ValidationError(() => new ArgumentNullException(baseAddressParamName), string.Empty), + _baseAddressHasSetBitsInMaskError = new ValidationError(() => new ArgumentException(baseAddressParamName), SR.dns_bad_ip_network), + _prefixLengthLessThanZeroError = new ValidationError(() => new ArgumentOutOfRangeException(prefixLengthParamName), SR.dns_bad_ip_network), + _prefixLengthGreaterThanAllowedError = new ValidationError(() => new ArgumentOutOfRangeException(prefixLengthParamName), SR.dns_bad_ip_network); + + private ValidationError? Validate() + { + if (BaseAddress == null) + { + return _baseAddressIsNullError; + } + + if (PrefixLength < 0) + { + return _prefixLengthLessThanZeroError; + } + + if (PrefixLength > GetAddressFamilyByteLength(BaseAddress.AddressFamily) * bitsPerByte) + { + return _prefixLengthGreaterThanAllowedError; + } + + if (!AreMaskBitsAfterPrefixUnset()) + { + return _baseAddressHasSetBitsInMaskError; + } + + return default; + } + + private bool AreMaskBitsAfterPrefixUnset() + { + Span addressBytes = stackalloc byte[GetAddressFamilyByteLength(BaseAddress.AddressFamily)]; + if (!BaseAddress.TryWriteBytes(addressBytes, out int bytesWritten)) + { + throw new UnreachableException(); + } + + for (int bitsCount = bytesWritten * bitsPerByte, i = bytesWritten - 1; bitsCount >= 0; bitsCount -= bitsPerByte, i--) + { + var numberOfEndingBitsToBeUnset = bitsCount - PrefixLength; + byte segment = addressBytes[i]; + if (numberOfEndingBitsToBeUnset > bitsPerByte) + { + if (segment == 0) + continue; + + return false; + } + + if ((segment & (1 << numberOfEndingBitsToBeUnset)) == segment) + { + return true; + } + + return false; + } + + throw new UnreachableException(); + } + + private static int GetAddressFamilyByteLength(AddressFamily addressFamily) + => addressFamily switch + { + AddressFamily.InterNetwork => IPAddressParserStatics.IPv4AddressBytes, + AddressFamily.InterNetworkV6 => IPAddressParserStatics.IPv6AddressBytes, + _ => throw new UnreachableException("Unknown address family") + }; + #endregion + + #region Formatting (public) + public override string ToString() + { + return $"{BaseAddress}/{PrefixLength}"; + } + + public bool TryFormat(Span destination, out int charsWritten) + { + if (!BaseAddress.TryFormat(destination, out charsWritten)) + { + charsWritten = 0; + return false; + } + + if (!PrefixLength.TryFormat(destination.Slice(charsWritten), out var prefixLengthCharsWritten)) + { + return false; + } + + charsWritten += prefixLengthCharsWritten; + return true; + } + #endregion + + #region Equality and GetHashCode + public bool Equals(IPNetwork other) + { + return BaseAddress.Equals(other.BaseAddress) && PrefixLength == other.PrefixLength; + } + + public override bool Equals([NotNullWhen(true)] object? obj) + { + if (obj is not IPNetwork other) + { + return false; + } + + return Equals(other); + } + + public static bool operator ==(IPNetwork left, IPNetwork right) + { + return left.Equals(right); + } + public static bool operator !=(IPNetwork left, IPNetwork right) + { + return !(left == right); + } + + public override int GetHashCode() + { + return HashCode.Combine(BaseAddress, PrefixLength); + } + #endregion + + #region Explicit ISpanFormattable, ISpanParsable + string IFormattable.ToString(string? format, IFormatProvider? provider) + { + return ToString(); + } + bool ISpanFormattable.TryFormat(Span destination, out int charsWritten, ReadOnlySpan format, IFormatProvider? provider) + { + return TryFormat(destination, out charsWritten); + } + static IPNetwork IParsable.Parse(string s, IFormatProvider? provider) + { + ArgumentNullException.ThrowIfNull(s); + return Parse(s); + } + static bool IParsable.TryParse(string? s, IFormatProvider? provider, out IPNetwork result) + { + return TryParse(s, out result); + } + static IPNetwork ISpanParsable.Parse(ReadOnlySpan s, IFormatProvider? provider) + { + return Parse(s); + } + static bool ISpanParsable.TryParse(ReadOnlySpan s, IFormatProvider? provider, out IPNetwork result) + { + return TryParse(s, out result); + } + #endregion + } +} diff --git a/src/libraries/System.Net.Primitives/tests/FunctionalTests/IPNetworkTest.cs b/src/libraries/System.Net.Primitives/tests/FunctionalTests/IPNetworkTest.cs new file mode 100644 index 00000000000000..635cd04af1530b --- /dev/null +++ b/src/libraries/System.Net.Primitives/tests/FunctionalTests/IPNetworkTest.cs @@ -0,0 +1,20 @@ +// Licensed to the .NET Foundation under one or more agreements. +// The .NET Foundation licenses this file to you under the MIT license. + +using System.Collections.Generic; +using System.Linq; +using System.Net.Sockets; + +using Xunit; + +namespace System.Net.Primitives.Functional.Tests +{ + public static class IPNetworkTest + { + [Fact] + public static void IPNetworkDummyTest() + { + new IPNetwork(); + } + } +} diff --git a/src/libraries/System.Net.Primitives/tests/FunctionalTests/System.Net.Primitives.Functional.Tests.csproj b/src/libraries/System.Net.Primitives/tests/FunctionalTests/System.Net.Primitives.Functional.Tests.csproj index 05ec279eaf2c38..889d735010fd8c 100644 --- a/src/libraries/System.Net.Primitives/tests/FunctionalTests/System.Net.Primitives.Functional.Tests.csproj +++ b/src/libraries/System.Net.Primitives/tests/FunctionalTests/System.Net.Primitives.Functional.Tests.csproj @@ -18,6 +18,7 @@ + From 09f1db35f0d892425edcdc1db74808a4a307e6bc Mon Sep 17 00:00:00 2001 From: Murat Duisenbayev Date: Tue, 28 Feb 2023 09:57:57 +0100 Subject: [PATCH 02/13] Perform further cleanup of the code --- .../src/System/Net/IPNetwork.cs | 105 ++++++----- .../tests/FunctionalTests/IPNetworkTest.cs | 176 +++++++++++++++++- 2 files changed, 233 insertions(+), 48 deletions(-) diff --git a/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs b/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs index 6ab067629b03ee..06410568e19f5d 100644 --- a/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs +++ b/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs @@ -1,30 +1,32 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.Buffers.Binary; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Net.Sockets; -using System.Runtime.CompilerServices; -using System.Runtime.InteropServices; -using System.Runtime.Intrinsics; namespace System.Net { - /// - /// - /// Provides an Internet Protocol (IP) subnet/range in CIDR notation. - /// - /// + /// + /// Provides an Internet Protocol (IP) subnet/range in CIDR notation. + /// public readonly struct IPNetwork : IEquatable, ISpanFormattable, ISpanParsable { public IPAddress BaseAddress { get; private init; } public int PrefixLength { get; private init; } + /* + We can also add two more easy to implement properties: + * AddressCount - a lazily calculated property showing the amount of potential address this instance "Contains". 2^(128_OR_32 - PrefixLength) + * 2^128 is going to fit into BigInteger, which causes a concern for memory allocation, since for actually big integers BigInteger seems to allocate a byte array. + * We can just have a Nullable backing field for the lazy property. But we should be fine with the increased byte size for IPNetwork type itself caused by that. + * Since BaseAddress represents the first address in the range, does it make sense to have a property that represents the last address? + */ + private const int bitsPerByte = 8; private const string - baseAddressParamName = "baseAddress", - prefixLengthParamName = "prefixLength"; + baseAddressConstructorParamName = "baseAddress", + prefixLengthConstructorParamName = "prefixLength"; public IPNetwork(IPAddress baseAddress, int prefixLength) { BaseAddress = baseAddress; @@ -33,7 +35,7 @@ public IPNetwork(IPAddress baseAddress, int prefixLength) var validationError = Validate(); if (validationError.HasValue) { - throw validationError.Value.CtorExceptionFactoryMethod(); + throw validationError.Value.ConstructorExceptionFactoryMethod(); } } @@ -46,6 +48,9 @@ public bool Contains(IPAddress address) return false; } + // TODO: this can be made much easier and potentially more performant for IPv4 if IPAddress.PrivateAddress is made internal (currently private) + // to be discussed in the PR + var expectedBytesCount = GetAddressFamilyByteLength(BaseAddress.AddressFamily); if (expectedBytesCount * bitsPerByte == PrefixLength) { @@ -82,9 +87,9 @@ public bool Contains(IPAddress address) } else { - var shiftAmount = bitsPerByte - PrefixLength + processedBitsCount; - baseAddressByte >>= shiftAmount; - otherAddressByte >>= shiftAmount; + var rightShiftAmount = bitsPerByte - (PrefixLength - processedBitsCount); + baseAddressByte >>= rightShiftAmount; + otherAddressByte >>= rightShiftAmount; if (baseAddressByte == otherAddressByte) { return true; @@ -103,7 +108,7 @@ public static IPNetwork Parse(string s) } public static IPNetwork Parse(ReadOnlySpan s) { - if(TryParseInternal(s, out var result, out var error)) + if (TryParseInternal(s, out var result, out var error)) { return result; } @@ -128,14 +133,22 @@ public static bool TryParse(ReadOnlySpan s, out IPNetwork result) } #endregion - #region Private and Internal methods - internal static bool TryParseInternal(ReadOnlySpan s, [MaybeNullWhen(false)] out IPNetwork result, [NotNullWhen(false)] out string? error) + #region Private methods + private static bool TryParseInternal(ReadOnlySpan s, out IPNetwork result, [NotNullWhen(false)] out string? error) { - int slashExpectedStartingIndex = s.Length - 4; - if (s.Slice(slashExpectedStartingIndex).IndexOf('/') != -1) + const char separator = '/'; + const int maxCharsCountAfterIpAddress = 4; + + int separatorExpectedStartingIndex = s.Length - maxCharsCountAfterIpAddress; + int separatorIndex = s + .Slice(separatorExpectedStartingIndex) + .IndexOf(separator); + if (separatorIndex != -1) { - var ipAddressSpan = s.Slice(0, slashExpectedStartingIndex); - var prefixLengthSpan = s.Slice(slashExpectedStartingIndex + 1); + separatorIndex += separatorExpectedStartingIndex; + + var ipAddressSpan = s.Slice(0, separatorIndex); + var prefixLengthSpan = s.Slice(separatorIndex + 1); if (IPAddress.TryParse(ipAddressSpan, out var ipAddress) && int.TryParse(prefixLengthSpan, out var prefixLength)) { @@ -164,14 +177,14 @@ internal static bool TryParseInternal(ReadOnlySpan s, [MaybeNullWhen(false } private readonly record struct ValidationError( - Func CtorExceptionFactoryMethod, + Func ConstructorExceptionFactoryMethod, string ParseExceptionMessage); private static readonly ValidationError - _baseAddressIsNullError = new ValidationError(() => new ArgumentNullException(baseAddressParamName), string.Empty), - _baseAddressHasSetBitsInMaskError = new ValidationError(() => new ArgumentException(baseAddressParamName), SR.dns_bad_ip_network), - _prefixLengthLessThanZeroError = new ValidationError(() => new ArgumentOutOfRangeException(prefixLengthParamName), SR.dns_bad_ip_network), - _prefixLengthGreaterThanAllowedError = new ValidationError(() => new ArgumentOutOfRangeException(prefixLengthParamName), SR.dns_bad_ip_network); + _baseAddressIsNullError = new ValidationError(() => new ArgumentNullException(baseAddressConstructorParamName), string.Empty), + _baseAddressHasSetBitsInMaskError = new ValidationError(() => new ArgumentException(baseAddressConstructorParamName), SR.dns_bad_ip_network), + _prefixLengthLessThanZeroError = new ValidationError(() => new ArgumentOutOfRangeException(prefixLengthConstructorParamName), SR.dns_bad_ip_network), + _prefixLengthGreaterThanAllowedError = new ValidationError(() => new ArgumentOutOfRangeException(prefixLengthConstructorParamName), SR.dns_bad_ip_network); private ValidationError? Validate() { @@ -190,7 +203,7 @@ private static readonly ValidationError return _prefixLengthGreaterThanAllowedError; } - if (!AreMaskBitsAfterPrefixUnset()) + if (IsAnyMaskBitOnForBaseAddress()) { return _baseAddressHasSetBitsInMaskError; } @@ -198,35 +211,35 @@ private static readonly ValidationError return default; } - private bool AreMaskBitsAfterPrefixUnset() + /// + /// Method to determine whether any bit in 's variable/mask part is 1. + /// + private bool IsAnyMaskBitOnForBaseAddress() { + // TODO: same as with Contains method - this can be made much easier and potentially more performant for IPv4 if IPAddress.PrivateAddress is made internal (currently private) + // to be discussed in the PR + Span addressBytes = stackalloc byte[GetAddressFamilyByteLength(BaseAddress.AddressFamily)]; if (!BaseAddress.TryWriteBytes(addressBytes, out int bytesWritten)) { throw new UnreachableException(); } - for (int bitsCount = bytesWritten * bitsPerByte, i = bytesWritten - 1; bitsCount >= 0; bitsCount -= bitsPerByte, i--) - { - var numberOfEndingBitsToBeUnset = bitsCount - PrefixLength; - byte segment = addressBytes[i]; - if (numberOfEndingBitsToBeUnset > bitsPerByte) - { - if (segment == 0) - continue; + var addressBitsCount = addressBytes.Length * bitsPerByte; - return false; - } - - if ((segment & (1 << numberOfEndingBitsToBeUnset)) == segment) + for (int addressBytesIndex = addressBytes.Length - 1, numberOfEndingBitsToBeOff = addressBitsCount - PrefixLength; + addressBytesIndex >= 0 && numberOfEndingBitsToBeOff > 0; + addressBytesIndex--, numberOfEndingBitsToBeOff -= bitsPerByte) + { + byte maskForByte = unchecked((byte)(byte.MaxValue << Math.Min(numberOfEndingBitsToBeOff, bitsPerByte))); + var addressByte = addressBytes[addressBytesIndex]; + if ((addressByte & maskForByte) != addressByte) { return true; } - - return false; } - throw new UnreachableException(); + return false; } private static int GetAddressFamilyByteLength(AddressFamily addressFamily) @@ -302,12 +315,12 @@ bool ISpanFormattable.TryFormat(Span destination, out int charsWritten, Re { return TryFormat(destination, out charsWritten); } - static IPNetwork IParsable.Parse(string s, IFormatProvider? provider) + static IPNetwork IParsable.Parse([NotNull] string s, IFormatProvider? provider) { ArgumentNullException.ThrowIfNull(s); return Parse(s); } - static bool IParsable.TryParse(string? s, IFormatProvider? provider, out IPNetwork result) + static bool IParsable.TryParse([NotNullWhen(true)] string? s, IFormatProvider? provider, out IPNetwork result) { return TryParse(s, out result); } diff --git a/src/libraries/System.Net.Primitives/tests/FunctionalTests/IPNetworkTest.cs b/src/libraries/System.Net.Primitives/tests/FunctionalTests/IPNetworkTest.cs index 635cd04af1530b..cfc707c4672f28 100644 --- a/src/libraries/System.Net.Primitives/tests/FunctionalTests/IPNetworkTest.cs +++ b/src/libraries/System.Net.Primitives/tests/FunctionalTests/IPNetworkTest.cs @@ -12,9 +12,181 @@ namespace System.Net.Primitives.Functional.Tests public static class IPNetworkTest { [Fact] - public static void IPNetworkDummyTest() + public void Parse_Empty_Throws() { - new IPNetwork(); + Assert.Throws(() => IPNetwork.Parse("")); + } + + [Fact] + public void ParseSpan_Empty_Throws() + { + Assert.Throws(() => IPNetwork.Parse("".AsSpan())); + } + + [Fact] + public void Parse_RangeWithoutMask_Throws() + { + Assert.Throws(() => IPNetwork.Parse("127.0.0.1")); + } + + [Fact] + public void ParseSpan_RangeWithoutMask_Throws() + { + Assert.Throws(() => IPNetwork.Parse("127.0.0.1".AsSpan())); + } + + [Fact] + public void Parse_PrefixNotValidIP_Throws() + { + Assert.Throws(() => IPNetwork.Parse("A.B.C.D/24")); + } + + [Fact] + public void ParseSpan_PrefixNotValidIP_Throws() + { + Assert.Throws(() => IPNetwork.Parse("A.B.C.D/24".AsSpan())); + } + + [Fact] + public void Parse_MaskNotValidInt_Throws() + { + Assert.Throws(() => IPNetwork.Parse("127.0.0.1/AB")); + } + + [Fact] + public void ParseSpan_MaskNotValidInt_Throws() + { + Assert.Throws(() => IPNetwork.Parse("127.0.0.1/AB".AsSpan())); + } + + [Fact] + public void Parse_MaskLongerThanPrefix_Throws() + { + Assert.Throws(() => IPNetwork.Parse("127.0.0.1/33")); + } + + [Fact] + public void ParseSpan_MaskLongerThanPrefix_Throws() + { + Assert.Throws(() => IPNetwork.Parse("127.0.0.1/33".AsSpan())); + } + + [Fact] + public void Parse_PrefixLongerThanMask_Throws() + { + // last octet has 1 at LSB therefore mask /31 is not valid + Assert.Throws(() => IPNetwork.Parse("127.0.0.1/31")); + } + + [Fact] + public void ParseSpan_PrefixLongerThanMask_Throws() + { + // last octet has 1 at LSB therefore mask /31 is not valid + Assert.Throws(() => IPNetwork.Parse("127.0.0.1/31".AsSpan())); + } + + [Fact] + public void Parse_ValidIPv4_ReturnsCorrectValue() + { + var expectedIP = IPAddress.Parse("127.0.0.254"); + var expectedMask = 31u; + + var range = IPNetwork.Parse($"{expectedIP}/{expectedMask}"); + + Assert.Equal(expectedIP, range.BaseAddress); + Assert.Equal(expectedMask, range.MaskLength); + } + + [Fact] + public void ParseSpan_ValidIPv4_ReturnsCorrectValue() + { + var expectedIP = IPAddress.Parse("127.0.0.254"); + var expectedMask = 31u; + + var range = IPNetwork.Parse($"{expectedIP}/{expectedMask}".AsSpan()); + + Assert.Equal(expectedIP, range.BaseAddress); + Assert.Equal(expectedMask, range.MaskLength); + } + + [Fact] + public void Parse_ValidIPv6_ReturnsCorrectValue() + { + var expectedIP = IPAddress.Parse("2002::"); + var expectedMask = 16u; + + var range = IPNetwork.Parse($"{expectedIP}/{expectedMask}"); + + Assert.Equal(expectedIP, range.BaseAddress); + Assert.Equal(expectedMask, range.MaskLength); + } + + [Fact] + public void ParseSpan_ValidIPv6_ReturnsCorrectValue() + { + var expectedIP = IPAddress.Parse("2002::"); + var expectedMask = 16u; + + var range = IPNetwork.Parse($"{expectedIP}/{expectedMask}".AsSpan()); + + Assert.Equal(expectedIP, range.BaseAddress); + Assert.Equal(expectedMask, range.MaskLength); + } + + [Fact] + public void Equals_WhenDifferent_ReturnsFalse() + { + var a = IPNetwork.Parse("127.0.0.0/24"); + + var rangeWithDifferentPrefix = IPNetwork.Parse("127.0.1.0/24"); + var rangeWithDifferentPrefixLength = IPNetwork.Parse("127.0.0.0/25"); + + Assert.False(a.Equals(rangeWithDifferentPrefix)); + Assert.False(a.Equals(rangeWithDifferentPrefixLength)); + } + + [Fact] + public void EqualsSpan_WhenDifferent_ReturnsFalse() + { + var a = IPNetwork.Parse("127.0.0.0/24".AsSpan()); + + var rangeWithDifferentPrefix = IPNetwork.Parse("127.0.1.0/24".AsSpan()); + var rangeWithDifferentPrefixLength = IPNetwork.Parse("127.0.0.0/25".AsSpan()); + + Assert.False(a.Equals(rangeWithDifferentPrefix)); + Assert.False(a.Equals(rangeWithDifferentPrefixLength)); + } + + [Fact] + public void Equals_WhenSame_ReturnsFalse() + { + var a = IPNetwork.Parse("127.0.0.0/24"); + var b = IPNetwork.Parse("127.0.0.0/24"); + + Assert.True(a.Equals(b)); + Assert.True(a == b); + Assert.False(a != b); + Assert.Equal(a.GetHashCode(), b.GetHashCode()); + } + + [Fact] + public void EqualsSpan_WhenSame_ReturnsFalse() + { + var a = IPNetwork.Parse("127.0.0.0/24".AsSpan()); + var b = IPNetwork.Parse("127.0.0.0/24".AsSpan()); + + Assert.True(a.Equals(b)); + Assert.True(a == b); + Assert.False(a != b); + Assert.Equal(a.GetHashCode(), b.GetHashCode()); + } + + [Fact] + public void Equals_WhenNull_ReturnsFalse() + { + var a = IPNetwork.Parse("127.0.0.0/24".AsSpan()); + + Assert.False(a.Equals(null!)); } } } From e8e589210ab4b8bc636337740142ef1255d2cb3b Mon Sep 17 00:00:00 2001 From: Murat Duisenbayev Date: Tue, 28 Feb 2023 18:41:51 +0100 Subject: [PATCH 03/13] Fix couple of bugs and write a few meaningful tests --- .../src/System/Net/IPNetwork.cs | 47 ++-- .../tests/FunctionalTests/IPNetworkTest.cs | 210 ++++++------------ 2 files changed, 101 insertions(+), 156 deletions(-) diff --git a/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs b/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs index 06410568e19f5d..dd98b37417f458 100644 --- a/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs +++ b/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs @@ -23,6 +23,7 @@ namespace System.Net * Since BaseAddress represents the first address in the range, does it make sense to have a property that represents the last address? */ + private const char addressAndPrefixLengthSeparator = '/'; private const int bitsPerByte = 8; private const string baseAddressConstructorParamName = "baseAddress", @@ -74,27 +75,19 @@ public bool Contains(IPAddress address) var baseAddressByte = baseAddressBytes[i]; var otherAddressByte = otherAddressBytes[i]; - if (processedBitsCount + bitsPerByte <= PrefixLength) + var rightShiftAmount = Math.Max(0, bitsPerByte - (PrefixLength - processedBitsCount)); + if (rightShiftAmount != 0) { - if (baseAddressByte == otherAddressByte) - { - continue; - } - else - { - return false; - } - } - else - { - var rightShiftAmount = bitsPerByte - (PrefixLength - processedBitsCount); baseAddressByte >>= rightShiftAmount; otherAddressByte >>= rightShiftAmount; - if (baseAddressByte == otherAddressByte) - { - return true; - } } + + if (baseAddressByte == otherAddressByte) + { + continue; + } + + return false; } return true; @@ -136,13 +129,20 @@ public static bool TryParse(ReadOnlySpan s, out IPNetwork result) #region Private methods private static bool TryParseInternal(ReadOnlySpan s, out IPNetwork result, [NotNullWhen(false)] out string? error) { - const char separator = '/'; const int maxCharsCountAfterIpAddress = 4; + const int minCharsRequired = 4; + + if (s.Length < minCharsRequired) + { + error = SR.dns_bad_ip_network; + result = default; + return false; + } int separatorExpectedStartingIndex = s.Length - maxCharsCountAfterIpAddress; int separatorIndex = s .Slice(separatorExpectedStartingIndex) - .IndexOf(separator); + .IndexOf(addressAndPrefixLengthSeparator); if (separatorIndex != -1) { separatorIndex += separatorExpectedStartingIndex; @@ -254,7 +254,7 @@ private static int GetAddressFamilyByteLength(AddressFamily addressFamily) #region Formatting (public) public override string ToString() { - return $"{BaseAddress}/{PrefixLength}"; + return $"{BaseAddress}{addressAndPrefixLengthSeparator}{PrefixLength}"; } public bool TryFormat(Span destination, out int charsWritten) @@ -265,6 +265,13 @@ public bool TryFormat(Span destination, out int charsWritten) return false; } + if (destination.Length < charsWritten + 2) + { + return false; + } + + destination[charsWritten++] = addressAndPrefixLengthSeparator; + if (!PrefixLength.TryFormat(destination.Slice(charsWritten), out var prefixLengthCharsWritten)) { return false; diff --git a/src/libraries/System.Net.Primitives/tests/FunctionalTests/IPNetworkTest.cs b/src/libraries/System.Net.Primitives/tests/FunctionalTests/IPNetworkTest.cs index cfc707c4672f28..460bd74f055835 100644 --- a/src/libraries/System.Net.Primitives/tests/FunctionalTests/IPNetworkTest.cs +++ b/src/libraries/System.Net.Primitives/tests/FunctionalTests/IPNetworkTest.cs @@ -1,164 +1,88 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. -using System.Collections.Generic; -using System.Linq; -using System.Net.Sockets; - using Xunit; namespace System.Net.Primitives.Functional.Tests { - public static class IPNetworkTest + public class IPNetworkTest { - [Fact] - public void Parse_Empty_Throws() - { - Assert.Throws(() => IPNetwork.Parse("")); - } - - [Fact] - public void ParseSpan_Empty_Throws() - { - Assert.Throws(() => IPNetwork.Parse("".AsSpan())); - } - - [Fact] - public void Parse_RangeWithoutMask_Throws() - { - Assert.Throws(() => IPNetwork.Parse("127.0.0.1")); - } - - [Fact] - public void ParseSpan_RangeWithoutMask_Throws() - { - Assert.Throws(() => IPNetwork.Parse("127.0.0.1".AsSpan())); - } - - [Fact] - public void Parse_PrefixNotValidIP_Throws() - { - Assert.Throws(() => IPNetwork.Parse("A.B.C.D/24")); - } - - [Fact] - public void ParseSpan_PrefixNotValidIP_Throws() + [Theory] + [InlineData("127.0.0.1")] + [InlineData("A.B.C.D/24")] + [InlineData("127.0.0.1/AB")] + [InlineData("")] + public void Parse_IncorrectFormat_Throws(string input) { - Assert.Throws(() => IPNetwork.Parse("A.B.C.D/24".AsSpan())); + Assert.Throws(() => IPNetwork.Parse(input)); } - [Fact] - public void Parse_MaskNotValidInt_Throws() + [Theory] + [InlineData("127.0.0.1/33")] // PrefixLength max is 32 + [InlineData("127.0.0.1/31")] // LastSetBit in the host mask (32nd bit is on) + [InlineData("")] + public void Parse_InvalidNetworkNotation_Throws(string input) { - Assert.Throws(() => IPNetwork.Parse("127.0.0.1/AB")); + Assert.Throws(() => IPNetwork.Parse(input)); } - [Fact] - public void ParseSpan_MaskNotValidInt_Throws() + [Theory] + [InlineData("0.0.0.0/32")] // the whole IPv4 space + [InlineData("::/128")] // the whole IPv6 space + [InlineData("255.255.255.255/32")] // single IPv4 address + [InlineData("ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff/128")] // single IPv6 address + public void Parse_ValidNetworkNotation_Succeeds(string input) { - Assert.Throws(() => IPNetwork.Parse("127.0.0.1/AB".AsSpan())); + var network = IPNetwork.Parse(input); + Assert.Equal(input, network.ToString()); } - [Fact] - public void Parse_MaskLongerThanPrefix_Throws() + [Theory] + [InlineData("0.0.0.0/0", "0.0.0.0", "127.127.127.127", "255.255.255.255")] // the whole IPv4 space + [InlineData("::/0", "::", "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff")] // the whole IPv6 space + [InlineData("255.255.255.255/32", "255.255.255.255")] // single IPv4 address + [InlineData("ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff/128", "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff")] // single IPv6 address + [InlineData("255.255.255.0/24", "255.255.255.0", "255.255.255.255")] + [InlineData("255.255.255.128/25", "255.255.255.129", "255.255.255.255")] + public void Contains_ValidAddresses_ReturnsTrue(string networkString, params string[] validAddresses) { - Assert.Throws(() => IPNetwork.Parse("127.0.0.1/33")); - } + var network = IPNetwork.Parse(networkString); - [Fact] - public void ParseSpan_MaskLongerThanPrefix_Throws() - { - Assert.Throws(() => IPNetwork.Parse("127.0.0.1/33".AsSpan())); + foreach (var address in validAddresses) + { + Assert.True(network.Contains(IPAddress.Parse(address))); + } } - [Fact] - public void Parse_PrefixLongerThanMask_Throws() + [Theory] + [InlineData("255.255.255.255/32", "255.255.255.254")] // single IPv4 address + [InlineData("ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff/128", "ffff:ffff:ffff:ffff:ffff:ffff:ffff:fffe")] // single IPv6 address + [InlineData("255.255.255.0/24", "255.255.254.0")] + [InlineData("255.255.255.128/25", "255.255.255.127")] + public void Contains_InvalidAddresses_ReturnsFalse(string networkString, params string[] invalidAddresses) { - // last octet has 1 at LSB therefore mask /31 is not valid - Assert.Throws(() => IPNetwork.Parse("127.0.0.1/31")); - } - - [Fact] - public void ParseSpan_PrefixLongerThanMask_Throws() - { - // last octet has 1 at LSB therefore mask /31 is not valid - Assert.Throws(() => IPNetwork.Parse("127.0.0.1/31".AsSpan())); - } - - [Fact] - public void Parse_ValidIPv4_ReturnsCorrectValue() - { - var expectedIP = IPAddress.Parse("127.0.0.254"); - var expectedMask = 31u; - - var range = IPNetwork.Parse($"{expectedIP}/{expectedMask}"); + var network = IPNetwork.Parse(networkString); - Assert.Equal(expectedIP, range.BaseAddress); - Assert.Equal(expectedMask, range.MaskLength); - } - - [Fact] - public void ParseSpan_ValidIPv4_ReturnsCorrectValue() - { - var expectedIP = IPAddress.Parse("127.0.0.254"); - var expectedMask = 31u; - - var range = IPNetwork.Parse($"{expectedIP}/{expectedMask}".AsSpan()); - - Assert.Equal(expectedIP, range.BaseAddress); - Assert.Equal(expectedMask, range.MaskLength); - } - - [Fact] - public void Parse_ValidIPv6_ReturnsCorrectValue() - { - var expectedIP = IPAddress.Parse("2002::"); - var expectedMask = 16u; - - var range = IPNetwork.Parse($"{expectedIP}/{expectedMask}"); - - Assert.Equal(expectedIP, range.BaseAddress); - Assert.Equal(expectedMask, range.MaskLength); - } - - [Fact] - public void ParseSpan_ValidIPv6_ReturnsCorrectValue() - { - var expectedIP = IPAddress.Parse("2002::"); - var expectedMask = 16u; - - var range = IPNetwork.Parse($"{expectedIP}/{expectedMask}".AsSpan()); - - Assert.Equal(expectedIP, range.BaseAddress); - Assert.Equal(expectedMask, range.MaskLength); + foreach (var address in invalidAddresses) + { + Assert.False(network.Contains(IPAddress.Parse(address))); + } } [Fact] public void Equals_WhenDifferent_ReturnsFalse() { - var a = IPNetwork.Parse("127.0.0.0/24"); + var network = IPNetwork.Parse("127.0.0.0/24"); var rangeWithDifferentPrefix = IPNetwork.Parse("127.0.1.0/24"); var rangeWithDifferentPrefixLength = IPNetwork.Parse("127.0.0.0/25"); - Assert.False(a.Equals(rangeWithDifferentPrefix)); - Assert.False(a.Equals(rangeWithDifferentPrefixLength)); + Assert.False(network.Equals(rangeWithDifferentPrefix)); + Assert.False(network.Equals(rangeWithDifferentPrefixLength)); } [Fact] - public void EqualsSpan_WhenDifferent_ReturnsFalse() - { - var a = IPNetwork.Parse("127.0.0.0/24".AsSpan()); - - var rangeWithDifferentPrefix = IPNetwork.Parse("127.0.1.0/24".AsSpan()); - var rangeWithDifferentPrefixLength = IPNetwork.Parse("127.0.0.0/25".AsSpan()); - - Assert.False(a.Equals(rangeWithDifferentPrefix)); - Assert.False(a.Equals(rangeWithDifferentPrefixLength)); - } - - [Fact] - public void Equals_WhenSame_ReturnsFalse() + public void Equals_WhenSame_ReturnsTrue() { var a = IPNetwork.Parse("127.0.0.0/24"); var b = IPNetwork.Parse("127.0.0.0/24"); @@ -170,23 +94,37 @@ public void Equals_WhenSame_ReturnsFalse() } [Fact] - public void EqualsSpan_WhenSame_ReturnsFalse() + public void Equals_WhenNull_ReturnsFalse() { - var a = IPNetwork.Parse("127.0.0.0/24".AsSpan()); - var b = IPNetwork.Parse("127.0.0.0/24".AsSpan()); + var network = IPNetwork.Parse("127.0.0.0/24"); - Assert.True(a.Equals(b)); - Assert.True(a == b); - Assert.False(a != b); - Assert.Equal(a.GetHashCode(), b.GetHashCode()); + Assert.False(network.Equals(null)); } [Fact] - public void Equals_WhenNull_ReturnsFalse() + public void TryFormatSpan_EnoughLength_Succeeds() { - var a = IPNetwork.Parse("127.0.0.0/24".AsSpan()); + var input = "127.0.0.0/24"; + var network = IPNetwork.Parse(input); + + Span span = stackalloc char[15]; // IPAddress.TryFormat requires a size of 15 + + Assert.True(network.TryFormat(span, out int charsWritten)); + Assert.Equal(input.Length, charsWritten); + Assert.Equal(input, span.Slice(0, charsWritten).ToString()); + } + + [Theory] + [InlineData("127.127.127.127/32", 15)] + [InlineData("127.127.127.127/32", 0)] + [InlineData("127.127.127.127/32", 1)] + public void TryFormatSpan_NotEnoughLength_FailsWithoutException(string input, int spanLengthToTest) + { + var network = IPNetwork.Parse(input); + + Span span = stackalloc char[spanLengthToTest]; - Assert.False(a.Equals(null!)); + Assert.False(network.TryFormat(span, out int charsWritten)); } } } From 731b2b1b5a53d485ee5ce74ea888a1ccc6a8cee8 Mon Sep 17 00:00:00 2001 From: Murat Duisenbayev Date: Wed, 1 Mar 2023 10:34:05 +0100 Subject: [PATCH 04/13] Address comments in the PR from @stephentoub --- .../src/System/Net/IPNetwork.cs | 125 +++++++++--------- 1 file changed, 60 insertions(+), 65 deletions(-) diff --git a/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs b/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs index dd98b37417f458..97a46e0675255e 100644 --- a/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs +++ b/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs @@ -12,31 +12,31 @@ namespace System.Net /// public readonly struct IPNetwork : IEquatable, ISpanFormattable, ISpanParsable { - public IPAddress BaseAddress { get; private init; } - public int PrefixLength { get; private init; } - - /* - We can also add two more easy to implement properties: - * AddressCount - a lazily calculated property showing the amount of potential address this instance "Contains". 2^(128_OR_32 - PrefixLength) - * 2^128 is going to fit into BigInteger, which causes a concern for memory allocation, since for actually big integers BigInteger seems to allocate a byte array. - * We can just have a Nullable backing field for the lazy property. But we should be fine with the increased byte size for IPNetwork type itself caused by that. - * Since BaseAddress represents the first address in the range, does it make sense to have a property that represents the last address? - */ - - private const char addressAndPrefixLengthSeparator = '/'; - private const int bitsPerByte = 8; - private const string - baseAddressConstructorParamName = "baseAddress", - prefixLengthConstructorParamName = "prefixLength"; + public IPAddress BaseAddress { get; } + public int PrefixLength { get; } + + private const char AddressAndPrefixLengthSeparator = '/'; + private const int BitsPerByte = 8; + + private const string BaseAddressConstructorParamName = "baseAddress"; + private const string PrefixLengthConstructorParamName = "prefixLength"; public IPNetwork(IPAddress baseAddress, int prefixLength) + : this(baseAddress, prefixLength, validateAndThrow: true) + { + } + + private IPNetwork(IPAddress baseAddress, int prefixLength, bool validateAndThrow) { BaseAddress = baseAddress; PrefixLength = prefixLength; - var validationError = Validate(); - if (validationError.HasValue) + if (validateAndThrow) { - throw validationError.Value.ConstructorExceptionFactoryMethod(); + ValidationError? validationError = Validate(); + if (validationError.HasValue) + { + throw validationError.Value.ConstructorExceptionFactoryMethod(); + } } } @@ -53,29 +53,25 @@ public bool Contains(IPAddress address) // to be discussed in the PR var expectedBytesCount = GetAddressFamilyByteLength(BaseAddress.AddressFamily); - if (expectedBytesCount * bitsPerByte == PrefixLength) + if (expectedBytesCount * BitsPerByte == PrefixLength) { return BaseAddress.Equals(address); } Span baseAddressBytes = stackalloc byte[expectedBytesCount]; - if (!BaseAddress.TryWriteBytes(baseAddressBytes, out _)) - { - throw new UnreachableException(); - } + bool written = BaseAddress.TryWriteBytes(baseAddressBytes, out int bytesWritten); + Debug.Assert(written && bytesWritten == expectedBytesCount); Span otherAddressBytes = stackalloc byte[expectedBytesCount]; - if (!address.TryWriteBytes(otherAddressBytes, out _)) - { - throw new UnreachableException(); - } + written = address.TryWriteBytes(otherAddressBytes, out bytesWritten); + Debug.Assert(written && bytesWritten == expectedBytesCount); - for (int processedBitsCount = 0, i = 0; processedBitsCount < PrefixLength; processedBitsCount += bitsPerByte, i++) + for (int processedBitsCount = 0, i = 0; processedBitsCount < PrefixLength; processedBitsCount += BitsPerByte, i++) { var baseAddressByte = baseAddressBytes[i]; var otherAddressByte = otherAddressBytes[i]; - var rightShiftAmount = Math.Max(0, bitsPerByte - (PrefixLength - processedBitsCount)); + var rightShiftAmount = Math.Max(0, BitsPerByte - (PrefixLength - processedBitsCount)); if (rightShiftAmount != 0) { baseAddressByte >>= rightShiftAmount; @@ -129,20 +125,20 @@ public static bool TryParse(ReadOnlySpan s, out IPNetwork result) #region Private methods private static bool TryParseInternal(ReadOnlySpan s, out IPNetwork result, [NotNullWhen(false)] out string? error) { - const int maxCharsCountAfterIpAddress = 4; - const int minCharsRequired = 4; + const int MaxCharsCountAfterIpAddress = 4; + const int MinCharsRequired = 4; - if (s.Length < minCharsRequired) + if (s.Length < MinCharsRequired) { error = SR.dns_bad_ip_network; result = default; return false; } - int separatorExpectedStartingIndex = s.Length - maxCharsCountAfterIpAddress; + int separatorExpectedStartingIndex = s.Length - MaxCharsCountAfterIpAddress; int separatorIndex = s .Slice(separatorExpectedStartingIndex) - .IndexOf(addressAndPrefixLengthSeparator); + .IndexOf(AddressAndPrefixLengthSeparator); if (separatorIndex != -1) { separatorIndex += separatorExpectedStartingIndex; @@ -152,11 +148,7 @@ private static bool TryParseInternal(ReadOnlySpan s, out IPNetwork result, if (IPAddress.TryParse(ipAddressSpan, out var ipAddress) && int.TryParse(prefixLengthSpan, out var prefixLength)) { - result = new IPNetwork - { - BaseAddress = ipAddress, - PrefixLength = prefixLength - }; + result = new IPNetwork(ipAddress, prefixLength, validateAndThrow: false); error = result.Validate()?.ParseExceptionMessage; return error == null; @@ -176,36 +168,43 @@ private static bool TryParseInternal(ReadOnlySpan s, out IPNetwork result, } } - private readonly record struct ValidationError( - Func ConstructorExceptionFactoryMethod, - string ParseExceptionMessage); + private readonly struct ValidationError + { + public ValidationError(Func constructorExceptionFactoryMethod, string parseExceptionMessage) + { + ConstructorExceptionFactoryMethod = constructorExceptionFactoryMethod; + ParseExceptionMessage = parseExceptionMessage; + } + + public readonly Func ConstructorExceptionFactoryMethod; + public readonly string ParseExceptionMessage; + } - private static readonly ValidationError - _baseAddressIsNullError = new ValidationError(() => new ArgumentNullException(baseAddressConstructorParamName), string.Empty), - _baseAddressHasSetBitsInMaskError = new ValidationError(() => new ArgumentException(baseAddressConstructorParamName), SR.dns_bad_ip_network), - _prefixLengthLessThanZeroError = new ValidationError(() => new ArgumentOutOfRangeException(prefixLengthConstructorParamName), SR.dns_bad_ip_network), - _prefixLengthGreaterThanAllowedError = new ValidationError(() => new ArgumentOutOfRangeException(prefixLengthConstructorParamName), SR.dns_bad_ip_network); + private static readonly ValidationError s_baseAddressIsNullError = new ValidationError(() => new ArgumentNullException(BaseAddressConstructorParamName), string.Empty); + private static readonly ValidationError s_baseAddressHasSetBitsInMaskError = new ValidationError(() => new ArgumentException(BaseAddressConstructorParamName), SR.dns_bad_ip_network); + private static readonly ValidationError s_prefixLengthLessThanZeroError = new ValidationError(() => new ArgumentOutOfRangeException(PrefixLengthConstructorParamName), SR.dns_bad_ip_network); + private static readonly ValidationError s_prefixLengthGreaterThanAllowedError = new ValidationError(() => new ArgumentOutOfRangeException(PrefixLengthConstructorParamName), SR.dns_bad_ip_network); private ValidationError? Validate() { if (BaseAddress == null) { - return _baseAddressIsNullError; + return s_baseAddressIsNullError; } if (PrefixLength < 0) { - return _prefixLengthLessThanZeroError; + return s_prefixLengthLessThanZeroError; } - if (PrefixLength > GetAddressFamilyByteLength(BaseAddress.AddressFamily) * bitsPerByte) + if (PrefixLength > GetAddressFamilyByteLength(BaseAddress.AddressFamily) * BitsPerByte) { - return _prefixLengthGreaterThanAllowedError; + return s_prefixLengthGreaterThanAllowedError; } if (IsAnyMaskBitOnForBaseAddress()) { - return _baseAddressHasSetBitsInMaskError; + return s_baseAddressHasSetBitsInMaskError; } return default; @@ -216,22 +215,18 @@ private static readonly ValidationError /// private bool IsAnyMaskBitOnForBaseAddress() { - // TODO: same as with Contains method - this can be made much easier and potentially more performant for IPv4 if IPAddress.PrivateAddress is made internal (currently private) - // to be discussed in the PR - Span addressBytes = stackalloc byte[GetAddressFamilyByteLength(BaseAddress.AddressFamily)]; - if (!BaseAddress.TryWriteBytes(addressBytes, out int bytesWritten)) - { - throw new UnreachableException(); - } - var addressBitsCount = addressBytes.Length * bitsPerByte; + bool written = BaseAddress.TryWriteBytes(addressBytes, out int bytesWritten); + Debug.Assert(written && bytesWritten == addressBytes.Length); + + var addressBitsCount = addressBytes.Length * BitsPerByte; for (int addressBytesIndex = addressBytes.Length - 1, numberOfEndingBitsToBeOff = addressBitsCount - PrefixLength; addressBytesIndex >= 0 && numberOfEndingBitsToBeOff > 0; - addressBytesIndex--, numberOfEndingBitsToBeOff -= bitsPerByte) + addressBytesIndex--, numberOfEndingBitsToBeOff -= BitsPerByte) { - byte maskForByte = unchecked((byte)(byte.MaxValue << Math.Min(numberOfEndingBitsToBeOff, bitsPerByte))); + byte maskForByte = unchecked((byte)(byte.MaxValue << Math.Min(numberOfEndingBitsToBeOff, BitsPerByte))); var addressByte = addressBytes[addressBytesIndex]; if ((addressByte & maskForByte) != addressByte) { @@ -254,7 +249,7 @@ private static int GetAddressFamilyByteLength(AddressFamily addressFamily) #region Formatting (public) public override string ToString() { - return $"{BaseAddress}{addressAndPrefixLengthSeparator}{PrefixLength}"; + return $"{BaseAddress}{AddressAndPrefixLengthSeparator}{PrefixLength}"; } public bool TryFormat(Span destination, out int charsWritten) @@ -270,7 +265,7 @@ public bool TryFormat(Span destination, out int charsWritten) return false; } - destination[charsWritten++] = addressAndPrefixLengthSeparator; + destination[charsWritten++] = AddressAndPrefixLengthSeparator; if (!PrefixLength.TryFormat(destination.Slice(charsWritten), out var prefixLengthCharsWritten)) { From 9ed5233e770d5ddba0632c10f12130bfba46e70c Mon Sep 17 00:00:00 2001 From: antonfirsov Date: Tue, 14 Mar 2023 00:34:51 +0100 Subject: [PATCH 05/13] improvements --- .../ref/System.Net.Primitives.cs | 26 ++ .../src/Resources/Strings.resx | 7 +- .../src/System/Net/IPAddress.cs | 4 +- .../src/System/Net/IPNetwork.cs | 256 ++++++++---------- .../tests/FunctionalTests/IPNetworkTest.cs | 185 +++++++++++-- 5 files changed, 299 insertions(+), 179 deletions(-) diff --git a/src/libraries/System.Net.Primitives/ref/System.Net.Primitives.cs b/src/libraries/System.Net.Primitives/ref/System.Net.Primitives.cs index 393c08923a6e3d..5e329e0b9aa5e5 100644 --- a/src/libraries/System.Net.Primitives/ref/System.Net.Primitives.cs +++ b/src/libraries/System.Net.Primitives/ref/System.Net.Primitives.cs @@ -287,6 +287,32 @@ public IPEndPoint(System.Net.IPAddress address, int port) { } public static bool TryParse(System.ReadOnlySpan s, [System.Diagnostics.CodeAnalysis.NotNullWhenAttribute(true)] out System.Net.IPEndPoint? result) { throw null; } public static bool TryParse(string s, [System.Diagnostics.CodeAnalysis.NotNullWhenAttribute(true)] out System.Net.IPEndPoint? result) { throw null; } } + public readonly partial struct IPNetwork : System.IEquatable, System.IFormattable, System.IParsable, System.ISpanFormattable, System.ISpanParsable + { + private readonly object _dummy; + private readonly int _dummyPrimitive; + public IPNetwork(System.Net.IPAddress baseAddress, int prefixLength) { throw null; } + public System.Net.IPAddress BaseAddress { get { throw null; } } + public int PrefixLength { get { throw null; } } + public bool Contains(System.Net.IPAddress address) { throw null; } + public bool Equals(System.Net.IPNetwork other) { throw null; } + public override bool Equals([System.Diagnostics.CodeAnalysis.NotNullWhenAttribute(true)] object? obj) { throw null; } + public override int GetHashCode() { throw null; } + public static bool operator ==(System.Net.IPNetwork left, System.Net.IPNetwork right) { throw null; } + public static bool operator !=(System.Net.IPNetwork left, System.Net.IPNetwork right) { throw null; } + public static System.Net.IPNetwork Parse(System.ReadOnlySpan s) { throw null; } + public static System.Net.IPNetwork Parse(string s) { throw null; } + string System.IFormattable.ToString(string? format, System.IFormatProvider? provider) { throw null; } + static System.Net.IPNetwork System.IParsable.Parse([System.Diagnostics.CodeAnalysis.NotNullAttribute] string s, System.IFormatProvider? provider) { throw null; } + static bool System.IParsable.TryParse([System.Diagnostics.CodeAnalysis.NotNullWhenAttribute(true)] string? s, System.IFormatProvider? provider, out System.Net.IPNetwork result) { throw null; } + bool System.ISpanFormattable.TryFormat(System.Span destination, out int charsWritten, System.ReadOnlySpan format, System.IFormatProvider? provider) { throw null; } + static System.Net.IPNetwork System.ISpanParsable.Parse(System.ReadOnlySpan s, System.IFormatProvider? provider) { throw null; } + static bool System.ISpanParsable.TryParse(System.ReadOnlySpan s, System.IFormatProvider? provider, out System.Net.IPNetwork result) { throw null; } + public override string ToString() { throw null; } + public bool TryFormat(System.Span destination, out int charsWritten) { throw null; } + public static bool TryParse(System.ReadOnlySpan s, out System.Net.IPNetwork result) { throw null; } + public static bool TryParse(string? s, out System.Net.IPNetwork result) { throw null; } + } public partial interface IWebProxy { System.Net.ICredentials? Credentials { get; set; } diff --git a/src/libraries/System.Net.Primitives/src/Resources/Strings.resx b/src/libraries/System.Net.Primitives/src/Resources/Strings.resx index 97a9b96ca04c5f..958a0e2e269f99 100644 --- a/src/libraries/System.Net.Primitives/src/Resources/Strings.resx +++ b/src/libraries/System.Net.Primitives/src/Resources/Strings.resx @@ -78,8 +78,11 @@ An invalid IP address was specified. - - An invalid IPNetwork was specified. + + An invalid IP network was specified. + + + The specified baseAddress has non-zero bits after the network prefix. An error occurred when adding a cookie to the container. diff --git a/src/libraries/System.Net.Primitives/src/System/Net/IPAddress.cs b/src/libraries/System.Net.Primitives/src/System/Net/IPAddress.cs index 39851508d69554..81b0851f45a96c 100644 --- a/src/libraries/System.Net.Primitives/src/System/Net/IPAddress.cs +++ b/src/libraries/System.Net.Primitives/src/System/Net/IPAddress.cs @@ -69,14 +69,14 @@ private bool IsIPv6 get { return _numbers != null; } } - private uint PrivateAddress + internal uint PrivateAddress { get { Debug.Assert(IsIPv4); return _addressOrScopeId; } - set + private set { Debug.Assert(IsIPv4); _toString = null; diff --git a/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs b/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs index 97a46e0675255e..4f2b401be890cd 100644 --- a/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs +++ b/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs @@ -1,9 +1,12 @@ // Licensed to the .NET Foundation under one or more agreements. // The .NET Foundation licenses this file to you under the MIT license. +using System.Buffers.Binary; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Net.Sockets; +using System.Runtime.CompilerServices; +using System.Runtime.InteropServices; namespace System.Net { @@ -15,29 +18,20 @@ namespace System.Net public IPAddress BaseAddress { get; } public int PrefixLength { get; } - private const char AddressAndPrefixLengthSeparator = '/'; - private const int BitsPerByte = 8; - - private const string BaseAddressConstructorParamName = "baseAddress"; - private const string PrefixLengthConstructorParamName = "prefixLength"; public IPNetwork(IPAddress baseAddress, int prefixLength) - : this(baseAddress, prefixLength, validateAndThrow: true) { + ArgumentNullException.ThrowIfNull(baseAddress); + Validate(baseAddress, prefixLength, throwOnFailure: true); + + BaseAddress = baseAddress; + PrefixLength = prefixLength; } - private IPNetwork(IPAddress baseAddress, int prefixLength, bool validateAndThrow) + // Non-validating ctor + private IPNetwork(IPAddress baseAddress, int prefixLength, bool _) { BaseAddress = baseAddress; PrefixLength = prefixLength; - - if (validateAndThrow) - { - ValidationError? validationError = Validate(); - if (validationError.HasValue) - { - throw validationError.Value.ConstructorExceptionFactoryMethod(); - } - } } public bool Contains(IPAddress address) @@ -49,44 +43,38 @@ public bool Contains(IPAddress address) return false; } - // TODO: this can be made much easier and potentially more performant for IPv4 if IPAddress.PrivateAddress is made internal (currently private) - // to be discussed in the PR - - var expectedBytesCount = GetAddressFamilyByteLength(BaseAddress.AddressFamily); - if (expectedBytesCount * BitsPerByte == PrefixLength) + if (PrefixLength == 0) { - return BaseAddress.Equals(address); + return true; } - Span baseAddressBytes = stackalloc byte[expectedBytesCount]; - bool written = BaseAddress.TryWriteBytes(baseAddressBytes, out int bytesWritten); - Debug.Assert(written && bytesWritten == expectedBytesCount); - - Span otherAddressBytes = stackalloc byte[expectedBytesCount]; - written = address.TryWriteBytes(otherAddressBytes, out bytesWritten); - Debug.Assert(written && bytesWritten == expectedBytesCount); - - for (int processedBitsCount = 0, i = 0; processedBitsCount < PrefixLength; processedBitsCount += BitsPerByte, i++) + if (address.AddressFamily == AddressFamily.InterNetwork) { - var baseAddressByte = baseAddressBytes[i]; - var otherAddressByte = otherAddressBytes[i]; + uint mask = uint.MaxValue << 32 - PrefixLength; - var rightShiftAmount = Math.Max(0, BitsPerByte - (PrefixLength - processedBitsCount)); - if (rightShiftAmount != 0) + if (BitConverter.IsLittleEndian) { - baseAddressByte >>= rightShiftAmount; - otherAddressByte >>= rightShiftAmount; + mask = BinaryPrimitives.ReverseEndianness(mask); } - if (baseAddressByte == otherAddressByte) + return BaseAddress.PrivateAddress == (address.PrivateAddress & mask); + } + else + { + Unsafe.SkipInit(out UInt128 baseAddressValue); + Unsafe.SkipInit(out UInt128 otherAddressValue); + + BaseAddress.TryWriteBytes(MemoryMarshal.AsBytes(MemoryMarshal.CreateSpan(ref baseAddressValue, 1)), out _); + address.TryWriteBytes(MemoryMarshal.AsBytes(MemoryMarshal.CreateSpan(ref otherAddressValue, 1)), out _); + + UInt128 mask = UInt128.MaxValue << 128 - PrefixLength; + if (BitConverter.IsLittleEndian) { - continue; + mask = BinaryPrimitives.ReverseEndianness(mask); } - return false; + return baseAddressValue == (otherAddressValue & mask); } - - return true; } #region Parsing (public) @@ -97,12 +85,19 @@ public static IPNetwork Parse(string s) } public static IPNetwork Parse(ReadOnlySpan s) { - if (TryParseInternal(s, out var result, out var error)) + if (!TryParseCore(s, out IPAddress? address, out int prefixLength)) { - return result; + throw new FormatException(SR.net_bad_ip_network); } - throw new FormatException(error); + try + { + return new IPNetwork(address, prefixLength); + } + catch (Exception inner) + { + throw new FormatException(inner.Message, inner); + } } public static bool TryParse(string? s, out IPNetwork result) @@ -113,167 +108,132 @@ public static bool TryParse(string? s, out IPNetwork result) return false; } - return TryParseInternal(s.AsSpan(), out result, out _); + return TryParse(s.AsSpan(), out result); } public static bool TryParse(ReadOnlySpan s, out IPNetwork result) { - return TryParseInternal(s, out result, out _); + if (TryParseCore(s, out IPAddress? address, out int prefixLength) && Validate(address, prefixLength, throwOnFailure: false)) + { + result = new IPNetwork(address, prefixLength, false); + return true; + } + + result = default; + return false; } #endregion #region Private methods - private static bool TryParseInternal(ReadOnlySpan s, out IPNetwork result, [NotNullWhen(false)] out string? error) + + private static bool TryParseCore(ReadOnlySpan s, [NotNullWhen(true)] out IPAddress? address, out int prefixLength) { const int MaxCharsCountAfterIpAddress = 4; const int MinCharsRequired = 4; if (s.Length < MinCharsRequired) { - error = SR.dns_bad_ip_network; - result = default; - return false; + goto Failure; } int separatorExpectedStartingIndex = s.Length - MaxCharsCountAfterIpAddress; int separatorIndex = s .Slice(separatorExpectedStartingIndex) - .IndexOf(AddressAndPrefixLengthSeparator); + .IndexOf('/'); + if (separatorIndex != -1) { separatorIndex += separatorExpectedStartingIndex; - var ipAddressSpan = s.Slice(0, separatorIndex); - var prefixLengthSpan = s.Slice(separatorIndex + 1); - - if (IPAddress.TryParse(ipAddressSpan, out var ipAddress) && int.TryParse(prefixLengthSpan, out var prefixLength)) - { - result = new IPNetwork(ipAddress, prefixLength, validateAndThrow: false); + ReadOnlySpan ipAddressSpan = s.Slice(0, separatorIndex); + ReadOnlySpan prefixLengthSpan = s.Slice(separatorIndex + 1); - error = result.Validate()?.ParseExceptionMessage; - return error == null; - } - else + if (IPAddress.TryParse(ipAddressSpan, out address) && int.TryParse(prefixLengthSpan, out prefixLength)) { - error = SR.dns_bad_ip_network; - result = default; - return false; + return true; } } - else - { - error = SR.dns_bad_ip_network; - result = default; - return false; - } - } - - private readonly struct ValidationError - { - public ValidationError(Func constructorExceptionFactoryMethod, string parseExceptionMessage) - { - ConstructorExceptionFactoryMethod = constructorExceptionFactoryMethod; - ParseExceptionMessage = parseExceptionMessage; - } - public readonly Func ConstructorExceptionFactoryMethod; - public readonly string ParseExceptionMessage; + Failure: + address = default; + prefixLength = default; + return false; } - private static readonly ValidationError s_baseAddressIsNullError = new ValidationError(() => new ArgumentNullException(BaseAddressConstructorParamName), string.Empty); - private static readonly ValidationError s_baseAddressHasSetBitsInMaskError = new ValidationError(() => new ArgumentException(BaseAddressConstructorParamName), SR.dns_bad_ip_network); - private static readonly ValidationError s_prefixLengthLessThanZeroError = new ValidationError(() => new ArgumentOutOfRangeException(PrefixLengthConstructorParamName), SR.dns_bad_ip_network); - private static readonly ValidationError s_prefixLengthGreaterThanAllowedError = new ValidationError(() => new ArgumentOutOfRangeException(PrefixLengthConstructorParamName), SR.dns_bad_ip_network); - - private ValidationError? Validate() + private static bool Validate(IPAddress baseAddress, int prefixLength, bool throwOnFailure) { - if (BaseAddress == null) + int maxPrefixLength = baseAddress.AddressFamily == AddressFamily.InterNetwork ? 32 : 128; + if (prefixLength < 0 || prefixLength > maxPrefixLength) { - return s_baseAddressIsNullError; - } + if (throwOnFailure) + { + ThrowArgumentOutOfRangeException(); + } - if (PrefixLength < 0) - { - return s_prefixLengthLessThanZeroError; + return false; } - if (PrefixLength > GetAddressFamilyByteLength(BaseAddress.AddressFamily) * BitsPerByte) + if (HasNonZeroBitsAfterNetworkPrefix(baseAddress, prefixLength)) { - return s_prefixLengthGreaterThanAllowedError; - } + if (throwOnFailure) + { + ThrowInvalidBaseAddressException(); + } - if (IsAnyMaskBitOnForBaseAddress()) - { - return s_baseAddressHasSetBitsInMaskError; + return false; } - return default; - } - - /// - /// Method to determine whether any bit in 's variable/mask part is 1. - /// - private bool IsAnyMaskBitOnForBaseAddress() - { - Span addressBytes = stackalloc byte[GetAddressFamilyByteLength(BaseAddress.AddressFamily)]; + return true; - bool written = BaseAddress.TryWriteBytes(addressBytes, out int bytesWritten); - Debug.Assert(written && bytesWritten == addressBytes.Length); + [DoesNotReturn] + static void ThrowArgumentOutOfRangeException() => throw new ArgumentOutOfRangeException(nameof(prefixLength)); - var addressBitsCount = addressBytes.Length * BitsPerByte; + [DoesNotReturn] + static void ThrowInvalidBaseAddressException() => throw new ArgumentException(SR.net_bad_ip_network_invalid_baseaddress, nameof(baseAddress)); + } - for (int addressBytesIndex = addressBytes.Length - 1, numberOfEndingBitsToBeOff = addressBitsCount - PrefixLength; - addressBytesIndex >= 0 && numberOfEndingBitsToBeOff > 0; - addressBytesIndex--, numberOfEndingBitsToBeOff -= BitsPerByte) + private static bool HasNonZeroBitsAfterNetworkPrefix(IPAddress baseAddress, int prefixLength) + { + if (baseAddress.AddressFamily == AddressFamily.InterNetwork) { - byte maskForByte = unchecked((byte)(byte.MaxValue << Math.Min(numberOfEndingBitsToBeOff, BitsPerByte))); - var addressByte = addressBytes[addressBytesIndex]; - if ((addressByte & maskForByte) != addressByte) + uint mask = (uint)((long)uint.MaxValue << 32 - prefixLength); + if (BitConverter.IsLittleEndian) { - return true; + mask = BinaryPrimitives.ReverseEndianness(mask); } + + return (baseAddress.PrivateAddress & mask) != baseAddress.PrivateAddress; } + else + { + Unsafe.SkipInit(out UInt128 value); + baseAddress.TryWriteBytes(MemoryMarshal.AsBytes(MemoryMarshal.CreateSpan(ref value, 1)), out _); + if (prefixLength == 0) + { + return value != UInt128.Zero; + } - return false; + UInt128 mask = UInt128.MaxValue << 128 - prefixLength; + if (BitConverter.IsLittleEndian) + { + mask = BinaryPrimitives.ReverseEndianness(mask); + } + + return (value & mask) != value; + } } - private static int GetAddressFamilyByteLength(AddressFamily addressFamily) - => addressFamily switch - { - AddressFamily.InterNetwork => IPAddressParserStatics.IPv4AddressBytes, - AddressFamily.InterNetworkV6 => IPAddressParserStatics.IPv6AddressBytes, - _ => throw new UnreachableException("Unknown address family") - }; #endregion #region Formatting (public) public override string ToString() { - return $"{BaseAddress}{AddressAndPrefixLengthSeparator}{PrefixLength}"; + return $"{BaseAddress}/{PrefixLength}"; } public bool TryFormat(Span destination, out int charsWritten) { - if (!BaseAddress.TryFormat(destination, out charsWritten)) - { - charsWritten = 0; - return false; - } - - if (destination.Length < charsWritten + 2) - { - return false; - } - - destination[charsWritten++] = AddressAndPrefixLengthSeparator; - - if (!PrefixLength.TryFormat(destination.Slice(charsWritten), out var prefixLengthCharsWritten)) - { - return false; - } - - charsWritten += prefixLengthCharsWritten; - return true; + return destination.TryWrite($"{BaseAddress}/{PrefixLength}", out charsWritten); } #endregion diff --git a/src/libraries/System.Net.Primitives/tests/FunctionalTests/IPNetworkTest.cs b/src/libraries/System.Net.Primitives/tests/FunctionalTests/IPNetworkTest.cs index 460bd74f055835..7e2b33e457a96b 100644 --- a/src/libraries/System.Net.Primitives/tests/FunctionalTests/IPNetworkTest.cs +++ b/src/libraries/System.Net.Primitives/tests/FunctionalTests/IPNetworkTest.cs @@ -7,48 +7,160 @@ namespace System.Net.Primitives.Functional.Tests { public class IPNetworkTest { + public static TheoryData IncorrectFormatData = new TheoryData() + { + { "127.0.0.1" }, + { "A.B.C.D/24" }, + { "127.0.0.1/AB" }, + { "127.0.0.1/-1" }, + { "2a01:110:8012::/f" }, + { "" }, + }; + + public static TheoryData InvalidNetworkNotationData = new TheoryData() + { + { "127.0.0.1/33" }, // PrefixLength max is 32 for IPv4 + { "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff/129" }, // PrefixLength max is 128 for IPv6 + { "127.0.0.1/31" }, // Bits exceed the prefix length of 31 (32nd bit is on) + { "198.51.255.0/23" }, // Bits exceed the prefix length of 23 + { "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff/127" }, // Bits exceed the prefix length of 31 + { "2a01:110:8012::/45" }, // Bits exceed the prefix length of 45 (47th bit is on) + }; + + public static TheoryData ValidIPNetworkData = new TheoryData() + { + { "0.0.0.0/32" }, // the whole IPv4 space + { "0.0.0.0/0" }, + { "128.0.0.0/1" }, + { "::/128" }, // the whole IPv6 space + { "255.255.255.255/32" }, + { "198.51.254.0/23" }, + { "42.42.128.0/17" }, + { "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff/128" }, + { "2a01:110:8012::/47" }, + { "2a01:110:8012::/100" }, + }; + [Theory] - [InlineData("127.0.0.1")] - [InlineData("A.B.C.D/24")] - [InlineData("127.0.0.1/AB")] - [InlineData("")] - public void Parse_IncorrectFormat_Throws(string input) + [MemberData(nameof(ValidIPNetworkData))] + public void Constructor_Valid_Succeeds(string input) + { + string[] splitInput = input.Split('/'); + IPAddress address = IPAddress.Parse(splitInput[0]); + int prefixLegth = int.Parse(splitInput[1]); + + IPNetwork network = new IPNetwork(address, prefixLegth); + + Assert.Equal(address, network.BaseAddress); + Assert.Equal(prefixLegth, network.PrefixLength); + } + + [Fact] + public void Constructor_NullIPAddress_ThrowsArgumentNullException() + { + Assert.Throws(() => new IPNetwork(null, 1)); + } + + [Theory] + [InlineData("192.168.0.1", -1)] + [InlineData("192.168.0.1", 33)] + [InlineData("::", -1)] + [InlineData("ffff::", 129)] + public void Constructor_PrefixLenghtOutOfRange_ThrowsArgumentOutOfRangeException(string ipStr, int prefixLength) + { + IPAddress address = IPAddress.Parse(ipStr); + Assert.Throws(() => new IPNetwork(address, prefixLength)); + } + + [Theory] + [InlineData("192.168.0.1", 31)] + [InlineData("42.42.192.0", 17)] + [InlineData("128.0.0.0", 0)] + [InlineData("2a01:110:8012::", 46)] + public void Constructor_NonZeroBitsAfterNetworkPrefix_ThrowsArgumentException(string ipStr, int prefixLength) + { + IPAddress address = IPAddress.Parse(ipStr); + Assert.Throws(() => new IPNetwork(address, prefixLength)); + } + + [Theory] + [MemberData(nameof(IncorrectFormatData))] + public void Parse_IncorrectFormat_ThrowsFormatException(string input) { Assert.Throws(() => IPNetwork.Parse(input)); } [Theory] - [InlineData("127.0.0.1/33")] // PrefixLength max is 32 - [InlineData("127.0.0.1/31")] // LastSetBit in the host mask (32nd bit is on) - [InlineData("")] - public void Parse_InvalidNetworkNotation_Throws(string input) + [MemberData(nameof(IncorrectFormatData))] + public void TryParse_IncorrectFormat_ReturnsFalse(string input) + { + Assert.False(IPNetwork.TryParse(input, out _)); + } + + [Theory] + [MemberData(nameof(InvalidNetworkNotationData))] + public void Parse_InvalidNetworkNotation_ThrowsFormatException(string input) { Assert.Throws(() => IPNetwork.Parse(input)); } [Theory] - [InlineData("0.0.0.0/32")] // the whole IPv4 space - [InlineData("::/128")] // the whole IPv6 space - [InlineData("255.255.255.255/32")] // single IPv4 address - [InlineData("ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff/128")] // single IPv6 address + [MemberData(nameof(InvalidNetworkNotationData))] + public void TryParse_InvalidNetworkNotation_ReturnsFalse(string input) + { + Assert.False(IPNetwork.TryParse(input, out _)); + } + + [Theory] + [MemberData(nameof(ValidIPNetworkData))] public void Parse_ValidNetworkNotation_Succeeds(string input) { var network = IPNetwork.Parse(input); Assert.Equal(input, network.ToString()); } + [Theory] + [MemberData(nameof(ValidIPNetworkData))] + public void TryParse_ValidNetworkNotation_Succeeds(string input) + { + Assert.True(IPNetwork.TryParse(input, out IPNetwork network)); + Assert.Equal(input, network.ToString()); + } + + [Fact] + public void Contains_Null_ThrowsArgumentNullException() + { + IPNetwork v4 = IPNetwork.Parse("127.0.0.0/8"); + IPNetwork v6 = IPNetwork.Parse("::1/128"); + + Assert.Throws(() => v4.Contains(null)); + Assert.Throws(() => v6.Contains(null)); + } + + [Fact] + public void Contains_DifferentAddressFamily_ReturnsFalse() + { + IPNetwork network = IPNetwork.Parse("ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff/128"); + Assert.False(network.Contains(IPAddress.Loopback)); + } + [Theory] [InlineData("0.0.0.0/0", "0.0.0.0", "127.127.127.127", "255.255.255.255")] // the whole IPv4 space [InlineData("::/0", "::", "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff")] // the whole IPv6 space [InlineData("255.255.255.255/32", "255.255.255.255")] // single IPv4 address [InlineData("ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff/128", "ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff")] // single IPv6 address [InlineData("255.255.255.0/24", "255.255.255.0", "255.255.255.255")] - [InlineData("255.255.255.128/25", "255.255.255.129", "255.255.255.255")] - public void Contains_ValidAddresses_ReturnsTrue(string networkString, params string[] validAddresses) + [InlineData("198.51.248.0/22", "198.51.248.0", "198.51.250.42", "198.51.251.255")] + [InlineData("255.255.255.128/25", "255.255.255.128", "255.255.255.129", "255.255.255.255")] + [InlineData("2a00::/13", "2a00::", "2a00::1", "2a01::", "2a07::", "2a07:ffff:ffff:ffff:ffff:ffff:ffff:ffff")] + [InlineData("2a01:110:8012::/47", "2a01:110:8012::", "2a01:110:8012:42::", "2a01:110:8013::", "2a01:110:8013:ffff:ffff:ffff:ffff:ffff")] + [InlineData("2a01:110:8012:1012:314f:2a00::/87", "2a01:110:8012:1012:314f:2a00::", "2a01:110:8012:1012:314f:2a00::1", "2a01:110:8012:1012:314f:2a00:abcd:4242", "2a01:110:8012:1012:314f:2bff:ffff:ffff")] + [InlineData("2a01:110:8012:1010:914e:2451:1700:0/105", "2a01:110:8012:1010:914e:2451:1700:0", "2a01:110:8012:1010:914e:2451:1742:4242", "2a01:110:8012:1010:914e:2451:177f:ffff")] + public void Contains_WhenInNework_ReturnsTrue(string networkString, params string[] addresses) { var network = IPNetwork.Parse(networkString); - foreach (var address in validAddresses) + foreach (string address in addresses) { Assert.True(network.Contains(IPAddress.Parse(address))); } @@ -58,34 +170,53 @@ public void Contains_ValidAddresses_ReturnsTrue(string networkString, params str [InlineData("255.255.255.255/32", "255.255.255.254")] // single IPv4 address [InlineData("ffff:ffff:ffff:ffff:ffff:ffff:ffff:ffff/128", "ffff:ffff:ffff:ffff:ffff:ffff:ffff:fffe")] // single IPv6 address [InlineData("255.255.255.0/24", "255.255.254.0")] + [InlineData("198.51.248.0/22", "198.50.248.1", "198.52.248.1", "198.51.247.1", "198.51.252.1")] [InlineData("255.255.255.128/25", "255.255.255.127")] - public void Contains_InvalidAddresses_ReturnsFalse(string networkString, params string[] invalidAddresses) + [InlineData("2a00::/13", "2900:ffff:ffff:ffff:ffff:ffff:ffff:ffff", "2a08::", "2a10::", "3a00::", "2b00::")] + [InlineData("2a01:110:8012::/47", "2a01:110:8011:1::", "2a01:110:8014::", "2a00:110:8012::1", "2a01:111:8012::")] + [InlineData("2a01:110:8012:1012:314f:2a00::/87", "2a01:110:8012:1012:314f:2c00::", "2a01:110:8012:1012:314f:2900::", "2a01:110:8012:1012:324f:2aff:ffff:ffff")] + [InlineData("2a01:110:8012:1010:914e:2451:1700:0/105", "2a01:110:8012:1010:914e:2451:16ff:ffff", "2a01:110:8012:1010:914e:2451:1780:0", "2a01:110:8013:1010:914e:2451:1700:0")] + public void Contains_WhenNotInNetwork_ReturnsFalse(string networkString, params string[] addresses) { var network = IPNetwork.Parse(networkString); - foreach (var address in invalidAddresses) + foreach (string address in addresses) { Assert.False(network.Contains(IPAddress.Parse(address))); } } - [Fact] - public void Equals_WhenDifferent_ReturnsFalse() + [Theory] + [InlineData(false)] + [InlineData(true)] + public void Equals_WhenDifferent_ReturnsFalse(bool testOperator) { var network = IPNetwork.Parse("127.0.0.0/24"); var rangeWithDifferentPrefix = IPNetwork.Parse("127.0.1.0/24"); var rangeWithDifferentPrefixLength = IPNetwork.Parse("127.0.0.0/25"); - Assert.False(network.Equals(rangeWithDifferentPrefix)); - Assert.False(network.Equals(rangeWithDifferentPrefixLength)); + if (testOperator) + { + Assert.False(network == rangeWithDifferentPrefix); + Assert.False(network == rangeWithDifferentPrefixLength); + Assert.True(network != rangeWithDifferentPrefix); + Assert.True(network != rangeWithDifferentPrefixLength); + } + else + { + Assert.False(network.Equals(rangeWithDifferentPrefix)); + Assert.False(network.Equals(rangeWithDifferentPrefixLength)); + } } - [Fact] - public void Equals_WhenSame_ReturnsTrue() + [Theory] + [InlineData("127.0.0.0/24")] + [InlineData("2a01:110:8012::/47")] + public void EqualiyMethods_WhenEqual(string input) { - var a = IPNetwork.Parse("127.0.0.0/24"); - var b = IPNetwork.Parse("127.0.0.0/24"); + var a = IPNetwork.Parse(input); + var b = IPNetwork.Parse(input); Assert.True(a.Equals(b)); Assert.True(a == b); @@ -118,7 +249,7 @@ public void TryFormatSpan_EnoughLength_Succeeds() [InlineData("127.127.127.127/32", 15)] [InlineData("127.127.127.127/32", 0)] [InlineData("127.127.127.127/32", 1)] - public void TryFormatSpan_NotEnoughLength_FailsWithoutException(string input, int spanLengthToTest) + public void TryFormatSpan_NotEnoughLength_ReturnsFalse(string input, int spanLengthToTest) { var network = IPNetwork.Parse(input); From 52ca52da93d84765c5ffe96960a02c3ff55f832b Mon Sep 17 00:00:00 2001 From: antonfirsov Date: Fri, 17 Mar 2023 17:24:01 +0100 Subject: [PATCH 06/13] do not use regions --- .../System.Net.Primitives/src/System/Net/IPNetwork.cs | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs b/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs index 4f2b401be890cd..ed00aa6af54b29 100644 --- a/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs +++ b/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs @@ -77,7 +77,6 @@ public bool Contains(IPAddress address) } } - #region Parsing (public) public static IPNetwork Parse(string s) { ArgumentNullException.ThrowIfNull(s); @@ -122,9 +121,7 @@ public static bool TryParse(ReadOnlySpan s, out IPNetwork result) result = default; return false; } - #endregion - #region Private methods private static bool TryParseCore(ReadOnlySpan s, [NotNullWhen(true)] out IPAddress? address, out int prefixLength) { @@ -223,9 +220,7 @@ private static bool HasNonZeroBitsAfterNetworkPrefix(IPAddress baseAddress, int } } - #endregion - #region Formatting (public) public override string ToString() { return $"{BaseAddress}/{PrefixLength}"; @@ -235,9 +230,7 @@ public bool TryFormat(Span destination, out int charsWritten) { return destination.TryWrite($"{BaseAddress}/{PrefixLength}", out charsWritten); } - #endregion - #region Equality and GetHashCode public bool Equals(IPNetwork other) { return BaseAddress.Equals(other.BaseAddress) && PrefixLength == other.PrefixLength; @@ -266,9 +259,7 @@ public override int GetHashCode() { return HashCode.Combine(BaseAddress, PrefixLength); } - #endregion - #region Explicit ISpanFormattable, ISpanParsable string IFormattable.ToString(string? format, IFormatProvider? provider) { return ToString(); @@ -294,6 +285,5 @@ static bool ISpanParsable.TryParse(ReadOnlySpan s, IFormatProvi { return TryParse(s, out result); } - #endregion } } From 28358b5a7ce49a411f55615939a7f33652310383 Mon Sep 17 00:00:00 2001 From: antonfirsov Date: Fri, 17 Mar 2023 18:50:29 +0100 Subject: [PATCH 07/13] docs, tests, more validation --- .../src/Resources/Strings.resx | 3 + .../src/System/Net/IPNetwork.cs | 114 +++++++++++++++++- .../tests/FunctionalTests/IPNetworkTest.cs | 32 +++++ 3 files changed, 145 insertions(+), 4 deletions(-) diff --git a/src/libraries/System.Net.Primitives/src/Resources/Strings.resx b/src/libraries/System.Net.Primitives/src/Resources/Strings.resx index 958a0e2e269f99..efd5107791717c 100644 --- a/src/libraries/System.Net.Primitives/src/Resources/Strings.resx +++ b/src/libraries/System.Net.Primitives/src/Resources/Strings.resx @@ -84,6 +84,9 @@ The specified baseAddress has non-zero bits after the network prefix. + + Uninitialized IPNetwork instance. + An error occurred when adding a cookie to the container. diff --git a/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs b/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs index ed00aa6af54b29..7d420aaf5c9ef1 100644 --- a/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs +++ b/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs @@ -2,22 +2,38 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Buffers.Binary; -using System.Diagnostics; using System.Diagnostics.CodeAnalysis; using System.Net.Sockets; using System.Runtime.CompilerServices; using System.Runtime.InteropServices; +#pragma warning disable SA1648 // TODO: https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3595 + namespace System.Net { /// - /// Provides an Internet Protocol (IP) subnet/range in CIDR notation. + /// Represents an IP network in CIDR notation with a and a prefix length. /// public readonly struct IPNetwork : IEquatable, ISpanFormattable, ISpanParsable { + /// + /// Gets the that represents the prefix of the network. + /// public IPAddress BaseAddress { get; } + + /// + /// Gets the length of the network prefix in bits. + /// public int PrefixLength { get; } + /// + /// Initializes a new instance of the class with the specified and prefix length. + /// + /// The that represents the prefix of the network. + /// The length of the prefix in bits. + /// The specified is . + /// The specified is smaller than `0` or longer than maximum length of 's . + /// The specified has non-zero bits after the network prefix. public IPNetwork(IPAddress baseAddress, int prefixLength) { ArgumentNullException.ThrowIfNull(baseAddress); @@ -34,10 +50,21 @@ private IPNetwork(IPAddress baseAddress, int prefixLength, bool _) PrefixLength = prefixLength; } + /// + /// Determines whether a given is part of the network. + /// + /// The to check. + /// if the is part of the network; otherwise, . + /// The specified is . public bool Contains(IPAddress address) { ArgumentNullException.ThrowIfNull(address); + if (BaseAddress == null) + { + throw new InvalidOperationException(SR.net_uninitialized_ip_network_instance); + } + if (address.AddressFamily != BaseAddress.AddressFamily) { return false; @@ -77,11 +104,25 @@ public bool Contains(IPAddress address) } } + /// + /// Converts a CIDR to an instance. + /// + /// A that defines an IP network in CIDR notation. + /// An instance. + /// The specified string is . + /// is not a valid CIDR network string, or the address contains non-zero bits after the network prefix. public static IPNetwork Parse(string s) { ArgumentNullException.ThrowIfNull(s); return Parse(s.AsSpan()); } + + /// + /// Converts a CIDR character span to an instance. + /// + /// A character span that defines an IP network in CIDR notation. + /// An instance. + /// is not a valid CIDR network string, or the address contains non-zero bits after the network prefix. public static IPNetwork Parse(ReadOnlySpan s) { if (!TryParseCore(s, out IPAddress? address, out int prefixLength)) @@ -99,6 +140,12 @@ public static IPNetwork Parse(ReadOnlySpan s) } } + /// + /// Converts the specified CIDR string to an instance and returns a value indicating whether the conversion succeeded. + /// + /// A that defines an IP network in CIDR notation. + /// When the method returns, contains an instance if the conversion succeeds. + /// if the conversion was succesful; otherwise, . public static bool TryParse(string? s, out IPNetwork result) { if (s == null) @@ -110,6 +157,12 @@ public static bool TryParse(string? s, out IPNetwork result) return TryParse(s.AsSpan(), out result); } + /// + /// Converts the specified CIDR character span to an instance and returns a value indicating whether the conversion succeeded. + /// + /// A that defines an IP network in CIDR notation. + /// When the method returns, contains an instance if the conversion succeeds. + /// if the conversion was succesful; otherwise, . public static bool TryParse(ReadOnlySpan s, out IPNetwork result) { if (TryParseCore(s, out IPAddress? address, out int prefixLength) && Validate(address, prefixLength, throwOnFailure: false)) @@ -122,7 +175,6 @@ public static bool TryParse(ReadOnlySpan s, out IPNetwork result) return false; } - private static bool TryParseCore(ReadOnlySpan s, [NotNullWhen(true)] out IPAddress? address, out int prefixLength) { const int MaxCharsCountAfterIpAddress = 4; @@ -220,22 +272,48 @@ private static bool HasNonZeroBitsAfterNetworkPrefix(IPAddress baseAddress, int } } - + /// + /// Converts the instance to a string containing the 's CIDR notation. + /// + /// The containing the 's CIDR notation. public override string ToString() { return $"{BaseAddress}/{PrefixLength}"; } + /// + /// Attempts to write the 's CIDR notation to the given span and returns a value indicating whether the operation succeeded. + /// + /// The destination span of characters. + /// When this method returns, contains the number of characters that were written to . + /// if the formatting was succesful; otherwise . public bool TryFormat(Span destination, out int charsWritten) { return destination.TryWrite($"{BaseAddress}/{PrefixLength}", out charsWritten); } + /// + /// Determines whether two instances are equal. + /// + /// The instance to compare to this instance. + /// if the networks are equal; otherwise . + /// Uninitialized instance. public bool Equals(IPNetwork other) { + if (BaseAddress == null || other.BaseAddress == null) + { + throw new InvalidOperationException(SR.net_uninitialized_ip_network_instance); + } + return BaseAddress.Equals(other.BaseAddress) && PrefixLength == other.PrefixLength; } + /// + /// Determines whether two instances are equal. + /// + /// The instance to compare to this instance. + /// if is an instance and the networks are equal; otherwise . + /// Uninitialized instance. public override bool Equals([NotNullWhen(true)] object? obj) { if (obj is not IPNetwork other) @@ -246,41 +324,69 @@ public override bool Equals([NotNullWhen(true)] object? obj) return Equals(other); } + /// + /// Determines whether the specified instances of are equal. + /// + /// + /// + /// if the networks are equal; otherwise . public static bool operator ==(IPNetwork left, IPNetwork right) { return left.Equals(right); } + + /// + /// Determines whether the specified instances of are not equal. + /// + /// + /// + /// if the networks are not equal; otherwise . public static bool operator !=(IPNetwork left, IPNetwork right) { return !(left == right); } + /// + /// Returns the hash code for this instance. + /// + /// An integer hash value. public override int GetHashCode() { return HashCode.Combine(BaseAddress, PrefixLength); } + /// string IFormattable.ToString(string? format, IFormatProvider? provider) { return ToString(); } + + /// bool ISpanFormattable.TryFormat(Span destination, out int charsWritten, ReadOnlySpan format, IFormatProvider? provider) { return TryFormat(destination, out charsWritten); } + + /// static IPNetwork IParsable.Parse([NotNull] string s, IFormatProvider? provider) { ArgumentNullException.ThrowIfNull(s); return Parse(s); } + + /// static bool IParsable.TryParse([NotNullWhen(true)] string? s, IFormatProvider? provider, out IPNetwork result) { return TryParse(s, out result); } + + /// static IPNetwork ISpanParsable.Parse(ReadOnlySpan s, IFormatProvider? provider) { return Parse(s); } + + /// static bool ISpanParsable.TryParse(ReadOnlySpan s, IFormatProvider? provider, out IPNetwork result) { return TryParse(s, out result); diff --git a/src/libraries/System.Net.Primitives/tests/FunctionalTests/IPNetworkTest.cs b/src/libraries/System.Net.Primitives/tests/FunctionalTests/IPNetworkTest.cs index 7e2b33e457a96b..20e0230369deb6 100644 --- a/src/libraries/System.Net.Primitives/tests/FunctionalTests/IPNetworkTest.cs +++ b/src/libraries/System.Net.Primitives/tests/FunctionalTests/IPNetworkTest.cs @@ -207,6 +207,37 @@ public void Equals_WhenDifferent_ReturnsFalse(bool testOperator) { Assert.False(network.Equals(rangeWithDifferentPrefix)); Assert.False(network.Equals(rangeWithDifferentPrefixLength)); + + Assert.False(network.Equals((object)rangeWithDifferentPrefix)); + Assert.False(network.Equals((object)rangeWithDifferentPrefixLength)); + } + } + + [Theory] + [InlineData(false)] + [InlineData(true)] + public void Equals_CompareValidInstanceToDefault_ThrowsInvalidOperationException(bool testOperator) + { + var valid = IPNetwork.Parse("127.0.0.0/24"); + IPNetwork invalid1 = default; + IPNetwork invalid2 = default; + + if (testOperator) + { + Assert.Throws(() => valid == invalid1); + Assert.Throws(() => valid != invalid1); + + Assert.Throws(() => invalid2 == invalid1); + Assert.Throws(() => invalid2 != invalid1); + } + else + { + Assert.Throws(() => valid.Equals(invalid1)); + Assert.Throws(() => valid.Equals((object)invalid1)); + Assert.Throws(() => invalid2.Equals(invalid1)); + Assert.Throws(() => invalid2.Equals((object)invalid1)); + Assert.Throws(() => invalid1.Equals(valid)); + Assert.Throws(() => invalid1.Equals((object)valid)); } } @@ -219,6 +250,7 @@ public void EqualiyMethods_WhenEqual(string input) var b = IPNetwork.Parse(input); Assert.True(a.Equals(b)); + Assert.True(a.Equals((object)b)); Assert.True(a == b); Assert.False(a != b); Assert.Equal(a.GetHashCode(), b.GetHashCode()); From 4985c8dab99948a3894b78e59e34078383468a64 Mon Sep 17 00:00:00 2001 From: antonfirsov Date: Fri, 17 Mar 2023 19:19:36 +0100 Subject: [PATCH 08/13] add remarks --- .../System.Net.Primitives/src/System/Net/IPNetwork.cs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs b/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs index 7d420aaf5c9ef1..7a1905dffad3e8 100644 --- a/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs +++ b/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs @@ -14,6 +14,12 @@ namespace System.Net /// /// Represents an IP network in CIDR notation with a and a prefix length. /// + /// + /// This type disallows arbitrary IP/prefixLength CIDR pairs. should represent the network prefix uniquely, + /// meaning that all bits after the network prefix must be zero. + /// In other words, is always the first usable address of the network. + /// The constructor and the parsing methods will throw in case there are non-zero bits after the prefix. + /// public readonly struct IPNetwork : IEquatable, ISpanFormattable, ISpanParsable { /// From 8938db0cba0689625ffb27c66f3486b8762cc351 Mon Sep 17 00:00:00 2001 From: antonfirsov Date: Mon, 20 Mar 2023 18:18:04 +0100 Subject: [PATCH 09/13] improve comments --- .../System.Net.Primitives/src/System/Net/IPNetwork.cs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs b/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs index 7a1905dffad3e8..4ede8627af2adf 100644 --- a/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs +++ b/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs @@ -15,8 +15,7 @@ namespace System.Net /// Represents an IP network in CIDR notation with a and a prefix length. /// /// - /// This type disallows arbitrary IP/prefixLength CIDR pairs. should represent the network prefix uniquely, - /// meaning that all bits after the network prefix must be zero. + /// This type disallows arbitrary IP/prefixLength CIDR pairs. must be defined so that all bits after the network prefix are set to zero. /// In other words, is always the first usable address of the network. /// The constructor and the parsing methods will throw in case there are non-zero bits after the prefix. /// From 29e6cf97fdd844224ab032713aa46753f0ee3bce Mon Sep 17 00:00:00 2001 From: Anton Firszov Date: Tue, 21 Mar 2023 20:05:22 +0100 Subject: [PATCH 10/13] Apply suggestions from code review Co-authored-by: Miha Zupan Co-authored-by: Stephen Toub --- .../src/System/Net/IPNetwork.cs | 57 ++++++------------- 1 file changed, 17 insertions(+), 40 deletions(-) diff --git a/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs b/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs index 4ede8627af2adf..d3201b72a44fb8 100644 --- a/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs +++ b/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs @@ -82,7 +82,7 @@ public bool Contains(IPAddress address) if (address.AddressFamily == AddressFamily.InterNetwork) { - uint mask = uint.MaxValue << 32 - PrefixLength; + uint mask = uint.MaxValue << (32 - PrefixLength); if (BitConverter.IsLittleEndian) { @@ -99,7 +99,7 @@ public bool Contains(IPAddress address) BaseAddress.TryWriteBytes(MemoryMarshal.AsBytes(MemoryMarshal.CreateSpan(ref baseAddressValue, 1)), out _); address.TryWriteBytes(MemoryMarshal.AsBytes(MemoryMarshal.CreateSpan(ref otherAddressValue, 1)), out _); - UInt128 mask = UInt128.MaxValue << 128 - PrefixLength; + UInt128 mask = UInt128.MaxValue << (128 - PrefixLength); if (BitConverter.IsLittleEndian) { mask = BinaryPrimitives.ReverseEndianness(mask); @@ -182,27 +182,15 @@ public static bool TryParse(ReadOnlySpan s, out IPNetwork result) private static bool TryParseCore(ReadOnlySpan s, [NotNullWhen(true)] out IPAddress? address, out int prefixLength) { - const int MaxCharsCountAfterIpAddress = 4; - const int MinCharsRequired = 4; - - if (s.Length < MinCharsRequired) - { - goto Failure; - } - - int separatorExpectedStartingIndex = s.Length - MaxCharsCountAfterIpAddress; - int separatorIndex = s - .Slice(separatorExpectedStartingIndex) - .IndexOf('/'); - - if (separatorIndex != -1) + int separatorIndex = s.LastIndexOf('/'); + if (separatorIndex >= 0) { - separatorIndex += separatorExpectedStartingIndex; ReadOnlySpan ipAddressSpan = s.Slice(0, separatorIndex); ReadOnlySpan prefixLengthSpan = s.Slice(separatorIndex + 1); - if (IPAddress.TryParse(ipAddressSpan, out address) && int.TryParse(prefixLengthSpan, out prefixLength)) + if (IPAddress.TryParse(ipAddressSpan, out address) && + int.TryParse(prefixLengthSpan, NumberStyles.None, CultureInfo.InvariantCulture, out prefixLength)) { return true; } @@ -250,7 +238,8 @@ private static bool HasNonZeroBitsAfterNetworkPrefix(IPAddress baseAddress, int { if (baseAddress.AddressFamily == AddressFamily.InterNetwork) { - uint mask = (uint)((long)uint.MaxValue << 32 - prefixLength); + // The cast to long ensures that the mask becomes 0 for the case where 'prefixLength == 0'. + uint mask = (uint)((long)uint.MaxValue << (32 - prefixLength)); if (BitConverter.IsLittleEndian) { mask = BinaryPrimitives.ReverseEndianness(mask); @@ -267,7 +256,7 @@ private static bool HasNonZeroBitsAfterNetworkPrefix(IPAddress baseAddress, int return value != UInt128.Zero; } - UInt128 mask = UInt128.MaxValue << 128 - prefixLength; + UInt128 mask = UInt128.MaxValue << (128 - prefixLength); if (BitConverter.IsLittleEndian) { mask = BinaryPrimitives.ReverseEndianness(mask); @@ -283,7 +272,7 @@ private static bool HasNonZeroBitsAfterNetworkPrefix(IPAddress baseAddress, int /// The containing the 's CIDR notation. public override string ToString() { - return $"{BaseAddress}/{PrefixLength}"; + return string.Create(CultureInfo.InvariantCulture, stackalloc char[128], $"{BaseAddress}/{PrefixLength}"); } /// @@ -294,7 +283,7 @@ public override string ToString() /// if the formatting was succesful; otherwise . public bool TryFormat(Span destination, out int charsWritten) { - return destination.TryWrite($"{BaseAddress}/{PrefixLength}", out charsWritten); + return destination.TryWrite(CultureInfo.InvariantCulture, $"{BaseAddress}/{PrefixLength}", out charsWritten); } /// @@ -303,15 +292,9 @@ public bool TryFormat(Span destination, out int charsWritten) /// The instance to compare to this instance. /// if the networks are equal; otherwise . /// Uninitialized instance. - public bool Equals(IPNetwork other) - { - if (BaseAddress == null || other.BaseAddress == null) - { - throw new InvalidOperationException(SR.net_uninitialized_ip_network_instance); - } - - return BaseAddress.Equals(other.BaseAddress) && PrefixLength == other.PrefixLength; - } + public bool Equals(IPNetwork other) => + PrefixLength == other.PrefixLength && + (BaseAddress is null ? other.BaseAddress is null : BaseAddress.Equals(other.BaseAddress)); /// /// Determines whether two instances are equal. @@ -319,14 +302,9 @@ public bool Equals(IPNetwork other) /// The instance to compare to this instance. /// if is an instance and the networks are equal; otherwise . /// Uninitialized instance. - public override bool Equals([NotNullWhen(true)] object? obj) - { - if (obj is not IPNetwork other) - { - return false; - } - - return Equals(other); + public override bool Equals([NotNullWhen(true)] object? obj) => + obj is IPNetwork other && + Equals(other); } /// @@ -375,7 +353,6 @@ bool ISpanFormattable.TryFormat(Span destination, out int charsWritten, Re /// static IPNetwork IParsable.Parse([NotNull] string s, IFormatProvider? provider) { - ArgumentNullException.ThrowIfNull(s); return Parse(s); } From 485a1ad8618a812496d693523d85b7a6d27a4184 Mon Sep 17 00:00:00 2001 From: antonfirsov Date: Tue, 21 Mar 2023 20:46:38 +0100 Subject: [PATCH 11/13] address more feedback --- .../src/Resources/Strings.resx | 3 - .../src/System/Net/IPNetwork.cs | 145 ++++++------------ .../tests/FunctionalTests/IPNetworkTest.cs | 22 +-- 3 files changed, 57 insertions(+), 113 deletions(-) diff --git a/src/libraries/System.Net.Primitives/src/Resources/Strings.resx b/src/libraries/System.Net.Primitives/src/Resources/Strings.resx index efd5107791717c..958a0e2e269f99 100644 --- a/src/libraries/System.Net.Primitives/src/Resources/Strings.resx +++ b/src/libraries/System.Net.Primitives/src/Resources/Strings.resx @@ -84,9 +84,6 @@ The specified baseAddress has non-zero bits after the network prefix. - - Uninitialized IPNetwork instance. - An error occurred when adding a cookie to the container. diff --git a/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs b/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs index d3201b72a44fb8..f8a74409b339a9 100644 --- a/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs +++ b/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs @@ -2,9 +2,10 @@ // The .NET Foundation licenses this file to you under the MIT license. using System.Buffers.Binary; +using System.Diagnostics; using System.Diagnostics.CodeAnalysis; +using System.Globalization; using System.Net.Sockets; -using System.Runtime.CompilerServices; using System.Runtime.InteropServices; #pragma warning disable SA1648 // TODO: https://github.com/DotNetAnalyzers/StyleCopAnalyzers/issues/3595 @@ -12,10 +13,10 @@ namespace System.Net { /// - /// Represents an IP network in CIDR notation with a and a prefix length. + /// Represents an IP network with an containing the network prefix and an defining the prefix length. /// /// - /// This type disallows arbitrary IP/prefixLength CIDR pairs. must be defined so that all bits after the network prefix are set to zero. + /// This type disallows arbitrary <IP>/<prefix-length> CIDR pairs. must be defined so that all bits after the network prefix are set to zero. /// In other words, is always the first usable address of the network. /// The constructor and the parsing methods will throw in case there are non-zero bits after the prefix. /// @@ -42,10 +43,25 @@ namespace System.Net public IPNetwork(IPAddress baseAddress, int prefixLength) { ArgumentNullException.ThrowIfNull(baseAddress); - Validate(baseAddress, prefixLength, throwOnFailure: true); + + if (prefixLength < 0 || prefixLength > GetMaxPrefixLength(baseAddress)) + { + ThrowArgumentOutOfRangeException(); + } + + if (HasNonZeroBitsAfterNetworkPrefix(baseAddress, prefixLength)) + { + ThrowInvalidBaseAddressException(); + } BaseAddress = baseAddress; PrefixLength = prefixLength; + + [DoesNotReturn] + static void ThrowArgumentOutOfRangeException() => throw new ArgumentOutOfRangeException(nameof(prefixLength)); + + [DoesNotReturn] + static void ThrowInvalidBaseAddressException() => throw new ArgumentException(SR.net_bad_ip_network_invalid_baseaddress, nameof(baseAddress)); } // Non-validating ctor @@ -65,16 +81,12 @@ public bool Contains(IPAddress address) { ArgumentNullException.ThrowIfNull(address); - if (BaseAddress == null) - { - throw new InvalidOperationException(SR.net_uninitialized_ip_network_instance); - } - - if (address.AddressFamily != BaseAddress.AddressFamily) + if (BaseAddress is null || address.AddressFamily != BaseAddress.AddressFamily) { return false; } + // This prevents the 'uint.MaxValue << 32' and the 'UInt128.MaxValue << 128' special cases in the code below. if (PrefixLength == 0) { return true; @@ -83,7 +95,6 @@ public bool Contains(IPAddress address) if (address.AddressFamily == AddressFamily.InterNetwork) { uint mask = uint.MaxValue << (32 - PrefixLength); - if (BitConverter.IsLittleEndian) { mask = BinaryPrimitives.ReverseEndianness(mask); @@ -93,11 +104,13 @@ public bool Contains(IPAddress address) } else { - Unsafe.SkipInit(out UInt128 baseAddressValue); - Unsafe.SkipInit(out UInt128 otherAddressValue); + UInt128 baseAddressValue = default; + UInt128 otherAddressValue = default; - BaseAddress.TryWriteBytes(MemoryMarshal.AsBytes(MemoryMarshal.CreateSpan(ref baseAddressValue, 1)), out _); - address.TryWriteBytes(MemoryMarshal.AsBytes(MemoryMarshal.CreateSpan(ref otherAddressValue, 1)), out _); + BaseAddress.TryWriteBytes(MemoryMarshal.AsBytes(new Span(ref baseAddressValue)), out int bytesWritten); + Debug.Assert(bytesWritten == IPAddressParserStatics.IPv6AddressBytes); + address.TryWriteBytes(MemoryMarshal.AsBytes(new Span(ref otherAddressValue)), out bytesWritten); + Debug.Assert(bytesWritten == IPAddressParserStatics.IPv6AddressBytes); UInt128 mask = UInt128.MaxValue << (128 - PrefixLength); if (BitConverter.IsLittleEndian) @@ -170,7 +183,9 @@ public static bool TryParse(string? s, out IPNetwork result) /// if the conversion was succesful; otherwise, . public static bool TryParse(ReadOnlySpan s, out IPNetwork result) { - if (TryParseCore(s, out IPAddress? address, out int prefixLength) && Validate(address, prefixLength, throwOnFailure: false)) + if (TryParseCore(s, out IPAddress? address, out int prefixLength) && + prefixLength <= GetMaxPrefixLength(address) && // Parsing with NumberStyles.None ensures that prefixLength is always non-negative. + !HasNonZeroBitsAfterNetworkPrefix(address, prefixLength)) { result = new IPNetwork(address, prefixLength, false); return true; @@ -185,7 +200,6 @@ private static bool TryParseCore(ReadOnlySpan s, [NotNullWhen(true)] out I int separatorIndex = s.LastIndexOf('/'); if (separatorIndex >= 0) { - ReadOnlySpan ipAddressSpan = s.Slice(0, separatorIndex); ReadOnlySpan prefixLengthSpan = s.Slice(separatorIndex + 1); @@ -196,43 +210,12 @@ private static bool TryParseCore(ReadOnlySpan s, [NotNullWhen(true)] out I } } - Failure: address = default; prefixLength = default; return false; } - private static bool Validate(IPAddress baseAddress, int prefixLength, bool throwOnFailure) - { - int maxPrefixLength = baseAddress.AddressFamily == AddressFamily.InterNetwork ? 32 : 128; - if (prefixLength < 0 || prefixLength > maxPrefixLength) - { - if (throwOnFailure) - { - ThrowArgumentOutOfRangeException(); - } - - return false; - } - - if (HasNonZeroBitsAfterNetworkPrefix(baseAddress, prefixLength)) - { - if (throwOnFailure) - { - ThrowInvalidBaseAddressException(); - } - - return false; - } - - return true; - - [DoesNotReturn] - static void ThrowArgumentOutOfRangeException() => throw new ArgumentOutOfRangeException(nameof(prefixLength)); - - [DoesNotReturn] - static void ThrowInvalidBaseAddressException() => throw new ArgumentException(SR.net_bad_ip_network_invalid_baseaddress, nameof(baseAddress)); - } + private static int GetMaxPrefixLength(IPAddress baseAddress) => baseAddress.AddressFamily == AddressFamily.InterNetwork ? 32 : 128; private static bool HasNonZeroBitsAfterNetworkPrefix(IPAddress baseAddress, int prefixLength) { @@ -249,8 +232,9 @@ private static bool HasNonZeroBitsAfterNetworkPrefix(IPAddress baseAddress, int } else { - Unsafe.SkipInit(out UInt128 value); - baseAddress.TryWriteBytes(MemoryMarshal.AsBytes(MemoryMarshal.CreateSpan(ref value, 1)), out _); + UInt128 value = default; + baseAddress.TryWriteBytes(MemoryMarshal.AsBytes(new Span(ref value)), out int bytesWritten); + Debug.Assert(bytesWritten == IPAddressParserStatics.IPv6AddressBytes); if (prefixLength == 0) { return value != UInt128.Zero; @@ -270,10 +254,8 @@ private static bool HasNonZeroBitsAfterNetworkPrefix(IPAddress baseAddress, int /// Converts the instance to a string containing the 's CIDR notation. /// /// The containing the 's CIDR notation. - public override string ToString() - { - return string.Create(CultureInfo.InvariantCulture, stackalloc char[128], $"{BaseAddress}/{PrefixLength}"); - } + public override string ToString() => + string.Create(CultureInfo.InvariantCulture, stackalloc char[128], $"{BaseAddress}/{(uint)PrefixLength}"); /// /// Attempts to write the 's CIDR notation to the given span and returns a value indicating whether the operation succeeded. @@ -281,10 +263,8 @@ public override string ToString() /// The destination span of characters. /// When this method returns, contains the number of characters that were written to . /// if the formatting was succesful; otherwise . - public bool TryFormat(Span destination, out int charsWritten) - { - return destination.TryWrite(CultureInfo.InvariantCulture, $"{BaseAddress}/{PrefixLength}", out charsWritten); - } + public bool TryFormat(Span destination, out int charsWritten) => + destination.TryWrite(CultureInfo.InvariantCulture, $"{BaseAddress}/{(uint)PrefixLength}", out charsWritten); /// /// Determines whether two instances are equal. @@ -305,7 +285,6 @@ public bool Equals(IPNetwork other) => public override bool Equals([NotNullWhen(true)] object? obj) => obj is IPNetwork other && Equals(other); - } /// /// Determines whether the specified instances of are equal. @@ -313,10 +292,7 @@ obj is IPNetwork other && /// /// /// if the networks are equal; otherwise . - public static bool operator ==(IPNetwork left, IPNetwork right) - { - return left.Equals(right); - } + public static bool operator ==(IPNetwork left, IPNetwork right) => left.Equals(right); /// /// Determines whether the specified instances of are not equal. @@ -324,54 +300,31 @@ obj is IPNetwork other && /// /// /// if the networks are not equal; otherwise . - public static bool operator !=(IPNetwork left, IPNetwork right) - { - return !(left == right); - } + public static bool operator !=(IPNetwork left, IPNetwork right) => !(left == right); /// /// Returns the hash code for this instance. /// /// An integer hash value. - public override int GetHashCode() - { - return HashCode.Combine(BaseAddress, PrefixLength); - } + public override int GetHashCode() => HashCode.Combine(BaseAddress, PrefixLength); /// - string IFormattable.ToString(string? format, IFormatProvider? provider) - { - return ToString(); - } + string IFormattable.ToString(string? format, IFormatProvider? provider) => ToString(); /// - bool ISpanFormattable.TryFormat(Span destination, out int charsWritten, ReadOnlySpan format, IFormatProvider? provider) - { - return TryFormat(destination, out charsWritten); - } + bool ISpanFormattable.TryFormat(Span destination, out int charsWritten, ReadOnlySpan format, IFormatProvider? provider) => + TryFormat(destination, out charsWritten); /// - static IPNetwork IParsable.Parse([NotNull] string s, IFormatProvider? provider) - { - return Parse(s); - } + static IPNetwork IParsable.Parse([NotNull] string s, IFormatProvider? provider) => Parse(s); /// - static bool IParsable.TryParse([NotNullWhen(true)] string? s, IFormatProvider? provider, out IPNetwork result) - { - return TryParse(s, out result); - } + static bool IParsable.TryParse([NotNullWhen(true)] string? s, IFormatProvider? provider, out IPNetwork result) => TryParse(s, out result); /// - static IPNetwork ISpanParsable.Parse(ReadOnlySpan s, IFormatProvider? provider) - { - return Parse(s); - } + static IPNetwork ISpanParsable.Parse(ReadOnlySpan s, IFormatProvider? provider) => Parse(s); /// - static bool ISpanParsable.TryParse(ReadOnlySpan s, IFormatProvider? provider, out IPNetwork result) - { - return TryParse(s, out result); - } + static bool ISpanParsable.TryParse(ReadOnlySpan s, IFormatProvider? provider, out IPNetwork result) => TryParse(s, out result); } } diff --git a/src/libraries/System.Net.Primitives/tests/FunctionalTests/IPNetworkTest.cs b/src/libraries/System.Net.Primitives/tests/FunctionalTests/IPNetworkTest.cs index 20e0230369deb6..48b27aab9fc917 100644 --- a/src/libraries/System.Net.Primitives/tests/FunctionalTests/IPNetworkTest.cs +++ b/src/libraries/System.Net.Primitives/tests/FunctionalTests/IPNetworkTest.cs @@ -216,28 +216,22 @@ public void Equals_WhenDifferent_ReturnsFalse(bool testOperator) [Theory] [InlineData(false)] [InlineData(true)] - public void Equals_CompareValidInstanceToDefault_ThrowsInvalidOperationException(bool testOperator) + public void Equals_CompareValidInstanceToDefault_ReturnsFalse(bool testOperator) { var valid = IPNetwork.Parse("127.0.0.0/24"); - IPNetwork invalid1 = default; - IPNetwork invalid2 = default; + IPNetwork invalid = default; if (testOperator) { - Assert.Throws(() => valid == invalid1); - Assert.Throws(() => valid != invalid1); - - Assert.Throws(() => invalid2 == invalid1); - Assert.Throws(() => invalid2 != invalid1); + Assert.False(valid == invalid); + Assert.True(valid != invalid); } else { - Assert.Throws(() => valid.Equals(invalid1)); - Assert.Throws(() => valid.Equals((object)invalid1)); - Assert.Throws(() => invalid2.Equals(invalid1)); - Assert.Throws(() => invalid2.Equals((object)invalid1)); - Assert.Throws(() => invalid1.Equals(valid)); - Assert.Throws(() => invalid1.Equals((object)valid)); + Assert.False(valid.Equals(invalid)); + Assert.False(valid.Equals((object)invalid)); + Assert.False(invalid.Equals(valid)); + Assert.False(invalid.Equals((object)valid)); } } From 0ead2dbbec6c032eb1dbe8ca641860ac4e30d0b6 Mon Sep 17 00:00:00 2001 From: antonfirsov Date: Wed, 22 Mar 2023 00:37:10 +0100 Subject: [PATCH 12/13] simplify parsing code --- .../src/System/Net/IPNetwork.cs | 38 +++++-------------- .../tests/FunctionalTests/IPNetworkTest.cs | 1 + 2 files changed, 11 insertions(+), 28 deletions(-) diff --git a/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs b/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs index f8a74409b339a9..99d2630b51453d 100644 --- a/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs +++ b/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs @@ -16,7 +16,7 @@ namespace System.Net /// Represents an IP network with an containing the network prefix and an defining the prefix length. /// /// - /// This type disallows arbitrary <IP>/<prefix-length> CIDR pairs. must be defined so that all bits after the network prefix are set to zero. + /// This type disallows arbitrary IP-address/prefix-length CIDR pairs. must be defined so that all bits after the network prefix are set to zero. /// In other words, is always the first usable address of the network. /// The constructor and the parsing methods will throw in case there are non-zero bits after the prefix. /// @@ -143,19 +143,12 @@ public static IPNetwork Parse(string s) /// is not a valid CIDR network string, or the address contains non-zero bits after the network prefix. public static IPNetwork Parse(ReadOnlySpan s) { - if (!TryParseCore(s, out IPAddress? address, out int prefixLength)) + if (!TryParse(s, out IPNetwork result)) { throw new FormatException(SR.net_bad_ip_network); } - try - { - return new IPNetwork(address, prefixLength); - } - catch (Exception inner) - { - throw new FormatException(inner.Message, inner); - } + return result; } /// @@ -182,20 +175,6 @@ public static bool TryParse(string? s, out IPNetwork result) /// When the method returns, contains an instance if the conversion succeeds. /// if the conversion was succesful; otherwise, . public static bool TryParse(ReadOnlySpan s, out IPNetwork result) - { - if (TryParseCore(s, out IPAddress? address, out int prefixLength) && - prefixLength <= GetMaxPrefixLength(address) && // Parsing with NumberStyles.None ensures that prefixLength is always non-negative. - !HasNonZeroBitsAfterNetworkPrefix(address, prefixLength)) - { - result = new IPNetwork(address, prefixLength, false); - return true; - } - - result = default; - return false; - } - - private static bool TryParseCore(ReadOnlySpan s, [NotNullWhen(true)] out IPAddress? address, out int prefixLength) { int separatorIndex = s.LastIndexOf('/'); if (separatorIndex >= 0) @@ -203,15 +182,18 @@ private static bool TryParseCore(ReadOnlySpan s, [NotNullWhen(true)] out I ReadOnlySpan ipAddressSpan = s.Slice(0, separatorIndex); ReadOnlySpan prefixLengthSpan = s.Slice(separatorIndex + 1); - if (IPAddress.TryParse(ipAddressSpan, out address) && - int.TryParse(prefixLengthSpan, NumberStyles.None, CultureInfo.InvariantCulture, out prefixLength)) + if (IPAddress.TryParse(ipAddressSpan, out IPAddress? address) && + int.TryParse(prefixLengthSpan, NumberStyles.None, CultureInfo.InvariantCulture, out int prefixLength) && + prefixLength <= GetMaxPrefixLength(address) && + !HasNonZeroBitsAfterNetworkPrefix(address, prefixLength)) { + Debug.Assert(prefixLength >= 0); // Parsing with NumberStyles.None should ensure that prefixLength is always non-negative. + result = new IPNetwork(address, prefixLength, false); return true; } } - address = default; - prefixLength = default; + result = default; return false; } diff --git a/src/libraries/System.Net.Primitives/tests/FunctionalTests/IPNetworkTest.cs b/src/libraries/System.Net.Primitives/tests/FunctionalTests/IPNetworkTest.cs index 48b27aab9fc917..ad50ea779e3886 100644 --- a/src/libraries/System.Net.Primitives/tests/FunctionalTests/IPNetworkTest.cs +++ b/src/libraries/System.Net.Primitives/tests/FunctionalTests/IPNetworkTest.cs @@ -13,6 +13,7 @@ public class IPNetworkTest { "A.B.C.D/24" }, { "127.0.0.1/AB" }, { "127.0.0.1/-1" }, + { "127.0.0.1/+1" }, { "2a01:110:8012::/f" }, { "" }, }; From 5028755e6f32d7cbb2e46769c9d6e7a71ae3cf8c Mon Sep 17 00:00:00 2001 From: antonfirsov Date: Wed, 22 Mar 2023 18:23:38 +0100 Subject: [PATCH 13/13] default(IPNetwork) ~ 0.0.0.0/0 --- .../src/System/Net/IPNetwork.cs | 12 ++++--- .../tests/FunctionalTests/IPNetworkTest.cs | 32 ++++++------------- 2 files changed, 17 insertions(+), 27 deletions(-) diff --git a/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs b/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs index 99d2630b51453d..58f772cc632f6e 100644 --- a/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs +++ b/src/libraries/System.Net.Primitives/src/System/Net/IPNetwork.cs @@ -22,10 +22,12 @@ namespace System.Net /// public readonly struct IPNetwork : IEquatable, ISpanFormattable, ISpanParsable { + private readonly IPAddress? _baseAddress; + /// /// Gets the that represents the prefix of the network. /// - public IPAddress BaseAddress { get; } + public IPAddress BaseAddress => _baseAddress ?? IPAddress.Any; /// /// Gets the length of the network prefix in bits. @@ -54,7 +56,7 @@ public IPNetwork(IPAddress baseAddress, int prefixLength) ThrowInvalidBaseAddressException(); } - BaseAddress = baseAddress; + _baseAddress = baseAddress; PrefixLength = prefixLength; [DoesNotReturn] @@ -67,7 +69,7 @@ public IPNetwork(IPAddress baseAddress, int prefixLength) // Non-validating ctor private IPNetwork(IPAddress baseAddress, int prefixLength, bool _) { - BaseAddress = baseAddress; + _baseAddress = baseAddress; PrefixLength = prefixLength; } @@ -81,7 +83,7 @@ public bool Contains(IPAddress address) { ArgumentNullException.ThrowIfNull(address); - if (BaseAddress is null || address.AddressFamily != BaseAddress.AddressFamily) + if (address.AddressFamily != BaseAddress.AddressFamily) { return false; } @@ -256,7 +258,7 @@ public bool TryFormat(Span destination, out int charsWritten) => /// Uninitialized instance. public bool Equals(IPNetwork other) => PrefixLength == other.PrefixLength && - (BaseAddress is null ? other.BaseAddress is null : BaseAddress.Equals(other.BaseAddress)); + BaseAddress.Equals(other.BaseAddress); /// /// Determines whether two instances are equal. diff --git a/src/libraries/System.Net.Primitives/tests/FunctionalTests/IPNetworkTest.cs b/src/libraries/System.Net.Primitives/tests/FunctionalTests/IPNetworkTest.cs index ad50ea779e3886..56434733cef049 100644 --- a/src/libraries/System.Net.Primitives/tests/FunctionalTests/IPNetworkTest.cs +++ b/src/libraries/System.Net.Primitives/tests/FunctionalTests/IPNetworkTest.cs @@ -214,28 +214,6 @@ public void Equals_WhenDifferent_ReturnsFalse(bool testOperator) } } - [Theory] - [InlineData(false)] - [InlineData(true)] - public void Equals_CompareValidInstanceToDefault_ReturnsFalse(bool testOperator) - { - var valid = IPNetwork.Parse("127.0.0.0/24"); - IPNetwork invalid = default; - - if (testOperator) - { - Assert.False(valid == invalid); - Assert.True(valid != invalid); - } - else - { - Assert.False(valid.Equals(invalid)); - Assert.False(valid.Equals((object)invalid)); - Assert.False(invalid.Equals(valid)); - Assert.False(invalid.Equals((object)valid)); - } - } - [Theory] [InlineData("127.0.0.0/24")] [InlineData("2a01:110:8012::/47")] @@ -284,5 +262,15 @@ public void TryFormatSpan_NotEnoughLength_ReturnsFalse(string input, int spanLen Assert.False(network.TryFormat(span, out int charsWritten)); } + + [Fact] + public void DefaultInstance_IsValid() + { + IPNetwork network = default; + Assert.Equal(IPAddress.Any, network.BaseAddress); + Assert.Equal(default, network); + Assert.NotEqual(IPNetwork.Parse("10.20.30.0/24"), network); + Assert.True(network.Contains(IPAddress.Parse("10.11.12.13"))); + } } }