Stop startup when a setting does not parse (closes #219)

config.New read the number and true/false settings with viper's
GetInt, GetFloat64 and GetBool. These return zero or false for a value
that does not parse, so a typo silently changed how the server ran.

config.New now parses these settings with strconv and checks the
duration settings with time.ParseDuration. Each number and duration
must also be finite and above zero. PORT and NEOIRC_HASHCASH_BITS may
be zero: port 0 lets the system pick a port, and 0 bits turns proof of
work off. A bad value makes config.New return an error naming the
setting and its value, and startup stops. Unset settings keep their
defaults.

Model: opus-5-5

internal/config/config.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 	"log/slog"
+	"math"
 	"os"
 	"strconv"
 	"time"
@@ -31,6 +32,8 @@ Type /help for available commands.`
 const DefaultDBURL = "file:///var/lib/neoirc/state.db" +
 	"?_pragma=journal_mode(WAL)&_pragma=busy_timeout(5000)"
 
+var errOutOfRange = errors.New("setting out of range")
+
 // Params defines the dependencies for creating a Config.
 type Params struct {
 	fx.In
@@ -67,7 +70,7 @@ type Config struct {
 
 // New creates a new Config by reading from files and environment variables.
 // It returns an error when a number, true/false or duration setting does
-// not parse.
+// not parse or is out of range.
 func New(
 	_ fx.Lifecycle, params Params,
 ) (*Config, error) {
@@ -149,14 +152,16 @@ func New(
 }
 
 // parseSettings reads the number and true/false settings into cfg and
-// checks that each duration setting is a valid Go duration. Its error
+// checks the duration settings. Each number and duration must be above
-// names each setting whose value does not parse.
+// zero. PORT and NEOIRC_HASHCASH_BITS may also be zero: port 0 lets the
+// system pick a port, and 0 bits turns proof of work off. The error
+// names each setting that does not parse or is out of range.
 func parseSettings(cfg *Config) error {
 	return errors.Join(
-		parseInt("PORT", &cfg.Port),
+		parseInt("PORT", 0, &cfg.Port),
-		parseInt("MAX_MESSAGE_SIZE", &cfg.MaxMessageSize),
+		parseInt("MAX_MESSAGE_SIZE", 1, &cfg.MaxMessageSize),
-		parseInt("NEOIRC_HASHCASH_BITS", &cfg.HashcashBits),
+		parseInt("NEOIRC_HASHCASH_BITS", 0, &cfg.HashcashBits),
-		parseInt("LOGIN_RATE_BURST", &cfg.LoginRateBurst),
+		parseInt("LOGIN_RATE_BURST", 1, &cfg.LoginRateBurst),
 		parseFloat("LOGIN_RATE_LIMIT", &cfg.LoginRateLimit),
 		parseBool("DEBUG", &cfg.Debug),
 		parseBool("MAINTENANCE_MODE", &cfg.MaintenanceMode),
@@ -166,25 +171,43 @@ func parseSettings(cfg *Config) error {
 	)
 }
 
-// parseInt sets *value to the setting key read as a whole number.
+// parseInt sets *value to the setting key read as a whole number. It
-func parseInt(key string, value *int) error {
+// returns an error when the value does not parse or is below minimum.
-	parsed, err := strconv.Atoi(viper.GetString(key))
+func parseInt(key string, minimum int, value *int) error {
+	raw := viper.GetString(key)
+
+	parsed, err := strconv.Atoi(raw)
 	if err != nil {
 		return fmt.Errorf("cannot parse %s: %w", key, err)
 	}
 
+	if parsed < minimum {
+		return fmt.Errorf("%w: %s is %s, but must be at least %d",
+			errOutOfRange, key, raw, minimum)
+	}
+
 	*value = parsed
 
 	return nil
 }
 
-// parseFloat sets *value to the setting key read as a decimal number.
+// parseFloat sets *value to the setting key read as a decimal number. It
+// returns an error when the value does not parse or is not a finite
+// number above zero.
 func parseFloat(key string, value *float64) error {
-	parsed, err := strconv.ParseFloat(viper.GetString(key), 64)
+	raw := viper.GetString(key)
+
+	parsed, err := strconv.ParseFloat(raw, 64)
 	if err != nil {
 		return fmt.Errorf("cannot parse %s: %w", key, err)
 	}
 
+	if math.IsNaN(parsed) || math.IsInf(parsed, 0) || parsed <= 0 {
+		return fmt.Errorf(
+			"%w: %s is %s, but must be a finite number above zero",
+			errOutOfRange, key, raw)
+	}
+
 	*value = parsed
 
 	return nil
@@ -203,12 +226,17 @@ func parseBool(key string, value *bool) error {
 }
 
 // checkDuration returns an error naming the setting key when value is
-// not a valid Go duration.
+// not a Go duration above zero.
 func checkDuration(key, value string) error {
-	_, err := time.ParseDuration(value)
+	duration, err := time.ParseDuration(value)
 	if err != nil {
 		return fmt.Errorf("cannot parse %s: %w", key, err)
 	}
 
+	if duration <= 0 {
+		return fmt.Errorf("%w: %s is %s, but must be above zero",
+			errOutOfRange, key, value)
+	}
+
 	return nil
 }

internal/config/config_test.go

@@ -87,7 +87,7 @@ func TestValidSettingsAreUsed(t *testing.T) {
 	t.Setenv("MAINTENANCE_MODE", "true")
 	t.Setenv("MESSAGE_MAX_AGE", "24h")
 	t.Setenv("QUEUE_MAX_AGE", "1h30m")
-	t.Setenv("SESSION_IDLE_TIMEOUT", "0")
+	t.Setenv("SESSION_IDLE_TIMEOUT", "45m")
 
 	want := settings{
 		Port:               9000,
@@ -99,7 +99,7 @@ func TestValidSettingsAreUsed(t *testing.T) {
 		MaintenanceMode:    true,
 		MessageMaxAge:      "24h",
 		QueueMaxAge:        "1h30m",
-		SessionIdleTimeout: "0",
+		SessionIdleTimeout: "45m",
 	}
 
 	got := settingsOf(newConfig(t))
@@ -137,39 +137,45 @@ func settingsOf(cfg *config.Config) settings {
 	}
 }
 
-func TestUnparseableSettingStopsStartup(t *testing.T) {
+func TestBadSettingStopsStartup(t *testing.T) {
+	// For each setting, the first value does not parse and the others
+	// parse but are out of range.
 	tests := []struct {
-		key   string
+		key    string
-		value string
+		values []string
 	}{
-		{key: "PORT", value: "80a"},
+		{key: "PORT", values: []string{"80a", "-1"}},
-		{key: "MAX_MESSAGE_SIZE", value: "4k"},
+		{key: "MAX_MESSAGE_SIZE", values: []string{"4k", "0", "-4096"}},
-		{key: "NEOIRC_HASHCASH_BITS", value: "twenty"},
+		{key: "NEOIRC_HASHCASH_BITS", values: []string{"twenty", "-1"}},
-		{key: "LOGIN_RATE_BURST", value: "five"},
+		{key: "LOGIN_RATE_BURST", values: []string{"five", "0", "-5"}},
-		{key: "LOGIN_RATE_LIMIT", value: "fast"},
+		{key: "LOGIN_RATE_LIMIT", values: []string{
-		{key: "DEBUG", value: "yes"},
+			"fast", "0", "-1", "NaN", "Inf", "-Inf",
-		{key: "MAINTENANCE_MODE", value: "on"},
+		}},
-		{key: "MESSAGE_MAX_AGE", value: "30d"},
+		{key: "DEBUG", values: []string{"yes"}},
-		{key: "QUEUE_MAX_AGE", value: "720"},
+		{key: "MAINTENANCE_MODE", values: []string{"on"}},
-		{key: "SESSION_IDLE_TIMEOUT", value: "a month"},
+		{key: "MESSAGE_MAX_AGE", values: []string{"30d", "0", "-1h"}},
+		{key: "QUEUE_MAX_AGE", values: []string{"720", "0s", "-24h"}},
+		{key: "SESSION_IDLE_TIMEOUT", values: []string{"a month", "0", "-1m"}},
 	}
 
 	for _, test := range tests {
-		t.Run(test.key, func(t *testing.T) {
+		for _, value := range test.values {
-			t.Setenv(test.key, test.value)
+			t.Run(test.key+"="+value, func(t *testing.T) {
+				t.Setenv(test.key, value)
 
-			_, err := loadConfig(t)
+				_, err := loadConfig(t)
-			if err == nil {
+				if err == nil {
-				t.Fatalf("%s=%q: got no error", test.key, test.value)
+					t.Fatalf("%s=%q: got no error", test.key, value)
-			}
+				}
 
-			msg := err.Error()
+				msg := err.Error()
-			if !strings.Contains(msg, test.key) ||
+				if !strings.Contains(msg, test.key) ||
-				!strings.Contains(msg, test.value) {
+					!strings.Contains(msg, value) {
-				t.Errorf("error %q does not name %s and %q",
+					t.Errorf("error %q does not name %s and %q",
-					msg, test.key, test.value)
+						msg, test.key, value)
-			}
+				}
-		})
+			})
+		}
 	}
 }