refactor(server): extract listen target resolution into testable functions

Restructure the web server startup to separate listen target
determination from server lifecycle management.

internal/http/server/httpd.go

@@ -1,201 +0,0 @@
-// SPDX-FileCopyrightText: Copyright The Miniflux Authors. All rights reserved.
-// SPDX-License-Identifier: Apache-2.0
-
-package server // import "miniflux.app/v2/internal/http/server"
-
-import (
-	"crypto/tls"
-	"fmt"
-	"log/slog"
-	"net"
-	"net/http"
-	"os"
-	"strconv"
-	"strings"
-
-	"miniflux.app/v2/internal/config"
-	"miniflux.app/v2/internal/storage"
-	"miniflux.app/v2/internal/worker"
-
-	"golang.org/x/crypto/acme"
-	"golang.org/x/crypto/acme/autocert"
-)
-
-func StartWebServer(store *storage.Storage, pool *worker.Pool) []*http.Server {
-	listenAddresses := config.Opts.ListenAddr()
-	var httpServers []*http.Server
-
-	certFile := config.Opts.CertFile()
-	keyFile := config.Opts.CertKeyFile()
-	certDomain := config.Opts.CertDomain()
-	var sharedAutocertTLSConfig *tls.Config
-
-	if certDomain != "" {
-		slog.Debug("Configuring autocert manager and shared TLS config", slog.String("domain", certDomain))
-		certManager := autocert.Manager{
-			Cache:      storage.NewCertificateCache(store),
-			Prompt:     autocert.AcceptTOS,
-			HostPolicy: autocert.HostWhitelist(certDomain),
-		}
-
-		sharedAutocertTLSConfig = &tls.Config{}
-		sharedAutocertTLSConfig.GetCertificate = certManager.GetCertificate
-		sharedAutocertTLSConfig.NextProtos = []string{"h2", "http/1.1", acme.ALPNProto}
-
-		challengeServer := &http.Server{
-			Handler: certManager.HTTPHandler(nil),
-			Addr:    ":http",
-		}
-		slog.Info("Starting ACME HTTP challenge server for autocert", slog.String("address", challengeServer.Addr))
-		go func() {
-			if err := challengeServer.ListenAndServe(); err != http.ErrServerClosed {
-				slog.Error("ACME HTTP challenge server failed", slog.Any("error", err))
-			}
-		}()
-		config.Opts.SetHTTPSValue(true)
-		httpServers = append(httpServers, challengeServer)
-	}
-
-	for i, listenAddr := range listenAddresses {
-		server := &http.Server{
-			ReadTimeout:  config.Opts.HTTPServerTimeout(),
-			WriteTimeout: config.Opts.HTTPServerTimeout(),
-			IdleTimeout:  config.Opts.HTTPServerTimeout(),
-			Handler:      newRouter(store, pool),
-		}
-
-		isUNIXSocket := strings.HasPrefix(listenAddr, "/")
-		isListenPID := os.Getenv("LISTEN_PID") == strconv.Itoa(os.Getpid())
-
-		if !isUNIXSocket && !isListenPID {
-			server.Addr = listenAddr
-		}
-
-		switch {
-		case isListenPID:
-			if i == 0 {
-				slog.Info("Starting server using systemd socket for the first listen address", slog.String("address_info", listenAddr))
-				startSystemdSocketServer(server)
-			} else {
-				slog.Warn("Systemd socket activation: Only the first listen address is used by systemd. Other addresses are ignored.", slog.String("skipped_address", listenAddr))
-				continue
-			}
-		case isUNIXSocket:
-			startUnixSocketServer(server, listenAddr)
-		case certDomain != "" && (listenAddr == ":https" || (i == 0 && strings.Contains(listenAddr, ":"))):
-			server.Addr = listenAddr
-			startAutoCertTLSServer(server, sharedAutocertTLSConfig)
-		case certFile != "" && keyFile != "":
-			server.Addr = listenAddr
-			startTLSServer(server, certFile, keyFile)
-			config.Opts.SetHTTPSValue(true)
-		default:
-			server.Addr = listenAddr
-			startHTTPServer(server)
-		}
-
-		httpServers = append(httpServers, server)
-	}
-
-	return httpServers
-}
-
-func startSystemdSocketServer(server *http.Server) {
-	go func() {
-		f := os.NewFile(3, "systemd socket")
-		listener, err := net.FileListener(f)
-		if err != nil {
-			printErrorAndExit(`Unable to create listener from systemd socket: %v`, err)
-		}
-
-		slog.Info(`Starting server using systemd socket`)
-		if err := server.Serve(listener); err != http.ErrServerClosed {
-			printErrorAndExit(`Systemd socket server failed to start: %v`, err)
-		}
-	}()
-}
-
-func startUnixSocketServer(server *http.Server, socketFile string) {
-	if err := os.Remove(socketFile); err != nil && !os.IsNotExist(err) {
-		printErrorAndExit("Unable to remove existing Unix socket %s: %v", socketFile, err)
-	}
-	listener, err := net.Listen("unix", socketFile)
-	if err != nil {
-		printErrorAndExit(`Server failed to listen on Unix socket %s: %v`, socketFile, err)
-	}
-
-	if err := os.Chmod(socketFile, 0666); err != nil {
-		printErrorAndExit(`Unable to change socket permission for %s: %v`, socketFile, err)
-	}
-
-	go func() {
-		certFile := config.Opts.CertFile()
-		keyFile := config.Opts.CertKeyFile()
-
-		if certFile != "" && keyFile != "" {
-			slog.Info("Starting TLS server using a Unix socket",
-				slog.String("socket", socketFile),
-				slog.String("cert_file", certFile),
-				slog.String("key_file", keyFile),
-			)
-			// Ensure HTTPS is marked as true if any listener uses TLS
-			config.Opts.SetHTTPSValue(true)
-			if err := server.ServeTLS(listener, certFile, keyFile); err != http.ErrServerClosed {
-				printErrorAndExit("TLS Unix socket server failed to start on %s: %v", socketFile, err)
-			}
-		} else {
-			slog.Info("Starting server using a Unix socket", slog.String("socket", socketFile))
-			if err := server.Serve(listener); err != http.ErrServerClosed {
-				printErrorAndExit("Unix socket server failed to start on %s: %v", socketFile, err)
-			}
-		}
-	}()
-}
-
-func startAutoCertTLSServer(server *http.Server, autoTLSConfig *tls.Config) {
-	if server.TLSConfig == nil {
-		server.TLSConfig = &tls.Config{}
-	}
-	server.TLSConfig.GetCertificate = autoTLSConfig.GetCertificate
-	server.TLSConfig.NextProtos = autoTLSConfig.NextProtos
-
-	go func() {
-		slog.Info("Starting TLS server using automatic certificate management",
-			slog.String("listen_address", server.Addr),
-		)
-		if err := server.ListenAndServeTLS("", ""); err != http.ErrServerClosed {
-			printErrorAndExit("Autocert server failed to start on %s: %v", server.Addr, err)
-		}
-	}()
-}
-
-func startTLSServer(server *http.Server, certFile, keyFile string) {
-	go func() {
-		slog.Info("Starting TLS server using a certificate",
-			slog.String("listen_address", server.Addr),
-			slog.String("cert_file", certFile),
-			slog.String("key_file", keyFile),
-		)
-		if err := server.ListenAndServeTLS(certFile, keyFile); err != http.ErrServerClosed {
-			printErrorAndExit("TLS server failed to start on %s: %v", server.Addr, err)
-		}
-	}()
-}
-
-func startHTTPServer(server *http.Server) {
-	go func() {
-		slog.Info("Starting HTTP server",
-			slog.String("listen_address", server.Addr),
-		)
-		if err := server.ListenAndServe(); err != http.ErrServerClosed {
-			printErrorAndExit("HTTP server failed to start on %s: %v", server.Addr, err)
-		}
-	}()
-}
-
-func printErrorAndExit(format string, a ...any) {
-	message := fmt.Sprintf(format, a...)
-	slog.Error(message)
-	fmt.Fprintf(os.Stderr, "%v\n", message)
-	os.Exit(1)
-}

