From 2281b8c997dff0fa148bf0f193b37664420aca3e Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Wed, 20 Jul 2022 14:29:30 +0200 Subject: [PATCH] Move away from using Collection, simplify code, add proper ordering --- Emby.Dlna/Main/DlnaEntryPoint.cs | 2 +- .../Configuration/NetworkConfiguration.cs | 4 +- Jellyfin.Networking/Manager/NetworkManager.cs | 54 +++++++++---------- .../CreateNetworkConfiguration.cs | 2 +- MediaBrowser.Common/Net/INetworkManager.cs | 13 +++-- MediaBrowser.Common/Net/NetworkExtensions.cs | 6 +-- .../NetworkParseTests.cs | 8 ++- 7 files changed, 42 insertions(+), 47 deletions(-) diff --git a/Emby.Dlna/Main/DlnaEntryPoint.cs b/Emby.Dlna/Main/DlnaEntryPoint.cs index 7bc761b71c..90c985a816 100644 --- a/Emby.Dlna/Main/DlnaEntryPoint.cs +++ b/Emby.Dlna/Main/DlnaEntryPoint.cs @@ -293,7 +293,7 @@ namespace Emby.Dlna.Main if (bindAddresses.Count == 0) { // No interfaces returned, so use loopback. - bindAddresses = _networkManager.GetLoopbacks(); + bindAddresses = _networkManager.GetLoopbacks().ToList(); } foreach (var address in bindAddresses) diff --git a/Jellyfin.Networking/Configuration/NetworkConfiguration.cs b/Jellyfin.Networking/Configuration/NetworkConfiguration.cs index 0ac55c986e..be8dc738d9 100644 --- a/Jellyfin.Networking/Configuration/NetworkConfiguration.cs +++ b/Jellyfin.Networking/Configuration/NetworkConfiguration.cs @@ -151,9 +151,9 @@ namespace Jellyfin.Networking.Configuration public bool IgnoreVirtualInterfaces { get; set; } = true; /// - /// Gets or sets a value indicating the interfaces that should be ignored. The list can be comma separated. . + /// Gets or sets a value indicating the interface name prefixes that should be ignored. The list can be comma separated and values are case-insensitive. . /// - public string VirtualInterfaceNames { get; set; } = "vEthernet*"; + public string VirtualInterfaceNames { get; set; } = "veth"; /// /// Gets or sets the time (in seconds) between the pings of SSDP gateway monitor. diff --git a/Jellyfin.Networking/Manager/NetworkManager.cs b/Jellyfin.Networking/Manager/NetworkManager.cs index e9e3e18022..757b5994b9 100644 --- a/Jellyfin.Networking/Manager/NetworkManager.cs +++ b/Jellyfin.Networking/Manager/NetworkManager.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Generic; -using System.Collections.ObjectModel; using System.Globalization; using System.Linq; using System.Net; @@ -47,7 +46,7 @@ namespace Jellyfin.Networking.Manager /// private readonly Dictionary _publishedServerUrls; - private Collection _remoteAddressFilter; + private List _remoteAddressFilter; /// /// Used to stop "event-racing conditions". @@ -58,12 +57,12 @@ namespace Jellyfin.Networking.Manager /// Unfiltered user defined LAN subnets () /// or internal interface network subnets if undefined by user. /// - private Collection _lanSubnets; + private List _lanSubnets; /// /// User defined list of subnets to excluded from the LAN. /// - private Collection _excludedSubnets; + private List _excludedSubnets; /// /// List of interfaces to bind to. @@ -95,7 +94,7 @@ namespace Jellyfin.Networking.Manager _macAddresses = new List(); _publishedServerUrls = new Dictionary(); _eventFireLock = new object(); - _remoteAddressFilter = new Collection(); + _remoteAddressFilter = new List(); UpdateSettings(_configurationManager.GetNetworkConfiguration()); @@ -225,8 +224,8 @@ namespace Jellyfin.Networking.Manager { try { - IPInterfaceProperties ipProperties = adapter.GetIPProperties(); - PhysicalAddress mac = adapter.GetPhysicalAddress(); + var ipProperties = adapter.GetIPProperties(); + var mac = adapter.GetPhysicalAddress(); // Populate MAC list if (adapter.NetworkInterfaceType != NetworkInterfaceType.Loopback && PhysicalAddress.None.Equals(mac)) @@ -235,7 +234,7 @@ namespace Jellyfin.Networking.Manager } // Populate interface list - foreach (UnicastIPAddressInformation info in ipProperties.UnicastAddresses) + foreach (var info in ipProperties.UnicastAddresses) { if (IsIpv4Enabled && info.Address.AddressFamily == AddressFamily.InterNetwork) { @@ -364,8 +363,8 @@ namespace Jellyfin.Networking.Manager // Use explicit bind addresses if (config.LocalNetworkAddresses.Length > 0) { - _bindAddresses = config.LocalNetworkAddresses.Select(p => IPAddress.TryParse(p, out var address) - ? address + _bindAddresses = config.LocalNetworkAddresses.Select(p => IPAddress.TryParse(p, out var addresses) + ? addresses : (_interfaces.Where(x => x.Name.Equals(p, StringComparison.OrdinalIgnoreCase)).Select(x => x.Address).FirstOrDefault() ?? IPAddress.None)).ToList(); _bindAddresses.RemoveAll(x => x == IPAddress.None); } @@ -525,13 +524,11 @@ namespace Jellyfin.Networking.Manager var address = IPAddress.Parse(split[0]); var network = new IPNetwork(address, int.Parse(split[1], CultureInfo.InvariantCulture)); var index = int.Parse(parts[1], CultureInfo.InvariantCulture); - if (address.AddressFamily == AddressFamily.InterNetwork) + if (address.AddressFamily == AddressFamily.InterNetwork || address.AddressFamily == AddressFamily.InterNetworkV6) { - _interfaces.Add(new IPData(address, network, parts[2])); - } - else if (address.AddressFamily == AddressFamily.InterNetworkV6) - { - _interfaces.Add(new IPData(address, network, parts[2])); + var data = new IPData(address, network, parts[2]); + data.Index = index; + _interfaces.Add(data); } } } @@ -562,9 +559,9 @@ namespace Jellyfin.Networking.Manager } /// - public bool TryParseInterface(string intf, out Collection result) + public bool TryParseInterface(string intf, out List result) { - result = new Collection(); + result = new List(); if (string.IsNullOrEmpty(intf)) { return false; @@ -573,7 +570,7 @@ namespace Jellyfin.Networking.Manager if (_interfaces != null) { // Match all interfaces starting with names starting with token - var matchedInterfaces = _interfaces.Where(s => s.Name.Equals(intf.ToLowerInvariant(), StringComparison.OrdinalIgnoreCase)); + var matchedInterfaces = _interfaces.Where(s => s.Name.Equals(intf.ToLowerInvariant(), StringComparison.OrdinalIgnoreCase)).OrderBy(x => x.Index); if (matchedInterfaces.Any()) { _logger.LogInformation("Interface {Token} used in settings. Using its interface addresses.", intf); @@ -626,14 +623,14 @@ namespace Jellyfin.Networking.Manager } /// - public IReadOnlyCollection GetMacAddresses() + public IReadOnlyList GetMacAddresses() { // Populated in construction - so always has values. return _macAddresses; } /// - public List GetLoopbacks() + public IReadOnlyList GetLoopbacks() { var loopbackNetworks = new List(); if (IsIpv4Enabled) @@ -650,7 +647,7 @@ namespace Jellyfin.Networking.Manager } /// - public List GetAllBindInterfaces(bool individualInterfaces = false) + public IReadOnlyList GetAllBindInterfaces(bool individualInterfaces = false) { if (_bindAddresses.Count == 0) { @@ -816,7 +813,7 @@ namespace Jellyfin.Networking.Manager } /// - public List GetInternalBindAddresses() + public IReadOnlyList GetInternalBindAddresses() { if (_bindAddresses.Count == 0) { @@ -833,7 +830,8 @@ namespace Jellyfin.Networking.Manager // Select all local bind addresses return _interfaces.Where(x => _bindAddresses.Contains(x.Address)) .Where(x => IsInLocalNetwork(x.Address)) - .OrderBy(x => x.Index).ToList(); + .OrderBy(x => x.Index) + .ToList(); } /// @@ -892,7 +890,7 @@ namespace Jellyfin.Networking.Manager interfaces = interfaces.Where(x => IsInLocalNetwork(x.Address)).ToList(); } - foreach (var intf in _interfaces) + foreach (var intf in interfaces) { if (intf.Subnet.Contains(address)) { @@ -942,7 +940,7 @@ namespace Jellyfin.Networking.Manager foreach (var data in validPublishedServerUrls) { // Get address interface - var intf = _interfaces.FirstOrDefault(s => s.Subnet.Contains(data.Key.Address)); + var intf = _interfaces.OrderBy(x => x.Index).FirstOrDefault(s => s.Subnet.Contains(data.Key.Address)); // Remaining. Match anything. if (data.Key.Address.Equals(IPAddress.Broadcast)) @@ -1017,7 +1015,7 @@ namespace Jellyfin.Networking.Manager defaultGateway = addr; } - var intf = _interfaces.Where(x => x.Subnet.Contains(addr)).FirstOrDefault(); + var intf = _interfaces.Where(x => x.Subnet.Contains(addr)).OrderBy(x => x.Index).FirstOrDefault(); if (bindAddress == null && intf != null && intf.Subnet.Contains(source)) { @@ -1082,7 +1080,7 @@ namespace Jellyfin.Networking.Manager { result = string.Empty; // Get the first WAN interface address that isn't a loopback. - var extResult = _interfaces.Where(p => !IsInLocalNetwork(p.Address)); + var extResult = _interfaces.Where(p => !IsInLocalNetwork(p.Address)).OrderBy(x => x.Index); IPAddress? hasResult = null; // Does the request originate in one of the interface subnets? diff --git a/Jellyfin.Server/Migrations/PreStartupRoutines/CreateNetworkConfiguration.cs b/Jellyfin.Server/Migrations/PreStartupRoutines/CreateNetworkConfiguration.cs index ceeaa26e62..b670172814 100644 --- a/Jellyfin.Server/Migrations/PreStartupRoutines/CreateNetworkConfiguration.cs +++ b/Jellyfin.Server/Migrations/PreStartupRoutines/CreateNetworkConfiguration.cs @@ -114,7 +114,7 @@ public class CreateNetworkConfiguration : IMigrationRoutine public bool IgnoreVirtualInterfaces { get; set; } = true; - public string VirtualInterfaceNames { get; set; } = "veth*"; + public string VirtualInterfaceNames { get; set; } = "veth"; public string[] PublishedServerUriBySubnet { get; set; } = Array.Empty(); diff --git a/MediaBrowser.Common/Net/INetworkManager.cs b/MediaBrowser.Common/Net/INetworkManager.cs index 9f48535576..bb0e2dcb36 100644 --- a/MediaBrowser.Common/Net/INetworkManager.cs +++ b/MediaBrowser.Common/Net/INetworkManager.cs @@ -1,6 +1,5 @@ using System; using System.Collections.Generic; -using System.Collections.ObjectModel; using System.Net; using System.Net.NetworkInformation; using Microsoft.AspNetCore.Http; @@ -34,13 +33,13 @@ namespace MediaBrowser.Common.Net /// If all the interfaces are specified, and none are excluded, it returns zero items /// to represent any address. /// When false, return or for all interfaces. - List GetAllBindInterfaces(bool individualInterfaces = false); + IReadOnlyList GetAllBindInterfaces(bool individualInterfaces = false); /// - /// Returns a collection containing the loopback interfaces. + /// Returns a list containing the loopback interfaces. /// /// List{IPData}. - List GetLoopbacks(); + IReadOnlyList GetLoopbacks(); /// /// Retrieves the bind address to use in system url's. (Server Discovery, PlayTo, LiveTV, SystemInfo) @@ -97,7 +96,7 @@ namespace MediaBrowser.Common.Net /// Get a list of all the MAC addresses associated with active interfaces. /// /// List of MAC addresses. - IReadOnlyCollection GetMacAddresses(); + IReadOnlyList GetMacAddresses(); /// /// Returns true if the address is part of the user defined LAN. @@ -122,13 +121,13 @@ namespace MediaBrowser.Common.Net /// Interface name. /// Resultant object's ip addresses, if successful. /// Success of the operation. - bool TryParseInterface(string intf, out Collection? result); + bool TryParseInterface(string intf, out List? result); /// /// Returns all the internal Bind interface addresses. /// /// An internal list of interfaces addresses. - List GetInternalBindAddresses(); + IReadOnlyList GetInternalBindAddresses(); /// /// Checks to see if has access. diff --git a/MediaBrowser.Common/Net/NetworkExtensions.cs b/MediaBrowser.Common/Net/NetworkExtensions.cs index ae1e47ccc2..316c2ebcd1 100644 --- a/MediaBrowser.Common/Net/NetworkExtensions.cs +++ b/MediaBrowser.Common/Net/NetworkExtensions.cs @@ -1,6 +1,6 @@ using Microsoft.AspNetCore.HttpOverrides; using System; -using System.Collections.ObjectModel; +using System.Collections.Generic; using System.Diagnostics.CodeAnalysis; using System.Net; using System.Net.Sockets; @@ -148,9 +148,9 @@ 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 TryParseSubnets(string[] values, out Collection result, bool negated = false) + public static bool TryParseSubnets(string[] values, out List result, bool negated = false) { - result = new Collection(); + result = new List(); if (values == null || values.Length == 0) { diff --git a/tests/Jellyfin.Networking.Tests/NetworkParseTests.cs b/tests/Jellyfin.Networking.Tests/NetworkParseTests.cs index 5b9739dd3d..c45a9c9f54 100644 --- a/tests/Jellyfin.Networking.Tests/NetworkParseTests.cs +++ b/tests/Jellyfin.Networking.Tests/NetworkParseTests.cs @@ -1,5 +1,5 @@ using System; -using System.Collections.ObjectModel; +using System.Collections.Generic; using System.Globalization; using System.Linq; using System.Net; @@ -49,8 +49,6 @@ namespace Jellyfin.Networking.Tests { EnableIPV6 = true, EnableIPV4 = true, - IgnoreVirtualInterfaces = true, - VirtualInterfaceNames = "veth", LocalNetworkSubnets = lan?.Split(';') ?? throw new ArgumentNullException(nameof(lan)) }; @@ -208,7 +206,7 @@ namespace Jellyfin.Networking.Tests using var nm = new NetworkManager(GetMockConfig(conf), new NullLogger()); NetworkManager.MockNetworkSettings = string.Empty; - _ = nm.TryParseInterface(result, out Collection? resultObj); + _ = nm.TryParseInterface(result, out List? resultObj); // Check to see if dns resolution is working. If not, skip test. _ = NetworkExtensions.TryParseHost(source, out var host); @@ -277,7 +275,7 @@ namespace Jellyfin.Networking.Tests using var nm = new NetworkManager(GetMockConfig(conf), new NullLogger()); NetworkManager.MockNetworkSettings = string.Empty; - if (nm.TryParseInterface(result, out Collection? resultObj) && resultObj != null) + if (nm.TryParseInterface(result, out List? resultObj) && resultObj != null) { // Parse out IPAddresses so we can do a string comparison. (Ignore subnet masks). result = resultObj.First().Address.ToString();