diff --git a/Makefile b/Makefile
index 9b823d82..685be056 100644
--- a/Makefile
+++ b/Makefile
@@ -253,6 +253,9 @@ bench:
coverage: test
go tool cover -html=/tmp/coverage.out -o=coverage.html
+integration-test-bridge:
+ ${MAKE} -C test test-bridge
+
mocks:
mockgen --package mocks github.com/ProtonMail/proton-bridge/internal/users Locator,PanicHandler,CredentialsStorer,StoreMaker > internal/users/mocks/mocks.go
mockgen --package mocks github.com/ProtonMail/proton-bridge/pkg/listener Listener > internal/users/mocks/listener_mocks.go
diff --git a/internal/imap/server.go b/internal/imap/server.go
index 4a07c0df..70161af4 100644
--- a/internal/imap/server.go
+++ b/internal/imap/server.go
@@ -32,8 +32,8 @@ import (
"github.com/ProtonMail/proton-bridge/internal/events"
"github.com/ProtonMail/proton-bridge/internal/imap/id"
"github.com/ProtonMail/proton-bridge/internal/imap/uidplus"
+ "github.com/ProtonMail/proton-bridge/internal/serverutil"
"github.com/ProtonMail/proton-bridge/pkg/listener"
- "github.com/ProtonMail/proton-bridge/pkg/ports"
"github.com/emersion/go-imap"
imapappendlimit "github.com/emersion/go-imap-appendlimit"
imapidle "github.com/emersion/go-imap-idle"
@@ -116,18 +116,20 @@ func NewIMAPServer(panicHandler panicHandler, debugClient, debugServer bool, por
return server
}
-// Starts the server.
-func (s *imapServer) ListenAndServe() {
- go s.monitorDisconnectedUsers()
- go s.monitorInternetConnection()
+func (s *imapServer) HandlePanic() { s.panicHandler.HandlePanic() }
+func (s *imapServer) IsRunning() bool { return s.isRunning.Load().(bool) }
+func (s *imapServer) Port() int { return s.port }
- // When starting the Bridge, we don't want to retry to notify user
- // quickly about the issue. Very probably retry will not help anyway.
- s.listenAndServe(0)
+// ListenAndServe starts the server and keeps it on based on internet
+// availability.
+func (s *imapServer) ListenAndServe() {
+ serverutil.ListenAndServe(s, s.eventListener)
}
-func (s *imapServer) listenAndServe(retries int) {
- if s.isRunning.Load().(bool) {
+// ListenRetryAndServe will start listener. If port is occupied it will try
+// again after coolDown time. Once listener is OK it will serve.
+func (s *imapServer) ListenRetryAndServe(retries int, retryAfter time.Duration) {
+ if s.IsRunning() {
return
}
s.isRunning.Store(true)
@@ -139,8 +141,8 @@ func (s *imapServer) listenAndServe(retries int) {
s.isRunning.Store(false)
if retries > 0 {
l.WithError(err).WithField("retries", retries).Warn("IMAP listener failed")
- time.Sleep(15 * time.Second)
- s.listenAndServe(retries - 1)
+ time.Sleep(retryAfter)
+ s.ListenRetryAndServe(retries-1, retryAfter)
return
}
@@ -157,7 +159,7 @@ func (s *imapServer) listenAndServe(retries int) {
// Serve returns error every time, even after closing the server.
// User shouldn't be notified about error if server shouldn't be running,
// but it should in case it was not closed by `s.Close()`.
- if err != nil && s.isRunning.Load().(bool) {
+ if err != nil && s.IsRunning() {
s.isRunning.Store(false)
l.WithError(err).Error("IMAP server failed")
s.eventListener.Emit(events.ErrorEvent, "IMAP failed: "+err.Error())
@@ -170,7 +172,7 @@ func (s *imapServer) listenAndServe(retries int) {
// Stops the server.
func (s *imapServer) Close() {
- if !s.isRunning.Load().(bool) {
+ if !s.IsRunning() {
return
}
s.isRunning.Store(false)
@@ -181,62 +183,16 @@ func (s *imapServer) Close() {
}
}
-func (s *imapServer) monitorInternetConnection() {
- on := make(chan string)
- s.eventListener.Add(events.InternetOnEvent, on)
- off := make(chan string)
- s.eventListener.Add(events.InternetOffEvent, off)
-
- for {
- var expectedIsPortFree bool
- select {
- case <-on:
- go func() {
- defer s.panicHandler.HandlePanic()
- // We had issues on Mac that from time to time something
- // blocked our port for a bit after we closed IMAP server
- // due to connection issues.
- // Restart always helped, so we do retry to not bother user.
- s.listenAndServe(10)
- }()
- expectedIsPortFree = false
- case <-off:
- s.Close()
- expectedIsPortFree = true
- }
-
- start := time.Now()
- for {
- if ports.IsPortFree(s.port) == expectedIsPortFree {
- break
- }
- // Safety stop if something went wrong.
- if time.Since(start) > 15*time.Second {
- log.WithField("expectedIsPortFree", expectedIsPortFree).Warn("Server start/stop check timeouted")
- break
- }
- time.Sleep(100 * time.Millisecond)
- }
- }
-}
-
-func (s *imapServer) monitorDisconnectedUsers() {
- ch := make(chan string)
- s.eventListener.Add(events.CloseConnectionEvent, ch)
-
- for address := range ch {
- address := address
- log.Info("Disconnecting all open IMAP connections for ", address)
- disconnectUser := func(conn imapserver.Conn) {
- connUser := conn.Context().User
- if connUser != nil && strings.EqualFold(connUser.Username(), address) {
- if err := conn.Close(); err != nil {
- log.WithError(err).Error("Failed to close the connection")
- }
+func (s *imapServer) DisconnectUser(address string) {
+ log.Info("Disconnecting all open IMAP connections for ", address)
+ s.server.ForEachConn(func(conn imapserver.Conn) {
+ connUser := conn.Context().User
+ if connUser != nil && strings.EqualFold(connUser.Username(), address) {
+ if err := conn.Close(); err != nil {
+ log.WithError(err).Error("Failed to close the connection")
}
}
- s.server.ForEachConn(disconnectUser)
- }
+ })
}
// connListener sets debug loggers on server containing fields with local
diff --git a/internal/imap/server_test.go b/internal/imap/server_test.go
index d8e497c0..1653cbd2 100644
--- a/internal/imap/server_test.go
+++ b/internal/imap/server_test.go
@@ -20,48 +20,33 @@ package imap
import (
"fmt"
"testing"
- "time"
"github.com/ProtonMail/proton-bridge/internal/bridge"
"github.com/ProtonMail/proton-bridge/internal/config/useragent"
- "github.com/ProtonMail/proton-bridge/internal/events"
- "github.com/ProtonMail/proton-bridge/pkg/listener"
- "github.com/ProtonMail/proton-bridge/pkg/ports"
+ "github.com/ProtonMail/proton-bridge/internal/serverutil/mocks"
imapserver "github.com/emersion/go-imap/server"
"github.com/stretchr/testify/require"
)
-type testPanicHandler struct{}
-
-func (ph *testPanicHandler) HandlePanic() {}
-
func TestIMAPServerTurnOffAndOnAgain(t *testing.T) {
- panicHandler := &testPanicHandler{}
+ r := require.New(t)
+ ts := mocks.NewTestServer(12345)
- eventListener := listener.New()
-
- port := ports.FindFreePortFrom(12345)
server := imapserver.New(nil)
- server.Addr = fmt.Sprintf("%v:%v", bridge.Host, port)
+ server.Addr = fmt.Sprintf("%v:%v", bridge.Host, ts.WantPort)
s := &imapServer{
- panicHandler: panicHandler,
+ panicHandler: ts.PanicHandler,
server: server,
- eventListener: eventListener,
+ port: ts.WantPort,
+ eventListener: ts.EventListener,
userAgent: useragent.New(),
}
s.isRunning.Store(false)
+ r.True(ts.IsPortFree())
+
go s.ListenAndServe()
- time.Sleep(5 * time.Second)
- require.False(t, ports.IsPortFree(port))
-
- eventListener.Emit(events.InternetOffEvent, "")
- time.Sleep(10 * time.Second)
- require.True(t, ports.IsPortFree(port))
-
- eventListener.Emit(events.InternetOnEvent, "")
- time.Sleep(10 * time.Second)
- require.False(t, ports.IsPortFree(port))
+ ts.RunServerTests(r)
}
diff --git a/internal/serverutil/mocks/server.go b/internal/serverutil/mocks/server.go
new file mode 100644
index 00000000..8e60c33c
--- /dev/null
+++ b/internal/serverutil/mocks/server.go
@@ -0,0 +1,150 @@
+// Copyright (c) 2021 Proton Technologies AG
+//
+// This file is part of ProtonMail Bridge.
+//
+// ProtonMail 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.
+//
+// ProtonMail 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 ProtonMail Bridge. If not, see .
+
+package mocks
+
+import (
+ "fmt"
+ "net/http"
+ "sync/atomic"
+ "time"
+
+ "github.com/ProtonMail/proton-bridge/internal/events"
+ "github.com/ProtonMail/proton-bridge/pkg/listener"
+ "github.com/ProtonMail/proton-bridge/pkg/ports"
+ "github.com/sirupsen/logrus"
+ "github.com/stretchr/testify/require"
+)
+
+type DummyPanicHandler struct{}
+
+func (ph *DummyPanicHandler) HandlePanic() {}
+
+type TestServer struct {
+ PanicHandler *DummyPanicHandler
+ WantPort int
+ EventListener listener.Listener
+
+ isRunning atomic.Value
+ srv *http.Server
+}
+
+func NewTestServer(port int) *TestServer {
+ s := &TestServer{
+ PanicHandler: &DummyPanicHandler{},
+ EventListener: listener.New(),
+ WantPort: ports.FindFreePortFrom(port),
+ }
+ s.isRunning.Store(false)
+ return s
+}
+
+func (s *TestServer) IsPortFree() bool {
+ return true
+}
+
+func (s *TestServer) IsPortOccupied() bool {
+ return true
+}
+
+func (s *TestServer) Emit(event string, try, iEvt int) int {
+ // Emit has separate go routine so it is needed to wait here to
+ // prevent event race condition.
+ time.Sleep(100 * time.Millisecond)
+ iEvt++
+ s.EventListener.Emit(event, fmt.Sprintf("%d:%d", try, iEvt))
+ return iEvt
+}
+
+func (s *TestServer) HandlePanic() {}
+func (s *TestServer) DisconnectUser(string) {}
+func (s *TestServer) Port() int { return s.WantPort }
+func (s *TestServer) IsRunning() bool { return s.isRunning.Load().(bool) }
+
+func (s *TestServer) ListenRetryAndServe(retries int, retryAfter time.Duration) {
+ if s.isRunning.Load().(bool) {
+ return
+ }
+ s.isRunning.Store(true)
+
+ // There can be delay when starting server
+ time.Sleep(200 * time.Millisecond)
+
+ s.srv = &http.Server{
+ Addr: fmt.Sprintf("127.0.0.1:%d", s.WantPort),
+ }
+
+ err := s.srv.ListenAndServe()
+ if err != nil {
+ s.isRunning.Store(false)
+ if retries > 0 {
+ time.Sleep(retryAfter)
+ s.ListenRetryAndServe(retries-1, retryAfter)
+ }
+ }
+
+ if s.IsRunning() {
+ logrus.Error("Not serving but isRunning is true")
+ s.isRunning.Store(false)
+ }
+}
+
+func (s *TestServer) Close() {
+ if !s.isRunning.Load().(bool) {
+ return
+ }
+ s.isRunning.Store(false)
+
+ // There can be delay when stopping server
+ time.Sleep(200 * time.Millisecond)
+ if err := s.srv.Close(); err != nil {
+ logrus.WithError(err).Error("Closing dummy server")
+ }
+}
+
+func (s *TestServer) RunServerTests(r *require.Assertions) {
+ // NOTE About choosing tick durations:
+ // In order to avoid ticks to synchronise and cause occasional race
+ // condition we choose the tick duration around 100ms but not exactly
+ // to have large common multiple.
+ r.Eventually(s.IsPortOccupied, 5*time.Second, 97*time.Millisecond)
+
+ // There was an issue where second time we were not able to restore server.
+ for try := 0; try < 3; try++ {
+ i := s.Emit(events.InternetOffEvent, try, 0)
+ r.Eventually(s.IsPortFree, 10*time.Second, 99*time.Millisecond, "signal off try %d : %d", try, i)
+
+ i = s.Emit(events.InternetOnEvent, try, i)
+ i = s.Emit(events.InternetOffEvent, try, i)
+ i = s.Emit(events.InternetOffEvent, try, i)
+ i = s.Emit(events.InternetOffEvent, try, i)
+ i = s.Emit(events.InternetOffEvent, try, i)
+ i = s.Emit(events.InternetOnEvent, try, i)
+ i = s.Emit(events.InternetOnEvent, try, i)
+ i = s.Emit(events.InternetOffEvent, try, i)
+ // Wait a bit longer if needed to process all events
+ r.Eventually(s.IsPortFree, 20*time.Second, 101*time.Millisecond, "again signal off number %d : %d", try, i)
+
+ i = s.Emit(events.InternetOnEvent, try, i)
+ r.Eventually(s.IsPortOccupied, 10*time.Second, 103*time.Millisecond, "signal on number %d : %d", try, i)
+
+ i = s.Emit(events.InternetOffEvent, try, i)
+ i = s.Emit(events.InternetOnEvent, try, i)
+ i = s.Emit(events.InternetOnEvent, try, i)
+ r.Eventually(s.IsPortOccupied, 10*time.Second, 107*time.Millisecond, "again signal on number %d : %d", try, i)
+ }
+}
diff --git a/internal/serverutil/server.go b/internal/serverutil/server.go
new file mode 100644
index 00000000..1a07805d
--- /dev/null
+++ b/internal/serverutil/server.go
@@ -0,0 +1,132 @@
+// Copyright (c) 2021 Proton Technologies AG
+//
+// This file is part of ProtonMail Bridge.
+//
+// ProtonMail 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.
+//
+// ProtonMail 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 ProtonMail Bridge. If not, see .
+
+package serverutil
+
+import (
+ "time"
+
+ "github.com/ProtonMail/proton-bridge/internal/events"
+ "github.com/ProtonMail/proton-bridge/pkg/listener"
+ "github.com/ProtonMail/proton-bridge/pkg/ports"
+ "github.com/sirupsen/logrus"
+)
+
+// Server which can handle disconnected users and lost internet connection.
+type Server interface {
+ HandlePanic()
+ DisconnectUser(string)
+ ListenRetryAndServe(int, time.Duration)
+ Close()
+ Port() int
+ IsRunning() bool
+}
+
+func monitorDisconnectedUsers(s Server, l listener.Listener) {
+ ch := make(chan string)
+ l.Add(events.CloseConnectionEvent, ch)
+ for address := range ch {
+ s.DisconnectUser(address)
+ }
+}
+
+func redirectInternetEventsToOneChannel(l listener.Listener) (isInternetOn chan bool) {
+ on := make(chan string)
+ l.Add(events.InternetOnEvent, on)
+ off := make(chan string)
+ l.Add(events.InternetOffEvent, off)
+
+ // Redirect two channels into one. When select was used the algorithm
+ // first read all on channels and then read all off channels.
+ isInternetOn = make(chan bool, 20)
+ go func() {
+ for {
+ logrus.WithField("try", <-on).Trace("Internet ON")
+ isInternetOn <- true
+ }
+ }()
+
+ go func() {
+ for {
+ logrus.WithField("try", <-off).Trace("Internet OFF")
+ isInternetOn <- false
+ }
+ }()
+ return
+}
+
+const (
+ recheckPortAfter = 50 * time.Millisecond
+ stopPortChecksAfter = 15 * time.Second
+ retryListnerAfter = 5 * time.Second
+)
+
+func monitorInternetConnection(s Server, l listener.Listener) {
+ isInternetOn := redirectInternetEventsToOneChannel(l)
+ for {
+ var expectedIsPortFree bool
+ if <-isInternetOn {
+ if s.IsRunning() {
+ continue
+ }
+ go func() {
+ defer s.HandlePanic()
+ // We had issues on Mac that from time to time something
+ // blocked our port for a bit after we closed IMAP server
+ // due to connection issues.
+ // Restart always helped, so we do retry to not bother user.
+ s.ListenRetryAndServe(10, retryListnerAfter)
+ }()
+ expectedIsPortFree = false
+ } else {
+ if !s.IsRunning() {
+ continue
+ }
+ s.Close()
+ expectedIsPortFree = true
+ }
+ start := time.Now()
+ for {
+ isPortFree := ports.IsPortFree(s.Port())
+ logrus.
+ WithField("port", s.Port()).
+ WithField("isFree", isPortFree).
+ WithField("wantToBeFree", expectedIsPortFree).
+ Trace("Check port")
+ if isPortFree == expectedIsPortFree {
+ break
+ }
+ // Safety stop if something went wrong.
+ if time.Since(start) > stopPortChecksAfter {
+ logrus.WithField("expectedIsPortFree", expectedIsPortFree).Warn("Server start/stop check timeouted")
+ break
+ }
+ time.Sleep(recheckPortAfter)
+ }
+ }
+}
+
+// ListenAndServe starts the server and keeps it on based on internet
+// availability. It also monitors and disconnect users if requested.
+func ListenAndServe(s Server, l listener.Listener) {
+ go monitorDisconnectedUsers(s, l)
+ go monitorInternetConnection(s, l)
+
+ // When starting the Bridge, we don't want to retry to notify user
+ // quickly about the issue. Very probably retry will not help anyway.
+ s.ListenRetryAndServe(0, 0)
+}
diff --git a/internal/serverutil/server_test.go b/internal/serverutil/server_test.go
new file mode 100644
index 00000000..fa04a06f
--- /dev/null
+++ b/internal/serverutil/server_test.go
@@ -0,0 +1,35 @@
+// Copyright (c) 2021 Proton Technologies AG
+//
+// This file is part of ProtonMail Bridge.
+//
+// ProtonMail 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.
+//
+// ProtonMail 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 ProtonMail Bridge. If not, see .
+
+package serverutil
+
+import (
+ "testing"
+
+ "github.com/ProtonMail/proton-bridge/internal/serverutil/mocks"
+ "github.com/stretchr/testify/require"
+)
+
+func TestServerTurnOffAndOnAgain(t *testing.T) {
+ r := require.New(t)
+ s := mocks.NewTestServer(12321)
+
+ r.True(s.IsPortFree())
+
+ go ListenAndServe(s, s.EventListener)
+ s.RunServerTests(r)
+}
diff --git a/internal/smtp/server.go b/internal/smtp/server.go
index 245506f4..ecb9f2f6 100644
--- a/internal/smtp/server.go
+++ b/internal/smtp/server.go
@@ -26,31 +26,28 @@ import (
"github.com/ProtonMail/proton-bridge/internal/bridge"
"github.com/ProtonMail/proton-bridge/internal/events"
+ "github.com/ProtonMail/proton-bridge/internal/serverutil"
"github.com/ProtonMail/proton-bridge/pkg/listener"
- "github.com/ProtonMail/proton-bridge/pkg/ports"
"github.com/emersion/go-sasl"
goSMTP "github.com/emersion/go-smtp"
"github.com/sirupsen/logrus"
)
-type smtpServer struct {
+// Server is Bridge SMTP server implementation.
+type Server struct {
panicHandler panicHandler
+ backend goSMTP.Backend
server *goSMTP.Server
eventListener listener.Listener
+ debug bool
useSSL bool
port int
+ tls *tls.Config
isRunning atomic.Value
}
// NewSMTPServer returns an SMTP server configured with the given options.
-func NewSMTPServer(panicHandler panicHandler, debug bool, port int, useSSL bool, tls *tls.Config, smtpBackend goSMTP.Backend, eventListener listener.Listener) *smtpServer { //nolint[golint]
- s := goSMTP.NewServer(smtpBackend)
- s.Addr = fmt.Sprintf("%v:%v", bridge.Host, port)
- s.TLSConfig = tls
- s.Domain = bridge.Host
- s.AllowInsecureAuth = true
- s.MaxLineLength = 2 << 16
-
+func NewSMTPServer(panicHandler panicHandler, debug bool, port int, useSSL bool, tls *tls.Config, smtpBackend goSMTP.Backend, eventListener listener.Listener) *Server {
if debug {
fmt.Println("THE LOG WILL CONTAIN **DECRYPTED** MESSAGE DATA")
log.Warning("================================================")
@@ -58,13 +55,38 @@ func NewSMTPServer(panicHandler panicHandler, debug bool, port int, useSSL bool,
log.Warning("================================================")
}
+ server := &Server{
+ panicHandler: panicHandler,
+ backend: smtpBackend,
+ eventListener: eventListener,
+ debug: debug,
+ useSSL: useSSL,
+ port: port,
+ tls: tls,
+ }
+ server.isRunning.Store(false)
+ return server
+}
+
+func (s *Server) HandlePanic() { s.panicHandler.HandlePanic() }
+func (s *Server) IsRunning() bool { return s.isRunning.Load().(bool) }
+func (s *Server) Port() int { return s.port }
+
+func newGoSMTPServer(debug bool, smtpBackend goSMTP.Backend, port int, tls *tls.Config) *goSMTP.Server {
+ newSMTP := goSMTP.NewServer(smtpBackend)
+ newSMTP.Addr = fmt.Sprintf("%v:%v", bridge.Host, port)
+ newSMTP.TLSConfig = tls
+ newSMTP.Domain = bridge.Host
+ newSMTP.AllowInsecureAuth = true
+ newSMTP.MaxLineLength = 1 << 16
+
if debug {
- s.Debug = logrus.
+ newSMTP.Debug = logrus.
WithField("pkg", "smtp/server").
WriterLevel(logrus.DebugLevel)
}
- s.EnableAuth(sasl.Login, func(conn *goSMTP.Conn) sasl.Server {
+ newSMTP.EnableAuth(sasl.Login, func(conn *goSMTP.Conn) sasl.Server {
return sasl.NewLoginServer(func(address, password string) error {
user, err := conn.Server().Backend.Login(nil, address, password)
if err != nil {
@@ -75,36 +97,26 @@ func NewSMTPServer(panicHandler panicHandler, debug bool, port int, useSSL bool,
return nil
})
})
-
- server := &smtpServer{
- panicHandler: panicHandler,
- server: s,
- eventListener: eventListener,
- useSSL: useSSL,
- port: port,
- }
- server.isRunning.Store(false)
- return server
+ return newSMTP
}
-// Starts the server.
-func (s *smtpServer) ListenAndServe() {
- go s.monitorDisconnectedUsers()
- go s.monitorInternetConnection()
-
- // When starting the Bridge, we don't want to retry to notify user
- // quickly about the issue. Very probably retry will not help anyway.
- s.listenAndServe(0)
+// ListenAndServe starts the server and keeps it on based on internet
+// availability.
+func (s *Server) ListenAndServe() {
+ serverutil.ListenAndServe(s, s.eventListener)
}
-func (s *smtpServer) listenAndServe(retries int) {
- if s.isRunning.Load().(bool) {
+func (s *Server) ListenRetryAndServe(retries int, retryAfter time.Duration) {
+ if s.IsRunning() {
return
}
s.isRunning.Store(true)
+ s.server = newGoSMTPServer(s.debug, s.backend, s.port, s.tls)
+
l := log.WithField("useSSL", s.useSSL).WithField("address", s.server.Addr)
l.Info("SMTP server is starting")
+
var listener net.Listener
var err error
if s.useSSL {
@@ -112,12 +124,13 @@ func (s *smtpServer) listenAndServe(retries int) {
} else {
listener, err = net.Listen("tcp", s.server.Addr)
}
+ l.WithError(err).Debug("Listener for SMTP created")
if err != nil {
s.isRunning.Store(false)
if retries > 0 {
l.WithError(err).WithField("retries", retries).Warn("SMTP listener failed")
- time.Sleep(15 * time.Second)
- s.listenAndServe(retries - 1)
+ time.Sleep(retryAfter)
+ s.ListenRetryAndServe(retries-1, retryAfter)
return
}
@@ -127,85 +140,49 @@ func (s *smtpServer) listenAndServe(retries int) {
}
err = s.server.Serve(listener)
+ l.WithError(err).Debug("GoSMTP not serving")
// Serve returns error every time, even after closing the server.
// User shouldn't be notified about error if server shouldn't be running,
// but it should in case it was not closed by `s.Close()`.
- if err != nil && s.isRunning.Load().(bool) {
+ if err != nil && s.IsRunning() {
s.isRunning.Store(false)
l.WithError(err).Error("SMTP server failed")
s.eventListener.Emit(events.ErrorEvent, "SMTP failed: "+err.Error())
return
}
- defer s.server.Close() //nolint[errcheck]
+ defer func() {
+ // Go SMTP server instance can be closed only once. Otherwise
+ // it returns an error. The error is not export therefore we
+ // will check the string value.
+ err := s.server.Close()
+ if err == nil || err.Error() != "smtp: server already closed" {
+ l.WithError(err).Warn("Server was not closed")
+ }
+ }()
- l.Info("SMTP server stopped")
+ l.Info("SMTP server closed")
}
-// Stops the server.
-func (s *smtpServer) Close() {
- if !s.isRunning.Load().(bool) {
+// Close stops the server.
+func (s *Server) Close() {
+ if !s.IsRunning() {
return
}
s.isRunning.Store(false)
if err := s.server.Close(); err != nil {
- log.WithError(err).Error("Failed to close the connection")
+ log.WithError(err).Error("Cannot close the server")
}
}
-func (s *smtpServer) monitorInternetConnection() {
- on := make(chan string)
- s.eventListener.Add(events.InternetOnEvent, on)
- off := make(chan string)
- s.eventListener.Add(events.InternetOffEvent, off)
-
- for {
- var expectedIsPortFree bool
- select {
- case <-on:
- go func() {
- defer s.panicHandler.HandlePanic()
- // We had issues on Mac that from time to time something
- // blocked our port for a bit after we closed IMAP server
- // due to connection issues.
- // Restart always helped, so we do retry to not bother user.
- s.listenAndServe(10)
- }()
- expectedIsPortFree = false
- case <-off:
- s.Close()
- expectedIsPortFree = true
- }
-
- start := time.Now()
- for {
- if ports.IsPortFree(s.port) == expectedIsPortFree {
- break
+func (s *Server) DisconnectUser(address string) {
+ log.Info("Disconnecting all open SMTP connections for ", address)
+ s.server.ForEachConn(func(conn *goSMTP.Conn) {
+ connUser := conn.Session()
+ if connUser != nil {
+ if err := conn.Close(); err != nil {
+ log.WithError(err).Error("Failed to close the connection")
}
- // Safety stop if something went wrong.
- if time.Since(start) > 15*time.Second {
- log.WithField("expectedIsPortFree", expectedIsPortFree).Warn("Server start/stop check timeouted")
- break
- }
- time.Sleep(100 * time.Millisecond)
}
- }
-}
-
-func (s *smtpServer) monitorDisconnectedUsers() {
- ch := make(chan string)
- s.eventListener.Add(events.CloseConnectionEvent, ch)
-
- for address := range ch {
- log.Info("Disconnecting all open SMTP connections for ", address)
- disconnectUser := func(conn *goSMTP.Conn) {
- connUser := conn.Session()
- if connUser != nil {
- if err := conn.Close(); err != nil {
- log.WithError(err).Error("Failed to close the connection")
- }
- }
- }
- s.server.ForEachConn(disconnectUser)
- }
+ })
}
diff --git a/internal/smtp/server_test.go b/internal/smtp/server_test.go
index d3f421c9..3342d21c 100644
--- a/internal/smtp/server_test.go
+++ b/internal/smtp/server_test.go
@@ -18,48 +18,26 @@
package smtp
import (
- "fmt"
"testing"
- "time"
- "github.com/ProtonMail/proton-bridge/internal/bridge"
- "github.com/ProtonMail/proton-bridge/internal/events"
- "github.com/ProtonMail/proton-bridge/pkg/listener"
- "github.com/ProtonMail/proton-bridge/pkg/ports"
- goSMTP "github.com/emersion/go-smtp"
+ "github.com/ProtonMail/proton-bridge/internal/serverutil/mocks"
"github.com/stretchr/testify/require"
)
-type testPanicHandler struct{}
-
-func (ph *testPanicHandler) HandlePanic() {}
-
func TestSMTPServerTurnOffAndOnAgain(t *testing.T) {
- panicHandler := &testPanicHandler{}
+ r := require.New(t)
+ ts := mocks.NewTestServer(12342)
- eventListener := listener.New()
-
- port := ports.FindFreePortFrom(12345)
- server := goSMTP.NewServer(nil)
- server.Addr = fmt.Sprintf("%v:%v", bridge.Host, port)
-
- s := &smtpServer{
- panicHandler: panicHandler,
- server: server,
- eventListener: eventListener,
+ s := &Server{
+ panicHandler: ts.PanicHandler,
+ port: ts.WantPort,
+ eventListener: ts.EventListener,
}
s.isRunning.Store(false)
+ r.True(ts.IsPortFree())
+
go s.ListenAndServe()
- time.Sleep(5 * time.Second)
- require.False(t, ports.IsPortFree(port))
-
- eventListener.Emit(events.InternetOffEvent, "")
- time.Sleep(10 * time.Second)
- require.True(t, ports.IsPortFree(port))
-
- eventListener.Emit(events.InternetOnEvent, "")
- time.Sleep(10 * time.Second)
- require.False(t, ports.IsPortFree(port))
+ ts.RunServerTests(r)
}
diff --git a/pkg/listener/listener.go b/pkg/listener/listener.go
index 10cb9dde..32d91561 100644
--- a/pkg/listener/listener.go
+++ b/pkg/listener/listener.go
@@ -88,7 +88,9 @@ func (l *listener) Add(eventName string, channel chan<- string) {
l.channels = make(map[string][]chan<- string)
}
+ log := log.WithField("name", eventName).WithField("i", len(l.channels[eventName]))
l.channels[eventName] = append(l.channels[eventName], channel)
+ log.Debug("Added event listner")
}
// Remove removes an event listener.
@@ -123,8 +125,10 @@ func (l *listener) emit(eventName, data string, isReEmit bool) {
if _, ok := l.channels[eventName]; ok {
for i, handler := range l.channels[eventName] {
go func(handler chan<- string, i int) {
+ log := log.WithField("name", eventName).WithField("i", i).WithField("data", data)
+ log.Debug("Send event")
handler <- data
- log.Debugf("emitted %s data %s -> %d", eventName, data, i)
+ log.Debug("Event sent")
}(handler, i)
}
} else if !isReEmit {
diff --git a/test/api_actions_test.go b/test/api_actions_test.go
index 34b9d0e5..a48ad13b 100644
--- a/test/api_actions_test.go
+++ b/test/api_actions_test.go
@@ -26,7 +26,7 @@ import (
func APIActionsFeatureContext(s *godog.Suite) {
s.Step(`^the internet connection is lost$`, theInternetConnectionIsLost)
s.Step(`^the internet connection is restored$`, theInternetConnectionIsRestored)
- s.Step(`^(\d+) seconds pass$`, secondsPass)
+ s.Step(`^(\d+) second[s]? pass$`, secondsPass)
}
func theInternetConnectionIsLost() error {
diff --git a/test/features/bridge/no_internet.feature b/test/features/bridge/no_internet.feature
new file mode 100644
index 00000000..470856b6
--- /dev/null
+++ b/test/features/bridge/no_internet.feature
@@ -0,0 +1,28 @@
+Feature: Servers are closed when no internet
+
+ Scenario: All connection are closed and then restored multiple times
+ Given there is connected user "user"
+ And there is IMAP client "i1" logged in as "user"
+ And there is SMTP client "s1" logged in as "user"
+ When there is no internet connection
+ And 1 second pass
+ Then IMAP client "i1" is logged out
+ And SMTP client "s1" is logged out
+ Given the internet connection is restored
+ And there is IMAP client "i2" logged in as "user"
+ And there is SMTP client "s2" logged in as "user"
+ When IMAP client "i2" gets info of "INBOX"
+ When SMTP client "s2" sends "HELO example.com"
+ Then IMAP response to "i2" is "OK"
+ Then SMTP response to "s2" is "OK"
+ When there is no internet connection
+ And 1 second pass
+ Then IMAP client "i2" is logged out
+ And SMTP client "s2" is logged out
+ Given the internet connection is restored
+ And there is IMAP client "i3" logged in as "user"
+ And there is SMTP client "s3" logged in as "user"
+ When IMAP client "i3" gets info of "INBOX"
+ When SMTP client "s3" sends "HELO example.com"
+ Then IMAP response to "i3" is "OK"
+ Then SMTP response to "s3" is "OK"
diff --git a/test/imap_actions_mailbox_test.go b/test/imap_actions_mailbox_test.go
index ef00f1e7..ed042863 100644
--- a/test/imap_actions_mailbox_test.go
+++ b/test/imap_actions_mailbox_test.go
@@ -30,6 +30,7 @@ func IMAPActionsMailboxFeatureContext(s *godog.Suite) {
s.Step(`^IMAP client lists mailboxes$`, imapClientListsMailboxes)
s.Step(`^IMAP client selects "([^"]*)"$`, imapClientSelects)
s.Step(`^IMAP client gets info of "([^"]*)"$`, imapClientGetsInfoOf)
+ s.Step(`^IMAP client "([^"]*)" gets info of "([^"]*)"$`, imapClientNamedGetsInfoOf)
s.Step(`^IMAP client gets status of "([^"]*)"$`, imapClientGetsStatusOf)
}
@@ -74,8 +75,12 @@ func imapClientSelects(mailboxName string) error {
}
func imapClientGetsInfoOf(mailboxName string) error {
- res := ctx.GetIMAPClient("imap").GetMailboxInfo(mailboxName)
- ctx.SetIMAPLastResponse("imap", res)
+ return imapClientNamedGetsInfoOf("imap", mailboxName)
+}
+
+func imapClientNamedGetsInfoOf(clientName, mailboxName string) error {
+ res := ctx.GetIMAPClient(clientName).GetMailboxInfo(mailboxName)
+ ctx.SetIMAPLastResponse(clientName, res)
return nil
}
diff --git a/test/imap_checks_test.go b/test/imap_checks_test.go
index 5f8ee06c..2604456d 100644
--- a/test/imap_checks_test.go
+++ b/test/imap_checks_test.go
@@ -40,6 +40,8 @@ func IMAPChecksFeatureContext(s *godog.Suite) {
s.Step(`^IMAP client receives update marking message seq "([^"]*)" as unread within (\d+) seconds$`, imapClientReceivesUpdateMarkingMessageSeqAsUnreadWithin)
s.Step(`^IMAP client "([^"]*)" receives update marking message seq "([^"]*)" as unread within (\d+) seconds$`, imapClientNamedReceivesUpdateMarkingMessageSeqAsUnreadWithin)
s.Step(`^IMAP client "([^"]*)" does not receive update for message seq "([^"]*)" within (\d+) seconds$`, imapClientDoesNotReceiveUpdateForMessageSeqWithin)
+ s.Step(`^IMAP client is logged out$`, imapClientIsLoggedOut)
+ s.Step(`^IMAP client "([^"]*)" is logged out$`, imapClientNamedIsLoggedOut)
}
func imapResponseIs(expectedResponse string) error {
@@ -136,3 +138,13 @@ func iterateOverSeqSet(seqSet string, callback func(string)) {
}
}
}
+
+func imapClientIsLoggedOut() error {
+ return imapClientNamedIsLoggedOut("imap")
+}
+
+func imapClientNamedIsLoggedOut(clientName string) error {
+ res := ctx.GetIMAPClient(clientName).SendCommand("CAPABILITY")
+ res.AssertError("read response failed:")
+ return ctx.GetTestingError()
+}
diff --git a/test/mocks/imap_client.go b/test/mocks/imap_client.go
index ea9786d6..8c932619 100644
--- a/test/mocks/imap_client.go
+++ b/test/mocks/imap_client.go
@@ -43,6 +43,9 @@ type IMAPClient struct {
func NewIMAPClient(t TestingT, tag string, imapAddr string) *IMAPClient {
conn, err := net.Dial("tcp", imapAddr)
require.NoError(t, err)
+ if err != nil {
+ return &IMAPClient{}
+ }
response := bufio.NewReader(conn)
// Read first response to opening connection.
diff --git a/test/mocks/imap_response.go b/test/mocks/imap_response.go
index b1e646e0..7c609bb7 100644
--- a/test/mocks/imap_response.go
+++ b/test/mocks/imap_response.go
@@ -117,7 +117,7 @@ func (ir *IMAPResponse) AssertResult(wantResult string) *IMAPResponse {
func (ir *IMAPResponse) AssertError(wantErrMsg string) *IMAPResponse {
ir.wait()
if ir.err == nil {
- a.Fail(ir.t, "Expected error %s", wantErrMsg)
+ a.Fail(ir.t, "Error is nil", "Expected to have %q", wantErrMsg)
} else {
a.Regexp(ir.t, wantErrMsg, ir.err.Error(), "Expected error %s but got %s", wantErrMsg, ir.err)
}
diff --git a/test/mocks/smtp.go b/test/mocks/smtp.go
index 59efb8c6..acd32dbd 100644
--- a/test/mocks/smtp.go
+++ b/test/mocks/smtp.go
@@ -30,6 +30,7 @@ import (
"github.com/ProtonMail/go-rfc5322"
"github.com/pkg/errors"
+ "github.com/sirupsen/logrus"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
@@ -46,6 +47,12 @@ type SMTPClient struct {
func NewSMTPClient(t TestingT, tag, smtpAddr string) *SMTPClient {
conn, err := net.Dial("tcp", smtpAddr)
require.NoError(t, err)
+ if err != nil {
+ return &SMTPClient{}
+ }
+ logrus.WithField("addr", conn.LocalAddr().String()).
+ WithField("tag", tag).
+ Debug("SMTP Dialed")
response := bufio.NewReader(conn)
// Read first response to opening connection.
@@ -85,6 +92,7 @@ func (c *SMTPClient) SendCommands(commands ...string) *SMTPResponse {
message, err := c.response.ReadString('\n')
if err != nil {
smtpResponse.err = fmt.Errorf("read response failed: %v", err)
+ c.debug.printErr(smtpResponse.err.Error() + "\n")
return smtpResponse
}
diff --git a/test/mocks/smtp_response.go b/test/mocks/smtp_response.go
index 8da699a3..26df1cf3 100644
--- a/test/mocks/smtp_response.go
+++ b/test/mocks/smtp_response.go
@@ -34,7 +34,7 @@ func (sr *SMTPResponse) AssertOK() *SMTPResponse {
func (sr *SMTPResponse) AssertError(wantErrMsg string) *SMTPResponse {
if sr.err == nil {
- a.Fail(sr.t, "Expected error %s", wantErrMsg)
+ a.Fail(sr.t, "Error is nil", "Expected to have %q", wantErrMsg)
} else {
a.Regexp(sr.t, wantErrMsg, sr.err.Error(), "Expected error %s but got %s", wantErrMsg, sr.err)
}
diff --git a/test/smtp_actions_test.go b/test/smtp_actions_test.go
index 82e403fe..21a599b7 100644
--- a/test/smtp_actions_test.go
+++ b/test/smtp_actions_test.go
@@ -37,6 +37,7 @@ func SMTPActionsAuthFeatureContext(s *godog.Suite) {
s.Step(`^SMTP client sends message with bcc "([^"]*)"$`, smtpClientSendsMessageWithBCC)
s.Step(`^SMTP client "([^"]*)" sends message with bcc "([^"]*)"$`, smtpClientNamedSendsMessageWithBCC)
s.Step(`^SMTP client sends "([^"]*)"$`, smtpClientSendsCommand)
+ s.Step(`^SMTP client "([^"]*)" sends "([^"]*)"$`, smtpClientNamedSendsCommand)
}
func smtpClientAuthenticates(bddUserID string) error {
@@ -108,9 +109,12 @@ func smtpClientNamedSendsMessageWithBCC(clientID, bcc string, message *gherkin.D
}
func smtpClientSendsCommand(command string) error {
+ return smtpClientNamedSendsCommand("smtp", command)
+}
+func smtpClientNamedSendsCommand(clientName, command string) error {
command = strings.ReplaceAll(command, "\\r", "\r")
command = strings.ReplaceAll(command, "\\n", "\n")
- res := ctx.GetSMTPClient("smtp").SendCommands(command)
- ctx.SetSMTPLastResponse("smtp", res)
+ res := ctx.GetSMTPClient(clientName).SendCommands(command)
+ ctx.SetSMTPLastResponse(clientName, res)
return nil
}
diff --git a/test/smtp_checks_test.go b/test/smtp_checks_test.go
index 44f8e3c3..9cd2e5e2 100644
--- a/test/smtp_checks_test.go
+++ b/test/smtp_checks_test.go
@@ -24,6 +24,8 @@ import (
func SMTPChecksFeatureContext(s *godog.Suite) {
s.Step(`^SMTP response is "([^"]*)"$`, smtpResponseIs)
s.Step(`^SMTP response to "([^"]*)" is "([^"]*)"$`, smtpResponseNamedIs)
+ s.Step(`^SMTP client is logged out`, smtpClientIsLoggedOut)
+ s.Step(`^SMTP client "([^"]*)" is logged out`, smtpClientNamedIsLoggedOut)
}
func smtpResponseIs(expectedResponse string) error {
@@ -39,3 +41,13 @@ func smtpResponseNamedIs(clientID, expectedResponse string) error {
}
return ctx.GetTestingError()
}
+
+func smtpClientIsLoggedOut() error {
+ return smtpClientNamedIsLoggedOut("smtp")
+}
+
+func smtpClientNamedIsLoggedOut(clientName string) error {
+ res := ctx.GetSMTPClient(clientName).SendCommands("HELO loggedOut.com")
+ res.AssertError("read response failed:")
+ return ctx.GetTestingError()
+}