Update to 3.5.2 and .net core 2.1

1 files changed, 62 insertions, 80 deletions

diff --git a/Emby.Server.Implementations/EntryPoints/ExternalPortForwarding.cs b/Emby.Server.Implementations/EntryPoints/ExternalPortForwarding.cs
index 903bb0ff4..6801b2823 100644
--- a/Emby.Server.Implementations/EntryPoints/ExternalPortForwarding.cs
+++ b/Emby.Server.Implementations/EntryPoints/ExternalPortForwarding.cs
@@ -26,9 +26,10 @@ namespace Emby.Server.Implementations.EntryPoints
         private readonly IDeviceDiscovery _deviceDiscovery;
 
         private ITimer _timer;
-        private bool _isStarted;
         private readonly ITimerFactory _timerFactory;
 
+        private NatManager _natManager;
+
         public ExternalPortForwarding(ILogManager logmanager, IServerApplicationHost appHost, IServerConfigurationManager config, IDeviceDiscovery deviceDiscovery, IHttpClient httpClient, ITimerFactory timerFactory)
         {
             _logger = logmanager.GetLogger("PortMapper");
@@ -37,6 +38,12 @@ namespace Emby.Server.Implementations.EntryPoints
             _deviceDiscovery = deviceDiscovery;
             _httpClient = httpClient;
             _timerFactory = timerFactory;
+            _config.ConfigurationUpdated += _config_ConfigurationUpdated1;
+        }
+
+        private void _config_ConfigurationUpdated1(object sender, EventArgs e)
+        {
+            _config_ConfigurationUpdated(sender, e);
         }
 
         private string _lastConfigIdentifier;
@@ -49,8 +56,8 @@ namespace Emby.Server.Implementations.EntryPoints
             values.Add(config.PublicPort.ToString(CultureInfo.InvariantCulture));
             values.Add(_appHost.HttpPort.ToString(CultureInfo.InvariantCulture));
             values.Add(_appHost.HttpsPort.ToString(CultureInfo.InvariantCulture));
-            values.Add((config.EnableHttps || config.RequireHttps).ToString());
             values.Add(_appHost.EnableHttps.ToString());
+            values.Add((config.EnableRemoteAccess).ToString());
 
             return string.Join("|", values.ToArray(values.Count));
         }
@@ -59,10 +66,7 @@ namespace Emby.Server.Implementations.EntryPoints
         {
             if (!string.Equals(_lastConfigIdentifier, GetConfigIdentifier(), StringComparison.OrdinalIgnoreCase))
             {
-                if (_isStarted)
-                {
-                    DisposeNat();
-                }
+                DisposeNat();
 
                 Run();
             }
@@ -70,10 +74,7 @@ namespace Emby.Server.Implementations.EntryPoints
 
         public void Run()
         {
-            NatUtility.Logger = _logger;
-            NatUtility.HttpClient = _httpClient;
-
-            if (_config.Configuration.EnableUPnP)
+            if (_config.Configuration.EnableUPnP && _config.Configuration.EnableRemoteAccess)
             {
                 Start();
             }
@@ -85,26 +86,18 @@ namespace Emby.Server.Implementations.EntryPoints
         private void Start()
         {
             _logger.Debug("Starting NAT discovery");
-            NatUtility.EnabledProtocols = new List<NatProtocol>
+            if (_natManager == null)
             {
-                NatProtocol.Pmp
-            };
-            NatUtility.DeviceFound += NatUtility_DeviceFound;
-
-            // Mono.Nat does never rise this event. The event is there however it is useless. 
-            // You could remove it with no risk. 
-            NatUtility.DeviceLost += NatUtility_DeviceLost;
-
-
-            NatUtility.StartDiscovery();
+                _natManager = new NatManager(_logger, _httpClient);
+                _natManager.DeviceFound += NatUtility_DeviceFound;
+                _natManager.StartDiscovery();
+            }
 
             _timer = _timerFactory.Create(ClearCreatedRules, null, TimeSpan.FromMinutes(10), TimeSpan.FromMinutes(10));
 
             _deviceDiscovery.DeviceDiscovered += _deviceDiscovery_DeviceDiscovered;
 
             _lastConfigIdentifier = GetConfigIdentifier();
-
-            _isStarted = true;
         }
 
         private async void _deviceDiscovery_DeviceDiscovered(object sender, GenericEventArgs<UpnpDeviceInfo> e)
@@ -182,8 +175,17 @@ namespace Emby.Server.Implementations.EntryPoints
                     return;
                 }
 
-                _logger.Debug("Calling Nat.Handle on " + identifier);
-                NatUtility.Handle(localAddress, info, endpoint, NatProtocol.Upnp);
+                // This should never happen, but the Handle method will throw ArgumentNullException if it does
+                if (localAddress == null)
+                {
+                    return;
+                }
+
+                var natManager = _natManager;
+                if (natManager != null)
+                {
+                    await natManager.Handle(localAddress, info, endpoint, NatProtocol.Upnp).ConfigureAwait(false);
+                }
             }
         }
 
