From 9b0e44019a350ee89befcc92edcb95ae20787cf9 Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Sun, 2 Jul 2023 12:34:50 +0200 Subject: [PATCH] Apply review suggestions --- Jellyfin.Networking/Manager/NetworkManager.cs | 7 ++-- MediaBrowser.Common/Net/INetworkManager.cs | 3 +- MediaBrowser.Common/Net/NetworkExtensions.cs | 35 ++++++++++--------- .../NetworkParseTests.cs | 11 +++--- 4 files changed, 30 insertions(+), 26 deletions(-) diff --git a/Jellyfin.Networking/Manager/NetworkManager.cs b/Jellyfin.Networking/Manager/NetworkManager.cs index 1b1e91e9fe..7d21c26cd0 100644 --- a/Jellyfin.Networking/Manager/NetworkManager.cs +++ b/Jellyfin.Networking/Manager/NetworkManager.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.Linq; using System.Net; @@ -380,7 +381,7 @@ namespace Jellyfin.Networking.Manager // Remove all interfaces matching any virtual machine interface prefix if (config.IgnoreVirtualInterfaces) { - // Remove potentially exisiting * and split config string into prefixes + // Remove potentially existing * and split config string into prefixes var virtualInterfacePrefixes = config.VirtualInterfaceNames .Select(i => i.Replace("*", string.Empty, StringComparison.OrdinalIgnoreCase)); @@ -587,7 +588,7 @@ namespace Jellyfin.Networking.Manager } /// - public bool TryParseInterface(string intf, out IReadOnlyList result) + public bool TryParseInterface(string intf, [NotNullWhen(true)] out IReadOnlyList? result) { var resultList = new List(); if (string.IsNullOrEmpty(intf) || _interfaces is null) @@ -660,7 +661,7 @@ namespace Jellyfin.Networking.Manager /// public IReadOnlyList GetLoopbacks() { - if (!(IsIPv4Enabled && IsIPv4Enabled)) + if (!IsIPv4Enabled && !IsIPv6Enabled) { return Array.Empty(); } diff --git a/MediaBrowser.Common/Net/INetworkManager.cs b/MediaBrowser.Common/Net/INetworkManager.cs index a92b751f2a..c51090e385 100644 --- a/MediaBrowser.Common/Net/INetworkManager.cs +++ b/MediaBrowser.Common/Net/INetworkManager.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Diagnostics.CodeAnalysis; using System.Net; using System.Net.NetworkInformation; using MediaBrowser.Model.Net; @@ -120,7 +121,7 @@ namespace MediaBrowser.Common.Net /// Interface name. /// Resulting object's IP addresses, if successful. /// Success of the operation. - bool TryParseInterface(string intf, out IReadOnlyList result); + bool TryParseInterface(string intf, [NotNullWhen(true)] out IReadOnlyList? result); /// /// Returns all internal (LAN) bind interface addresses. diff --git a/MediaBrowser.Common/Net/NetworkExtensions.cs b/MediaBrowser.Common/Net/NetworkExtensions.cs index 8a14bf48db..ae85583a56 100644 --- a/MediaBrowser.Common/Net/NetworkExtensions.cs +++ b/MediaBrowser.Common/Net/NetworkExtensions.cs @@ -166,28 +166,30 @@ namespace MediaBrowser.Common.Net /// Collection of . /// Boolean signaling if negated or not negated values should be parsed. /// True if parsing was successful. - public static bool TryParseToSubnets(string[] values, out List result, bool negated = false) + public static bool TryParseToSubnets(string[] values, [NotNullWhen(true)] out IReadOnlyList? result, bool negated = false) { - result = new List(); - if (values is null || values.Length == 0) { + result = null; return false; } + var tmpResult = new List(); for (int a = 0; a < values.Length; a++) { if (TryParseToSubnet(values[a], out var innerResult, negated)) { - result.Add(innerResult); + tmpResult.Add(innerResult); } } - if (result.Count > 0) + if (tmpResult.Count > 0) { + result = tmpResult; return true; } + result = null; return false; } @@ -199,9 +201,8 @@ namespace MediaBrowser.Common.Net /// An . /// Boolean signaling if negated or not negated values should be parsed. /// True if parsing was successful. - public static bool TryParseToSubnet(ReadOnlySpan value, out IPNetwork result, bool negated = false) + public static bool TryParseToSubnet(ReadOnlySpan value, [NotNullWhen(true)] out IPNetwork? result, bool negated = false) { - result = new IPNetwork(IPAddress.None, 32); var splitString = value.Trim().Split('/'); if (splitString.MoveNext()) { @@ -224,25 +225,28 @@ namespace MediaBrowser.Common.Net if (int.TryParse(subnetBlock, out var netmask)) { result = new IPNetwork(address, netmask); + return true; } else if (IPAddress.TryParse(subnetBlock, out var netmaskAddress)) { result = new IPNetwork(address, NetworkExtensions.MaskToCidr(netmaskAddress)); + return true; } } else if (address.AddressFamily == AddressFamily.InterNetwork) { result = new IPNetwork(address, 32); + return true; } else if (address.AddressFamily == AddressFamily.InterNetworkV6) { result = new IPNetwork(address, 128); + return true; } - - return true; } } + result = null; return false; } @@ -254,11 +258,11 @@ namespace MediaBrowser.Common.Net /// true if IPv4 is enabled. /// true if IPv6 is enabled. /// true if the parsing is successful, false if not. - public static bool TryParseHost(ReadOnlySpan host, [NotNullWhen(true)] out IPAddress[] addresses, bool isIPv4Enabled = true, bool isIPv6Enabled = false) + public static bool TryParseHost(ReadOnlySpan host, [NotNullWhen(true)] out IPAddress[]? addresses, bool isIPv4Enabled = true, bool isIPv6Enabled = false) { if (host.IsEmpty) { - addresses = Array.Empty(); + addresses = null; return false; } @@ -301,9 +305,7 @@ namespace MediaBrowser.Common.Net } // Is an IP4 or IP4:port - host = hosts[0].Split('/')[0]; - - if (IPAddress.TryParse(host, out var address)) + if (IPAddress.TryParse(hosts[0].AsSpan().LeftPart('/'), out var address)) { if (((address.AddressFamily == AddressFamily.InterNetwork) && (!isIPv4Enabled && isIPv6Enabled)) || ((address.AddressFamily == AddressFamily.InterNetworkV6) && (isIPv4Enabled && !isIPv6Enabled))) @@ -318,10 +320,9 @@ namespace MediaBrowser.Common.Net return true; } } - else if (hosts.Count <= 9) // 8 octets + port + else if (hosts.Count > 0 && hosts.Count <= 9) // 8 octets + port { - var splitSpan = host.Split('/'); - if (splitSpan.MoveNext() && IPAddress.TryParse(splitSpan.Current, out var address)) + if (IPAddress.TryParse(host.LeftPart('/'), out var address)) { addresses = new[] { address }; return true; diff --git a/tests/Jellyfin.Networking.Tests/NetworkParseTests.cs b/tests/Jellyfin.Networking.Tests/NetworkParseTests.cs index 8b7df0470d..77f18c5445 100644 --- a/tests/Jellyfin.Networking.Tests/NetworkParseTests.cs +++ b/tests/Jellyfin.Networking.Tests/NetworkParseTests.cs @@ -203,12 +203,13 @@ namespace Jellyfin.Networking.Tests using var nm = new NetworkManager(GetMockConfig(conf), new NullLogger()); NetworkManager.MockNetworkSettings = string.Empty; - _ = nm.TryParseInterface(result, out IReadOnlyList? resultObj); + // Check to see if DNS resolution is working. If not, skip test. + if (!NetworkExtensions.TryParseHost(source, out var host)) + { + return; + } - // Check to see if dns resolution is working. If not, skip test. - _ = NetworkExtensions.TryParseHost(source, out var host); - - if (resultObj is not null && host.Length > 0) + if (nm.TryParseInterface(result, out var resultObj)) { result = resultObj.First().Address.ToString(); var intf = nm.GetBindAddress(source, out _);