Merge pull request #2774 from mark-monteiro/simplify-https-config

Simplify HTTPS Properties

Emby.Server.Implementations/ApplicationHost.cs

@@ -94,7 +94,6 @@ using MediaBrowser.Model.Serialization;
 using MediaBrowser.Model.Services;
 using MediaBrowser.Model.System;
 using MediaBrowser.Model.Tasks;
-using MediaBrowser.Model.Updates;
 using MediaBrowser.Providers.Chapters;
 using MediaBrowser.Providers.Manager;
 using MediaBrowser.Providers.Plugins.TheTvdb;
@@ -1143,9 +1142,6 @@ namespace Emby.Server.Implementations
                 ItemsByNamePath = ApplicationPaths.InternalMetadataPath,
                 InternalMetadataPath = ApplicationPaths.InternalMetadataPath,
                 CachePath = ApplicationPaths.CachePath,
-                HttpServerPortNumber = HttpPort,
-                SupportsHttps = SupportsHttps,
-                HttpsPortNumber = HttpsPort,
                 OperatingSystem = OperatingSystem.Id.ToString(),
                 OperatingSystemDisplayName = OperatingSystem.Name,
                 CanSelfRestart = CanSelfRestart,
@@ -1181,23 +1177,22 @@ namespace Emby.Server.Implementations
             };
         }
 
-        public bool EnableHttps => SupportsHttps && ServerConfigurationManager.Configuration.EnableHttps;
-
-        public bool SupportsHttps => Certificate != null || ServerConfigurationManager.Configuration.IsBehindProxy;
+        /// <inheritdoc/>
+        public bool ListenWithHttps => Certificate != null && ServerConfigurationManager.Configuration.EnableHttps;
 
-        public async Task<string> GetLocalApiUrl(CancellationToken cancellationToken, bool forceHttp = false)
+        /// <inheritdoc/>
+        public async Task<string> GetLocalApiUrl(CancellationToken cancellationToken)
         {
             try
             {
                 // Return the first matched address, if found, or the first known local address
                 var addresses = await GetLocalIpAddressesInternal(false, 1, cancellationToken).ConfigureAwait(false);
-
-                foreach (var address in addresses)
+                if (addresses.Count == 0)
                 {
-                    return GetLocalApiUrl(address, forceHttp);
+                    return null;
                 }
 
-                return null;
+                return GetLocalApiUrl(addresses.First());
             }
             catch (Exception ex)
             {
@@ -1224,7 +1219,7 @@ namespace Emby.Server.Implementations
         }
 
         /// <inheritdoc />
-        public string GetLocalApiUrl(IPAddress ipAddress, bool forceHttp = false)
+        public string GetLocalApiUrl(IPAddress ipAddress)
         {
             if (ipAddress.AddressFamily == AddressFamily.InterNetworkV6)
             {
@@ -1234,29 +1229,30 @@ namespace Emby.Server.Implementations
                 str.CopyTo(span.Slice(1));
                 span[^1] = ']';
 
-                return GetLocalApiUrl(span, forceHttp);
+                return GetLocalApiUrl(span);
             }
 
-            return GetLocalApiUrl(ipAddress.ToString(), forceHttp);
+            return GetLocalApiUrl(ipAddress.ToString());
         }
 
-        /// <inheritdoc />
-        public string GetLocalApiUrl(ReadOnlySpan<char> host, bool forceHttp = false)
+        /// <inheritdoc/>
+        public string GetLoopbackHttpApiUrl()
         {
-            var url = new StringBuilder(64);
-            bool useHttps = EnableHttps && !forceHttp;
-            url.Append(useHttps ? "https://" : "http://")
-                .Append(host)
-                .Append(':')
-                .Append(useHttps ? HttpsPort : HttpPort);
-
-            string baseUrl = ServerConfigurationManager.Configuration.BaseUrl;
-            if (baseUrl.Length != 0)
-            {
-                url.Append(baseUrl);
-            }
+            return GetLocalApiUrl("127.0.0.1", Uri.UriSchemeHttp, HttpPort);
+        }
 
-            return url.ToString();
+        /// <inheritdoc/>
+        public string GetLocalApiUrl(ReadOnlySpan<char> host, string scheme = null, int? port = null)
+        {
+            // NOTE: If no BaseUrl is set then UriBuilder appends a trailing slash, but if there is no BaseUrl it does
+            // not. For consistency, always trim the trailing slash.
+            return new UriBuilder
+            {
+                Scheme = scheme ?? (ListenWithHttps ? Uri.UriSchemeHttps : Uri.UriSchemeHttp),
+                Host = host.ToString(),
+                Port = port ?? (ListenWithHttps ? HttpsPort : HttpPort),
+                Path = ServerConfigurationManager.Configuration.BaseUrl
+            }.ToString().TrimEnd('/');
         }
 
         public Task<List<IPAddress>> GetLocalIpAddresses(CancellationToken cancellationToken)
@@ -1290,7 +1286,7 @@ namespace Emby.Server.Implementations
                     }
                 }
 
