diff --git a/internal/imapx/conn.go b/internal/imapx/conn.go new file mode 100644 index 0000000..57f359c --- /dev/null +++ b/internal/imapx/conn.go @@ -0,0 +1,30 @@ +package imapx + +import ( + "net" + "time" +) + +// idleReadTimeout bounds how long a connection may go WITHOUT receiving any +// bytes from the server before the read is aborted. It is an *idle* timeout, +// not a total deadline: every successful read pushes it forward, so a slow but +// live transfer never trips it — only a genuinely dead/mute socket does. This +// is what stops an account from wedging in "running" forever when a server +// accepts a command (e.g. a large FETCH) and then goes silent. +var idleReadTimeout = 120 * time.Second + +// idleConn wraps a net.Conn and arms a fresh read deadline before every Read. +// It lives beneath any TLS layer (the raw TCP conn), so the deadline governs +// the actual blocking network read regardless of encryption. +type idleConn struct { + net.Conn + timeout time.Duration +} + +func (c *idleConn) Read(b []byte) (int, error) { + if c.timeout > 0 { + // Ignore the error: a closed conn will surface it from Read below. + _ = c.Conn.SetReadDeadline(time.Now().Add(c.timeout)) + } + return c.Conn.Read(b) +} diff --git a/internal/imapx/conn_test.go b/internal/imapx/conn_test.go new file mode 100644 index 0000000..528525a --- /dev/null +++ b/internal/imapx/conn_test.go @@ -0,0 +1,60 @@ +package imapx + +import ( + "io" + "net" + "testing" + "time" + + "context" +) + +// A server that sends the IMAP greeting, accepts one command, then goes silent +// forever must NOT wedge the client: the idle read-timeout has to abort the +// blocked read so the command returns an error instead of hanging. This is the +// exact production failure (account stuck in "running" with zero progress). +func TestConnectIdleReadTimeoutUnwedgesSilentServer(t *testing.T) { + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listen: %v", err) + } + defer ln.Close() + + go func() { + c, err := ln.Accept() + if err != nil { + return + } + defer c.Close() + _, _ = io.WriteString(c, "* OK IMAP4rev2 ready\r\n") + // Read the LOGIN command bytes, then never answer. + buf := make([]byte, 512) + _, _ = c.Read(buf) + // Block until the client gives up and closes the connection. + _, _ = c.Read(buf) + }() + + old := idleReadTimeout + idleReadTimeout = 150 * time.Millisecond + defer func() { idleReadTimeout = old }() + + port := ln.Addr().(*net.TCPAddr).Port + ep := Endpoint{Host: "127.0.0.1", Port: port, TLSMode: "plain"} + + c, err := Connect(context.Background(), ep) + if err != nil { + t.Fatalf("Connect: %v", err) + } + + done := make(chan error, 1) + go func() { done <- c.Login("user", "pass").Wait() }() + + select { + case err := <-done: + if err == nil { + t.Fatal("expected an error from Login against a silent server, got nil") + } + case <-time.After(3 * time.Second): + t.Fatal("Login did not return: idle read-timeout was not enforced (connection wedged)") + } +} diff --git a/internal/imapx/dial.go b/internal/imapx/dial.go index 8de1b20..459d973 100644 --- a/internal/imapx/dial.go +++ b/internal/imapx/dial.go @@ -4,6 +4,7 @@ import ( "context" "crypto/tls" "fmt" + "net" "time" "github.com/emersion/go-imap/v2/imapclient" @@ -17,23 +18,59 @@ type Endpoint struct { func (e Endpoint) addr() string { return fmt.Sprintf("%s:%d", e.Host, e.Port) } -func dialOnce(ep Endpoint) (*imapclient.Client, error) { +// dialTimeout bounds establishing the TCP connection (matches go-imap's own +// default). The subsequent idleReadTimeout governs reads once connected. +const dialTimeout = 30 * time.Second + +// dialOnce establishes one connection and returns a ready *Client whose reads +// are guarded by idleReadTimeout. Unlike imapclient.Dial*, the underlying TCP +// conn is wrapped in idleConn so a server that stops responding mid-command +// unblocks the read instead of hanging forever. ctx bounds the TCP dial. +func dialOnce(ctx context.Context, ep Endpoint) (*imapclient.Client, error) { + d := &net.Dialer{Timeout: dialTimeout} + raw, err := d.DialContext(ctx, "tcp", ep.addr()) + if err != nil { + return nil, err + } + conn := &idleConn{Conn: raw, timeout: idleReadTimeout} + switch ep.TLSMode { case "ssl": - return imapclient.DialTLS(ep.addr(), &imapclient.Options{ - TLSConfig: &tls.Config{ServerName: ep.Host}, - }) + // NextProtos mirrors imapclient.DialTLS's ALPN advertisement. + tlsConn := tls.Client(conn, &tls.Config{ServerName: ep.Host, NextProtos: []string{"imap"}}) + if err := tlsConn.HandshakeContext(ctx); err != nil { + _ = conn.Close() + return nil, err + } + c := imapclient.New(tlsConn, nil) + return waitGreeting(c) case "starttls": - return imapclient.DialStartTLS(ep.addr(), &imapclient.Options{ - TLSConfig: &tls.Config{ServerName: ep.Host}, - }) + opts := &imapclient.Options{TLSConfig: &tls.Config{ServerName: ep.Host}} + c, err := imapclient.NewStartTLS(conn, opts) + if err != nil { + return nil, err + } + return c, nil case "plain": - return imapclient.DialInsecure(ep.addr(), nil) + c := imapclient.New(conn, nil) + return waitGreeting(c) default: + _ = conn.Close() return nil, fmt.Errorf("unknown tls_mode %q", ep.TLSMode) } } +// waitGreeting blocks for the server's initial greeting so a mute server is +// caught at connect time (bounded by idleReadTimeout) rather than at the first +// command. NewStartTLS already awaits the greeting during its STARTTLS upgrade. +func waitGreeting(c *imapclient.Client) (*imapclient.Client, error) { + if err := c.WaitGreeting(); err != nil { + _ = c.Close() + return nil, err + } + return c, nil +} + func Connect(ctx context.Context, ep Endpoint) (*imapclient.Client, error) { const attempts = 3 var lastErr error @@ -41,7 +78,7 @@ func Connect(ctx context.Context, ep Endpoint) (*imapclient.Client, error) { if err := ctx.Err(); err != nil { return nil, err } - c, err := dialOnce(ep) + c, err := dialOnce(ctx, ep) if err == nil { return c, nil }