Honor PreferHostingUrls #1575

src/Microsoft.AspNetCore.Server.Kestrel.Core/KestrelServer.cs

@@ -119,14 +119,36 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core
                 var hasListenOptions = listenOptions.Any();
                 var hasServerAddresses = _serverAddresses.Addresses.Any();
 
-                if (hasListenOptions && hasServerAddresses)
+                if (_serverAddresses.PreferHostingUrls && hasServerAddresses)
                 {
-                    var joined = string.Join(", ", _serverAddresses.Addresses);
+                    if (hasListenOptions)
-                    _logger.LogWarning($"Overriding address(es) '{joined}'. Binding to endpoints defined in UseKestrel() instead.");
+                    {
+                        var joined = string.Join(", ", _serverAddresses.Addresses);
+                        _logger.LogInformation($"Overriding endpoints defined in UseKestrel() since {nameof(IServerAddressesFeature.PreferHostingUrls)} is set to true. Binding to address(es) '{joined}' instead.");
 
-                    _serverAddresses.Addresses.Clear();
+                        listenOptions.Clear();
+                    }
+
+                    await BindToServerAddresses(listenOptions, serviceContext, application, cancellationToken).ConfigureAwait(false);
                 }
-                else if (!hasListenOptions && !hasServerAddresses)
+                else if (hasListenOptions)
+                {
+                    if (hasServerAddresses)
+                    {
+                        var joined = string.Join(", ", _serverAddresses.Addresses);
+                        _logger.LogWarning($"Overriding address(es) '{joined}'. Binding to endpoints defined in UseKestrel() instead.");
+
+                        _serverAddresses.Addresses.Clear();
+                    }
+
+                    await BindToEndpoints(listenOptions, serviceContext, application).ConfigureAwait(false);
+                }
+                else if (hasServerAddresses)
+                {
+                    // If no endpoints are configured directly using KestrelServerOptions, use those configured via the IServerAddressesFeature.
+                    await BindToServerAddresses(listenOptions, serviceContext, application, cancellationToken).ConfigureAwait(false);
+                }
+                else
                 {
                     _logger.LogDebug($"No listening endpoints were configured. Binding to {Constants.DefaultServerAddress} by default.");
 
@@ -136,78 +158,6 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core
                     // If StartLocalhost doesn't throw, there is at least one listener.
                     // The port cannot change for "localhost".
                     _serverAddresses.Addresses.Add(Constants.DefaultServerAddress);
-
-                    return;
-                }
-                else if (!hasListenOptions)
-                {
-                    // If no endpoints are configured directly using KestrelServerOptions, use those configured via the IServerAddressesFeature.
-                    var copiedAddresses = _serverAddresses.Addresses.ToArray();
-                    _serverAddresses.Addresses.Clear();
-
-                    foreach (var address in copiedAddresses)
-                    {
-                        var parsedAddress = ServerAddress.FromUrl(address);
-
-                        if (parsedAddress.Scheme.Equals("https", StringComparison.OrdinalIgnoreCase))
-                        {
-                            throw new InvalidOperationException($"HTTPS endpoints can only be configured using {nameof(KestrelServerOptions)}.{nameof(KestrelServerOptions.Listen)}().");
-                        }
-                        else if (!parsedAddress.Scheme.Equals("http", StringComparison.OrdinalIgnoreCase))
-                        {
-                            throw new InvalidOperationException($"Unrecognized scheme in server address '{address}'. Only 'http://' is supported.");
-                        }
-
-                        if (!string.IsNullOrEmpty(parsedAddress.PathBase))
-                        {
-                            throw new InvalidOperationException($"A path base can only be configured using {nameof(IApplicationBuilder)}.UsePathBase().");
-                        }
-
-                        if (!string.IsNullOrEmpty(parsedAddress.PathBase))
-                        {
-                            _logger.LogWarning($"Path base in address {address} is not supported and will be ignored. To specify a path base, use {nameof(IApplicationBuilder)}.UsePathBase().");
-                        }
-
-                        if (parsedAddress.IsUnixPipe)
-                        {
-                            listenOptions.Add(new ListenOptions(parsedAddress.UnixPipePath));
-                        }
-                        else
-                        {
-                            if (string.Equals(parsedAddress.Host, "localhost", StringComparison.OrdinalIgnoreCase))
-                            {
-                                // "localhost" for both IPv4 and IPv6 can't be represented as an IPEndPoint.
-                                await StartLocalhostAsync(parsedAddress, serviceContext, application, cancellationToken).ConfigureAwait(false);
-
-                                // If StartLocalhost doesn't throw, there is at least one listener.
-                                // The port cannot change for "localhost".
-                                _serverAddresses.Addresses.Add(parsedAddress.ToString());
-                            }
-                            else
-                            {
-                                // These endPoints will be added later to _serverAddresses.Addresses
-                                listenOptions.Add(new ListenOptions(CreateIPEndPoint(parsedAddress)));
-                            }
-                        }
-                    }
-                }
-
-                foreach (var endPoint in listenOptions)
-                {
-                    var connectionHandler = new ConnectionHandler<TContext>(endPoint, serviceContext, application);
-                    var transport = _transportFactory.Create(endPoint, connectionHandler);
-                    _transports.Add(transport);
-
-                    try
-                    {
-                        await transport.BindAsync().ConfigureAwait(false);
-                    }
-                    catch (AddressInUseException ex)
-                    {
-                        throw new IOException($"Failed to bind to address {endPoint}: address already in use.", ex);
-                    }
-
-                    _serverAddresses.Addresses.Add(endPoint.GetDisplayName());
                 }
             }
             catch (Exception ex)