@@ -209,19 +211,11 @@ namespace Emby.Server.Implementations.EntryPoints
             try
             {
                 var device = e.Device;
-                _logger.Debug("NAT device found: {0}", device.LocalAddress.ToString());
 
                 CreateRules(device);
             }
             catch
             {
-                // I think it could be a good idea to log the exception because 
-                //   you are using permanent portmapping here (never expire) and that means that next time
-                //   CreatePortMap is invoked it can fails with a 718-ConflictInMappingEntry or not. That depends
-                //   on the router's upnp implementation (specs says it should fail however some routers don't do it)
-                //   It also can fail with others like 727-ExternalPortOnlySupportsWildcard, 728-NoPortMapsAvailable
-                // and those errors (upnp errors) could be useful for diagnosting.  
-
                 // Commenting out because users are reporting problems out of our control
                 //_logger.ErrorException("Error creating port forwarding rules", ex);
             }
@@ -238,14 +232,15 @@ namespace Emby.Server.Implementations.EntryPoints
 
             // On some systems the device discovered event seems to fire repeatedly
             // This check will help ensure we're not trying to port map the same device over and over
+            var address = device.LocalAddress;
 
-            var address = device.LocalAddress.ToString();
+            var addressString = address.ToString();
 
             lock (_createdRules)
             {
-                if (!_createdRules.Contains(address))
+                if (!_createdRules.Contains(addressString))
                 {
-                    _createdRules.Add(address);
+                    _createdRules.Add(addressString);
                 }
                 else
                 {
@@ -253,41 +248,32 @@ namespace Emby.Server.Implementations.EntryPoints
                 }
             }
 
-            var success = await CreatePortMap(device, _appHost.HttpPort, _config.Configuration.PublicPort).ConfigureAwait(false);
-
-            if (success)
+            try
             {
-                await CreatePortMap(device, _appHost.HttpsPort, _config.Configuration.PublicHttpsPort).ConfigureAwait(false);
+                await CreatePortMap(device, _appHost.HttpPort, _config.Configuration.PublicPort).ConfigureAwait(false);
+            }
+            catch (Exception ex)
+            {
+                return;
             }
-        }
-
-        private async Task<bool> CreatePortMap(INatDevice device, int privatePort, int publicPort)
-        {
-            _logger.Debug("Creating port map on port {0}", privatePort);
 
             try
             {
-                await device.CreatePortMap(new Mapping(Protocol.Tcp, privatePort, publicPort)
-                {
-                    Description = _appHost.Name
-
-                }).ConfigureAwait(false);
-
-                return true;
+                await CreatePortMap(device, _appHost.HttpsPort, _config.Configuration.PublicHttpsPort).ConfigureAwait(false);
             }
             catch (Exception ex)
             {
-                _logger.Error("Error creating port map: " + ex.Message);
-
-                return false;
             }
         }
 
-        // As I said before, this method will be never invoked. You can remove it.
-        void NatUtility_DeviceLost(object sender, DeviceEventArgs e)
+        private Task CreatePortMap(INatDevice device, int privatePort, int publicPort)
         {
-            var device = e.Device;
-            _logger.Debug("NAT device lost: {0}", device.LocalAddress.ToString());
+            _logger.Debug("Creating port map on local port {0} to public port {1} with device {2}", privatePort, publicPort, device.LocalAddress.ToString());
+
+            return device.CreatePortMap(new Mapping(Protocol.Tcp, privatePort, publicPort)
+            {
+                Description = _appHost.Name
+            });
         }
 
         private bool _disposed = false;
@@ -295,7 +281,6 @@ namespace Emby.Server.Implementations.EntryPoints
         {
             _disposed = true;
             DisposeNat();
-            GC.SuppressFinalize(this);
         }
 
         private void DisposeNat()
@@ -310,27 +295,24 @@ namespace Emby.Server.Implementations.EntryPoints
 
             _deviceDiscovery.DeviceDiscovered -= _deviceDiscovery_DeviceDiscovered;
 
-            try
-            {
-                // This is not a significant improvement
-                NatUtility.StopDiscovery();
-                NatUtility.DeviceFound -= NatUtility_DeviceFound;
-                NatUtility.DeviceLost -= NatUtility_DeviceLost;
-            }
-            // Statements in try-block will no fail because StopDiscovery is a one-line 
-            // method that was no chances to fail.
-            //		public static void StopDiscovery ()
-            //      {
-            //          searching.Reset();
-            //      }
-            // IMO you could remove the catch-block
-            catch (Exception ex)
-            {
-                _logger.ErrorException("Error stopping NAT Discovery", ex);
-            }
-            finally
+            var natManager = _natManager;
+
+            if (natManager != null)
             {
-                _isStarted = false;
+                _natManager = null;
+
+                using (natManager)
+                {
+                    try
+                    {
+                        natManager.StopDiscovery();
+                        natManager.DeviceFound -= NatUtility_DeviceFound;
+                    }
+                    catch (Exception ex)
+                    {
+                        _logger.ErrorException("Error stopping NAT Discovery", ex);
+                    }
+                }
             }
         }
     }