fix: return error if parsing header fails GODT-502

General issue template

.github/ISSUE_TEMPLATE/general-issue-template.md

@@ -0,0 +1,38 @@
+---
+name: General issue template
+about: Template for detailed report of issues
+title: ''
+labels: ''
+assignees: ''
+
+---
+
+Issue tracker is ONLY used for reporting bugs with technical details. "It doesn't work" or new features should be discussed with our customer support. Please use bug report function in Bridge or contact bridge@protonmail.ch.
+<!--- Provide a general summary of the issue in the Title above -->
+
+## Expected Behavior
+<!--- Tell us what should happen -->
+
+## Current Behavior
+<!--- Tell us what happens instead of the expected behavior -->
+
+## Possible Solution
+<!--- Not obligatory, but suggest a fix/reason for the bug, -->
+
+## Steps to Reproduce
+<!--- Provide a link to a live example, or an unambiguous set of steps to -->
+<!--- reproduce this bug. Include code to reproduce, if relevant -->
+1.
+2.
+3.
+4.
+
+## Context (Environment)
+<!--- How has this issue affected you? What are you trying to accomplish? -->
+<!--- Providing context helps us come up with a solution that is most useful in the real world -->
+
+## Detailed Description
+<!--- Provide a detailed description of the change or addition you are proposing -->
+
+## Possible Implementation
+<!--- Not obligatory, but suggest an idea for implementing addition or change -->

.gitlab-ci.yml