@@ -218,6 +168,72 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core
             }
         }
 
+        private async Task BindToServerAddresses<TContext>(List<ListenOptions> listenOptions, ServiceContext serviceContext, IHttpApplication<TContext> application, CancellationToken cancellationToken)
+        {
+            var copiedAddresses = _serverAddresses.Addresses.ToArray();
+            _serverAddresses.Addresses.Clear();
+            foreach (var address in copiedAddresses)
+            {
+                var parsedAddress = ServerAddress.FromUrl(address);
+
+                if (parsedAddress.Scheme.Equals("https", StringComparison.OrdinalIgnoreCase))
+                {
+                    throw new InvalidOperationException($"HTTPS endpoints can only be configured using {nameof(KestrelServerOptions)}.{nameof(KestrelServerOptions.Listen)}().");
+                }
+                else if (!parsedAddress.Scheme.Equals("http", StringComparison.OrdinalIgnoreCase))
+                {
+                    throw new InvalidOperationException($"Unrecognized scheme in server address '{address}'. Only 'http://' is supported.");
+                }
+
+                if (!string.IsNullOrEmpty(parsedAddress.PathBase))
+                {
+                    throw new InvalidOperationException($"A path base can only be configured using {nameof(IApplicationBuilder)}.UsePathBase().");
+                }
+
+                if (parsedAddress.IsUnixPipe)
+                {
+                    listenOptions.Add(new ListenOptions(parsedAddress.UnixPipePath));
+                }
+                else if (string.Equals(parsedAddress.Host, "localhost", StringComparison.OrdinalIgnoreCase))
+                {
+                    // "localhost" for both IPv4 and IPv6 can't be represented as an IPEndPoint.
+                    await StartLocalhostAsync(parsedAddress, serviceContext, application, cancellationToken).ConfigureAwait(false);
+                    // If StartLocalhost doesn't throw, there is at least one listener.
+                    // The port cannot change for "localhost".
+                    _serverAddresses.Addresses.Add(parsedAddress.ToString());
+                }
+                else
+                {
+                    // These endPoints will be added later to _serverAddresses.Addresses
+                    listenOptions.Add(new ListenOptions(CreateIPEndPoint(parsedAddress)));
+                }
+            }
+
+            await BindToEndpoints(listenOptions, serviceContext, application).ConfigureAwait(false);
+        }
+
+        private async Task BindToEndpoints<TContext>(List<ListenOptions> listenOptions, ServiceContext serviceContext, IHttpApplication<TContext> application)
+        {
+            foreach (var endPoint in listenOptions)
+            {
+                var connectionHandler = new ConnectionHandler<TContext>(endPoint, serviceContext, application);
+                var transport = _transportFactory.Create(endPoint, connectionHandler);
+                _transports.Add(transport);
+
+                try
+                {
+                    await transport.BindAsync().ConfigureAwait(false);
+                }
+                catch (AddressInUseException ex)
+                {
+                    throw new IOException($"Failed to bind to address {endPoint}: address already in use.", ex);
+                }
+
+                // If requested port was "0", replace with assigned dynamic port.
+                _serverAddresses.Addresses.Add(endPoint.GetDisplayName());
+            }
+        }
+
         // Graceful shutdown if possible
         public async Task StopAsync(CancellationToken cancellationToken)
         {
@@ -340,4 +356,4 @@ namespace Microsoft.AspNetCore.Server.Kestrel.Core
             return new IPEndPoint(ip, address.Port);
         }
     }
