From 2e832520e68efa1cfed3c4b3a456021ed0cd348f Mon Sep 17 00:00:00 2001 From: James Houlahan Date: Wed, 1 Feb 2023 13:11:57 +0100 Subject: [PATCH] chore: Remove panics from SetGluonDir --- internal/bridge/bridge.go | 28 +++++++++---------- internal/bridge/imap.go | 18 ++++++------ internal/bridge/settings.go | 55 +++++++++++-------------------------- internal/bridge/smtp.go | 18 ++++++------ internal/events/serve.go | 17 ++++++++++++ 5 files changed, 66 insertions(+), 70 deletions(-) diff --git a/internal/bridge/bridge.go b/internal/bridge/bridge.go index 634181d7..699cf551 100644 --- a/internal/bridge/bridge.go +++ b/internal/bridge/bridge.go @@ -371,14 +371,12 @@ func (bridge *Bridge) init(tlsReporter TLSReporter) error { }) }) - // Attempt to lazy load users when triggered. Only load once. - var loaded bool - bridge.goLoad = bridge.tasks.Trigger(func(ctx context.Context) { - if loaded { - logrus.Debug("All users are already loaded, skipping") - return - } + // We need to load users before we can start the IMAP and SMTP servers. + // We must only start the servers once. + var once sync.Once + // Attempt to load users from the vault when triggered. + bridge.goLoad = bridge.tasks.Trigger(func(ctx context.Context) { logrus.Info("Loading users") if err := bridge.loadUsers(ctx); err != nil { @@ -389,15 +387,15 @@ func (bridge *Bridge) init(tlsReporter TLSReporter) error { bridge.publish(events.AllUsersLoaded{}) // Once all users have been loaded, start the bridge's IMAP and SMTP servers. - if err := bridge.serveIMAP(); err != nil { - logrus.WithError(err).Error("Failed to start IMAP server") - } + once.Do(func() { + if err := bridge.serveIMAP(); err != nil { + logrus.WithError(err).Error("Failed to start IMAP server") + } - if err := bridge.serveSMTP(); err != nil { - logrus.WithError(err).Error("Failed to start SMTP server") - } - - loaded = true + if err := bridge.serveSMTP(); err != nil { + logrus.WithError(err).Error("Failed to start SMTP server") + } + }) }) defer bridge.goLoad() diff --git a/internal/bridge/imap.go b/internal/bridge/imap.go index 358da57f..8141cbf9 100644 --- a/internal/bridge/imap.go +++ b/internal/bridge/imap.go @@ -46,7 +46,7 @@ const ( ) func (bridge *Bridge) serveIMAP() error { - if port, err := func() (int, error) { + port, err := func() (int, error) { if bridge.imapServer == nil { return 0, fmt.Errorf("no IMAP server instance running") } @@ -69,19 +69,21 @@ func (bridge *Bridge) serveIMAP() error { } return getPort(imapListener.Addr()), nil - }(); err != nil { + }() + + if err != nil { bridge.publish(events.IMAPServerError{ Error: err, }) return err - } else { - bridge.publish(events.IMAPServerReady{ - Port: port, - }) - - return nil } + + bridge.publish(events.IMAPServerReady{ + Port: port, + }) + + return nil } func (bridge *Bridge) restartIMAP() error { diff --git a/internal/bridge/settings.go b/internal/bridge/settings.go index 1e0b1a0f..667406b2 100644 --- a/internal/bridge/settings.go +++ b/internal/bridge/settings.go @@ -131,26 +131,21 @@ func (bridge *Bridge) SetGluonDir(ctx context.Context, newGluonDir string) error return fmt.Errorf("new gluon dir is the same as the old one") } - if err := bridge.stopEventLoops(); err != nil { - return err + if err := bridge.closeIMAP(context.Background()); err != nil { + return fmt.Errorf("failed to close IMAP: %w", err) } - defer func() { - err := bridge.startEventLoops(ctx) - if err != nil { - panic(err) - } - }() if err := bridge.moveGluonCacheDir(currentGluonDir, newGluonDir); err != nil { logrus.WithError(err).Error("failed to move GluonCacheDir") + if err := bridge.vault.SetGluonDir(currentGluonDir); err != nil { - panic(err) + return fmt.Errorf("failed to revert GluonCacheDir: %w", err) } } gluonDataDir, err := bridge.GetGluonDataDir() if err != nil { - panic(fmt.Errorf("failed to get Gluon Database directory: %w", err)) + return fmt.Errorf("failed to get Gluon Database directory: %w", err) } imapServer, err := newIMAPServer( @@ -166,11 +161,21 @@ func (bridge *Bridge) SetGluonDir(ctx context.Context, newGluonDir string) error bridge.uidValidityGenerator, ) if err != nil { - panic(fmt.Errorf("failed to create new IMAP server: %w", err)) + return fmt.Errorf("failed to create new IMAP server: %w", err) } bridge.imapServer = imapServer + for _, user := range bridge.users { + if err := bridge.addIMAPUser(ctx, user); err != nil { + return fmt.Errorf("failed to add users to new IMAP server: %w", err) + } + } + + if err := bridge.serveIMAP(); err != nil { + return fmt.Errorf("failed to serve IMAP: %w", err) + } + return nil }, bridge.usersLock) } @@ -192,34 +197,6 @@ func (bridge *Bridge) moveGluonCacheDir(oldGluonDir, newGluonDir string) error { return nil } -func (bridge *Bridge) stopEventLoops() error { - if err := bridge.closeIMAP(context.Background()); err != nil { - return fmt.Errorf("failed to close IMAP: %w", err) - } - - if err := bridge.closeSMTP(); err != nil { - return fmt.Errorf("failed to close SMTP: %w", err) - } - return nil -} - -func (bridge *Bridge) startEventLoops(ctx context.Context) error { - for _, user := range bridge.users { - if err := bridge.addIMAPUser(ctx, user); err != nil { - return fmt.Errorf("failed to add users to new IMAP server: %w", err) - } - } - - if err := bridge.serveIMAP(); err != nil { - panic(fmt.Errorf("failed to serve IMAP: %w", err)) - } - - if err := bridge.serveSMTP(); err != nil { - panic(fmt.Errorf("failed to serve SMTP: %w", err)) - } - return nil -} - func (bridge *Bridge) GetProxyAllowed() bool { return bridge.vault.GetProxyAllowed() } diff --git a/internal/bridge/smtp.go b/internal/bridge/smtp.go index 0ecf269d..b6d95158 100644 --- a/internal/bridge/smtp.go +++ b/internal/bridge/smtp.go @@ -32,7 +32,7 @@ import ( ) func (bridge *Bridge) serveSMTP() error { - if port, err := func() (int, error) { + port, err := func() (int, error) { logrus.Info("Starting SMTP server") smtpListener, err := newListener(bridge.vault.GetSMTPPort(), bridge.vault.GetSMTPSSL(), bridge.tlsConfig) @@ -53,19 +53,21 @@ func (bridge *Bridge) serveSMTP() error { } return getPort(smtpListener.Addr()), nil - }(); err != nil { + }() + + if err != nil { bridge.publish(events.SMTPServerError{ Error: err, }) return err - } else { - bridge.publish(events.SMTPServerReady{ - Port: port, - }) - - return nil } + + bridge.publish(events.SMTPServerReady{ + Port: port, + }) + + return nil } func (bridge *Bridge) restartSMTP() error { diff --git a/internal/events/serve.go b/internal/events/serve.go index ca6c3500..a8f866b1 100644 --- a/internal/events/serve.go +++ b/internal/events/serve.go @@ -1,3 +1,20 @@ +// Copyright (c) 2023 Proton AG +// +// This file is part of Proton Mail Bridge. +// +// Proton Mail Bridge is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Proton Mail Bridge is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Proton Mail Bridge. If not, see . + package events import "fmt"