Keep a configured Kestrel endpoint unless --port is given [patch]

GitLfsCache.Tests/Tool/PortFlagTests.cs

@@ -0,0 +1,56 @@
+// Copyright (c) 2023-2026 ktsu-dev contributors
+
+namespace ktsu.GitLfsCache.Tests.Tool;
+
+using ktsu.GitLfsCache.Tool;
+using Microsoft.Extensions.Configuration;
+
+[TestClass]
+public class PortFlagTests
+{
+	private static string? ResolveEndpoint(int? port, Dictionary<string, string?> configured)
+	{
+		IConfiguration configuration = new ConfigurationBuilder().AddInMemoryCollection(configured).Build();
+		Dictionary<string, string?> overrides = [];
+
+		Program.ApplyListenPort(configuration, overrides, port);
+
+		// The overrides are layered over configuration, so this is the endpoint Kestrel ends up reading.
+		return new ConfigurationBuilder()
+			.AddInMemoryCollection(configured)
+			.AddInMemoryCollection(overrides)
+			.Build()[Program.HttpEndpointUrlKey];
+	}
+
+	[TestMethod]
+	public void NoFlag_ConfiguredEndpoint_IsKept()
+	{
+		// The endpoint used to be overwritten with http://*:8080 whether or not --port was given (issue #140).
+		string? endpoint = ResolveEndpoint(null, new() { [Program.HttpEndpointUrlKey] = "http://127.0.0.1:9123" });
+
+		Assert.AreEqual("http://127.0.0.1:9123", endpoint);
+	}
+
+	[TestMethod]
+	public void NoFlag_ConfiguredUrlsOrPorts_AddsNoEndpoint()
+	{
+		foreach (string key in (string[])["urls", "http_ports", "https_ports", "Kestrel:Endpoints:Https:Url"])
+		{
+			Assert.IsNull(ResolveEndpoint(null, new() { [key] = "http://127.0.0.1:9123" }), key);
+		}
+	}
+
+	[TestMethod]
+	public void Flag_OverridesTheConfiguredEndpoint()
+	{
+		string? endpoint = ResolveEndpoint(9000, new() { [Program.HttpEndpointUrlKey] = "http://127.0.0.1:9123" });
+
+		Assert.AreEqual("http://*:9000", endpoint);
+	}
+
+	[TestMethod]
+	public void NoFlag_NoConfiguredEndpoint_DefaultsTo8080()
+	{
+		Assert.AreEqual("http://*:8080", ResolveEndpoint(null, []));
+	}
+}

GitLfsCache.Tool/Program.cs

@@ -29,11 +29,18 @@
 	/// <summary>The configuration key the <c>--token-key</c> flag overrides.</summary>
 	private const string TokenKeyKey = "GitLfsCache:TokenKeys:0";
 
+	/// <summary>The configuration key the <c>--port</c> flag overrides.</summary>
+	internal const string HttpEndpointUrlKey = "Kestrel:Endpoints:Http:Url";
+
+	/// <summary>The port listened on when neither <c>--port</c> nor configuration names an endpoint.</summary>
+	internal const int DefaultPort = 8080;
+
 	internal static async Task<int> Main(string[] args)
 	{
 		Option<int?> port = new("--port", "-p")
 		{
-			Description = "Port to listen on. Defaults to 8080.",
+			Description =
+				"Port to listen on. Defaults to 8080 unless configuration already names an endpoint, urls or http_ports.",
 		};
 
 		Option<string?> store = new("--store", "-s")
@@ -92,7 +99,7 @@
 		root.SetAction(async (parseResult, cancellationToken) =>
 		{
 			Dictionary<string, string?> overrides = [];
-			int listenPort = parseResult.GetValue(port) ?? 8080;
+			int? listenPort = parseResult.GetValue(port);
 			string? configPath = null;
 
 			if (parseResult.GetValue(configFile) is string requestedConfig)
@@ -157,7 +164,7 @@
 	private static async Task<int> RunAsync(
 		Dictionary<string, string?> overrides,
 		Dictionary<string, List<string>> allowLists,
-		int port,
+		int? port,
 		string? configPath,
 		CancellationToken cancellationToken)
 	{
@@ -178,8 +185,8 @@
 		}
 
 		ApplyDefaults(builder.Configuration, overrides);
+		ApplyListenPort(builder.Configuration, overrides, port);
 		builder.Configuration.AddInMemoryCollection(overrides);
-		builder.Configuration["Kestrel:Endpoints:Http:Url"] = $"http://*:{port}";
 
 		builder.Services.AddGitLfsCache(builder.Configuration);
 		builder.Services.PostConfigure<GitLfsCacheOptions>(options => ReplaceAllowLists(options, allowLists));
@@ -367,6 +374,41 @@
 		}
 	}
 
+	/// <summary>
+	/// Points the HTTP endpoint at <paramref name="port"/> when the flag was given, or at
+	/// <see cref="DefaultPort"/> when configuration names no endpoint of its own.
+	/// </summary>
+	/// <remarks>
+	/// The endpoint used to be overwritten after every other source had been added, so a port or bind address
+	/// set in appsettings, a <c>--config</c> file or <c>Kestrel__Endpoints__Http__Url</c> was ignored and the
+	/// proxy always listened on every interface (issue #140).
+	/// </remarks>
+	/// <param name="configuration">Configuration from every source but the flags.</param>
+	/// <param name="overrides">The flag overrides, layered over <paramref name="configuration"/>.</param>
+	/// <param name="port">The <c>--port</c> flag, or <see langword="null"/> when it was not given.</param>
+	[System.Diagnostics.CodeAnalysis.SuppressMessage(
+		"Security",
+		"S5332:Using clear-text protocols is security-sensitive",
+		Justification = "These are the proxy's own listen addresses, not requests it makes. The default endpoint has always been plain HTTP; an HTTPS endpoint is configured through Kestrel:Endpoints, which this method leaves alone.")]
+	internal static void ApplyListenPort(IConfiguration configuration, Dictionary<string, string?> overrides, int? port)
+	{
+		if (port is int requested)
+		{
+			overrides[HttpEndpointUrlKey] = $"http://*:{requested}";
+			return;
+		}
+
+		bool hasEndpoint = configuration.GetSection("Kestrel:Endpoints").GetChildren().Any()
+			|| !string.IsNullOrWhiteSpace(configuration["urls"])
+			|| !string.IsNullOrWhiteSpace(configuration["http_ports"])
+			|| !string.IsNullOrWhiteSpace(configuration["https_ports"]);
+
+		if (!hasEndpoint)
+		{
+			overrides[HttpEndpointUrlKey] = $"http://*:{DefaultPort}";
+		}
+	}
+
 	private static void ApplyDefaults(ConfigurationManager configuration, Dictionary<string, string?> overrides)
 	{
 		bool hasStore = overrides.ContainsKey("GitLfsCache:Store:Root")