From 3dadad51315ac99ad6b762bfcaf1134b0dd27e60 Mon Sep 17 00:00:00 2001 From: James Houlahan Date: Wed, 12 May 2021 16:29:43 +0200 Subject: [PATCH] GODT-1161: Guarantee order of responses when creating new message --- go.mod | 3 -- go.sum | 7 --- internal/imap/idle/extension.go | 85 +++++++++++++++++++++++++++++++ internal/imap/server.go | 4 +- internal/imap/updates.go | 6 +-- internal/store/mailbox_message.go | 28 +++++++--- 6 files changed, 110 insertions(+), 23 deletions(-) create mode 100644 internal/imap/idle/extension.go diff --git a/go.mod b/go.mod index 7834e12e..205dabf7 100644 --- a/go.mod +++ b/go.mod @@ -27,7 +27,6 @@ require ( github.com/chzyer/test v0.0.0-20180213035817-a1ea475d72b1 // indirect github.com/cucumber/godog v0.8.1 github.com/emersion/go-imap-appendlimit v0.0.0-20190308131241-25671c986a6a - github.com/emersion/go-imap-idle v0.0.0-20200601154248-f05f54664cc4 github.com/emersion/go-imap-move v0.0.0-20190710073258-6e5a51a5b342 github.com/emersion/go-imap-quota v0.0.0-20210203125329-619074823f3c github.com/emersion/go-imap-unselect v0.0.0-20171113212723-b985794e5f26 @@ -59,8 +58,6 @@ require ( github.com/ssor/bom v0.0.0-20170718123548-6386211fdfcf // indirect github.com/stretchr/testify v1.6.1 github.com/therecipe/qt v0.0.0-20200701200531-7f61353ee73e - github.com/therecipe/qt/internal/binding/files/docs/5.12.0 v0.0.0-20200904063919-c0c124a5770d // indirect - github.com/therecipe/qt/internal/binding/files/docs/5.13.0 v0.0.0-20200904063919-c0c124a5770d // indirect github.com/urfave/cli/v2 v2.2.0 github.com/vmihailenco/msgpack/v5 v5.1.3 go.etcd.io/bbolt v1.3.5 diff --git a/go.sum b/go.sum index 3d4d4a44..09da5138 100644 --- a/go.sum +++ b/go.sum @@ -73,8 +73,6 @@ github.com/dustin/go-humanize v1.0.0/go.mod h1:HtrtbFcZ19U5GC7JDqmcUSB87Iq5E25Kn github.com/eknkc/amber v0.0.0-20171010120322-cdade1c07385/go.mod h1:0vRUJqYpeSZifjYj7uP3BG/gKcuzL9xWVV/Y+cK33KM= github.com/emersion/go-imap-appendlimit v0.0.0-20190308131241-25671c986a6a h1:bMdSPm6sssuOFpIaveu3XGAijMS3Tq2S3EqFZmZxidc= github.com/emersion/go-imap-appendlimit v0.0.0-20190308131241-25671c986a6a/go.mod h1:ikgISoP7pRAolqsVP64yMteJa2FIpS6ju88eBT6K1yQ= -github.com/emersion/go-imap-idle v0.0.0-20200601154248-f05f54664cc4 h1:/JIALzmCduf5o8TWJSiOBzTb9+R0SChwElUrJLlp2po= -github.com/emersion/go-imap-idle v0.0.0-20200601154248-f05f54664cc4/go.mod h1:o14zPKCmEH5WC1vU5SdPoZGgNvQx7zzKSnxPQlobo78= github.com/emersion/go-imap-move v0.0.0-20190710073258-6e5a51a5b342 h1:5p1t3e1PomYgLWwEwhwEU5kVBwcyAcVrOpexv8AeZx0= github.com/emersion/go-imap-move v0.0.0-20190710073258-6e5a51a5b342/go.mod h1:QuMaZcKFDVI0yCrnAbPLfbwllz1wtOrZH8/vZ5yzp4w= github.com/emersion/go-imap-quota v0.0.0-20210203125329-619074823f3c h1:khcEdu1yFiZjBgi7gGnQiLhpSgghJ0YTnKD0l4EUqqc= @@ -262,11 +260,6 @@ github.com/stretchr/testify v1.6.1 h1:hDPOHmpOpP40lSULcqw7IrRb/u7w6RpDC9399XyoNd github.com/stretchr/testify v1.6.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= github.com/therecipe/qt v0.0.0-20200701200531-7f61353ee73e h1:G0DQ/TRQyrEZjtLlLwevFjaRiG8eeCMlq9WXQ2OO2bk= github.com/therecipe/qt v0.0.0-20200701200531-7f61353ee73e/go.mod h1:SUUR2j3aE1z6/g76SdD6NwACEpvCxb3fvG82eKbD6us= -github.com/therecipe/qt v0.0.0-20200904063919-c0c124a5770d h1:T+d8FnaLSvM/1BdlDXhW4d5dr2F07bAbB+LpgzMxx+o= -github.com/therecipe/qt/internal/binding/files/docs/5.12.0 v0.0.0-20200904063919-c0c124a5770d h1:hAZyEG2swPRWjF0kqqdGERXUazYnRJdAk4a58f14z7Y= -github.com/therecipe/qt/internal/binding/files/docs/5.12.0 v0.0.0-20200904063919-c0c124a5770d/go.mod h1:7m8PDYDEtEVqfjoUQc2UrFqhG0CDmoVJjRlQxexndFc= -github.com/therecipe/qt/internal/binding/files/docs/5.13.0 v0.0.0-20200904063919-c0c124a5770d h1:AJRoBel/g9cDS+yE8BcN3E+TDD/xNAguG21aoR8DAIE= -github.com/therecipe/qt/internal/binding/files/docs/5.13.0 v0.0.0-20200904063919-c0c124a5770d/go.mod h1:mH55Ek7AZcdns5KPp99O0bg+78el64YCYWHiQKrOdt4= github.com/ugorji/go v1.1.4/go.mod h1:uQMGLiO92mf5W77hV/PUCpI3pbzQx3CRekS0kk+RGrc= github.com/ugorji/go v1.1.7/go.mod h1:kZn38zHttfInRq0xu/PH0az30d+z6vm202qpg1oXVMw= github.com/ugorji/go/codec v0.0.0-20181204163529-d75b2dcb6bc8/go.mod h1:VFNgLljTbGfSG7qAOspJ7OScBnGdDN/yBr0sguwnwf0= diff --git a/internal/imap/idle/extension.go b/internal/imap/idle/extension.go new file mode 100644 index 00000000..4590409e --- /dev/null +++ b/internal/imap/idle/extension.go @@ -0,0 +1,85 @@ +// 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 idle + +import ( + "bufio" + "errors" + "strings" + + "github.com/emersion/go-imap" + "github.com/emersion/go-imap/server" +) + +const ( + idleCommand = "IDLE" // Capability and Command identificator + doneLine = "DONE" +) + +// Handler for IDLE extension. +type Handler struct{} + +// Command for IDLE handler. +func (h *Handler) Command() *imap.Command { + return &imap.Command{Name: idleCommand} +} + +// Parse for IDLE handler. +func (h *Handler) Parse(fields []interface{}) error { + return nil +} + +// Handle the IDLE request. +func (h *Handler) Handle(conn server.Conn) error { + cont := &imap.ContinuationReq{Info: "idling"} + if err := conn.WriteResp(cont); err != nil { + return err + } + + // Wait for DONE + scanner := bufio.NewScanner(conn) + scanner.Scan() + if err := scanner.Err(); err != nil { + return err + } + + if strings.ToUpper(scanner.Text()) != doneLine { + return errors.New("expected DONE") + } + return nil +} + +type extension struct{} + +func (ext *extension) Capabilities(c server.Conn) []string { + return []string{idleCommand} +} + +func (ext *extension) Command(name string) server.HandlerFactory { + if name != idleCommand { + return nil + } + + return func() server.Handler { + return &Handler{} + } +} + +func NewExtension() server.Extension { + return &extension{} +} diff --git a/internal/imap/server.go b/internal/imap/server.go index 70161af4..7dfb3caf 100644 --- a/internal/imap/server.go +++ b/internal/imap/server.go @@ -31,12 +31,12 @@ import ( "github.com/ProtonMail/proton-bridge/internal/config/useragent" "github.com/ProtonMail/proton-bridge/internal/events" "github.com/ProtonMail/proton-bridge/internal/imap/id" + "github.com/ProtonMail/proton-bridge/internal/imap/idle" "github.com/ProtonMail/proton-bridge/internal/imap/uidplus" "github.com/ProtonMail/proton-bridge/internal/serverutil" "github.com/ProtonMail/proton-bridge/pkg/listener" "github.com/emersion/go-imap" imapappendlimit "github.com/emersion/go-imap-appendlimit" - imapidle "github.com/emersion/go-imap-idle" imapmove "github.com/emersion/go-imap-move" imapquota "github.com/emersion/go-imap-quota" imapunselect "github.com/emersion/go-imap-unselect" @@ -94,7 +94,7 @@ func NewIMAPServer(panicHandler panicHandler, debugClient, debugServer bool, por }) s.Enable( - imapidle.NewExtension(), + idle.NewExtension(), imapmove.NewExtension(), id.NewExtension(serverID, userAgent), imapquota.NewExtension(), diff --git a/internal/imap/updates.go b/internal/imap/updates.go index 0e77d084..83fd87b0 100644 --- a/internal/imap/updates.go +++ b/internal/imap/updates.go @@ -188,10 +188,10 @@ func (iu *imapUpdates) MailboxStatus(address, mailboxName string, total, unread, update.MailboxStatus.Messages = total update.MailboxStatus.Unseen = unread update.MailboxStatus.UnseenSeqNum = unreadSeqNum - iu.sendIMAPUpdate(update, false) + iu.sendIMAPUpdate(update, true) } -func (iu *imapUpdates) sendIMAPUpdate(update goIMAPBackend.Update, block bool) { +func (iu *imapUpdates) sendIMAPUpdate(update goIMAPBackend.Update, isBlocking bool) { if iu.ch == nil { log.Trace("IMAP IDLE unavailable") return @@ -207,7 +207,7 @@ func (iu *imapUpdates) sendIMAPUpdate(update goIMAPBackend.Update, block bool) { } }() - if !block { + if !isBlocking { return } diff --git a/internal/store/mailbox_message.go b/internal/store/mailbox_message.go index 57d129c0..914e720a 100644 --- a/internal/store/mailbox_message.go +++ b/internal/store/mailbox_message.go @@ -355,6 +355,10 @@ func (storeMailbox *Mailbox) txCreateOrUpdateMessages(tx *bolt.Tx, msgs []*pmapi // Buckets are not initialized right away because it's a heavy operation. // The best option is to get the same bucket only once and only when needed. var apiBucket, imapBucket, deletedBucket *bolt.Bucket + + // Collect updates to send them later, after possibly sending the status/EXISTS update. + updates := make([]func(), 0, len(msgs)) + for _, msg := range msgs { if storeMailbox.txSkipAndRemoveFromMailbox(tx, msg) { continue @@ -417,14 +421,18 @@ func (storeMailbox *Mailbox) txCreateOrUpdateMessages(tx *bolt.Tx, msgs []*pmapi if err != nil { return errors.Wrap(err, "cannot get sequence number from UID") } - storeMailbox.store.notifyUpdateMessage( - storeMailbox.storeAddress.address, - storeMailbox.labelName, - uid, - seqNum, - msg, - false, // new message is never marked as deleted - ) + + updates = append(updates, func() { + storeMailbox.store.notifyUpdateMessage( + storeMailbox.storeAddress.address, + storeMailbox.labelName, + uid, + seqNum, + msg, + false, // new message is never marked as deleted + ) + }) + shouldSendMailboxUpdate = true } @@ -434,6 +442,10 @@ func (storeMailbox *Mailbox) txCreateOrUpdateMessages(tx *bolt.Tx, msgs []*pmapi } } + for _, update := range updates { + update() + } + return nil }