diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9e13b63..bdc9527 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -11,8 +11,8 @@ jobs: runs-on: docker container: git.nakama.town/fmartingr/ci-images/ci-base:1.0.0 steps: - - uses: actions/checkout@v6 - - uses: actions/setup-go@v6 + - uses: actions/checkout@v7 + - uses: actions/setup-go@v7 with: go-version-file: go.mod - run: make format @@ -22,17 +22,15 @@ jobs: runs-on: docker container: git.nakama.town/fmartingr/ci-images/ci-base:1.0.0 steps: - - uses: actions/checkout@v6 - - uses: actions/goreleaser-action@v6.4.0 - with: - args: check + - uses: actions/checkout@v7 + - run: make check lint: runs-on: docker container: git.nakama.town/fmartingr/ci-images/ci-base:1.0.0 steps: - - uses: actions/checkout@v6 - - uses: actions/setup-go@v6 + - uses: actions/checkout@v7 + - uses: actions/setup-go@v7 with: go-version-file: go.mod - run: make ci-lint @@ -41,8 +39,8 @@ jobs: runs-on: docker container: git.nakama.town/fmartingr/ci-images/ci-base:1.0.0 steps: - - uses: actions/checkout@v6 - - uses: actions/setup-go@v6 + - uses: actions/checkout@v7 + - uses: actions/setup-go@v7 with: go-version-file: go.mod - run: make test @@ -51,8 +49,8 @@ jobs: runs-on: docker container: git.nakama.town/fmartingr/ci-images/ci-base:1.0.0 steps: - - uses: actions/checkout@v6 - - uses: actions/setup-go@v6 + - uses: actions/checkout@v7 + - uses: actions/setup-go@v7 with: go-version-file: go.mod - run: make build diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 1f41606..1e3ab84 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -9,12 +9,12 @@ jobs: runs-on: docker container: git.nakama.town/fmartingr/ci-images/ci-base:1.0.0 steps: - - uses: actions/checkout@v6 + - uses: actions/checkout@v7 with: fetch-depth: 0 - name: Install Go - uses: actions/setup-go@v6 + uses: actions/setup-go@v7 with: go-version-file: go.mod @@ -22,9 +22,7 @@ jobs: run: echo "${{ secrets.FORGEJO_TOKEN }}" | docker login git.nakama.town -u ${{ github.actor }} --password-stdin - name: Run GoReleaser - uses: actions/goreleaser-action@v6.4.0 - with: - args: release --clean + run: goreleaser release --clean env: GORELEASER_FORCE_TOKEN: gitea GITEA_TOKEN: ${{ secrets.FORGEJO_TOKEN }} diff --git a/Containerfile b/Containerfile index 4472a6e..0526ef5 100644 --- a/Containerfile +++ b/Containerfile @@ -1,5 +1,5 @@ # Build dependencies on native arch (no QEMU needed) -FROM --platform=$BUILDPLATFORM alpine:3.20 AS base +FROM --platform=$BUILDPLATFORM alpine:3.24 AS base RUN apk add --no-cache ca-certificates tzdata RUN addgroup -g 1000 smtp2shoutrrr && adduser -u 1000 -G smtp2shoutrrr -s /bin/sh -D smtp2shoutrrr diff --git a/Makefile b/Makefile index 879fad3..c5b7d47 100644 --- a/Makefile +++ b/Makefile @@ -4,7 +4,8 @@ CGO_ENABLED := 0 DIST_PATH := ./dist TEST_OPTIONS := -v -failfast -race -bench=. -benchtime=100000x -cover -coverprofile=coverage.out -GOLANGCI_LINT_VERSION := v2.8.0 +GOLANGCI_LINT_VERSION := v2.13.2 +GORELEASER_VERSION := v2.18.1 .DEFAULT_GOAL := help @@ -14,9 +15,13 @@ help: @echo "Available targets:" @grep -E '^## ' $(MAKEFILE_LIST) | sed 's/## / /' | sort +.PHONY: ensure-goreleaser +ensure-goreleaser: + @which goreleaser > /dev/null 2>&1 || go install github.com/goreleaser/goreleaser/v2@$(GORELEASER_VERSION) + ## build: Build project using goreleaser (snapshot) .PHONY: build -build: +build: ensure-goreleaser goreleaser build --snapshot --clean ## quick-run: Execute project directly using go run @@ -35,6 +40,11 @@ format: go fmt ./... go mod tidy +## check: Validate the goreleaser configuration +.PHONY: check +check: ensure-goreleaser + goreleaser check + ## ci-lint: Run golangci-lint (installs if missing) .PHONY: ci-lint ci-lint: diff --git a/README.md b/README.md index ad3a5d8..3666932 100644 --- a/README.md +++ b/README.md @@ -44,6 +44,11 @@ Target = "ntfy://ntfy.sh/legacy-topic?tags=thing" # Will show deprecation warni Targets = ["ntfy://ntfy.sh/catch-all-topic?tags=unmatched"] ``` +The server refuses to start if the configuration cannot forward anything — no +recipients and no catch-all, a recipient without addresses or usable targets, or +a mistyped table name such as `[[Recipient]]`, which TOML would otherwise accept +silently. + ### From releases - Grab the latest release from the [releases page](https://git.nakama.town/fmartingr/smtp2shoutrrr/releases) diff --git a/backend_test.go b/backend_test.go index d9c5cc2..330519c 100644 --- a/backend_test.go +++ b/backend_test.go @@ -1,8 +1,6 @@ package smtp2shoutrrr import ( - "context" - "fmt" "io" "net/http" "net/http/httptest" @@ -36,7 +34,7 @@ func TestEmailForwarding(t *testing.T) { // Configure the SMTP server config := &Config{ - Port: 2525, + Port: 0, Username: "testuser", Password: "testpass", Recipients: []ConfigRecipient{ @@ -47,28 +45,12 @@ func TestEmailForwarding(t *testing.T) { }, } - ctx := context.Background() - smtpServer := NewSMTPServer(config) - - // Start the server - go func() { - if err := smtpServer.Start(ctx); err != nil { - t.Errorf("failed to start server: %v", err) - } - }() - - // Give the server time to start - time.Sleep(100 * time.Millisecond) - defer func() { - if err := smtpServer.Stop(ctx); err != nil { - t.Errorf("failed to stop server: %v", err) - } - }() + addr, _, _ := startServer(t, NewSMTPServer(config)) // Send test email auth := smtp.PlainAuth("", config.Username, config.Password, "localhost") err := smtp.SendMail( - fmt.Sprintf("localhost:%d", config.Port), + addr, auth, "sender@example.com", []string{"test@example.com"}, @@ -76,7 +58,6 @@ func TestEmailForwarding(t *testing.T) { ) require.NoError(t, err) - // Give some time for the notification to be processed time.Sleep(100 * time.Millisecond) // Verify the notification was received @@ -106,7 +87,7 @@ func TestMultipleTargets(t *testing.T) { // Configure with multiple targets config := &Config{ - Port: 2526, + Port: 0, Username: "testuser", Password: "testpass", Recipients: []ConfigRecipient{ @@ -120,26 +101,12 @@ func TestMultipleTargets(t *testing.T) { }, } - ctx := context.Background() - smtpServer := NewSMTPServer(config) - - go func() { - if err := smtpServer.Start(ctx); err != nil { - t.Errorf("failed to start server: %v", err) - } - }() - - time.Sleep(100 * time.Millisecond) - defer func() { - if err := smtpServer.Stop(ctx); err != nil { - t.Errorf("failed to stop server: %v", err) - } - }() + addr, _, _ := startServer(t, NewSMTPServer(config)) // Send test email auth := smtp.PlainAuth("", config.Username, config.Password, "localhost") err := smtp.SendMail( - fmt.Sprintf("localhost:%d", config.Port), + addr, auth, "sender@example.com", []string{"test@example.com"}, @@ -172,7 +139,7 @@ func TestTargetAndTargetsMerging(t *testing.T) { // Configure with both Target (deprecated) and Targets config := &Config{ - Port: 2527, + Port: 0, Username: "testuser", Password: "testpass", Recipients: []ConfigRecipient{ @@ -187,26 +154,12 @@ func TestTargetAndTargetsMerging(t *testing.T) { }, } - ctx := context.Background() - smtpServer := NewSMTPServer(config) - - go func() { - if err := smtpServer.Start(ctx); err != nil { - t.Errorf("failed to start server: %v", err) - } - }() - - time.Sleep(100 * time.Millisecond) - defer func() { - if err := smtpServer.Stop(ctx); err != nil { - t.Errorf("failed to stop server: %v", err) - } - }() + addr, _, _ := startServer(t, NewSMTPServer(config)) // Send test email auth := smtp.PlainAuth("", config.Username, config.Password, "localhost") err := smtp.SendMail( - fmt.Sprintf("localhost:%d", config.Port), + addr, auth, "sender@example.com", []string{"test@example.com"}, @@ -244,7 +197,7 @@ func TestPartialTargetFailure(t *testing.T) { // Configure with mix of good and bad targets config := &Config{ - Port: 2528, + Port: 0, Username: "testuser", Password: "testpass", Recipients: []ConfigRecipient{ @@ -259,26 +212,12 @@ func TestPartialTargetFailure(t *testing.T) { }, } - ctx := context.Background() - smtpServer := NewSMTPServer(config) - - go func() { - if err := smtpServer.Start(ctx); err != nil { - t.Errorf("failed to start server: %v", err) - } - }() - - time.Sleep(100 * time.Millisecond) - defer func() { - if err := smtpServer.Stop(ctx); err != nil { - t.Errorf("failed to stop server: %v", err) - } - }() + addr, _, _ := startServer(t, NewSMTPServer(config)) // Send test email auth := smtp.PlainAuth("", config.Username, config.Password, "localhost") err := smtp.SendMail( - fmt.Sprintf("localhost:%d", config.Port), + addr, auth, "sender@example.com", []string{"test@example.com"}, @@ -309,7 +248,7 @@ func TestInvalidTargetURLs(t *testing.T) { // Configure with invalid and valid URLs config := &Config{ - Port: 2529, + Port: 0, Username: "testuser", Password: "testpass", Recipients: []ConfigRecipient{ @@ -325,26 +264,12 @@ func TestInvalidTargetURLs(t *testing.T) { }, } - ctx := context.Background() - smtpServer := NewSMTPServer(config) - - go func() { - if err := smtpServer.Start(ctx); err != nil { - t.Errorf("failed to start server: %v", err) - } - }() - - time.Sleep(100 * time.Millisecond) - defer func() { - if err := smtpServer.Stop(ctx); err != nil { - t.Errorf("failed to stop server: %v", err) - } - }() + addr, _, _ := startServer(t, NewSMTPServer(config)) // Send test email auth := smtp.PlainAuth("", config.Username, config.Password, "localhost") err := smtp.SendMail( - fmt.Sprintf("localhost:%d", config.Port), + addr, auth, "sender@example.com", []string{"test@example.com"}, diff --git a/cmd/sendmail/main.go b/cmd/sendmail/main.go index df894b3..ede4b78 100644 --- a/cmd/sendmail/main.go +++ b/cmd/sendmail/main.go @@ -1,108 +1,51 @@ package main import ( + "errors" "fmt" - "log" "log/slog" "net/smtp" "os" - "git.nakama.town/fmartingr/gotoolkit/encoding" - "github.com/emersion/go-sasl" - "git.nakama.town/fmartingr/smtp2shoutrrr" ) -// The ANONYMOUS mechanism name. -const Anonymous = "ANONYMOUS" +const ( + configPath = "config.toml" -type anonymousClient struct { - Trace string -} - -func (c *anonymousClient) Start(si *smtp.ServerInfo) (mech string, ir []byte, err error) { - mech = Anonymous - ir = []byte(c.Trace) - return -} - -func (c *anonymousClient) Next(challenge []byte, b bool) (response []byte, err error) { - return nil, sasl.ErrUnexpectedServerChallenge -} - -// A client implementation of the ANONYMOUS authentication mechanism, as -// described in RFC 4505. -func NewAnonymousClient(trace string) smtp.Auth { - return &anonymousClient{trace} -} - -// The PLAIN mechanism name. -const Plain = "PLAIN" - -type plainClient struct { - Identity string - Username string - Password string -} - -func (a *plainClient) Start(si *smtp.ServerInfo) (mech string, ir []byte, err error) { - mech = "PLAIN" - ir = []byte(a.Identity + "\x00" + a.Username + "\x00" + a.Password) - return -} - -func (a *plainClient) Next(challenge []byte, b bool) (response []byte, err error) { - slog.Info("Next: %v", slog.String("challenge", string(challenge))) - return nil, nil -} - -// A client implementation of the PLAIN authentication mechanism, as described -// in RFC 4616. Authorization identity may be left blank to indicate that it is -// the same as the username. -func NewPlainClient(identity, username, password string) smtp.Auth { - return &plainClient{identity, username, password} -} + // PlainAuth refuses to send credentials over an unencrypted connection + // unless the host is a loopback address, which is what the development + // server listens on. + hostname = "localhost" +) func main() { - var config smtp2shoutrrr.Config - configPath := "config.toml" + if err := run(); err != nil { + slog.Error("sendmail failed", slog.String("err", err.Error())) + os.Exit(1) + } +} - f, err := os.Open(configPath) +func run() error { + config, err := smtp2shoutrrr.LoadConfig(configPath) if err != nil { - slog.Error("Error opening config file", slog.String("err", err.Error()), slog.String("path", configPath)) - return + return fmt.Errorf("loading configuration: %w", err) } - enc := encoding.NewTOMLEncoding() - if err := enc.DecodeReader(f, &config); err != nil { - slog.Error("Error decoding config file", slog.String("err", err.Error()), slog.String("path", configPath)) - return - } - - config.SetDefaults() - - // hostname is used by PlainAuth to validate the TLS certificate. - hostname := "localhost" - auth := NewPlainClient("", config.Username, config.Password) - // auth := NewAnonymousClient("test") - slog.Info("Using first recipient configuration to send a test email") if len(config.Recipients) == 0 { - slog.Error("No recipients found in configuration") - return + return errors.New("no recipients found in configuration") } if len(config.Recipients[0].Addresses) == 0 { - slog.Error("No email addresses found in first recipient configuration") - return + return errors.New("no email addresses found in first recipient configuration") } + auth := smtp.PlainAuth("", config.Username, config.Password, hostname) recipients := []string{config.Recipients[0].Addresses[0]} msg := []byte("Subject: Test notification\r\n\r\nThis is a test notification") from := "hello@localhost" - err = smtp.SendMail(fmt.Sprintf("%s:%d", hostname, config.Port), auth, from, recipients, msg) - if err != nil { - log.Fatal(err) - } + + return smtp.SendMail(fmt.Sprintf("%s:%d", hostname, config.Port), auth, from, recipients, msg) } diff --git a/cmd/smtp2shoutrrr/main.go b/cmd/smtp2shoutrrr/main.go index 0d92081..73ca2f1 100644 --- a/cmd/smtp2shoutrrr/main.go +++ b/cmd/smtp2shoutrrr/main.go @@ -2,54 +2,43 @@ package main import ( "context" + "fmt" "log/slog" "os" + "os/signal" + "syscall" - "git.nakama.town/fmartingr/gotoolkit/encoding" - "git.nakama.town/fmartingr/gotoolkit/model" - "git.nakama.town/fmartingr/gotoolkit/service" "git.nakama.town/fmartingr/smtp2shoutrrr" _ "golang.org/x/crypto/x509roots/fallback" ) +const configPath = "config.toml" + func main() { - var config *smtp2shoutrrr.Config - configPath := "config.toml" + if err := run(); err != nil { + slog.Error("smtp2shoutrrr stopped", slog.String("err", err.Error())) + os.Exit(1) + } +} - f, err := os.Open(configPath) +func run() error { + config, err := smtp2shoutrrr.LoadConfig(configPath) if err != nil { - slog.Error("Error opening config file", slog.String("err", err.Error()), slog.String("path", configPath)) - return + return fmt.Errorf("loading configuration: %w", err) } - enc := encoding.NewTOMLEncoding() - if err := enc.DecodeReader(f, &config); err != nil { - slog.Error("Error decoding config file", slog.String("err", err.Error()), slog.String("path", configPath)) - return - } - - config.SetDefaults() - slog.Info("config loaded", slog.Int("recipients", len(config.Recipients))) - ctx := context.Background() - smtpServer := smtp2shoutrrr.NewSMTPServer(config) + ctx, stop := signal.NotifyContext(context.Background(), syscall.SIGINT, syscall.SIGTERM) + defer stop() - svc, err := service.NewService([]model.Server{ - smtpServer, - }) - if err != nil { - slog.Error("Error creating service:", slog.String("err", err.Error())) - return - } + // Restore default signal handling as soon as the first signal arrives, so + // a second one aborts a drain instead of being swallowed. + go func() { + <-ctx.Done() + stop() + }() - if err := svc.Start(ctx); err != nil { - slog.Error("Error starting service:", slog.String("err", err.Error())) - return - } - - if err := svc.WaitStop(ctx); err != nil { - slog.Error("Error waiting for service interruption:", slog.String("err", err.Error())) - } + return smtp2shoutrrr.NewSMTPServer(config).Start(ctx) } diff --git a/config.go b/config.go index 232fd41..7fc4ffb 100644 --- a/config.go +++ b/config.go @@ -1,11 +1,60 @@ package smtp2shoutrrr import ( + "bytes" + "errors" + "fmt" "log/slog" "net/url" + "os" "strings" + + "github.com/pelletier/go-toml/v2" ) +// LoadConfig reads the TOML configuration at path, applies its defaults and +// checks that it can actually deliver something. +func LoadConfig(path string) (*Config, error) { + raw, err := os.ReadFile(path) + if err != nil { + return nil, fmt.Errorf("reading config file %q: %w", path, err) + } + + var config Config + if err := toml.Unmarshal(raw, &config); err != nil { + return nil, fmt.Errorf("decoding config file %q: %w", path, err) + } + + warnUnknownKeys(path, raw) + + config.SetDefaults() + + if err := config.Validate(); err != nil { + return nil, fmt.Errorf("invalid config file %q: %w", path, err) + } + + return &config, nil +} + +// warnUnknownKeys reports keys the decode silently ignored. Rejecting them +// would break configurations that already carry a stray key, but dropping them +// without a word hides the mistypes ([[Recipient]], Adresses) that otherwise +// produce a server which accepts mail and forwards none of it. +func warnUnknownKeys(path string, raw []byte) { + var discard Config + + var strict *toml.StrictMissingError + if !errors.As(toml.NewDecoder(bytes.NewReader(raw)).DisallowUnknownFields().Decode(&discard), &strict) { + return + } + + for i := range strict.Errors { + slog.Warn("ignoring unknown key in config file", + slog.String("path", path), + slog.String("key", strings.Join(strict.Errors[i].Key(), "."))) + } +} + type Config struct { Port int Username string @@ -30,6 +79,33 @@ func (c *Config) SetDefaults() { } } +// Validate rejects configuration that would leave the server accepting mail it +// can never forward. A TOML decode ignores unknown keys, so a mistyped table or +// field name ([[Recipient]], Adresses) otherwise yields a server that answers +// 250 OK to everything and sends nothing. +func (c *Config) Validate() error { + for i := range c.Recipients { + r := &c.Recipients[i] + if len(r.Addresses) == 0 { + return fmt.Errorf("recipient %d has no Addresses", i) + } + if len(r.GetTargetURLs()) == 0 { + return fmt.Errorf("recipient %d (%s) has no usable Targets", + i, strings.Join(r.Addresses, ",")) + } + } + + if c.CatchAll != nil && len(c.CatchAll.GetTargetURLs()) == 0 { + return errors.New("CatchAll has no usable Targets") + } + + if len(c.Recipients) == 0 && c.CatchAll == nil { + return errors.New("no Recipients and no CatchAll configured") + } + + return nil +} + type ConfigRecipient struct { Addresses []string // email addresses Target string // deprecated: use Targets instead @@ -63,6 +139,13 @@ func (cr *ConfigRecipient) GetTargetURLs() []*url.URL { slog.String("err", err.Error())) continue // Skip invalid URLs } + // url.Parse accepts almost any string, so the scheme is what + // actually distinguishes a shoutrrr target from a typo. + if parsedURL.Scheme == "" { + slog.Error("shoutrrr target URL has no scheme", + slog.String("target", target)) + continue + } cr.targetURLs = append(cr.targetURLs, parsedURL) } } diff --git a/config_test.go b/config_test.go new file mode 100644 index 0000000..f5a1820 --- /dev/null +++ b/config_test.go @@ -0,0 +1,156 @@ +package smtp2shoutrrr + +import ( + "bytes" + "log/slog" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/require" +) + +func writeConfig(t *testing.T, contents string) string { + t.Helper() + + path := filepath.Join(t.TempDir(), "config.toml") + require.NoError(t, os.WriteFile(path, []byte(contents), 0o600)) + + return path +} + +func TestLoadConfig(t *testing.T) { + path := writeConfig(t, ` +Port = 2525 +Username = "user" +Password = "secret" + +[[Recipients]] +Addresses = ["user@example.com"] +Targets = ["ntfy://ntfy.sh/topic"] + +[[Recipients]] +Addresses = ["legacy@example.com"] +Target = "ntfy://ntfy.sh/legacy" + +[CatchAll] +Targets = ["ntfy://ntfy.sh/catch-all"] +`) + + config, err := LoadConfig(path) + require.NoError(t, err) + + require.Equal(t, 2525, config.Port) + require.Equal(t, "user", config.Username) + require.Equal(t, "secret", config.Password) + + require.Len(t, config.Recipients, 2) + require.Equal(t, []string{"user@example.com"}, config.Recipients[0].Addresses) + require.Equal(t, []string{"ntfy://ntfy.sh/topic"}, config.Recipients[0].Targets) + require.Equal(t, "ntfy://ntfy.sh/legacy", config.Recipients[1].Target) + + require.NotNil(t, config.CatchAll) + require.Equal(t, []string{"ntfy://ntfy.sh/catch-all"}, config.CatchAll.Targets) +} + +func TestLoadConfigAppliesDefaults(t *testing.T) { + path := writeConfig(t, ` +[[Recipients]] +Addresses = ["user@example.com"] +Targets = ["ntfy://ntfy.sh/topic"] +`) + + config, err := LoadConfig(path) + require.NoError(t, err) + + require.Equal(t, 11125, config.Port) + require.Equal(t, "username", config.Username) + require.Equal(t, "password", config.Password) + require.Nil(t, config.CatchAll) +} + +func TestLoadConfigMissingFile(t *testing.T) { + _, err := LoadConfig(filepath.Join(t.TempDir(), "does-not-exist.toml")) + require.Error(t, err) +} + +func TestLoadConfigInvalidTOML(t *testing.T) { + _, err := LoadConfig(writeConfig(t, "Port = ")) + require.Error(t, err) +} + +func TestLoadConfigRejectsUnusableConfigurations(t *testing.T) { + // A TOML decode ignores unknown keys, so each of these parses cleanly and + // would otherwise start a server that forwards nothing. + for name, contents := range map[string]string{ + "mistyped recipients table": ` +[[Recipient]] +Addresses = ["user@example.com"] +Targets = ["ntfy://ntfy.sh/topic"] +`, + "mistyped addresses key": ` +[[Recipients]] +Adresses = ["user@example.com"] +Targets = ["ntfy://ntfy.sh/topic"] +`, + "recipient without targets": ` +[[Recipients]] +Addresses = ["user@example.com"] +`, + "recipient with only unparseable targets": ` +[[Recipients]] +Addresses = ["user@example.com"] +Targets = ["://invalid-url"] +`, + "recipient whose target is not a URL at all": ` +[[Recipients]] +Addresses = ["user@example.com"] +Targets = ["this is not a url at all"] +`, + "catch-all without targets": ` +[CatchAll] +Addresses = ["user@example.com"] +`, + "empty configuration": ` +Port = 2525 +`, + } { + t.Run(name, func(t *testing.T) { + _, err := LoadConfig(writeConfig(t, contents)) + require.Error(t, err) + }) + } +} + +func TestLoadConfigAcceptsCatchAllOnly(t *testing.T) { + config, err := LoadConfig(writeConfig(t, ` +[CatchAll] +Targets = ["ntfy://ntfy.sh/catch-all"] +`)) + require.NoError(t, err) + require.Empty(t, config.Recipients) + require.NotNil(t, config.CatchAll) +} + +// A mistyped table name is silently dropped by the decoder. It is only fatal +// when nothing else is configured, so with a catch-all present the warning is +// the sole signal that the block was ignored. +func TestLoadConfigWarnsAboutIgnoredKeys(t *testing.T) { + var logged bytes.Buffer + restore := slog.Default() + slog.SetDefault(slog.New(slog.NewTextHandler(&logged, &slog.HandlerOptions{Level: slog.LevelWarn}))) + t.Cleanup(func() { slog.SetDefault(restore) }) + + config, err := LoadConfig(writeConfig(t, ` +[[Recipient]] +Addresses = ["user@example.com"] +Targets = ["ntfy://ntfy.sh/topic"] + +[CatchAll] +Targets = ["ntfy://ntfy.sh/catch-all"] +`)) + require.NoError(t, err) + require.Empty(t, config.Recipients, "the mistyped table is still ignored") + require.Contains(t, logged.String(), "ignoring unknown key in config file") + require.Contains(t, logged.String(), "Recipient") +} diff --git a/go.mod b/go.mod index d04bc12..0892ec1 100644 --- a/go.mod +++ b/go.mod @@ -1,34 +1,27 @@ module git.nakama.town/fmartingr/smtp2shoutrrr -go 1.25.6 +go 1.27 + +toolchain go1.27.1 require ( - git.nakama.town/fmartingr/gotoolkit v0.2.4 github.com/containrrr/shoutrrr v0.8.0 github.com/emersion/go-sasl v0.0.0-20241020182733-b788ff22d5a6 - github.com/emersion/go-smtp v0.24.0 - github.com/stretchr/testify v1.9.0 - golang.org/x/crypto/x509roots/fallback v0.0.0-20250214233241-911360c8a4f4 + github.com/emersion/go-smtp v0.25.0 + github.com/pelletier/go-toml/v2 v2.4.3 + github.com/stretchr/testify v1.12.1 + golang.org/x/crypto/x509roots/fallback v0.0.0-20260902180247-86efde54dc70 ) require ( - github.com/davecgh/go-spew v1.1.1 // indirect - github.com/fatih/color v1.18.0 // indirect - github.com/go-logr/logr v1.4.1 // indirect + github.com/fatih/color v1.19.0 // indirect + github.com/go-logr/logr v1.4.4 // indirect github.com/golang/protobuf v1.5.4 // indirect - github.com/google/go-cmp v0.6.0 // indirect - github.com/kr/pretty v0.3.0 // indirect - github.com/mattn/go-colorable v0.1.14 // indirect - github.com/mattn/go-isatty v0.0.20 // indirect - github.com/pelletier/go-toml/v2 v2.2.3 // indirect - github.com/pmezard/go-difflib v1.0.0 // indirect - github.com/rogpeppe/go-internal v1.8.1 // indirect - golang.org/x/net v0.25.0 // indirect - golang.org/x/sys v0.40.0 // indirect - golang.org/x/text v0.21.0 // indirect - golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d // indirect - gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c // indirect - gopkg.in/yaml.v3 v3.0.1 // indirect + github.com/google/go-cmp v0.7.0 // indirect + github.com/mattn/go-colorable v0.1.15 // indirect + github.com/mattn/go-isatty v0.0.24 // indirect + go.yaml.in/yaml/v3 v3.0.5 // indirect + golang.org/x/net v0.58.0 // indirect + golang.org/x/sys v0.47.0 // indirect + golang.org/x/tools v0.49.0 // indirect ) - -//replace git.nakama.town/fmartingr/gotoolkit => ../gotoolkit diff --git a/go.sum b/go.sum index f92faca..5b3ff23 100644 --- a/go.sum +++ b/go.sum @@ -1,71 +1,48 @@ -git.nakama.town/fmartingr/gotoolkit v0.2.4 h1:mnq15OXzHdtTjr4Ows5WMqQfqHraAnRwIhhuOtuqBlU= -git.nakama.town/fmartingr/gotoolkit v0.2.4/go.mod h1:Ak68T/qEx0xwhB/9hzE+cbQRTAiWre3Tj1VdTK9gKyo= github.com/containrrr/shoutrrr v0.8.0 h1:mfG2ATzIS7NR2Ec6XL+xyoHzN97H8WPjir8aYzJUSec= github.com/containrrr/shoutrrr v0.8.0/go.mod h1:ioyQAyu1LJY6sILuNyKaQaw+9Ttik5QePU8atnAdO2o= -github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= -github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= -github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/emersion/go-sasl v0.0.0-20241020182733-b788ff22d5a6 h1:oP4q0fw+fOSWn3DfFi4EXdT+B+gTtzx8GC9xsc26Znk= github.com/emersion/go-sasl v0.0.0-20241020182733-b788ff22d5a6/go.mod h1:iL2twTeMvZnrg54ZoPDNfJaJaqy0xIQFuBdrLsmspwQ= -github.com/emersion/go-smtp v0.24.0 h1:g6AfoF140mvW0vLNPD/LuCBLEAdlxOjIXqbIkJIS6Wk= -github.com/emersion/go-smtp v0.24.0/go.mod h1:ZtRRkbTyp2XTHCA+BmyTFTrj8xY4I+b4McvHxCU2gsQ= -github.com/fatih/color v1.18.0 h1:S8gINlzdQ840/4pfAwic/ZE0djQEH3wM94VfqLTZcOM= -github.com/fatih/color v1.18.0/go.mod h1:4FelSpRwEGDpQ12mAdzqdOukCy4u8WUtOY6lkT/6HfU= -github.com/go-logr/logr v1.4.1 h1:pKouT5E8xu9zeFC39JXRDukb6JFQPXM5p5I91188VAQ= -github.com/go-logr/logr v1.4.1/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= +github.com/emersion/go-smtp v0.25.0 h1:krfiHrme2JbJYDh0DGuSRbvPpbnQTH/v9CIfPincl1I= +github.com/emersion/go-smtp v0.25.0/go.mod h1:ZtRRkbTyp2XTHCA+BmyTFTrj8xY4I+b4McvHxCU2gsQ= +github.com/fatih/color v1.19.0 h1:Zp3PiM21/9Ld6FzSKyL5c/BULoe/ONr9KlbYVOfG8+w= +github.com/fatih/color v1.19.0/go.mod h1:zNk67I0ZUT1bEGsSGyCZYZNrHuTkJJB+r6Q9VuMi0LE= +github.com/go-logr/logr v1.4.4 h1:tG4xh9yMsRCAiodLVTxyrkzSZ9+o0L1Kg/+cPVcbP/8= +github.com/go-logr/logr v1.4.4/go.mod h1:9T104GzyrTigFIr8wt5mBrctHMim0Nb2HLGrmQ40KvY= github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572 h1:tfuBGBXKqDEevZMzYi5KSi8KkcZtzBcTgAUUtapy0OI= github.com/go-task/slim-sprig v0.0.0-20230315185526-52ccab3ef572/go.mod h1:9Pwr4B2jHnOSGXyyzV8ROjYa2ojvAY6HCGYYfMoC3Ls= github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek= github.com/golang/protobuf v1.5.4/go.mod h1:lnTiLA8Wa4RWRcIUkrtSVa5nRhsEGBg48fD6rSs7xps= -github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= -github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= +github.com/google/go-cmp v0.7.0 h1:wk8382ETsv4JYUZwIsn6YpYiWiBsYLSJiTsyBybVuN8= +github.com/google/go-cmp v0.7.0/go.mod h1:pXiqmnSA92OHEEa9HXL2W4E7lf9JzCmGVUdgjX3N/iU= github.com/google/pprof v0.0.0-20210407192527-94a9f03dee38 h1:yAJXTCF9TqKcTiHJAE8dj7HMvPfh66eeA2JYW7eFpSE= github.com/google/pprof v0.0.0-20210407192527-94a9f03dee38/go.mod h1:kpwsk12EmLew5upagYY7GY0pfYCcupk39gWOCRROcvE= github.com/jarcoal/httpmock v1.3.0 h1:2RJ8GP0IIaWwcC9Fp2BmVi8Kog3v2Hn7VXM3fTd+nuc= github.com/jarcoal/httpmock v1.3.0/go.mod h1:3yb8rc4BI7TCBhFY8ng0gjuLKJNquuDNiPaZjnENuYg= -github.com/kr/pretty v0.1.0/go.mod h1:dAy3ld7l9f0ibDNOQOHHMYYIIbhfbHSm3C4ZsoJORNo= -github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI= -github.com/kr/pretty v0.3.0 h1:WgNl7dwNpEZ6jJ9k1snq4pZsg7DOEN8hP9Xw0Tsjwk0= -github.com/kr/pretty v0.3.0/go.mod h1:640gp4NfQd8pI5XOwp5fnNeVWj67G7CFk/SaSQn7NBk= -github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= -github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= -github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= -github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= -github.com/mattn/go-colorable v0.1.14 h1:9A9LHSqF/7dyVVX6g0U9cwm9pG3kP9gSzcuIPHPsaIE= -github.com/mattn/go-colorable v0.1.14/go.mod h1:6LmQG8QLFO4G5z1gPvYEzlUgJ2wF+stgPZH1UqBm1s8= -github.com/mattn/go-isatty v0.0.20 h1:xfD0iDuEKnDkl03q4limB+vH+GxLEtL/jb4xVJSWWEY= -github.com/mattn/go-isatty v0.0.20/go.mod h1:W+V8PltTTMOvKvAeJH7IuucS94S2C6jfK/D7dTCTo3Y= +github.com/mattn/go-colorable v0.1.15 h1:+u9SLTRGnXv73cEsnsmoZBom+dMU88B2M0aDcWy0/jY= +github.com/mattn/go-colorable v0.1.15/go.mod h1:6LmQG8QLFO4G5z1gPvYEzlUgJ2wF+stgPZH1UqBm1s8= +github.com/mattn/go-isatty v0.0.24 h1:tGZZoVgT/KiqK1c8ocVLeDS8BSWMRd47J3Lbz7vsReI= +github.com/mattn/go-isatty v0.0.24/go.mod h1:nMCL3Zebbrt45jsMDgnfIwz6ydEQApk5oEI3HqDio6A= github.com/onsi/ginkgo/v2 v2.9.2 h1:BA2GMJOtfGAfagzYtrAlufIP0lq6QERkFmHLMLPwFSU= github.com/onsi/ginkgo/v2 v2.9.2/go.mod h1:WHcJJG2dIlcCqVfBAwUCrJxSPFb6v4azBwgxeMeDuts= github.com/onsi/gomega v1.27.6 h1:ENqfyGeS5AX/rlXDd/ETokDz93u0YufY1Pgxuy/PvWE= github.com/onsi/gomega v1.27.6/go.mod h1:PIQNjfQwkP3aQAH7lf7j87O/5FiNr+ZR8+ipb+qQlhg= -github.com/pelletier/go-toml/v2 v2.2.3 h1:YmeHyLY8mFWbdkNWwpr+qIL2bEqT0o95WSdkNHvL12M= -github.com/pelletier/go-toml/v2 v2.2.3/go.mod h1:MfCQTFTvCcUyyvvwm1+G6H/jORL20Xlb6rzQu9GuUkc= -github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e/go.mod h1:pJLUxLENpZxwdsKMEsNbx1VGcRFpLqf3715MtcvvzbA= -github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= -github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= -github.com/rogpeppe/go-internal v1.6.1/go.mod h1:xXDCJY+GAPziupqXw64V24skbSoqbTEfhy4qGm1nDQc= -github.com/rogpeppe/go-internal v1.8.1 h1:geMPLpDpQOgVyCg5z5GoRwLHepNdb71NXb67XFkP+Eg= -github.com/rogpeppe/go-internal v1.8.1/go.mod h1:JeRgkft04UBgHMgCIwADu4Pn6Mtm5d4nPKWu0nJ5d+o= -github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg= -github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= -golang.org/x/crypto/x509roots/fallback v0.0.0-20250214233241-911360c8a4f4 h1:QDiVWrFJ2lyXzr3pJnIREQWR8S7jkjzuWJPJda8Ic8E= -golang.org/x/crypto/x509roots/fallback v0.0.0-20250214233241-911360c8a4f4/go.mod h1:lxN5T34bK4Z/i6cMaU7frUU57VkDXFD4Kamfl/cp9oU= -golang.org/x/net v0.25.0 h1:d/OCCoBEUq33pjydKrGQhw7IlUPI2Oylr+8qLx49kac= -golang.org/x/net v0.25.0/go.mod h1:JkAGAh7GEvH74S6FOH42FLoXpXbE/aqXSrIQjXgsiwM= -golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.40.0 h1:DBZZqJ2Rkml6QMQsZywtnjnnGvHza6BTfYFWY9kjEWQ= -golang.org/x/sys v0.40.0/go.mod h1:OgkHotnGiDImocRcuBABYBEXf8A9a87e/uXjp9XT3ks= -golang.org/x/text v0.21.0 h1:zyQAAkrwaneQ066sspRyJaG9VNi/YJ1NfzcGB3hZ/qo= -golang.org/x/text v0.21.0/go.mod h1:4IBbMaMmOPCJ8SecivzSH54+73PCFmPWxNTLm+vZkEQ= -golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d h1:vU5i/LfpvrRCpgM/VPfJLg5KjxD3E+hfT1SH+d9zLwg= -golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d/go.mod h1:aiJjzUbINMkxbQROHiO6hDPo2LHcIPhhQsa9DLh0yGk= +github.com/pelletier/go-toml/v2 v2.4.3 h1:GTRvJQutkOSftxIFD5xw9aepkYNuPWmVJpffdDPYVpY= +github.com/pelletier/go-toml/v2 v2.4.3/go.mod h1:2gIqNv+qfxSVS7cM2xJQKtLSTLUE9V8t9Stt+h56mCY= +github.com/stretchr/testify v1.12.1 h1:EuwCh5fleGS7H32xRwO3wRGT7DxrDhLAT6FF8MpWDWE= +github.com/stretchr/testify v1.12.1/go.mod h1:MDEgiDPPsNp5cuIrHPPCyornHKgEVbtFUmoNlxoYthg= +go.yaml.in/yaml/v3 v3.0.5 h1:N6y/pJk8buWs9NY5ERU2HSMfm+IuD/OtfdAnq6kESPw= +go.yaml.in/yaml/v3 v3.0.5/go.mod h1:HVTZu1O7/Vkt2N+BFy8Zza+lnLsABggaTM2ZpNIGuKg= +golang.org/x/crypto/x509roots/fallback v0.0.0-20260902180247-86efde54dc70 h1:VwViOGcd7C8/Gs18efVlMxvUCS0un6KRKBPFEBvpUXg= +golang.org/x/crypto/x509roots/fallback v0.0.0-20260902180247-86efde54dc70/go.mod h1:HPze8vhfG6fO06AM+VSvxRm4E3+5Yk375mgrJ5M2z1E= +golang.org/x/net v0.58.0 h1:ynWG7rqYi4ccpTEuPZ2QGWHktVEM9DMCj9yzDE0Q7To= +golang.org/x/net v0.58.0/go.mod h1:YwCddHnFlT7eLQqVprV19OnhLGtc5xOKgE0RyqgfWAU= +golang.org/x/sys v0.47.0 h1:o7XGOvZQCADBQQ4Y7VNq2dRWQR7JmOUW8Kxx4ZsNgWs= +golang.org/x/sys v0.47.0/go.mod h1:4GL1E5IUh+htKOUEOaiffhrAeqysfVGipDYzABqnCmw= +golang.org/x/text v0.41.0 h1:vz/seA0lnX87Othu2f/0L24RcgrXD9/YFTSuGjj3rH8= +golang.org/x/text v0.41.0/go.mod h1:jvf1O8ajNzZqhSrQBPbutR/EB83Cc0CFrezNQIwbb5M= +golang.org/x/tools v0.49.0 h1:3NI7VXzL9+1WZD52Dx2ttoPwD5DWrFGpl9mFZDlmisI= +golang.org/x/tools v0.49.0/go.mod h1:SJNXV9DBKT0UbdttsQjbfJlAE/q+y36++zo3uL3N0Oo= google.golang.org/protobuf v1.33.0 h1:uNO2rsAINq/JlFpSdYEKIZ0uKD/R9cpdv0T+yoGwGmI= google.golang.org/protobuf v1.33.0/go.mod h1:c6P6GXX6sHbq/GpV6MGZEdwhWPcYBgnhAHhKbcUYpos= -gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= -gopkg.in/check.v1 v1.0.0-20180628173108-788fd7840127/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= -gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= -gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= -gopkg.in/errgo.v2 v2.1.0/go.mod h1:hNsd1EY+bozCKY1Ytp96fpM3vjJbqLJn88ws8XvfDNI= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/server.go b/server.go index 7262576..21b648b 100644 --- a/server.go +++ b/server.go @@ -2,48 +2,155 @@ package smtp2shoutrrr import ( "context" + "errors" "fmt" "log/slog" + "net" + "sync" "time" - "git.nakama.town/fmartingr/gotoolkit/model" "github.com/emersion/go-smtp" ) -var _ model.Server = (*smtpServer)(nil) +const ( + readTimeout = 10 * time.Second + writeTimeout = 10 * time.Second -type smtpServer struct { + // defaultShutdownTimeout bounds the drain of in-flight sessions. It must + // stay well clear of readTimeout: an idle client is only dropped once its + // read deadline expires, so a budget near readTimeout would report an + // ordinary drain as a failed shutdown. + defaultShutdownTimeout = readTimeout + 20*time.Second +) + +type Server struct { backend *smtp.Server + + // shutdownTimeout is a field rather than the constant so tests can drive a + // real budget overrun without waiting out readTimeout. + shutdownTimeout time.Duration + + mu sync.Mutex + listener net.Listener + addr string } -func (s *smtpServer) IsEnabled() bool { - return true +// Addr reports the address the server is bound to, which is only known after +// Start has bound it — relevant when the configured port is 0. +func (s *Server) Addr() string { + s.mu.Lock() + defer s.mu.Unlock() + return s.addr } -func (s *smtpServer) Start(_ context.Context) error { - slog.Info("Started SMTP server", slog.String("addr", s.backend.Addr)) - return s.backend.ListenAndServe() +func (s *Server) setListener(l net.Listener) { + s.mu.Lock() + defer s.mu.Unlock() + s.listener = l + s.addr = l.Addr().String() } -func (s *smtpServer) Stop(ctx context.Context) error { +func (s *Server) closeListener() { + s.mu.Lock() + l := s.listener + s.listener = nil + s.mu.Unlock() + + if l != nil { + _ = l.Close() + } +} + +// Start binds the configured address and serves connections until ctx is +// cancelled or Stop is called, then drains in-flight sessions. It returns nil +// on a clean shutdown. +func (s *Server) Start(ctx context.Context) error { + // Binding here rather than inside ListenAndServe keeps the listener under + // our control: go-smtp's Shutdown only closes listeners Serve has already + // registered, so a stop that arrives first would otherwise leave Accept + // running forever. + l, err := net.Listen("tcp", s.backend.Addr) + if err != nil { + return fmt.Errorf("listen on %q: %w", s.backend.Addr, err) + } + s.setListener(l) + + slog.Info("Started SMTP server", slog.String("addr", l.Addr().String())) + + served := make(chan error, 1) + go func() { + served <- s.backend.Serve(l) + }() + + select { + case err := <-served: + // Serve gave up on its own; still release the listener and drain any + // sessions it left running before reporting why. + s.closeListener() + s.drain(context.WithoutCancel(ctx)) + return err + case <-ctx.Done(): + } + + // Stop accepting and let Serve return before draining. go-smtp registers + // each accepted session on a WaitGroup that Shutdown waits on from another + // goroutine, so an Accept still in flight would Add to that WaitGroup + // concurrently with the Wait — a data race, and a session the drain then + // fails to wait for. + s.closeListener() + if err := <-served; err != nil && !errors.Is(err, net.ErrClosed) { + return err + } + + s.drain(context.WithoutCancel(ctx)) + + return nil +} + +// drain stops the backend and waits for in-flight sessions to finish. Running +// past the budget is still an intentional stop, so it is logged rather than +// reported: turning it into an error exits the process non-zero on a routine +// signal. +func (s *Server) drain(ctx context.Context) { + shutdownCtx, cancel := context.WithTimeout(ctx, s.shutdownTimeout) + defer cancel() + + // Shutdown also closes the listener, which we have already closed, so its + // close error is expected rather than a sign of a session still running. + if err := s.Stop(shutdownCtx); err != nil && + !errors.Is(err, smtp.ErrServerClosed) && !errors.Is(err, net.ErrClosed) { + slog.Warn("SMTP server stopped with sessions still in flight", + slog.String("err", err.Error())) + } +} + +// Stop shuts the server down and releases its listener. A Server is single +// use: the backend cannot be restarted once stopped, so a later Start binds +// the address and then immediately gives it up. +func (s *Server) Stop(ctx context.Context) error { slog.Info("Stopping SMTP server") - return s.backend.Shutdown(ctx) + + err := s.backend.Shutdown(ctx) + s.closeListener() + + return err } -func NewSMTPServer(config *Config) model.Server { +func NewSMTPServer(config *Config) *Server { be := &Backend{ config: config, } smtpBackend := smtp.NewServer(be) smtpBackend.Addr = fmt.Sprintf(":%d", config.Port) - smtpBackend.WriteTimeout = 10 * time.Second - smtpBackend.ReadTimeout = 10 * time.Second + smtpBackend.WriteTimeout = writeTimeout + smtpBackend.ReadTimeout = readTimeout smtpBackend.MaxMessageBytes = 1024 * 1024 smtpBackend.MaxRecipients = 50 smtpBackend.AllowInsecureAuth = true - return &smtpServer{ - backend: smtpBackend, + return &Server{ + backend: smtpBackend, + shutdownTimeout: defaultShutdownTimeout, } } diff --git a/server_test.go b/server_test.go new file mode 100644 index 0000000..bebf864 --- /dev/null +++ b/server_test.go @@ -0,0 +1,205 @@ +package smtp2shoutrrr + +import ( + "context" + "net" + "testing" + "time" + + "github.com/stretchr/testify/require" +) + +// loopbackAddr rewrites the bound address to a dialable loopback address. +// The host stays "localhost" because smtp.PlainAuth refuses to authenticate +// when the dialed host differs from the one it was constructed with. It +// reports "" rather than failing the test, because it runs inside +// require.Eventually conditions, which execute on their own goroutine where +// t.FailNow would deadlock instead of reporting. +func loopbackAddr(s *Server) string { + _, port, err := net.SplitHostPort(s.Addr()) + if err != nil { + return "" + } + return net.JoinHostPort("localhost", port) +} + +func canDial(addr string) bool { + if addr == "" { + return false + } + conn, err := net.DialTimeout("tcp", addr, 200*time.Millisecond) + if err != nil { + return false + } + return conn.Close() == nil +} + +// startServer starts srv on an ephemeral port and blocks until it is bound, +// returning the dial address and the channel Start returns on. Readiness is +// the bind rather than a probe connection: the address is set before Serve +// runs, connections queue in the backlog either way, and a probe that opens +// and drops a connection only adds accept churn and server-side log noise. +func startServer(t *testing.T, srv *Server) (string, context.CancelFunc, <-chan error) { + t.Helper() + + ctx, cancel := context.WithCancel(context.Background()) + t.Cleanup(cancel) + + stopped := make(chan error, 1) + go func() { + stopped <- srv.Start(ctx) + }() + + require.Eventually(t, func() bool { + return srv.Addr() != "" || len(stopped) > 0 + }, 5*time.Second, 10*time.Millisecond, "server never started listening") + + // Report why Start gave up rather than letting it look like a slow bind. + select { + case err := <-stopped: + require.NoError(t, err, "server failed to start") + t.Fatal("server stopped before it began serving") + default: + } + + return loopbackAddr(srv), cancel, stopped +} + +func testConfig() *Config { + return &Config{Port: 0, Username: "testuser", Password: "testpass"} +} + +func TestServerShutsDownOnContextCancel(t *testing.T) { + srv := NewSMTPServer(testConfig()) + addr, cancel, stopped := startServer(t, srv) + + cancel() + + select { + case err := <-stopped: + require.NoError(t, err) + case <-time.After(defaultShutdownTimeout + time.Second): + t.Fatal("server did not shut down after context cancellation") + } + + require.False(t, canDial(addr), "listener should be released once Start returns") +} + +func TestServerStartReturnsListenError(t *testing.T) { + blocker, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + t.Cleanup(func() { _ = blocker.Close() }) + + _, port, err := net.SplitHostPort(blocker.Addr().String()) + require.NoError(t, err) + + config := testConfig() + config.Port = mustAtoi(t, port) + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + t.Cleanup(cancel) + + require.Error(t, NewSMTPServer(config).Start(ctx)) +} + +// Regression: go-smtp's Shutdown only closes listeners Serve has registered. +// When the stop won that race, Accept kept running and Start never returned — +// a SIGTERM during startup left a process that only SIGKILL could clear. +func TestServerStartReturnsWhenCancelledBeforeListening(t *testing.T) { + srv := NewSMTPServer(testConfig()) + + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + stopped := make(chan error, 1) + go func() { + stopped <- srv.Start(ctx) + }() + + select { + case err := <-stopped: + require.NoError(t, err) + case <-time.After(10 * time.Second): + t.Fatal("Start did not return when its context was cancelled before the listener registered") + } + + require.False(t, canDial(loopbackAddr(srv)), "listener should not be left accepting connections") +} + +// Regression: an idle client is only dropped once its read deadline expires, +// so the drain lasts about ReadTimeout. With the shutdown budget set to the +// same value, an ordinary stop surfaced as "context deadline exceeded" and the +// command turned that into a non-zero exit. +func TestServerDrainsIdleConnectionWithoutError(t *testing.T) { + srv := NewSMTPServer(testConfig()) + srv.backend.ReadTimeout = 300 * time.Millisecond + + addr, cancel, stopped := startServer(t, srv) + + conn, err := net.DialTimeout("tcp", addr, time.Second) + require.NoError(t, err) + t.Cleanup(func() { _ = conn.Close() }) + + cancel() + + select { + case err := <-stopped: + require.NoError(t, err, "an intentional shutdown must not report an error") + case <-time.After(defaultShutdownTimeout + time.Second): + t.Fatal("server did not drain the idle connection") + } +} + +// The drain waits out a client's read deadline, so the budget needs real room +// above it. A strict inequality is not enough: a budget one millisecond over +// readTimeout is the same coin flip as one equal to it. +func TestShutdownBudgetLeavesMarginOverReadTimeout(t *testing.T) { + require.GreaterOrEqual(t, defaultShutdownTimeout, readTimeout+10*time.Second) +} + +// Regression: overrunning the drain budget is still an intentional stop. +// Returning that deadline as an error made the command exit non-zero on a +// routine SIGTERM. Driven with a real overrun — a client that stays connected +// past a deliberately short budget — rather than asserted on the constant. +func TestServerReportsNoErrorWhenDrainOverrunsBudget(t *testing.T) { + srv := NewSMTPServer(testConfig()) + // Long enough that the session outlives the budget, short enough that it + // does not outlive the test binary. + srv.backend.ReadTimeout = 3 * time.Second + srv.shutdownTimeout = 300 * time.Millisecond + + addr, cancel, stopped := startServer(t, srv) + + conn, err := net.DialTimeout("tcp", addr, time.Second) + require.NoError(t, err) + t.Cleanup(func() { _ = conn.Close() }) + + // Wait for the greeting so the session is registered and genuinely in + // flight when the drain starts. + _ = conn.SetReadDeadline(time.Now().Add(2 * time.Second)) + buf := make([]byte, 64) + _, err = conn.Read(buf) + require.NoError(t, err) + + start := time.Now() + cancel() + + select { + case err := <-stopped: + require.NoError(t, err, "a drain that outruns its budget is still an intentional stop") + case <-time.After(srv.backend.ReadTimeout + 5*time.Second): + t.Fatal("Start did not return after the drain budget expired") + } + + require.Less(t, time.Since(start), srv.backend.ReadTimeout, + "Start should return on the budget, not wait out the read deadline") +} + +func mustAtoi(t *testing.T, s string) int { + t.Helper() + + n, err := net.LookupPort("tcp", s) + require.NoError(t, err) + + return n +}