-                var valid = await IsIpAddressValidAsync(address, cancellationToken).ConfigureAwait(false);
+                var valid = await IsLocalIpAddressValidAsync(address, cancellationToken).ConfigureAwait(false);
                 if (valid)
                 {
                     resultList.Add(address);
@@ -1324,7 +1320,7 @@ namespace Emby.Server.Implementations
 
         private readonly ConcurrentDictionary<string, bool> _validAddressResults = new ConcurrentDictionary<string, bool>(StringComparer.OrdinalIgnoreCase);
 
-        private async Task<bool> IsIpAddressValidAsync(IPAddress address, CancellationToken cancellationToken)
+        private async Task<bool> IsLocalIpAddressValidAsync(IPAddress address, CancellationToken cancellationToken)
         {
             if (address.Equals(IPAddress.Loopback)
                 || address.Equals(IPAddress.IPv6Loopback))
@@ -1332,8 +1328,7 @@ namespace Emby.Server.Implementations
                 return true;
             }
 
-            var apiUrl = GetLocalApiUrl(address);
-            apiUrl += "/system/ping";
+            var apiUrl = GetLocalApiUrl(address) + "/system/ping";
 
             if (_validAddressResults.TryGetValue(apiUrl, out var cachedResult))
             {

Emby.Server.Implementations/Browser/BrowserLauncher.cs

@@ -31,18 +31,18 @@ namespace Emby.Server.Implementations.Browser
         /// Opens the specified URL in an external browser window. Any exceptions will be logged, but ignored.
         /// </summary>
         /// <param name="appHost">The application host.</param>
-        /// <param name="url">The URL.</param>
-        private static void TryOpenUrl(IServerApplicationHost appHost, string url)
+        /// <param name="relativeUrl">The URL to open, relative to the server base URL.</param>
+        private static void TryOpenUrl(IServerApplicationHost appHost, string relativeUrl)
         {
             try
             {
                 string baseUrl = appHost.GetLocalApiUrl("localhost");
-                appHost.LaunchUrl(baseUrl + url);
+                appHost.LaunchUrl(baseUrl + relativeUrl);
             }
             catch (Exception ex)
             {
                 var logger = appHost.Resolve<ILogger>();
-                logger?.LogError(ex, "Failed to open browser window with URL {URL}", url);
+                logger?.LogError(ex, "Failed to open browser window with URL {URL}", relativeUrl);
             }
         }
     }

Emby.Server.Implementations/EntryPoints/ExternalPortForwarding.cs

@@ -64,7 +64,7 @@ namespace Emby.Server.Implementations.EntryPoints
                 .Append(config.PublicHttpsPort).Append(Separator)
                 .Append(_appHost.HttpPort).Append(Separator)
                 .Append(_appHost.HttpsPort).Append(Separator)
-                .Append(_appHost.EnableHttps).Append(Separator)
+                .Append(_appHost.ListenWithHttps).Append(Separator)
                 .Append(config.EnableRemoteAccess).Append(Separator)
                 .ToString();
         }
@@ -158,7 +158,7 @@ namespace Emby.Server.Implementations.EntryPoints
         {
             yield return CreatePortMap(device, _appHost.HttpPort, _config.Configuration.PublicPort);
 
-            if (_appHost.EnableHttps)
+            if (_appHost.ListenWithHttps)
             {
                 yield return CreatePortMap(device, _appHost.HttpsPort, _config.Configuration.PublicHttpsPort);
             }

Emby.Server.Implementations/HttpServer/HttpListenerHost.cs

@@ -424,9 +424,13 @@ namespace Emby.Server.Implementations.HttpServer
             return true;
         }
 