internal/http/server/server.go

@@ -0,0 +1,274 @@
+// SPDX-FileCopyrightText: Copyright The Miniflux Authors. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package server // import "miniflux.app/v2/internal/http/server"
+
+import (
+	"crypto/tls"
+	"fmt"
+	"log/slog"
+	"net"
+	"net/http"
+	"os"
+	"strconv"
+	"strings"
+
+	"miniflux.app/v2/internal/config"
+	"miniflux.app/v2/internal/storage"
+	"miniflux.app/v2/internal/worker"
+
+	"golang.org/x/crypto/acme"
+	"golang.org/x/crypto/acme/autocert"
+)
+
+func StartWebServer(store *storage.Storage, pool *worker.Pool) []*http.Server {
+	var servers []*http.Server
+
+	autocertTLSConfig, challengeServer := setupAutocert(store)
+	if challengeServer != nil {
+		servers = append(servers, challengeServer)
+	}
+
+	certFile := config.Opts.CertFile()
+	keyFile := config.Opts.CertKeyFile()
+	certDomain := config.Opts.CertDomain()
+
+	targets := determineListenTargets(config.Opts.ListenAddr(), certDomain, certFile, keyFile)
+
+	if autocertTLSConfig != nil || anyTLS(targets) {
+		config.Opts.SetHTTPSValue(true)
+	}
+
+	for _, t := range targets {
+		srv := &http.Server{
+			Addr:         t.address,
+			ReadTimeout:  config.Opts.HTTPServerTimeout(),
+			WriteTimeout: config.Opts.HTTPServerTimeout(),
+			IdleTimeout:  config.Opts.HTTPServerTimeout(),
+			Handler:      newRouter(store, pool),
+		}
+
+		switch t.mode {
+		case modeSystemd:
+			startSystemdSocketServer(srv)
+		case modeUnixSocket:
+			startUnixSocketServer(srv, t.address)
+		case modeUnixSocketTLS:
+			startUnixSocketTLSServer(srv, t.address, t.certFile, t.keyFile)
+		case modeAutocertTLS:
+			startAutoCertTLSServer(srv, autocertTLSConfig)
+		case modeTLS:
+			startTLSServer(srv, t.certFile, t.keyFile)
+		default:
+			startHTTPServer(srv)
+		}
+
+		servers = append(servers, srv)
+	}
+
+	return servers
+}
+
+type listenerMode int
+
+const (
+	modeHTTP listenerMode = iota
+	modeTLS
+	modeAutocertTLS
+	modeUnixSocket
+	modeUnixSocketTLS
+	modeSystemd
+)
+
+type listenTarget struct {
+	address  string
+	mode     listenerMode
+	certFile string
+	keyFile  string
+}
+
+func determineListenTargets(addresses []string, certDomain, certFile, keyFile string) []listenTarget {
+	isSystemd := os.Getenv("LISTEN_PID") == strconv.Itoa(os.Getpid())
+	hasCertFiles := certFile != "" && keyFile != ""
+	hasAutocert := certDomain != ""
+
+	var targets []listenTarget
+
+	for i, addr := range addresses {
+		if isSystemd {
+			if i == 0 {
+				targets = append(targets, listenTarget{address: addr, mode: modeSystemd})
+			} else {
+				slog.Warn("Systemd socket activation: only the first listen address is used, others are ignored",
+					slog.String("skipped_address", addr),
+				)
+			}
+			continue
+		}
+
+		isUnix := strings.HasPrefix(addr, "/")
+
+		switch {
+		case isUnix && hasCertFiles:
+			targets = append(targets, listenTarget{address: addr, mode: modeUnixSocketTLS, certFile: certFile, keyFile: keyFile})
+		case isUnix:
+			targets = append(targets, listenTarget{address: addr, mode: modeUnixSocket})
+		case hasAutocert && (addr == ":https" || (i == 0 && strings.Contains(addr, ":"))):
+			targets = append(targets, listenTarget{address: addr, mode: modeAutocertTLS})
+		case hasCertFiles:
+			targets = append(targets, listenTarget{address: addr, mode: modeTLS, certFile: certFile, keyFile: keyFile})
+		default:
+			targets = append(targets, listenTarget{address: addr, mode: modeHTTP})
+		}
+	}
+
+	return targets
+}
+
+func anyTLS(targets []listenTarget) bool {
+	for _, t := range targets {
+		switch t.mode {
+		case modeTLS, modeAutocertTLS, modeUnixSocketTLS:
+			return true
+		}
+	}
+	return false
+}
+
+func setupAutocert(store *storage.Storage) (*tls.Config, *http.Server) {
+	certDomain := config.Opts.CertDomain()
+	if certDomain == "" {
+		return nil, nil
+	}
+
+	slog.Debug("Configuring autocert manager", slog.String("domain", certDomain))
+	certManager := autocert.Manager{
+		Cache:      storage.NewCertificateCache(store),
+		Prompt:     autocert.AcceptTOS,
+		HostPolicy: autocert.HostWhitelist(certDomain),
+	}
+
+	tlsConfig := &tls.Config{
+		NextProtos: []string{"h2", "http/1.1", acme.ALPNProto},
+	}
+	tlsConfig.GetCertificate = certManager.GetCertificate
+
+	challengeServer := &http.Server{
+		Handler: certManager.HTTPHandler(nil),
+		Addr:    ":http",
+	}
+
+	slog.Info("Starting ACME HTTP challenge server", slog.String("address", challengeServer.Addr))
+	go func() {
+		if err := challengeServer.ListenAndServe(); err != http.ErrServerClosed {
+			slog.Error("ACME HTTP challenge server failed", slog.Any("error", err))
+		}
+	}()
+
+	return tlsConfig, challengeServer
+}
+
+func startSystemdSocketServer(server *http.Server) {
+	go func() {
+		f := os.NewFile(3, "systemd socket")
+		listener, err := net.FileListener(f)
+		if err != nil {
+			printErrorAndExit(`Unable to create listener from systemd socket: %v`, err)
+		}
+
+		slog.Info(`Starting server using systemd socket`)
+		if err := server.Serve(listener); err != http.ErrServerClosed {
+			printErrorAndExit(`Systemd socket server failed to start: %v`, err)
+		}
+	}()
+}
+
+func startUnixSocketServer(server *http.Server, socketFile string) {
+	listener := createUnixSocketListener(socketFile)
+
+	go func() {
+		slog.Info("Starting server using a Unix socket", slog.String("socket", socketFile))
+		if err := server.Serve(listener); err != http.ErrServerClosed {
+			printErrorAndExit("Unix socket server failed to start on %s: %v", socketFile, err)
+		}
+	}()
+}
+
+func startUnixSocketTLSServer(server *http.Server, socketFile, certFile, keyFile string) {
+	listener := createUnixSocketListener(socketFile)
+
+	go func() {
+		slog.Info("Starting TLS server using a Unix socket",
+			slog.String("socket", socketFile),
+			slog.String("cert_file", certFile),
+			slog.String("key_file", keyFile),
+		)
+		if err := server.ServeTLS(listener, certFile, keyFile); err != http.ErrServerClosed {
+			printErrorAndExit("TLS Unix socket server failed to start on %s: %v", socketFile, err)
+		}
+	}()
+}
+
+func createUnixSocketListener(socketFile string) net.Listener {
+	if err := os.Remove(socketFile); err != nil && !os.IsNotExist(err) {
+		printErrorAndExit("Unable to remove existing Unix socket %s: %v", socketFile, err)
+	}
+	listener, err := net.Listen("unix", socketFile)
+	if err != nil {
+		printErrorAndExit(`Server failed to listen on Unix socket %s: %v`, socketFile, err)
+	}
+
+	if err := os.Chmod(socketFile, 0666); err != nil {
+		printErrorAndExit(`Unable to change socket permission for %s: %v`, socketFile, err)
+	}
+
+	return listener
+}
+
+func startAutoCertTLSServer(server *http.Server, autoTLSConfig *tls.Config) {
+	if server.TLSConfig == nil {
+		server.TLSConfig = &tls.Config{}
+	}
+	server.TLSConfig.GetCertificate = autoTLSConfig.GetCertificate
+	server.TLSConfig.NextProtos = autoTLSConfig.NextProtos
+
+	go func() {
+		slog.Info("Starting TLS server using automatic certificate management",
+			slog.String("listen_address", server.Addr),
+		)
+		if err := server.ListenAndServeTLS("", ""); err != http.ErrServerClosed {
+			printErrorAndExit("Autocert server failed to start on %s: %v", server.Addr, err)
+		}
+	}()
+}
+
+func startTLSServer(server *http.Server, certFile, keyFile string) {
+	go func() {
+		slog.Info("Starting TLS server using a certificate",
+			slog.String("listen_address", server.Addr),
+			slog.String("cert_file", certFile),
+			slog.String("key_file", keyFile),
+		)
+		if err := server.ListenAndServeTLS(certFile, keyFile); err != http.ErrServerClosed {
+			printErrorAndExit("TLS server failed to start on %s: %v", server.Addr, err)
+		}
+	}()
+}
+
+func startHTTPServer(server *http.Server) {
+	go func() {
+		slog.Info("Starting HTTP server",
+			slog.String("listen_address", server.Addr),
+		)
+		if err := server.ListenAndServe(); err != http.ErrServerClosed {
+			printErrorAndExit("HTTP server failed to start on %s: %v", server.Addr, err)
+		}
+	}()
+}
+
+func printErrorAndExit(format string, a ...any) {
+	message := fmt.Sprintf(format, a...)
+	slog.Error(message)
+	fmt.Fprintf(os.Stderr, "%v\n", message)
+	os.Exit(1)
+}

