From e46fff0aa59d9adc82062091cbe6b6e33168fc01 Mon Sep 17 00:00:00 2001 From: James Houlahan Date: Wed, 8 Jul 2020 09:20:31 +0200 Subject: [PATCH 1/4] Error when selecting should unselect --- server/cmd_auth.go | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/server/cmd_auth.go b/server/cmd_auth.go index 9d847d29..e7b658b8 100644 --- a/server/cmd_auth.go +++ b/server/cmd_auth.go @@ -24,8 +24,21 @@ func (cmd *Select) Handle(conn Conn) error { return ErrNotAuthenticated } + // As per RFC1730#6.3.1, + // The SELECT command automatically deselects any + // currently selected mailbox before attempting the new selection. + // Consequently, if a mailbox is selected and a SELECT command that + // fails is attempted, no mailbox is selected. + // Thus if this GetMailbox call fails to find a mailbox, this is not an + // error and we should simply unselect. + // For example, some clients (e.g. Apple Mail) perform SELECT "" when the + // server doesn't announce the UNSELECT capability. mbox, err := ctx.User.GetMailbox(cmd.Mailbox) - if err != nil { + if errors.Is(err, backend.ErrNoSuchMailbox) { + ctx.Mailbox = nil + ctx.MailboxReadOnly = false + return nil + } else if err != nil { return err } @@ -149,7 +162,7 @@ func (cmd *List) Handle(conn Conn) error { go (func() { done <- conn.WriteResp(res) // Make sure to drain the channel. - for _ = range ch { + for range ch { } })() From 53f0d0dc74984b1eb2856b682aa1be7334f1e1cf Mon Sep 17 00:00:00 2001 From: James Houlahan Date: Wed, 8 Jul 2020 09:41:20 +0200 Subject: [PATCH 2/4] Always unselect on any SELECT failure --- server/cmd_auth.go | 32 +++++++++++++++++--------------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/server/cmd_auth.go b/server/cmd_auth.go index e7b658b8..39d6e8d4 100644 --- a/server/cmd_auth.go +++ b/server/cmd_auth.go @@ -18,28 +18,30 @@ type Select struct { commands.Select } -func (cmd *Select) Handle(conn Conn) error { +func (cmd *Select) Handle(conn Conn) (err error) { ctx := conn.Context() - if ctx.User == nil { - return ErrNotAuthenticated - } // As per RFC1730#6.3.1, // The SELECT command automatically deselects any // currently selected mailbox before attempting the new selection. // Consequently, if a mailbox is selected and a SELECT command that // fails is attempted, no mailbox is selected. - // Thus if this GetMailbox call fails to find a mailbox, this is not an - // error and we should simply unselect. + // Thus if SELECT fails for whatever reason we should unselect. // For example, some clients (e.g. Apple Mail) perform SELECT "" when the // server doesn't announce the UNSELECT capability. + defer func() { + if err != nil { + ctx.Mailbox = nil + ctx.MailboxReadOnly = false + } + }() + + if ctx.User == nil { + return ErrNotAuthenticated + } mbox, err := ctx.User.GetMailbox(cmd.Mailbox) - if errors.Is(err, backend.ErrNoSuchMailbox) { - ctx.Mailbox = nil - ctx.MailboxReadOnly = false - return nil - } else if err != nil { - return err + if err != nil { + return } items := []imap.StatusItem{ @@ -49,15 +51,15 @@ func (cmd *Select) Handle(conn Conn) error { status, err := mbox.Status(items) if err != nil { - return err + return } ctx.Mailbox = mbox ctx.MailboxReadOnly = cmd.ReadOnly || status.ReadOnly res := &responses.Select{Mailbox: status} - if err := conn.WriteResp(res); err != nil { - return err + if err = conn.WriteResp(res); err != nil { + return } var code imap.StatusRespCode = imap.CodeReadWrite From 639332e0c9733db96c0b66179f8206bb35564b1f Mon Sep 17 00:00:00 2001 From: James Houlahan Date: Wed, 8 Jul 2020 10:18:45 +0200 Subject: [PATCH 3/4] Always unselect before progressing with SELECT --- server/cmd_auth.go | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/server/cmd_auth.go b/server/cmd_auth.go index 39d6e8d4..ff0f76f2 100644 --- a/server/cmd_auth.go +++ b/server/cmd_auth.go @@ -26,15 +26,10 @@ func (cmd *Select) Handle(conn Conn) (err error) { // currently selected mailbox before attempting the new selection. // Consequently, if a mailbox is selected and a SELECT command that // fails is attempted, no mailbox is selected. - // Thus if SELECT fails for whatever reason we should unselect. // For example, some clients (e.g. Apple Mail) perform SELECT "" when the // server doesn't announce the UNSELECT capability. - defer func() { - if err != nil { - ctx.Mailbox = nil - ctx.MailboxReadOnly = false - } - }() + ctx.Mailbox = nil + ctx.MailboxReadOnly = false if ctx.User == nil { return ErrNotAuthenticated From 57302646448a8d1a013e4f706ac8c294a51c5bcd Mon Sep 17 00:00:00 2001 From: James Houlahan Date: Wed, 8 Jul 2020 10:29:11 +0200 Subject: [PATCH 4/4] Revert to unnamed return values in Select.Handle --- server/cmd_auth.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/server/cmd_auth.go b/server/cmd_auth.go index ff0f76f2..8ebf68bf 100644 --- a/server/cmd_auth.go +++ b/server/cmd_auth.go @@ -18,7 +18,7 @@ type Select struct { commands.Select } -func (cmd *Select) Handle(conn Conn) (err error) { +func (cmd *Select) Handle(conn Conn) error { ctx := conn.Context() // As per RFC1730#6.3.1, @@ -36,7 +36,7 @@ func (cmd *Select) Handle(conn Conn) (err error) { } mbox, err := ctx.User.GetMailbox(cmd.Mailbox) if err != nil { - return + return err } items := []imap.StatusItem{ @@ -46,15 +46,15 @@ func (cmd *Select) Handle(conn Conn) (err error) { status, err := mbox.Status(items) if err != nil { - return + return err } ctx.Mailbox = mbox ctx.MailboxReadOnly = cmd.ReadOnly || status.ReadOnly res := &responses.Select{Mailbox: status} - if err = conn.WriteResp(res); err != nil { - return + if err := conn.WriteResp(res); err != nil { + return err } var code imap.StatusRespCode = imap.CodeReadWrite