@@ -37,6 +37,8 @@ build-ci-image:
     - ci/*
   services:
   - docker:dind
+  variables:
+    DOCKER_HOST: tcp://docker:2375
   script:
   - docker login -u $CI_REGISTRY_USER -p $CI_REGISTRY_PASSWORD $CI_REGISTRY
   - docker info

BUILDS.md

@@ -35,5 +35,5 @@ In order to be able to run following commands please install the development dep
 
 * `make test` will run all unit tests
 * `make lint` will lint the whole project
-* `make -C ./tests test` will run the integration tests
+* `make -C ./test test` will run the integration tests
 * `make run` will build Bridge without a GUI and start it in CLI mode

Changelog.md

@@ -2,16 +2,40 @@
 
 Changelog [format](http://keepachangelog.com/en/1.0.0/)
 
-## Unpublished
-
-### Added
-* IMAP extension Unselect
+## [v1.2.8] Donghai-fix-append  (2020-06-XXX)
 
 ### Changed
-* GODT-165 Optimization of RebuildMailboxes
-* Adding DSN Sentry as build time parameter
+* GODT-396 reduce number of EXISTS calls
+* GODT-143 Allow appending to Sent folder when sender matches account address
 
-## [v1.2.6] Donghai - beta (2020-03-XXX)
+### Fixed
+* Do not crash when `nil` message header is parsed, it fails instead.
+
+## [v1.2.7] Donghai-fix-sync - (beta 2020-05-07 live 2020-04-20)
+
+### Added
+* IMAP extension MOVE with UIDPLUS support
+* IMAP extension Unselect
+* More logs about event loop activity
+
+### Changed
+* GODT-313 Reduce number of synchronizations
+    * do not trigger sync by counts
+    * cooldown timer for sync retries
+    * poll interval randomization
+* GODT-225 Do not send an EXISTS reposnse after EXPUNGE or when nothing changed (fixes rebuild of mailboxes in Outlook for Mac)
+* GODT-165 Optimization of RebuildMailboxes
+* GODT-282 Completely delete old draft instead moving to trash when user updates draft
+* Adding DSN Sentry as build time parameter
+* GODT-124 bump go-appdir from v1.0.0 to v1.1.0
+* CSB-72 Skip processing message update event if http statuscode is 422
+
+### Fixed
+* Use correct binary name when finding location of addcert.scpt
+
+
+
+## [v1.2.6] Donghai - beta (2020-03-31)
 
 ### Added
 * GODT-145 support drafts

Makefile

@@ -2,9 +2,9 @@ export GO111MODULE=on
 GOOS:=$(shell go env GOOS)
 
 ## Build
-.PHONY: build check-has-go
+.PHONY: build build-nogui check-has-go
 
-VERSION?=1.2.6-git
+VERSION?=1.2.7-git
 REVISION:=$(shell git rev-parse --short=10 HEAD)
 BUILD_TIME:=$(shell date +%FT%T%z)
 
@@ -36,6 +36,9 @@ TGZ_TARGET:=bridge_${GOOS}_${REVISION}.tgz
 
 build: ${TGZ_TARGET}
 
+build-nogui:
+	go build ${BUILD_FLAGS_NOGUI} -o Desktop-Bridge cmd/Desktop-Bridge/main.go
+
 ${TGZ_TARGET}: ${DEPLOY_DIR}/${GOOS}
 	rm -f $@
 	cd ${DEPLOY_DIR} && tar czf ../../../$@ ${GOOS}
@@ -55,6 +58,7 @@ ${DEPLOY_DIR}/darwin: ${EXE_TARGET}
 
 ${DEPLOY_DIR}/windows: ${EXE_TARGET}
 	cp ./internal/frontend/share/icons/logo.ico ${DEPLOY_DIR}/windows/
+	cp LICENSE ${DEPLOY_DIR}/windows/
 
 ${EXE_TARGET}: check-has-go gofiles ${ICO_FILES} update-vendor
 	rm -rf deploy ${GOOS} ${DEPLOY_DIR}

cmd/Desktop-Bridge/main.go

@@ -359,7 +359,7 @@ func migratePreferencesFromC10(cfg *config.Config) {
 		return
 	}
 
-	err = ioutil.WriteFile(pref11Path, data, 0644)
+	err = ioutil.WriteFile(pref11Path, data, 0644) //nolint[gosec]
 	if err != nil {
 		log.WithError(err).Error("Problem to migrate preferences")
 		return

go.mod

@@ -14,7 +14,7 @@ require (
 
 require (
 	github.com/0xAX/notificator v0.0.0-20191016112426-3962a5ea8da1
-	github.com/ProtonMail/go-appdir v1.0.0
+	github.com/ProtonMail/go-appdir v1.1.0
 	github.com/ProtonMail/go-apple-mobileconfig v0.0.0-20160701194735-7ea9927a11f6
 	github.com/ProtonMail/go-autostart v0.0.0-20181114175602-c5272053443a
 	github.com/ProtonMail/go-imap-id v0.0.0-20171219160728-ed0baee567ee
@@ -31,6 +31,7 @@ require (
 	github.com/danieljoos/wincred v1.0.2 // indirect
 	github.com/emersion/go-imap-appendlimit v0.0.0-20160923165328-beeb382f2a42
 	github.com/emersion/go-imap-idle v0.0.0-20161227184850-e03ba1e0ed89
+	github.com/emersion/go-imap-move v0.0.0-20161227183138-88aef42b0f1d
 	github.com/emersion/go-imap-specialuse v0.0.0-20161227184202-ba031ced6a62
 	github.com/emersion/go-imap-unselect v0.0.0-20161227183655-1e6dc73ac8fe
 	github.com/emersion/go-sasl v0.0.0-20191210011802-430746ea8b9b

go.sum

@@ -9,6 +9,8 @@ github.com/ProtonMail/docker-credential-helpers v1.0.0 h1:0DQXbZNvUszWgXUuP7TzvQ
 github.com/ProtonMail/docker-credential-helpers v1.0.0/go.mod h1:R1gQindzdYFcWJuuGXteYHDJzUCVtyU+EpEqp9aWcFs=
 github.com/ProtonMail/go-appdir v1.0.0 h1:PZXQ0HkveuEugga3LeDycxWtybrXQfKR0ThxURd6ojw=
 github.com/ProtonMail/go-appdir v1.0.0/go.mod h1:3d8Y9F5mbEUjrYbcJ3rcDxcWbqbttF+011nVZmdRdzc=
+github.com/ProtonMail/go-appdir v1.1.0 h1:9hdNDlU9kTqRKVNzmoqah8qqrj5QZyLByQdwQNlFWig=
+github.com/ProtonMail/go-appdir v1.1.0/go.mod h1:3d8Y9F5mbEUjrYbcJ3rcDxcWbqbttF+011nVZmdRdzc=
 github.com/ProtonMail/go-apple-mobileconfig v0.0.0-20160701194735-7ea9927a11f6 h1:YsSJ/mvZFYydQm/hRrt8R8UtgETixN2y3LK98f5LT60=
 github.com/ProtonMail/go-apple-mobileconfig v0.0.0-20160701194735-7ea9927a11f6/go.mod h1:EtDfBMIDWmVe4viZCuBTEfe3OIIo0ghbpOaAZVO+hVg=
 github.com/ProtonMail/go-autostart v0.0.0-20181114175602-c5272053443a h1:fXK2KsfnkBV9Nh+9SKzHchYjuE9s0vI20JG1mbtEAcc=
@@ -65,6 +67,12 @@ github.com/emersion/go-imap-appendlimit v0.0.0-20160923165328-beeb382f2a42 h1:3T
 github.com/emersion/go-imap-appendlimit v0.0.0-20160923165328-beeb382f2a42/go.mod h1:ikgISoP7pRAolqsVP64yMteJa2FIpS6ju88eBT6K1yQ=
 github.com/emersion/go-imap-idle v0.0.0-20161227184850-e03ba1e0ed89 h1:AzbVhcrxgJO5MfSvzG5q4IfrYVm0Jw4AHNPz47+DiR0=
 github.com/emersion/go-imap-idle v0.0.0-20161227184850-e03ba1e0ed89/go.mod h1:o14zPKCmEH5WC1vU5SdPoZGgNvQx7zzKSnxPQlobo78=
+github.com/emersion/go-imap-move v0.0.0-20161227173100-88aef42b0f1d h1:STRZFC+5HZITdsSFkhFfyYRb+tkiTwhxFz3sRW1lYjk=
+github.com/emersion/go-imap-move v0.0.0-20161227173100-88aef42b0f1d/go.mod h1:QuMaZcKFDVI0yCrnAbPLfbwllz1wtOrZH8/vZ5yzp4w=
+github.com/emersion/go-imap-move v0.0.0-20161227183138-88aef42b0f1d h1:E/ezdheD3QUe47cM0LpAPuJ6Pk1x0EFDmjoysaZhtaw=
+github.com/emersion/go-imap-move v0.0.0-20161227183138-88aef42b0f1d/go.mod h1:QuMaZcKFDVI0yCrnAbPLfbwllz1wtOrZH8/vZ5yzp4w=
+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-specialuse v0.0.0-20161227184202-ba031ced6a62 h1:4ZAfwfc8aDlj26kkEap1UDSwwDnJp9Ie8Uj1MSXAkPk=
 github.com/emersion/go-imap-specialuse v0.0.0-20161227184202-ba031ced6a62/go.mod h1:/nybxhI8kXom8Tw6BrHMl42usALvka6meORflnnYwe4=
 github.com/emersion/go-imap-unselect v0.0.0-20161227183655-1e6dc73ac8fe h1:2R2XpJkmbyy7PcSjnCPOnNfu+GuRzgWR9U2+j/d1O+0=

internal/bridge/credits.go

@@ -15,8 +15,8 @@
 // You should have received a copy of the GNU General Public License
 // along with ProtonMail Bridge.  If not, see <https://www.gnu.org/licenses/>.
 
-// Code generated by ./credits.sh at Thu Apr 9 13:39:29 CEST 2020. DO NOT EDIT.
+// Code generated by ./credits.sh at Thu Apr 16 13:43:04 CEST 2020. DO NOT EDIT.
 
 package bridge
 
-const Credits = "github.com/0xAX/notificator;github.com/ProtonMail/bcrypt;github.com/ProtonMail/crypto;github.com/ProtonMail/docker-credential-helpers;github.com/ProtonMail/go-appdir;github.com/ProtonMail/go-apple-mobileconfig;github.com/ProtonMail/go-autostart;github.com/ProtonMail/go-imap;github.com/ProtonMail/go-imap-id;github.com/ProtonMail/go-imap-quota;github.com/ProtonMail/go-smtp;github.com/ProtonMail/go-vcard;github.com/ProtonMail/gopenpgp;github.com/abiosoft/ishell;github.com/abiosoft/readline;github.com/allan-simon/go-singleinstance;github.com/andybalholm/cascadia;github.com/certifi/gocertifi;github.com/chzyer/logex;github.com/chzyer/test;github.com/cucumber/godog;github.com/danieljoos/wincred;github.com/docker/docker-credential-helpers;github.com/emersion/go-imap;github.com/emersion/go-imap-appendlimit;github.com/emersion/go-imap-idle;github.com/emersion/go-imap-quota;github.com/emersion/go-imap-specialuse;github.com/emersion/go-imap-unselect;github.com/emersion/go-sasl;github.com/emersion/go-smtp;github.com/emersion/go-textwrapper;github.com/emersion/go-vcard;github.com/fatih/color;github.com/flynn-archive/go-shlex;github.com/getsentry/raven-go;github.com/go-resty/resty/v2;github.com/golang/mock;github.com/google/go-cmp;github.com/gopherjs/gopherjs;github.com/hashicorp/go-multierror;github.com/jameskeane/bcrypt;github.com/jaytaylor/html2text;github.com/jhillyerd/enmime;github.com/kardianos/osext;github.com/keybase/go-keychain;github.com/logrusorgru/aurora;github.com/miekg/dns;github.com/myesui/uuid;github.com/nsf/jsondiff;github.com/pkg/errors;github.com/sirupsen/logrus;github.com/skratchdot/open-golang;github.com/stretchr/testify;github.com/therecipe/qt;github.com/twinj/uuid;github.com/urfave/cli;go.etcd.io/bbolt;golang.org/x/crypto;golang.org/x/net;golang.org/x/text;gopkg.in/stretchr/testify.v1;;Font Awesome 4.7.0;;Qt 5.13 by Qt group;"
+const Credits = "github.com/0xAX/notificator;github.com/ProtonMail/bcrypt;github.com/ProtonMail/crypto;github.com/ProtonMail/docker-credential-helpers;github.com/ProtonMail/go-appdir;github.com/ProtonMail/go-apple-mobileconfig;github.com/ProtonMail/go-autostart;github.com/ProtonMail/go-imap;github.com/ProtonMail/go-imap-id;github.com/ProtonMail/go-imap-quota;github.com/ProtonMail/go-smtp;github.com/ProtonMail/go-vcard;github.com/ProtonMail/gopenpgp;github.com/abiosoft/ishell;github.com/abiosoft/readline;github.com/allan-simon/go-singleinstance;github.com/andybalholm/cascadia;github.com/certifi/gocertifi;github.com/chzyer/logex;github.com/chzyer/test;github.com/cucumber/godog;github.com/danieljoos/wincred;github.com/docker/docker-credential-helpers;github.com/emersion/go-imap;github.com/emersion/go-imap-appendlimit;github.com/emersion/go-imap-idle;github.com/emersion/go-imap-move;github.com/emersion/go-imap-quota;github.com/emersion/go-imap-specialuse;github.com/emersion/go-imap-unselect;github.com/emersion/go-sasl;github.com/emersion/go-smtp;github.com/emersion/go-textwrapper;github.com/emersion/go-vcard;github.com/fatih/color;github.com/flynn-archive/go-shlex;github.com/getsentry/raven-go;github.com/go-resty/resty/v2;github.com/golang/mock;github.com/google/go-cmp;github.com/gopherjs/gopherjs;github.com/hashicorp/go-multierror;github.com/jameskeane/bcrypt;github.com/jaytaylor/html2text;github.com/jhillyerd/enmime;github.com/kardianos/osext;github.com/keybase/go-keychain;github.com/logrusorgru/aurora;github.com/miekg/dns;github.com/myesui/uuid;github.com/nsf/jsondiff;github.com/pkg/errors;github.com/sirupsen/logrus;github.com/skratchdot/open-golang;github.com/stretchr/testify;github.com/therecipe/qt;github.com/twinj/uuid;github.com/urfave/cli;go.etcd.io/bbolt;golang.org/x/crypto;golang.org/x/net;golang.org/x/text;gopkg.in/stretchr/testify.v1;;Font Awesome 4.7.0;;Qt 5.13 by Qt group;"

internal/bridge/mocks/mocks.go

@@ -5,12 +5,13 @@
 package mocks
 
 import (
-	credentials "github.com/ProtonMail/proton-bridge/internal/bridge/credentials"
-	pmapi "github.com/ProtonMail/proton-bridge/pkg/pmapi"
-	crypto "github.com/ProtonMail/gopenpgp/crypto"
-	gomock "github.com/golang/mock/gomock"
 	io "io"
 	reflect "reflect"
+
+	crypto "github.com/ProtonMail/gopenpgp/crypto"
+	credentials "github.com/ProtonMail/proton-bridge/internal/bridge/credentials"
+	pmapi "github.com/ProtonMail/proton-bridge/pkg/pmapi"
+	gomock "github.com/golang/mock/gomock"
 )
 
 // MockConfiger is a mock of Configer interface

internal/bridge/release_notes.go

@@ -15,20 +15,12 @@
 // You should have received a copy of the GNU General Public License
 // along with ProtonMail Bridge.  If not, see <https://www.gnu.org/licenses/>.
 
-// Code generated by ./release-notes.sh at Mon Apr 6 08:14:14 CEST 2020. DO NOT EDIT.
+// Code generated by ./release-notes.sh at Thu 21 May 2020 07:59:59 AM CEST. DO NOT EDIT.
 
 package bridge
 
-const ReleaseNotes = `NOTE: We recommend to reconfigure your email client after upgrading to ensure the best results with the new draft folder support
-
-• Faster and more resilient mail synchronization process, especially for large mailboxes
-• Added "Alternate Routing" feature to mitigate blocking of Proton Servers
-• Added synchronization of draft folder
-• Improved event handling when there are frequent changes
-• Security improvements for loading dependent libraries
-• Minor UI & API communication tweaks
+const ReleaseNotes = `
 `
 
-const ReleaseFixedBugs = `• Fixed rare case of sending the same message multiple times in Outlook
-• Fixed bug in macOS update process; available from next update
+const ReleaseFixedBugs = `• Fixed ignored import to Sent folder
 `

internal/frontend/autoconfig/applemail.go

@@ -28,9 +28,9 @@ import (
 	"strings"
 	"time"
 
+	mobileconfig "github.com/ProtonMail/go-apple-mobileconfig"
 	"github.com/ProtonMail/proton-bridge/internal/bridge"
 	"github.com/ProtonMail/proton-bridge/internal/frontend/types"
-	mobileconfig "github.com/ProtonMail/go-apple-mobileconfig"
 )
 
 func init() { //nolint[gochecknoinit]

internal/frontend/qt/frontend.go

@@ -36,6 +36,7 @@ import (
 	"sync"
 	"time"
 
+	"github.com/ProtonMail/go-autostart"
 	"github.com/ProtonMail/proton-bridge/internal/bridge"
 	"github.com/ProtonMail/proton-bridge/internal/events"
 	"github.com/ProtonMail/proton-bridge/internal/frontend/autoconfig"
@@ -44,7 +45,6 @@ import (
 	"github.com/ProtonMail/proton-bridge/pkg/config"
 	"github.com/ProtonMail/proton-bridge/pkg/ports"
 	"github.com/ProtonMail/proton-bridge/pkg/useragent"
-	"github.com/ProtonMail/go-autostart"
 
 	//"github.com/ProtonMail/proton-bridge/pkg/keychain"
 	"github.com/ProtonMail/proton-bridge/pkg/listener"

internal/imap/mailbox_message.go

@@ -126,10 +126,6 @@ func (im *imapMailbox) CreateMessage(flags []string, date time.Time, body imap.L
 
 			// We didn't find the message in the store, so we are currently sending it.
 			logEntry.WithField("time", date).Info("No matching UID, continuing APPEND to Sent")
-
-			// For now we don't import user's own messages to Sent because GetUIDByHeader is not smart enough.
-			// This will be fixed in GODT-143.
-			return nil
 		}
 
 		// This is an APPEND to the Sent folder, so we will set the sent flag

internal/imap/mailbox_messages.go

@@ -102,6 +102,21 @@ func (im *imapMailbox) CopyMessages(uid bool, seqSet *imap.SeqSet, targetLabel s
 	// Called from go-imap in goroutines - we need to handle panics for each function.
 	defer im.panicHandler.HandlePanic()
 
+	return im.labelMessages(uid, seqSet, targetLabel, false)
+}
+
+// MoveMessages adds dest's label and removes this mailbox' label from each message.
+//
+// This should not be used until MOVE extension has option to send UIDPLUS
+// responses.
+func (im *imapMailbox) MoveMessages(uid bool, seqSet *imap.SeqSet, targetLabel string) error {
+	// Called from go-imap in goroutines - we need to handle panics for each function.
+	defer im.panicHandler.HandlePanic()
+
+	return im.labelMessages(uid, seqSet, targetLabel, true)
+}
+
+func (im *imapMailbox) labelMessages(uid bool, seqSet *imap.SeqSet, targetLabel string, move bool) error {
 	messageIDs, err := im.apiIDsFromSeqSet(uid, seqSet)
 	if err != nil || len(messageIDs) == 0 {
 		return err
@@ -111,40 +126,24 @@ func (im *imapMailbox) CopyMessages(uid bool, seqSet *imap.SeqSet, targetLabel s
 	// messages can be removed from source during labeling (e.g. folder1 -> folder2).
 	sourceSeqSet := im.storeMailbox.GetUIDList(messageIDs)
 
-	targetStoreMBX, err := im.storeAddress.GetMailbox(targetLabel)
+	targetStoreMailbox, err := im.storeAddress.GetMailbox(targetLabel)
 	if err != nil {
 		return err
 	}
-	if err = targetStoreMBX.LabelMessages(messageIDs); err != nil {
-		return err
-	}
 
-	targetSeqSet := targetStoreMBX.GetUIDList(messageIDs)
-	return uidplus.CopyResponse(im.storeMailbox.UIDValidity(), sourceSeqSet, targetSeqSet)
-}
-
-// MoveMessages adds dest's label and removes this mailbox' label from each message.
-//
-// This should not be used until MOVE extension has option to send UIDPLUS
-// responses.
-func (im *imapMailbox) MoveMessages(uid bool, seqSet *imap.SeqSet, newLabel string) error {
-	// Called from go-imap in goroutines - we need to handle panics for each function.
-	defer im.panicHandler.HandlePanic()
-
-	messageIDs, err := im.apiIDsFromSeqSet(uid, seqSet)
-	if err != nil || len(messageIDs) == 0 {
-		return err
-	}
-	storeMailbox, err := im.storeAddress.GetMailbox(newLabel)
-	if err != nil {
-		return err
-	}
-	// Label messages first to not loss them. If message is only in trash and we unlabel
+	// Label messages first to not lose them. If message is only in trash and we unlabel
 	// it, it will be removed completely and we cannot label it back.
-	if err := storeMailbox.LabelMessages(messageIDs); err != nil {
+	if err := targetStoreMailbox.LabelMessages(messageIDs); err != nil {
 		return err
 	}
-	return im.storeMailbox.UnlabelMessages(messageIDs)
+	if move {
+		if err := im.storeMailbox.UnlabelMessages(messageIDs); err != nil {
+			return err
+		}
+	}
+
+	targetSeqSet := targetStoreMailbox.GetUIDList(messageIDs)
+	return uidplus.CopyResponse(targetStoreMailbox.UIDValidity(), sourceSeqSet, targetSeqSet)
 }
 
 // SearchMessages searches messages. The returned list must contain UIDs if

internal/imap/server.go

@@ -32,6 +32,7 @@ import (
 	"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"
 	imapspecialuse "github.com/emersion/go-imap-specialuse"
 	imapunselect "github.com/emersion/go-imap-unselect"
@@ -96,7 +97,7 @@ func NewIMAPServer(debugClient, debugServer bool, port int, tls *tls.Config, ima
 
 	s.Enable(
 		imapidle.NewExtension(),
-		//imapmove.NewExtension(), // extension is not fully implemented: if UIDPLUS exists it MUST return COPYUID and EXPUNGE continuous responses
+		imapmove.NewExtension(),
 		imapspecialuse.NewExtension(),
 		imapid.NewExtension(serverID),
 		imapquota.NewExtension(),

internal/imap/uidplus/extension.go

@@ -38,11 +38,11 @@ const Capability = "UIDPLUS"
 const (
 	copyuid      = "COPYUID"
 	appenduid    = "APPENDUID"
-	copySuccess  = "COPY successful"
-	appendSucess = "APPEND successful"
+	copySuccess  = "COPY completed"
+	appendSucess = "APPEND completed"
 )
 
-var log = logrus.WithField("pkg", "impa/uidplus") //nolint[gochecknoglobals]
+var log = logrus.WithField("pkg", "imap/uidplus") //nolint[gochecknoglobals]
 
 // OrderedSeq to remember Seq in order they are added.
 // We didn't find any restriction in RFC that server must respond with ranges

internal/store/cooldown.go

@@ -0,0 +1,65 @@
+// Copyright (c) 2020 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 <https://www.gnu.org/licenses/>.
+
+package store
+
+import "time"
+
+type cooldown struct {
+	waitTimes []time.Duration
+	waitIndex int
+	lastTry   time.Time
+}
+
+func (c *cooldown) setExponentialWait(initial time.Duration, base int, maximum time.Duration) {
+	waitTimes := []time.Duration{}
+	t := initial
+	if base > 1 {
+		for t < maximum {
+			waitTimes = append(waitTimes, t)
+			t *= time.Duration(base)
+		}
+	}
+	waitTimes = append(waitTimes, maximum)
+	c.setWaitTimes(waitTimes...)
+}
+
+func (c *cooldown) setWaitTimes(newTimes ...time.Duration) {
+	c.waitTimes = newTimes
+	c.reset()
+}
+
+// isTooSoon™ returns whether the cooldown period is not yet over.
+func (c *cooldown) isTooSoon() bool {
+	if time.Since(c.lastTry) < c.waitTimes[c.waitIndex] {
+		return true
+	}
+	c.lastTry = time.Now()
+	return false
+}
+
+func (c *cooldown) increaseWaitTime() {
+	c.lastTry = time.Now()
+	if c.waitIndex+1 < len(c.waitTimes) {
+		c.waitIndex++
+	}
+}
+
+func (c *cooldown) reset() {
+	c.waitIndex = 0
+	c.lastTry = time.Time{}
+}

internal/store/cooldown_test.go

@@ -0,0 +1,133 @@
+// Copyright (c) 2020 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 <https://www.gnu.org/licenses/>.
+
+package store
+
+import (
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestCooldownExponentialWait(t *testing.T) {
+	ms := time.Millisecond
+	sec := time.Second
+
+	testData := []struct {
+		haveInitial, haveMax time.Duration
+		haveBase             int
+		wantWaitTimes        []time.Duration
+	}{
+		{
+			haveInitial:   1 * sec,
+			haveBase:      0,
+			haveMax:       0 * sec,
+			wantWaitTimes: []time.Duration{0 * sec},
+		},
+		{
+			haveInitial:   0 * sec,
+			haveBase:      1,
+			haveMax:       0 * sec,
+			wantWaitTimes: []time.Duration{0 * sec},
+		},
+		{
+			haveInitial:   0 * sec,
+			haveBase:      0,
+			haveMax:       1 * sec,
+			wantWaitTimes: []time.Duration{1 * sec},
+		},
+		{
+			haveInitial:   0 * sec,
+			haveBase:      1,
+			haveMax:       1 * sec,
+			wantWaitTimes: []time.Duration{1 * sec},
+		},
+		{
+			haveInitial:   1 * sec,
+			haveBase:      0,
+			haveMax:       1 * sec,
+			wantWaitTimes: []time.Duration{1 * sec},
+		},
+		{
+			haveInitial:   1 * sec,
+			haveBase:      2,
+			haveMax:       1 * sec,
+			wantWaitTimes: []time.Duration{1 * sec},
+		},
+		{
+			haveInitial:   500 * ms,
+			haveBase:      2,
+			haveMax:       5 * sec,
+			wantWaitTimes: []time.Duration{500 * ms, 1 * sec, 2 * sec, 4 * sec, 5 * sec},
+		},
+	}
+
+	var testCooldown cooldown
+
+	for _, td := range testData {
+		testCooldown.setExponentialWait(td.haveInitial, td.haveBase, td.haveMax)
+		assert.Equal(t, td.wantWaitTimes, testCooldown.waitTimes)
+	}
+}
+
+func TestCooldownIncreaseAndReset(t *testing.T) {
+	var testCooldown cooldown
+	testCooldown.setWaitTimes(1*time.Second, 2*time.Second, 3*time.Second)
+	assert.Equal(t, 0, testCooldown.waitIndex)
+
+	assert.False(t, testCooldown.isTooSoon())
+	assert.True(t, testCooldown.isTooSoon())
+	assert.Equal(t, 0, testCooldown.waitIndex)
+
+	testCooldown.reset()
+	assert.Equal(t, 0, testCooldown.waitIndex)
+
+	assert.False(t, testCooldown.isTooSoon())
+	assert.True(t, testCooldown.isTooSoon())
+	assert.Equal(t, 0, testCooldown.waitIndex)
+
+	// increase at least N+1 times to check overflow
+	testCooldown.increaseWaitTime()
+	assert.True(t, testCooldown.isTooSoon())
+	testCooldown.increaseWaitTime()
+	assert.True(t, testCooldown.isTooSoon())
+	testCooldown.increaseWaitTime()
+	assert.True(t, testCooldown.isTooSoon())
+	testCooldown.increaseWaitTime()
+	assert.True(t, testCooldown.isTooSoon())
+
+	assert.Equal(t, 2, testCooldown.waitIndex)
+}
+
+func TestCooldownNotSooner(t *testing.T) {
+	var testCooldown cooldown
+	waitTime := 100 * time.Millisecond
+	retries := int64(10)
+	retryWait := time.Duration(waitTime.Milliseconds()/retries) * time.Millisecond
+	testCooldown.setWaitTimes(waitTime)
+
+	// first time it should never be too soon
+	assert.False(t, testCooldown.isTooSoon())
+	// these retries should be too soon
+	for i := retries; i > 0; i-- {
+		assert.True(t, testCooldown.isTooSoon())
+		time.Sleep(retryWait)
+	}
+	// after given wait time it shouldn't be soon anymore
+	assert.False(t, testCooldown.isTooSoon())
+}

internal/store/event_loop.go

@@ -18,6 +18,7 @@
 package store
 
 import (
+	"math/rand"
 	"time"
 
 	bridgeEvents "github.com/ProtonMail/proton-bridge/internal/events"
@@ -28,6 +29,7 @@ import (
 )
 
 const pollInterval = 30 * time.Second
+const pollIntervalSpread = 5 * time.Second
 
 type eventLoop struct {
 	cache          *Cache
@@ -38,6 +40,8 @@ type eventLoop struct {
 	isRunning      bool
 	hasInternet    bool
 
+	pollCounter int
+
 	log *logrus.Entry
 
 	store     *Store
@@ -70,7 +74,7 @@ func (loop *eventLoop) IsRunning() bool {
 }
 
 func (loop *eventLoop) setFirstEventID() (err error) {
-	loop.log.Trace("Setting first event ID")
+	loop.log.Info("Setting first event ID")
 
 	event, err := loop.apiClient.GetEvent("")
 	if err != nil {
@@ -104,7 +108,7 @@ func (loop *eventLoop) stop() {
 
 		select {
 		case <-loop.notifyStopCh:
-			loop.log.Info("Event loop was stopped")
+			loop.log.Warn("Event loop was stopped")
 		case <-time.After(1 * time.Second):
 			loop.log.Warn("Timed out waiting for event loop to stop")
 		}
@@ -127,10 +131,10 @@ func (loop *eventLoop) start() { // nolint[funlen]
 
 	loop.log.WithField("lastEventID", loop.currentEventID).Info("Subscribed to events")
 	defer func() {
-		loop.log.WithField("lastEventID", loop.currentEventID).Info("Subscription stopped")
+		loop.log.WithField("lastEventID", loop.currentEventID).Warn("Subscription stopped")
 	}()
 
-	t := time.NewTicker(pollInterval)
+	t := time.NewTicker(pollInterval - pollIntervalSpread)
 	defer t.Stop()
 
 	loop.hasInternet = true
@@ -143,8 +147,11 @@ func (loop *eventLoop) start() { // nolint[funlen]
 		case <-loop.stopCh:
 			close(loop.notifyStopCh)
 			return
-		case eventProcessedCh = <-loop.pollCh:
 		case <-t.C:
+			// Randomise periodic calls within range pollInterval ± pollSpread to reduces potential load spikes on API.
+			time.Sleep(time.Duration(rand.Intn(2*int(pollIntervalSpread.Milliseconds()))) * time.Millisecond)
+		case eventProcessedCh = <-loop.pollCh:
+			// We don't want to wait here. Polling should happen instantly.
 		}
 
 		// Before we fetch the first event, check whether this is the first time we've
@@ -194,7 +201,9 @@ func (loop *eventLoop) isBeforeFirstStart() bool {
 // (disk). It will filter out in defer all errors except invalid token error.
 // Invalid error will be returned and stop the event loop.
 func (loop *eventLoop) processNextEvent() (more bool, err error) { // nolint[funlen]
-	l := loop.log.WithField("currentEventID", loop.currentEventID)
+	l := loop.log.
+		WithField("currentEventID", loop.currentEventID).
+		WithField("pollCounter", loop.pollCounter)
 
 	// We only want to consider invalid tokens as real errors because all other errors might fix themselves eventually
 	// (e.g. no internet, ulimit reached etc.)
@@ -222,12 +231,19 @@ func (loop *eventLoop) processNextEvent() (more bool, err error) { // nolint[fun
 
 		// All errors except Invalid Token (which is not possible to recover from) are ignored.
 		if err != nil && !errUnauthorized && errors.Cause(err) != pmapi.ErrInvalidToken {
-			l.WithError(err).Trace("Error skipped")
+			l.WithError(err).Error("Error skipped")
 			err = nil
 		}
 	}()
 
 	l.Trace("Polling next event")
+	// Log activity of event loop each 100. poll which means approx. 28
+	// lines per day
+	if loop.pollCounter%100 == 0 {
+		l.Info("Polling next event")
+	}
+	loop.pollCounter++
+
 	var event *pmapi.Event
 	if event, err = loop.apiClient.GetEvent(loop.currentEventID); err != nil {
 		return false, errors.Wrap(err, "failed to get event")
@@ -245,6 +261,7 @@ func (loop *eventLoop) processNextEvent() (more bool, err error) { // nolint[fun
 	}
 
 	if loop.currentEventID != event.EventID {
+		l.WithField("newID", event.EventID).Info("New event processed")
 		// In case new event ID cannot be saved to cache, we update it in event loop
 		// anyway and continue processing new events to prevent the loop from repeatedly
 		// processing the same event.
@@ -382,7 +399,7 @@ func (loop *eventLoop) processLabels(eventLog *logrus.Entry, labels []*pmapi.Eve
 	return nil
 }
 
-func (loop *eventLoop) processMessages(eventLog *logrus.Entry, messages []*pmapi.EventMessage) (err error) {
+func (loop *eventLoop) processMessages(eventLog *logrus.Entry, messages []*pmapi.EventMessage) (err error) { // nolint[funlen]
 	eventLog.Debug("Processing message change event")
 
 	for _, message := range messages {
@@ -394,7 +411,7 @@ func (loop *eventLoop) processMessages(eventLog *logrus.Entry, messages []*pmapi
 
 			if message.Created == nil {
 				msgLog.Error("Got EventCreate with nil message")
-				break
+				continue
 			}
 
 			if err = loop.store.createOrUpdateMessageEvent(message.Created); err != nil {
@@ -405,23 +422,28 @@ func (loop *eventLoop) processMessages(eventLog *logrus.Entry, messages []*pmapi
 			msgLog.Debug("Processing EventUpdate(Flags) for message")
 
 			if message.Updated == nil {
-				msgLog.Errorf("Got EventUpdate(Flags) with nil message")
-				break
+				msgLog.Error("Got EventUpdate(Flags) with nil message")
+				continue
 			}
 
 			var msg *pmapi.Message
-			msg, err = loop.store.getMessageFromDB(message.ID)
-			if err == ErrNoSuchAPIID {
-				msgLog.WithError(err).Warning("Cannot get message from DB for updating. Trying fetch...")
-				msg, err = loop.store.fetchMessage(message.ID)
-				// If message does not exist anywhere, update event is probably old and off topic - skip it.
-				if err == ErrNoSuchAPIID {
-					msgLog.Warn("Skipping message update, because message does not exist nor in local DB or on API")
-					continue
+
+			if msg, err = loop.store.getMessageFromDB(message.ID); err != nil {
+				if err != ErrNoSuchAPIID {
+					return errors.Wrap(err, "failed to get message from DB for updating")
+				}
+
+				msgLog.WithError(err).Warning("Message was not present in DB. Trying fetch...")
+
+				if msg, err = loop.apiClient.GetMessage(message.ID); err != nil {
+					if _, ok := err.(*pmapi.ErrUnprocessableEntity); ok {
+						msgLog.WithError(err).Warn("Skipping message update because message exists neither in local DB nor on API")
+						err = nil
+						continue
+					}
+
+					return errors.Wrap(err, "failed to get message from API for updating")
 				}
-			}
-			if err != nil {
-				return errors.Wrap(err, "failed to get message from DB for updating")
 			}
 
 			updateMessage(msgLog, msg, message.Updated)
@@ -528,7 +550,7 @@ func (loop *eventLoop) processMessageCounts(l *logrus.Entry, messageCounts []*pm
 		return err
 	}
 	if !isSynced {
-		loop.store.triggerSync()
+		log.Error("The counts between DB and API are not matching")
 	}
 
 	return nil

internal/store/event_loop_test.go

@@ -64,7 +64,7 @@ func TestEventLoopProcessMoreEvents(t *testing.T) {
 	}, time.Second, 10*time.Millisecond)
 
 	// For normal event we need to wait to next polling.
-	time.Sleep(pollInterval)
+	time.Sleep(pollInterval + pollIntervalSpread)
 	require.Eventually(t, func() bool {
 		return m.store.eventLoop.currentEventID == "event71"
 	}, time.Second, 10*time.Millisecond)

internal/store/mailbox_ids.go

@@ -219,6 +219,11 @@ func (storeMailbox *Mailbox) GetUIDByHeader(header *mail.Header) (foundUID uint3
 	// in PM message. Message-Id in normal copy/move will be the PM internal ID.
 	messageID := header.Get("Message-Id")
 
+	// There is nothing to find, when no Message-Id given.
+	if messageID == "" {
+		return uint32(0)
+	}
+
 	// The most often situation is that message is APPENDed after it was sent so the
 	// Message-ID will be reflected by ExternalID in API message meta-data.
 	externalID := strings.Trim(messageID, "<> ") // remove '<>' to improve match

internal/store/mailbox_message.go

@@ -37,7 +37,7 @@ func (storeMailbox *Mailbox) GetMessage(apiID string) (*Message, error) {
 // FetchMessage fetches the message with the given `apiID`, stores it in the database, and returns a new store message
 // wrapping it.
 func (storeMailbox *Mailbox) FetchMessage(apiID string) (*Message, error) {
-	msg, err := storeMailbox.store.fetchMessage(apiID)
+	msg, err := storeMailbox.api().GetMessage(apiID)
 	if err != nil {
 		return nil, err
 	}
@@ -206,6 +206,10 @@ func (storeMailbox *Mailbox) DeleteMessages(apiIDs []string) error {
 				return err
 			}
 		}
+	case pmapi.DraftLabel:
+		if err := storeMailbox.api().DeleteMessages(apiIDs); err != nil {
+			return err
+		}
 	default:
 		if err := storeMailbox.api().UnlabelMessages(apiIDs, storeMailbox.labelID); err != nil {
 			return err
@@ -249,6 +253,8 @@ func (storeMailbox *Mailbox) txSkipAndRemoveFromMailbox(tx *bolt.Tx, msg *pmapi.
 
 // txCreateOrUpdateMessages will delete, create or update message from mailbox.
 func (storeMailbox *Mailbox) txCreateOrUpdateMessages(tx *bolt.Tx, msgs []*pmapi.Message) error { //nolint[funlen]
+	shouldSendMailboxUpdate := false
+
 	// 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 *bolt.Bucket
@@ -264,7 +270,7 @@ func (storeMailbox *Mailbox) txCreateOrUpdateMessages(tx *bolt.Tx, msgs []*pmapi
 
 		// Draft bodies can change and bodies are not re-fetched by IMAP clients.
 		// Every change has to be a new message; we need to delete the old one and always recreate it.
-		if storeMailbox.labelID == pmapi.DraftLabel {
+		if msg.Type == pmapi.MessageTypeDraft {
 			if err := storeMailbox.txDeleteMessage(tx, msg.ID); err != nil {
 				return errors.Wrap(err, "cannot delete old draft")
 			}
@@ -317,9 +323,16 @@ func (storeMailbox *Mailbox) txCreateOrUpdateMessages(tx *bolt.Tx, msgs []*pmapi
 			seqNum,
 			msg,
 		)
+		shouldSendMailboxUpdate = true
 	}
 
-	return storeMailbox.txMailboxStatusUpdate(tx)
+	if shouldSendMailboxUpdate {
+		if err := storeMailbox.txMailboxStatusUpdate(tx); err != nil {
+			return err
+		}
+	}
+
+	return nil
 }
 
 // txDeleteMessage deletes the message from the mailbox bucket.
@@ -353,9 +366,10 @@ func (storeMailbox *Mailbox) txDeleteMessage(tx *bolt.Tx, apiID string) error {
 			storeMailbox.labelName,
 			seqNum,
 		)
-		if err := storeMailbox.txMailboxStatusUpdate(tx); err != nil {
-			return err
-		}
+		// Outlook for Mac has problems with sending an EXISTS after deleting
+		// messages, mostly after moving message to other folder. It causes
+		// Outlook to rebuild the whole mailbox. [RFC-3501] says it's not
+		// necessary to send an EXISTS response with the new value.
 	}
 	return nil
 }

internal/store/mocks/mocks.go

@@ -5,8 +5,9 @@
 package mocks
 
 import (
-	gomock "github.com/golang/mock/gomock"
 	reflect "reflect"
+
+	gomock "github.com/golang/mock/gomock"
 )
 
 // MockPanicHandler is a mock of PanicHandler interface

internal/store/mocks/utils_mocks.go

@@ -5,9 +5,10 @@
 package mocks
 
 import (
-	gomock "github.com/golang/mock/gomock"
 	reflect "reflect"
 	time "time"
+
+	gomock "github.com/golang/mock/gomock"
 )
 
 // MockListener is a mock of Listener interface

internal/store/store.go

@@ -104,6 +104,7 @@ type Store struct {
 	imapUpdates chan interface{}
 
 	isSyncRunning bool
+	syncCooldown  cooldown
 	addressMode   addressMode
 }
 
@@ -148,6 +149,9 @@ func New(
 		log:          l,
 	}
 
+	// Minimal increase is event pollInterval, doubles every failed retry up to 5 minutes.
+	store.syncCooldown.setExponentialWait(pollInterval, 2, 5*time.Minute)
+
 	if err = store.init(firstInit); err != nil {
 		l.WithError(err).Error("Could not initialise store, attempting to close")
 		if storeCloseErr := store.Close(); storeCloseErr != nil {

internal/store/user_message.go

@@ -142,19 +142,6 @@ func (store *Store) getMessageFromDB(apiID string) (msg *pmapi.Message, err erro
 	return
 }
 
-// fetchMessage returns pmapi struct of message by API ID. If the requested
-// message is not in the database, it will try to fetch it from the server.
-// NOTE: Do not update the database here to prevent issues (extreme edge case).
-// The database will be updated by the event loop anyway.
-func (store *Store) fetchMessage(apiID string) (msg *pmapi.Message, err error) {
-	if msg, err = store.api.GetMessage(apiID); err != nil {
-		if err.Error() == "Message does not exist" {
-			return nil, ErrNoSuchAPIID
-		}
-	}
-	return
-}
-
 func (store *Store) txGetMessage(tx *bolt.Tx, apiID string) (*pmapi.Message, error) {
 	b := tx.Bucket(metadataBucket)
 

internal/store/user_message_test.go

@@ -80,7 +80,7 @@ func TestCreateOrUpdateMessageMetadata(t *testing.T) {
 	a.Equal(t, []*pmapi.Attachment(nil), msg.Attachments)
 	a.Equal(t, int64(-1), msg.Size)
 	a.Equal(t, "", msg.MIMEType)
-	a.Equal(t, mail.Header(nil), msg.Header)
+	a.Equal(t, make(mail.Header), msg.Header)
 
 	// Change the calculated data.
 	wantSize := int64(42)

internal/store/user_sync.go

@@ -128,11 +128,19 @@ func (store *Store) triggerSync() {
 		store.log.Debug("Store sync triggered")
 
 		store.lock.Lock()
+
 		if store.isSyncRunning {
 			store.lock.Unlock()
 			store.log.Info("Store sync is already ongoing")
 			return
 		}
+
+		if store.syncCooldown.isTooSoon() {
+			store.lock.Unlock()
+			store.log.Info("Skipping sync: store tries to resync too often")
+			return
+		}
+
 		store.isSyncRunning = true
 		store.lock.Unlock()
 
@@ -147,9 +155,11 @@ func (store *Store) triggerSync() {
 		err := syncAllMail(store.panicHandler, store, store.api, syncState)
 		if err != nil {
 			log.WithError(err).Error("Store sync failed")
+			store.syncCooldown.increaseWaitTime()
 			return
 		}
 
+		store.syncCooldown.reset()
 		syncState.setFinishTime()
 	}()
 }

pkg/config/tls.go

@@ -29,11 +29,9 @@ import (
 	"net"
 	"os"
 	"os/exec"
+	"path/filepath"
 	"runtime"
-	"strings"
 	"time"
-
-	"github.com/kardianos/osext"
 )
 
 type tlsConfiger interface {
@@ -74,10 +72,11 @@ func GetTLSConfig(cfg tlsConfiger) (tlsConfig *tls.Config, err error) {
 
 		if runtime.GOOS == "darwin" {
 			// If this fails, log the error but continue to load.
-			if p, err := osext.Executable(); err == nil {
-				p = strings.TrimSuffix(p, "MacOS/Desktop-Bridge") // This needs to match the executable name.
-				p += "Resources/addcert.scpt"
-				if err := exec.Command("/usr/bin/osascript", p).Run(); err != nil { // nolint[gosec]
+			if binaryPath, err := os.Executable(); err == nil {
+				macOSPath := filepath.Dir(binaryPath)
+				contentsPath := filepath.Dir(macOSPath)
+				resourcesPath := filepath.Join(contentsPath, "Resources", "addcert.scpt")
+				if err := exec.Command("/usr/bin/osascript", resourcesPath).Run(); err != nil { // nolint[gosec]
 					log.WithError(err).Error("Failed to add cert to system keychain")
 				}
 			}

pkg/pmapi/client.go

@@ -58,6 +58,14 @@ var (
 	ErrUpgradeApplication = errors.New("application upgrade required")
 )
 
+type ErrUnprocessableEntity struct {
+	error
+}
+
+func (err *ErrUnprocessableEntity) Error() string {
+	return err.error.Error()
+}
+
 type ErrUnauthorized struct {
 	error
 }

pkg/pmapi/events_test.go

@@ -20,6 +20,7 @@ package pmapi
 import (
 	"fmt"
 	"net/http"
+	"net/mail"
 	"regexp"
 	"strconv"
 	"strings"
@@ -113,6 +114,7 @@ var (
 				EventItem: EventItem{ID: "hdI7aIgUO1hFplCIcJHB0jShRVsAzS0AB75wGCaiNVeIHXLmaUnt4eJ8l7c7L6uk4g0ZdXhGWG5gfh6HHgAZnw==", Action: EventCreate},
 				Created: &Message{
 					ID:      "hdI7aIgUO1hFplCIcJHB0jShRVsAzS0AB75wGCaiNVeIHXLmaUnt4eJ8l7c7L6uk4g0ZdXhGWG5gfh6HHgAZnw==",
+					Header:  make(mail.Header),
 					Subject: "Hey there",
 				},
 			},
@@ -153,6 +155,7 @@ var (
 				EventItem: EventItem{ID: "msgID1", Action: EventCreate},
 				Created: &Message{
 					ID:      "id",
+					Header:  make(mail.Header),
 					Subject: "Hey there",
 				},
 			},
@@ -160,6 +163,7 @@ var (
 				EventItem: EventItem{ID: "msgID2", Action: EventCreate},
 				Created: &Message{
 					ID:      "id",
+					Header:  make(mail.Header),
 					Subject: "Hey there again",
 				},
 			},

pkg/pmapi/messages.go

@@ -24,6 +24,7 @@ import (
 	"encoding/base64"
 	"encoding/json"
 	"errors"
+	"fmt"
 	"io"
 	"net/http"
 	"net/mail"
@@ -32,6 +33,7 @@ import (
 	"strings"
 
 	pmcrypto "github.com/ProtonMail/gopenpgp/crypto"
+	"github.com/sirupsen/logrus"
 	"golang.org/x/crypto/openpgp/packet"
 )
 
@@ -211,9 +213,13 @@ func (m *Message) UnmarshalJSON(b []byte) error {
 
 	if raw.Header != "" && raw.Header != "(No Header)" {
 		msg, err := mail.ReadMessage(strings.NewReader(raw.Header + "\r\n\r\n"))
-		if err == nil {
-			m.Header = msg.Header
+		if err != nil {
+			logrus.WithField("rawHeader", raw.Header).Trace("Failed to parse header")
+			return fmt.Errorf("failed to parse header of message %v: %v", m.ID, err.Error())
 		}
+		m.Header = msg.Header
+	} else {
+		m.Header = make(mail.Header)
 	}
 
 	return nil
@@ -532,8 +538,7 @@ func (c *Client) GetMessage(id string) (msg *Message, err error) {
 		return
 	}
 
-	msg, err = res.Message, res.Err()
-	return
+	return res.Message, res.Err()
 }
 
 type SendMessageReq struct {

pkg/pmapi/res.go

@@ -17,6 +17,12 @@
 
 package pmapi
 
+import (
+	"net/http"
+
+	"github.com/pkg/errors"
+)
+
 // Common response codes.
 const (
 	CodeOk = 1000
@@ -35,6 +41,10 @@ type Res struct {
 
 // Err returns error if the response is an error. Otherwise, returns nil.
 func (res Res) Err() error {
+	if res.StatusCode == http.StatusUnprocessableEntity {
+		return &ErrUnprocessableEntity{errors.New(res.Error)}
+	}
+
 	if res.ResError == nil {
 		return nil
 	}

pkg/updates/updates.go

@@ -103,7 +103,7 @@ func (u *Updates) CreateJSONAndSign(deployDir, goos string) error {
 		return err
 	}
 
-	if err = ioutil.WriteFile(versionFilePath, txt, 0644); err != nil {
+	if err = ioutil.WriteFile(versionFilePath, txt, 0644); err != nil { //nolint[gosec]
 		return err
 	}
 

release-notes/bugs.txt

@@ -1,2 +1 @@
-• Fixed rare case of sending the same message multiple times in Outlook
-• Fixed bug in macOS update process; available from next update
+• Fixed ignored import to Sent folder

release-notes/notes.txt

@@ -1,8 +0,0 @@
-NOTE: We recommend to reconfigure your email client after upgrading to ensure the best results with the new draft folder support
-
-• Faster and more resilient mail synchronization process, especially for large mailboxes
-• Added "Alternate Routing" feature to mitigate blocking of Proton Servers
-• Added synchronization of draft folder
-• Improved event handling when there are frequent changes
-• Security improvements for loading dependent libraries
-• Minor UI & API communication tweaks

test/README.md

@@ -36,6 +36,14 @@ graph LR
 We want to test Bridge app from outside as much as possible. So we mock server (API),
 credentials store and call commands to IMAP or SMTP the same way as client would do.
 
+## Running tests
+
+In order to run Integration tests just go into the test folder `cd test`
+and run `make test`.
+
+You can also test only specific feature (or subset of features) by using `FEATURES` environment 
+variable: `FEATURES=features/imap/message/create.feature make test`.
+
 ## Example test
 
 BDD test in gherkin (cucumber) format (https://cucumber.io/docs/gherkin/reference/).
@@ -99,6 +107,10 @@ we can always be sure what each steps does or should do.
 In the code, we separate those parts in its own files to make sure
 it's clear how the function should be implemented.
 
+In the `Given` phase is also generally better to setup data (as `there are messages...`) 
+first, then users (`there is connected user...`) and then connections (`there is IMAP client...`). 
+This can prevent some hitches in internal implementation of integration tests.
+
 ## API faked by fakeapi or liveapi
 
 We need to control what server returns. Instead of using raw JSONs,

test/features/imap/message/create.feature

@@ -18,7 +18,6 @@ Feature: IMAP create messages
       | from      | to                 | subject | read |
       | [primary] | john.doe@email.com | foo     | true |
 
-  @ignore
   Scenario: Creates message sent from user's primary address
     Given there is IMAP client selected in "Sent"
     When IMAP client creates message "foo" from address "primary" of "userMoreAddresses" to "john.doe@email.com" with body "hello world" in "Sent"
@@ -29,7 +28,6 @@ Feature: IMAP create messages
       | [primary] | john.doe@email.com | foo     | true |
     And mailbox "INBOX" for "userMoreAddresses" has no messages
 
-  @ignore
   Scenario: Creates message sent from user's secondary address
     Given there is IMAP client selected in "Sent"
     When IMAP client creates message "foo" from address "secondary" of "userMoreAddresses" to "john.doe@email.com" with body "hello world" in "Sent"
@@ -57,3 +55,16 @@ Feature: IMAP create messages
       | from              | to                    | subject | read |
       | notuser@gmail.com | alsonotuser@gmail.com | foo     | true |
     And mailbox "INBOX" for "userMoreAddresses" has no messages
+
+  # Importing duplicate messages when messageID cannot be found in Sent already.
+  # 
+  # Previously, we discarded messages for which sender matches account address to
+  # avoid duplicates, but this led to discarding messages imported through mail client.
+  Scenario: Imports a similar (duplicate) message to sent
+    Given there are messages in mailbox "Sent" for "userMoreAddresses"
+      | from      | to             | subject        | body                |
+      | [primary] | chosen@one.com | Meet the Twins | Hello, Mr. Anderson |
+    And there is IMAP client selected in "Sent"
+    When IMAP client creates message "Meet the Twins" from address "primary" of "userMoreAddresses" to "chosen@one.com" with body "Hello, Mr. Anderson" in "Sent"
+    Then IMAP response is "OK"
+    And mailbox "Sent" for "userMoreAddresses" has 2 messages

test/features/imap/message/delete.feature

@@ -4,8 +4,6 @@ Feature: IMAP delete messages
     And there is "user" with mailbox "Folders/mbox"
     And there is "user" with mailbox "Labels/label"
 
-  # https://gitlab.protontech.ch/ProtonMail/Slim-API/issues/1420
-  @ignore-live
   Scenario Outline: Delete message
     Given there are 10 messages in mailbox "<mailbox>" for "user"
     And there is IMAP client logged in as "user"
@@ -19,11 +17,8 @@ Feature: IMAP delete messages
       | INBOX        |
       | Folders/mbox |
       | Labels/label |
-      | Drafts       |
       | Trash        |
 
-  # https://gitlab.protontech.ch/ProtonMail/Slim-API/issues/1420
-  @ignore-live
   Scenario Outline: Delete all messages
     Given there are 10 messages in mailbox "<mailbox>" for "user"
     And there is IMAP client logged in as "user"
@@ -37,5 +32,4 @@ Feature: IMAP delete messages
       | INBOX        |
       | Folders/mbox |
       | Labels/label |
-      | Drafts       |
       | Trash        |

test/features/imap/message/move.feature

@@ -9,9 +9,8 @@ Feature: IMAP move messages
     And there is IMAP client logged in as "user"
     And there is IMAP client selected in "INBOX"
 
-  @ignore
   Scenario: Move message
-    When IMAP client moves messages "1" to "Folders/mbox"
+    When IMAP client moves messages "2" to "Folders/mbox"
     Then IMAP response is "OK"
     And mailbox "INBOX" for "user" has messages
       | from              | to         | subject |
@@ -20,7 +19,6 @@ Feature: IMAP move messages
       | from              | to         | subject |
       | john.doe@mail.com | user@pm.me | foo     |
 
-  @ignore
   Scenario: Move all messages
     When IMAP client moves messages "1:*" to "Folders/mbox"
     Then IMAP response is "OK"
@@ -30,20 +28,8 @@ Feature: IMAP move messages
       | john.doe@mail.com | user@pm.me | foo     |
       | jane.doe@mail.com | name@pm.me | bar     |
 
-  @ignore
-  Scenario: Move message to All Mail
-    When IMAP client moves messages "1" to "All Mail"
-    Then IMAP response is "OK"
-    And mailbox "INBOX" for "user" has messages
-      | from              | to         | subject |
-      | jane.doe@mail.com | name@pm.me | bar     |
-    And mailbox "All Mail" for "user" has messages
-      | from              | to         | subject |
-      | john.doe@mail.com | user@pm.me | foo     |
-
-  @ignore
   Scenario: Move message from All Mail is not possible
-    When IMAP client moves messages "1" to "Folders/mbox"
+    When IMAP client moves messages "2" to "Folders/mbox"
     Then IMAP response is "OK"
     And mailbox "All Mail" for "user" has messages
       | from              | to         | subject |

test/liveapi/cleanup.go

@@ -38,7 +38,7 @@ func cleanup(client *pmapi.Client) error {
 }
 
 func cleanSystemFolders(client *pmapi.Client) error {
-	for _, labelID := range []string{pmapi.InboxLabel, pmapi.SentLabel, pmapi.ArchiveLabel} {
+	for _, labelID := range []string{pmapi.InboxLabel, pmapi.SentLabel, pmapi.ArchiveLabel, pmapi.AllMailLabel, pmapi.DraftLabel} {
 		for {
 			messages, total, err := client.ListMessages(&pmapi.MessagesFilter{
 				PageSize: 150,