internal/http/server/server_test.go

@@ -0,0 +1,189 @@
+// SPDX-FileCopyrightText: Copyright The Miniflux Authors. All rights reserved.
+// SPDX-License-Identifier: Apache-2.0
+
+package server
+
+import (
+	"testing"
+)
+
+func TestDetermineListenTargets(t *testing.T) {
+	tests := []struct {
+		name       string
+		addresses  []string
+		certDomain string
+		certFile   string
+		keyFile    string
+		expected   []listenTarget
+	}{
+		{
+			name:      "single HTTP listener",
+			addresses: []string{":8080"},
+			expected: []listenTarget{
+				{address: ":8080", mode: modeHTTP},
+			},
+		},
+		{
+			name:      "multiple HTTP listeners",
+			addresses: []string{":8080", ":9090"},
+			expected: []listenTarget{
+				{address: ":8080", mode: modeHTTP},
+				{address: ":9090", mode: modeHTTP},
+			},
+		},
+		{
+			name:      "TLS with cert files",
+			addresses: []string{":443"},
+			certFile:  "/path/to/cert.pem",
+			keyFile:   "/path/to/key.pem",
+			expected: []listenTarget{
+				{address: ":443", mode: modeTLS, certFile: "/path/to/cert.pem", keyFile: "/path/to/key.pem"},
+			},
+		},
+		{
+			name:      "cert file without key file falls back to HTTP",
+			addresses: []string{":8080"},
+			certFile:  "/path/to/cert.pem",
+			expected: []listenTarget{
+				{address: ":8080", mode: modeHTTP},
+			},
+		},
+		{
+			name:      "key file without cert file falls back to HTTP",
+			addresses: []string{":8080"},
+			keyFile:   "/path/to/key.pem",
+			expected: []listenTarget{
+				{address: ":8080", mode: modeHTTP},
+			},
+		},
+		{
+			name:       "autocert with :https address",
+			addresses:  []string{":https"},
+			certDomain: "example.com",
+			expected: []listenTarget{
+				{address: ":https", mode: modeAutocertTLS},
+			},
+		},
+		{
+			name:       "autocert with first address containing colon",
+			addresses:  []string{":443"},
+			certDomain: "example.com",
+			expected: []listenTarget{
+				{address: ":443", mode: modeAutocertTLS},
+			},
+		},
+		{
+			name:       "autocert does not apply to second non-https address",
+			addresses:  []string{":https", ":8080"},
+			certDomain: "example.com",
+			expected: []listenTarget{
+				{address: ":https", mode: modeAutocertTLS},
+				{address: ":8080", mode: modeHTTP},
+			},
+		},
+		{
+			name:      "unix socket",
+			addresses: []string{"/var/run/miniflux.sock"},
+			expected: []listenTarget{
+				{address: "/var/run/miniflux.sock", mode: modeUnixSocket},
+			},
+		},
+		{
+			name:      "unix socket with TLS",
+			addresses: []string{"/var/run/miniflux.sock"},
+			certFile:  "/path/to/cert.pem",
+			keyFile:   "/path/to/key.pem",
+			expected: []listenTarget{
+				{address: "/var/run/miniflux.sock", mode: modeUnixSocketTLS, certFile: "/path/to/cert.pem", keyFile: "/path/to/key.pem"},
+			},
+		},
+		{
+			name:      "mixed unix socket and TCP",
+			addresses: []string{"/var/run/miniflux.sock", ":8080"},
+			certFile:  "/path/to/cert.pem",
+			keyFile:   "/path/to/key.pem",
+			expected: []listenTarget{
+				{address: "/var/run/miniflux.sock", mode: modeUnixSocketTLS, certFile: "/path/to/cert.pem", keyFile: "/path/to/key.pem"},
+				{address: ":8080", mode: modeTLS, certFile: "/path/to/cert.pem", keyFile: "/path/to/key.pem"},
+			},
+		},
+		{
+			name:      "empty address list",
+			addresses: []string{},
+			expected:  nil,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			got := determineListenTargets(tc.addresses, tc.certDomain, tc.certFile, tc.keyFile)
+
+			if len(got) != len(tc.expected) {
+				t.Fatalf("got %d targets, want %d", len(got), len(tc.expected))
+			}
+
+			for i := range got {
+				if got[i] != tc.expected[i] {
+					t.Errorf("target[%d] = %+v, want %+v", i, got[i], tc.expected[i])
+				}
+			}
+		})
+	}
+}
+
+func TestAnyTLS(t *testing.T) {
+	tests := []struct {
+		name     string
+		targets  []listenTarget
+		expected bool
+	}{
+		{
+			name:     "empty list",
+			targets:  nil,
+			expected: false,
+		},
+		{
+			name:     "HTTP only",
+			targets:  []listenTarget{{mode: modeHTTP}},
+			expected: false,
+		},
+		{
+			name:     "systemd only",
+			targets:  []listenTarget{{mode: modeSystemd}},
+			expected: false,
+		},
+		{
+			name:     "unix socket without TLS",
+			targets:  []listenTarget{{mode: modeUnixSocket}},
+			expected: false,
+		},
+		{
+			name:     "TLS mode",
+			targets:  []listenTarget{{mode: modeTLS}},
+			expected: true,
+		},
+		{
+			name:     "autocert TLS mode",
+			targets:  []listenTarget{{mode: modeAutocertTLS}},
+			expected: true,
+		},
+		{
+			name:     "unix socket TLS mode",
+			targets:  []listenTarget{{mode: modeUnixSocketTLS}},
+			expected: true,
+		},
+		{
+			name:     "mixed with one TLS",
+			targets:  []listenTarget{{mode: modeHTTP}, {mode: modeTLS}, {mode: modeUnixSocket}},
+			expected: true,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			if got := anyTLS(tc.targets); got != tc.expected {
+				t.Errorf("anyTLS() = %v, want %v", got, tc.expected)
+			}
+		})
+	}
+}