-}
+}

test/Microsoft.AspNetCore.Server.Kestrel.FunctionalTests/AddressRegistrationTests.cs

@@ -168,10 +168,7 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
 
             var hostBuilder = new WebHostBuilder()
                .UseKestrel()
-               .ConfigureServices(services =>
+               .UseLoggerFactory(_ => new KestrelTestLoggerFactory(testLogger))
-               {
-                   services.AddSingleton<ILoggerFactory>(new KestrelTestLoggerFactory(testLogger));
-               })
                .Configure(ConfigureEchoAddress);
 
             using (var host = hostBuilder.Build())
@@ -232,6 +229,73 @@ namespace Microsoft.AspNetCore.Server.Kestrel.FunctionalTests
                 }
             }
         }
+        [ConditionalFact]
+        [PortSupportedCondition(5002)]
+        public async Task OverrideDirectConfigurationWithIServerAddressesFeature_Succeeds()
+        {
+            var overrideAddress = "http://localhost:5002";
+            var testLogger = new TestApplicationErrorLogger();
+
+            var hostBuilder = new WebHostBuilder()
+               .UseKestrel(options => options.Listen(IPAddress.Loopback, 5001))
+               .UseUrls(overrideAddress)
+               .PreferHostingUrls(true)
+               .UseLoggerFactory(_ => new KestrelTestLoggerFactory(testLogger))
+               .Configure(ConfigureEchoAddress);
+
+            using (var host = hostBuilder.Build())
+            {
+                host.Start();
+
+                Assert.Equal(5002, host.GetPort());
+                Assert.Single(testLogger.Messages, log => log.LogLevel == LogLevel.Information &&
+                    string.Equals($"Overriding endpoints defined in UseKestrel() since {nameof(IServerAddressesFeature.PreferHostingUrls)} is set to true. Binding to address(es) '{overrideAddress}' instead.",
+                    log.Message, StringComparison.Ordinal));
+
+                Assert.Equal(new Uri(overrideAddress).ToString(), await HttpClientSlim.GetStringAsync(overrideAddress));
+            }
+        }
+
+        [ConditionalFact]
+        [PortSupportedCondition(5001)]
+        public async Task DoesNotOverrideDirectConfigurationWithIServerAddressesFeature_IfPreferHostingUrlsFalse()
+        {
+            var endPointAddress = "http://localhost:5001";
+
+            var hostBuilder = new WebHostBuilder()
+               .UseKestrel(options => options.Listen(IPAddress.Loopback, 5001))
+               .UseUrls("http://localhost:5002")
+               .PreferHostingUrls(false)
+               .Configure(ConfigureEchoAddress);
+
+            using (var host = hostBuilder.Build())
+            {
+                host.Start();
+
+                Assert.Equal(5001, host.GetPort());
+                Assert.Equal(new Uri(endPointAddress).ToString(), await HttpClientSlim.GetStringAsync(endPointAddress));
+            }
+        }
+
+        [ConditionalFact]
+        [PortSupportedCondition(5001)]
+        public async Task DoesNotOverrideDirectConfigurationWithIServerAddressesFeature_IfAddressesEmpty()
+        {
+            var endPointAddress = "http://localhost:5001";
+
+            var hostBuilder = new WebHostBuilder()
+               .UseKestrel(options => options.Listen(IPAddress.Loopback, 5001))
+               .PreferHostingUrls(true)
+               .Configure(ConfigureEchoAddress);
+
+            using (var host = hostBuilder.Build())
+            {
+                host.Start();
+
+                Assert.Equal(5001, host.GetPort());
+                Assert.Equal(new Uri(endPointAddress).ToString(), await HttpClientSlim.GetStringAsync(endPointAddress));
+            }
+        }
 
         [Fact]
         public void ThrowsWhenBindingLocalhostToIPv4AddressInUse()