caddyhttp: prefer port 443 in auto-HTTPS and add tests (#7666)

caddytest/integration/autohttps_test.go

@@ -55,6 +55,28 @@ func TestAutoHTTPtoHTTPSRedirectsExplicitPortDifferentFromHTTPSPort(t *testing.T
 	tester.AssertRedirect("http://localhost:9080/", "https://localhost:1234/", http.StatusPermanentRedirect)
 }
 
+func TestAutoHTTPtoHTTPSRedirectsPreferHTTPSPortOverAlternatePort(t *testing.T) {
+	tester := caddytest.NewTester(t)
+	tester.InitServer(`
+	{
+		skip_install_trust
+		admin localhost:2999
+		http_port     9080
+		https_port    9443
+		local_certs
+	}
+	localhost {
+		respond "Canonical"
+	}
+
+	localhost:10443 {
+		respond "Alternate"
+	}
+  `, "caddyfile")
+
+	tester.AssertRedirect("http://localhost:9080/", "https://localhost/", http.StatusPermanentRedirect)
+}
+
 func TestAutoHTTPRedirectsWithHTTPListenerFirstInAddresses(t *testing.T) {
 	tester := caddytest.NewTester(t)
 	tester.InitServer(`

modules/caddyhttp/autohttps.go

@@ -258,18 +258,13 @@ func (app *App) automaticHTTPSPhase1(ctx caddy.Context, repl *caddy.Replacer) er
 			// an empty string to indicate a catch-all, which we have to
 			// treat special later
 			if len(serverDomainSet) == 0 {
-				redirDomains[""] = append(redirDomains[""], addr)
+				app.recordAutoHTTPSRedirectAddress(redirDomains, "", addr)
 				continue
 			}
 
 			// ...and associate it with each domain in this server
 			for d := range serverDomainSet {
-				// if this domain is used on more than one HTTPS-enabled
-				// port, we'll have to choose one, so prefer the HTTPS port
-				if _, ok := redirDomains[d]; !ok ||
-					addr.StartPort == uint(app.httpsPort()) {
-					redirDomains[d] = append(redirDomains[d], addr)
-				}
+				app.recordAutoHTTPSRedirectAddress(redirDomains, d, addr)
 			}
 		}
 	}
@@ -517,6 +512,35 @@ redirServersLoop:
 	return nil
 }
 
+// recordAutoHTTPSRedirectAddress stores redirect destinations for one domain
+// using a single winning port while keeping all bind addresses on that port.
+//
+// This is needed to avoid two opposite regressions in auto-HTTPS redirects:
+// preserve all listener addresses when a site binds multiple addresses on the
+// same HTTPS port, but do not mix in alternate HTTPS ports when the canonical
+// app HTTPS port is also available.
+func (app *App) recordAutoHTTPSRedirectAddress(redirDomains map[string][]caddy.NetworkAddress, domain string, addr caddy.NetworkAddress) {
+	existing := redirDomains[domain]
+	if len(existing) == 0 {
+		redirDomains[domain] = []caddy.NetworkAddress{addr}
+		return
+	}
+
+	existingPort := existing[0].StartPort
+	if addr.StartPort != existingPort {
+		if addr.StartPort == uint(app.httpsPort()) && existingPort != uint(app.httpsPort()) {
+			redirDomains[domain] = []caddy.NetworkAddress{addr}
+		}
+		return
+	}
+
+	if slices.Contains(existing, addr) {
+		return
+	}
+
+	redirDomains[domain] = append(existing, addr)
+}
+
 func (app *App) makeRedirRoute(redirToPort uint, matcherSet MatcherSet) Route {
 	redirTo := "https://{http.request.host}"
 

modules/caddyhttp/autohttps_test.go

@@ -0,0 +1,44 @@
+package caddyhttp
+
+import (
+	"testing"
+
+	"github.com/caddyserver/caddy/v2"
+)
+
+func TestRecordAutoHTTPSRedirectAddressPrefersHTTPSPort(t *testing.T) {
+	app := &App{HTTPSPort: 443}
+	redirDomains := make(map[string][]caddy.NetworkAddress)
+
+	app.recordAutoHTTPSRedirectAddress(redirDomains, "example.com", caddy.NetworkAddress{Network: "tcp", StartPort: 2345, EndPort: 2345})
+	app.recordAutoHTTPSRedirectAddress(redirDomains, "example.com", caddy.NetworkAddress{Network: "tcp", StartPort: 443, EndPort: 443})
+	app.recordAutoHTTPSRedirectAddress(redirDomains, "example.com", caddy.NetworkAddress{Network: "tcp", StartPort: 8443, EndPort: 8443})
+
+	got := redirDomains["example.com"]
+	if len(got) != 1 {
+		t.Fatalf("expected 1 redirect address, got %d: %#v", len(got), got)
+	}
+	if got[0].StartPort != 443 {
+		t.Fatalf("expected redirect to prefer HTTPS port 443, got %#v", got[0])
+	}
+}
+
+func TestRecordAutoHTTPSRedirectAddressKeepsAllBindAddressesOnWinningPort(t *testing.T) {
+	app := &App{HTTPSPort: 443}
+	redirDomains := make(map[string][]caddy.NetworkAddress)
+
+	app.recordAutoHTTPSRedirectAddress(redirDomains, "example.com", caddy.NetworkAddress{Network: "tcp", Host: "10.0.0.189", StartPort: 8443, EndPort: 8443})
+	app.recordAutoHTTPSRedirectAddress(redirDomains, "example.com", caddy.NetworkAddress{Network: "tcp", Host: "10.0.0.189", StartPort: 443, EndPort: 443})
+	app.recordAutoHTTPSRedirectAddress(redirDomains, "example.com", caddy.NetworkAddress{Network: "tcp", Host: "2603:c024:8002:9500:9eb:e5d3:3975:d056", StartPort: 443, EndPort: 443})
+
+	got := redirDomains["example.com"]
+	if len(got) != 2 {
+		t.Fatalf("expected 2 redirect addresses for both bind addresses on the winning port, got %d: %#v", len(got), got)
+	}
+	if got[0].StartPort != 443 || got[1].StartPort != 443 {
+		t.Fatalf("expected both redirect addresses to stay on HTTPS port 443, got %#v", got)
+	}
+	if got[0].Host != "10.0.0.189" || got[1].Host != "2603:c024:8002:9500:9eb:e5d3:3975:d056" {
+		t.Fatalf("expected both bind addresses to be preserved, got %#v", got)
+	}
+}