+        /// <summary>
+        /// Validate a connection from a remote IP address to a URL to see if a redirection to HTTPS is required.
+        /// </summary>
+        /// <returns>True if the request is valid, or false if the request is not valid and an HTTPS redirect is required.</returns>
         private bool ValidateSsl(string remoteIp, string urlString)
         {
-            if (_config.Configuration.RequireHttps && _appHost.EnableHttps && !_config.Configuration.IsBehindProxy)
+            if (_config.Configuration.RequireHttps && _appHost.ListenWithHttps)
             {
                 if (urlString.IndexOf("https://", StringComparison.OrdinalIgnoreCase) == -1)
                 {

Emby.Server.Implementations/LiveTv/EmbyTV/EmbyTV.cs

@@ -1059,7 +1059,7 @@ namespace Emby.Server.Implementations.LiveTv.EmbyTV
         {
             var stream = new MediaSourceInfo
             {
-                EncoderPath = _appHost.GetLocalApiUrl("127.0.0.1", true) + "/LiveTv/LiveRecordings/" + info.Id + "/stream",
+                EncoderPath = _appHost.GetLoopbackHttpApiUrl() + "/LiveTv/LiveRecordings/" + info.Id + "/stream",
                 EncoderProtocol = MediaProtocol.Http,
                 Path = info.Path,
                 Protocol = MediaProtocol.File,

Emby.Server.Implementations/LiveTv/TunerHosts/HdHomerun/HdHomerunUdpStream.cs

@@ -121,7 +121,7 @@ namespace Emby.Server.Implementations.LiveTv.TunerHosts.HdHomerun
             //OpenedMediaSource.Path = tempFile;
             //OpenedMediaSource.ReadAtNativeFramerate = true;
 
-            MediaSource.Path = _appHost.GetLocalApiUrl("127.0.0.1", true) + "/LiveTv/LiveStreamFiles/" + UniqueId + "/stream.ts";
+            MediaSource.Path = _appHost.GetLoopbackHttpApiUrl() + "/LiveTv/LiveStreamFiles/" + UniqueId + "/stream.ts";
             MediaSource.Protocol = MediaProtocol.Http;
             //OpenedMediaSource.SupportsDirectPlay = false;
             //OpenedMediaSource.SupportsDirectStream = true;

Emby.Server.Implementations/LiveTv/TunerHosts/SharedHttpStream.cs

@@ -106,7 +106,7 @@ namespace Emby.Server.Implementations.LiveTv.TunerHosts
             //OpenedMediaSource.Path = tempFile;
             //OpenedMediaSource.ReadAtNativeFramerate = true;
 
-            MediaSource.Path = _appHost.GetLocalApiUrl("127.0.0.1", true) + "/LiveTv/LiveStreamFiles/" + UniqueId + "/stream.ts";
+            MediaSource.Path = _appHost.GetLoopbackHttpApiUrl() + "/LiveTv/LiveStreamFiles/" + UniqueId + "/stream.ts";
             MediaSource.Protocol = MediaProtocol.Http;
 
             //OpenedMediaSource.Path = TempFilePath;

Jellyfin.Server/Program.cs

@@ -10,14 +10,11 @@ using System.Text.RegularExpressions;
 using System.Threading;
 using System.Threading.Tasks;
 using CommandLine;
-using Emby.Drawing;
 using Emby.Server.Implementations;
 using Emby.Server.Implementations.HttpServer;
 using Emby.Server.Implementations.IO;
 using Emby.Server.Implementations.Networking;
-using Jellyfin.Drawing.Skia;
 using MediaBrowser.Common.Configuration;
-using MediaBrowser.Controller.Drawing;
 using MediaBrowser.Controller.Extensions;
 using MediaBrowser.WebDashboard.Api;
 using Microsoft.AspNetCore.Hosting;
@@ -297,7 +294,7 @@ namespace Jellyfin.Server
                         {
                             _logger.LogInformation("Kestrel listening on {IpAddress}", address);
                             options.Listen(address, appHost.HttpPort);
-                            if (appHost.EnableHttps && appHost.Certificate != null)
+                            if (appHost.ListenWithHttps)
                             {
                                 options.Listen(address, appHost.HttpsPort, listenOptions =>
                                 {
@@ -327,7 +324,7 @@ namespace Jellyfin.Server
                         _logger.LogInformation("Kestrel listening on all interfaces");
                         options.ListenAnyIP(appHost.HttpPort);
 
-                        if (appHost.EnableHttps && appHost.Certificate != null)
+                        if (appHost.ListenWithHttps)
                         {
                             options.ListenAnyIP(appHost.HttpsPort, listenOptions =>
                             {

MediaBrowser.Controller/IServerApplicationHost.cs

@@ -39,10 +39,9 @@ namespace MediaBrowser.Controller
         int HttpsPort { get; }
 
         /// <summary>
-        /// Gets a value indicating whether [supports HTTPS].
+        /// Gets a value indicating whether the server should listen on an HTTPS port.
         /// </summary>
-        /// <value><c>true</c> if [supports HTTPS]; otherwise, <c>false</c>.</value>
-        bool EnableHttps { get; }
+        bool ListenWithHttps { get; }
 
         /// <summary>
         /// Gets a value indicating whether this instance has update available.
@@ -57,34 +56,50 @@ namespace MediaBrowser.Controller
         string FriendlyName { get; }
 
         /// <summary>
-        /// Gets the local ip address.
+        /// Gets all the local IP addresses of this API instance. Each address is validated by sending a 'ping' request
+        /// to the API that should exist at the address.
         /// </summary>
-        /// <value>The local ip address.</value>
+        /// <param name="cancellationToken">A cancellation token that can be used to cancel the task.</param>
+        /// <returns>A list containing all the local IP addresses of the server.</returns>
         Task<List<IPAddress>> GetLocalIpAddresses(CancellationToken cancellationToken);
 
         /// <summary>
-        /// Gets the local API URL.
+        /// Gets a local (LAN) URL that can be used to access the API. The hostname used is the first valid configured
+        /// IP address that can be found via <see cref="GetLocalIpAddresses"/>. HTTPS will be preferred when available.
         /// </summary>
-        /// <param name="cancellationToken">Token to cancel the request if needed.</param>
-        /// <param name="forceHttp">Whether to force usage of plain HTTP protocol.</param>
-        /// <value>The local API URL.</value>
-        Task<string> GetLocalApiUrl(CancellationToken cancellationToken, bool forceHttp = false);
+        /// <param name="cancellationToken">A cancellation token that can be used to cancel the task.</param>
+        /// <returns>The server URL.</returns>
+        Task<string> GetLocalApiUrl(CancellationToken cancellationToken);
 
         /// <summary>
-        /// Gets the local API URL.
+        /// Gets a localhost URL that can be used to access the API using the loop-back IP address (127.0.0.1)
+        /// over HTTP (not HTTPS).
         /// </summary>
-        /// <param name="hostname">The hostname.</param>
-        /// <param name="forceHttp">Whether to force usage of plain HTTP protocol.</param>
-        /// <returns>The local API URL.</returns>
-        string GetLocalApiUrl(ReadOnlySpan<char> hostname, bool forceHttp = false);
+        /// <returns>The API URL.</returns>
+        string GetLoopbackHttpApiUrl();
 
         /// <summary>
-        /// Gets the local API URL.
+        /// Gets a local (LAN) URL that can be used to access the API. HTTPS will be preferred when available.
         /// </summary>
-        /// <param name="address">The IP address.</param>
-        /// <param name="forceHttp">Whether to force usage of plain HTTP protocol.</param>
-        /// <returns>The local API URL.</returns>
-        string GetLocalApiUrl(IPAddress address, bool forceHttp = false);
+        /// <param name="address">The IP address to use as the hostname in the URL.</param>
+        /// <returns>The API URL.</returns>
+        string GetLocalApiUrl(IPAddress address);
+
+        /// <summary>
+        /// Gets a local (LAN) URL that can be used to access the API.
+        /// Note: if passing non-null scheme or port it is up to the caller to ensure they form the correct pair.
+        /// </summary>
+        /// <param name="hostname">The hostname to use in the URL.</param>
+        /// <param name="scheme">
+        /// The scheme to use for the URL. If null, the scheme will be selected automatically,
+        /// preferring HTTPS, if available.
+        /// </param>
+        /// <param name="port">
+        /// The port to use for the URL. If null, the port will be selected automatically,
+        /// preferring the HTTPS port, if available.
+        /// </param>
+        /// <returns>The API URL.</returns>
+        string GetLocalApiUrl(ReadOnlySpan<char> hostname, string scheme = null, int? port = null);
 
         /// <summary>
         /// Open a URL in an external browser window.

MediaBrowser.Model/Configuration/ServerConfiguration.cs

@@ -49,17 +49,24 @@ namespace MediaBrowser.Model.Configuration
         public int HttpsPortNumber { get; set; }
 
         /// <summary>
-        /// Gets or sets a value indicating whether [use HTTPS].
+        /// Gets or sets a value indicating whether to use HTTPS.
         /// </summary>
-        /// <value><c>true</c> if [use HTTPS]; otherwise, <c>false</c>.</value>
+        /// <remarks>
+        /// In order for HTTPS to be used, in addition to setting this to true, valid values must also be
+        /// provided for <see cref="CertificatePath"/> and <see cref="CertificatePassword"/>.
+        /// </remarks>
         public bool EnableHttps { get; set; }
+
         public bool EnableNormalizedItemByNameIds { get; set; }
 
         /// <summary>
-        /// Gets or sets the value pointing to the file system where the ssl certificate is located..
+        /// Gets or sets the filesystem path of an X.509 certificate to use for SSL.
         /// </summary>
-        /// <value>The value pointing to the file system where the ssl certificate is located..</value>
         public string CertificatePath { get; set; }
+
+        /// <summary>
+        /// Gets or sets the password required to access the X.509 certificate data in the file specified by <see cref="CertificatePath"/>.
+        /// </summary>
         public string CertificatePassword { get; set; }
 
         /// <summary>
@@ -69,8 +76,11 @@ namespace MediaBrowser.Model.Configuration
         public bool IsPortAuthorized { get; set; }
 
         public bool AutoRunWebApp { get; set; }
+
         public bool EnableRemoteAccess { get; set; }
+
         public bool CameraUploadUpgraded { get; set; }
+
         public bool CollectionsUpgraded { get; set; }
 
         /// <summary>
@@ -86,6 +96,7 @@ namespace MediaBrowser.Model.Configuration
         /// </summary>
         /// <value>The metadata path.</value>
         public string MetadataPath { get; set; }
+
         public string MetadataNetworkPath { get; set; }
 
         /// <summary>
@@ -208,15 +219,26 @@ namespace MediaBrowser.Model.Configuration
         public int RemoteClientBitrateLimit { get; set; }
 
         public bool EnableFolderView { get; set; }
+
         public bool EnableGroupingIntoCollections { get; set; }
+
         public bool DisplaySpecialsWithinSeasons { get; set; }
+
         public string[] LocalNetworkSubnets { get; set; }
+
         public string[] LocalNetworkAddresses { get; set; }
+
         public string[] CodecsUsed { get; set; }
+
         public bool IgnoreVirtualInterfaces { get; set; }
+
         public bool EnableExternalContentInSuggestions { get; set; }
+
+        /// <summary>
+        /// Gets or sets a value indicating whether the server should force connections over HTTPS.
+        /// </summary>
         public bool RequireHttps { get; set; }
-        public bool IsBehindProxy { get; set; }
+
         public bool EnableNewOmdbSupport { get; set; }
 
         public string[] RemoteIPFilter { get; set; }

MediaBrowser.Model/System/SystemInfo.cs

@@ -115,24 +115,6 @@ namespace MediaBrowser.Model.System
         /// <value>The transcode path.</value>
         public string TranscodingTempPath { get; set; }
 
-        /// <summary>
-        /// Gets or sets the HTTP server port number.
-        /// </summary>
-        /// <value>The HTTP server port number.</value>
-        public int HttpServerPortNumber { get; set; }
-
-        /// <summary>
-        /// Gets or sets a value indicating whether [enable HTTPS].
-        /// </summary>
-        /// <value><c>true</c> if [enable HTTPS]; otherwise, <c>false</c>.</value>
-        public bool SupportsHttps { get; set; }
-
-        /// <summary>
-        /// Gets or sets the HTTPS server port number.
-        /// </summary>
-        /// <value>The HTTPS server port number.</value>
-        public int HttpsPortNumber { get; set; }
-
         /// <summary>
         /// Gets or sets a value indicating whether this instance has update available.
         /// </summary>