Merge branch 'release/forth' into release/danube

Changelog.md

@@ -11,19 +11,45 @@ Changelog [format](http://keepachangelog.com/en/1.0.0/)
 * Bump crypto version to v0.0.0-20200818122824-ed5d25e28db8
 
 
+## [Bridge 1.4.4] Forth
+
+### Fixed
+* GODT-798 Replace, don't add, transfer encoding when making body 7-bit clean.
+* Move/Copy duplicate for emails with References in Outlook
+* CSB-247 Cannot update from 1.4.0
+
+
+## [Bridge 1.4.3] Forth
+
+### Changed
+* Reverted sending IMAP updates to be not blocking again.
+
+### Fixed
+* GODT-783 Settings flags by FLAGS (not using +/-FLAGS) do not change spam state.
+
+
+## [Bridge 1.4.2] Forth
+
+### Changed
+* GODT-761 Use label.Path instead of Name to partially support subfolders for webapp beta release.
+* GODT-765 Improve speed of checking whether message is deleted.
+
+
 ## [IE 1.1.1] Danube (beta 2020-09-xx) [Bridge 1.4.1] Forth (beta 2020-09-xx)
 
 ### Fixed
 * GODT-752 Parsing message with empty addresses.
 * GODT-752 Parsing non-utf8 multipart/alternative message.
 * GODT-752 Parsing message with duplicate charset parameter.
 
+
 ## [IE 1.1.0] Danube
 
 ### Fixed
 * GODT-703 Import-Export showed always at least one total message.
 * GODT-738 Fix for mbox files with long lines.
 
+
 ## [Bridge 1.4.0] Forth
 
 ### Added

internal/imap/mailbox_message.go

@@ -24,7 +24,6 @@ import (
 	"mime/multipart"
 	"net/mail"
 	"net/textproto"
-	"regexp"
 	"sort"
 	"strings"
 	"time"
@@ -141,18 +140,19 @@ func (im *imapMailbox) CreateMessage(flags []string, date time.Time, body imap.L
 	references := m.Header.Get("References")
 	referenceList := strings.Fields(references)
 
-	if len(referenceList) > 0 {
+	// In case there is a mail client which corrupts headers, try
+	// "References" too.
+	if internalID == "" && len(referenceList) > 0 {
 		lastReference := referenceList[len(referenceList)-1]
-		// In case we are using a mail client which corrupts headers, try "References" too.
-		re := regexp.MustCompile(pmapi.InternalReferenceFormat)
-		match := re.FindStringSubmatch(lastReference)
-		if len(match) > 0 {
-			internalID = match[0]
+		match := pmapi.RxInternalReferenceFormat.FindStringSubmatch(lastReference)
+		if len(match) == 2 {
+			internalID = match[1]
 		}
 	}
 
-	// Avoid appending a message which is already on the server. Apply the new
-	// label instead. This sometimes happens which Outlook (it uses APPEND instead of COPY).
+	// Avoid appending a message which is already on the server. Apply the
+	// new label instead. This always happens with Outlook (it uses APPEND
+	// instead of COPY).
 	if internalID != "" {
 		// Check to see if this belongs to a different address in split mode or another ProtonMail account.
 		msg, err := im.storeMailbox.GetMessage(internalID)

internal/imap/mailbox_messages.go

@@ -57,6 +57,10 @@ func (im *imapMailbox) UpdateMessagesFlags(uid bool, seqSet *imap.SeqSet, operat
 	return im.addOrRemoveFlags(operation, messageIDs, flags)
 }
 
+// setFlags is used for FLAGS command (not +FLAGS or -FLAGS), which means
+// to set flags passed as an argument and unset the rest. For example,
+// if message is not read, is flagged and is not deleted, call FLAGS \Seen
+// should flag message as read, unflagged and keep undeleted.
 func (im *imapMailbox) setFlags(messageIDs, flags []string) error { //nolint
 	seen := false
 	flagged := false
@@ -106,16 +110,17 @@ func (im *imapMailbox) setFlags(messageIDs, flags []string) error { //nolint
 		}
 	}
 
-	spamMailbox, err := im.storeAddress.GetMailbox("Spam")
-	if err != nil {
-		return err
-	}
+	// Spam should not be taken into action here as Outlook is using FLAGS
+	// without preserving junk flag. Probably it's because junk is not standard
+	// in the rfc3501 and thus Outlook expects calling FLAGS \Seen will not
+	// change the state of junk or other non-standard flags.
+	// Still, its safe to label as spam once any client sends the request.
 	if spam {
-		if err := spamMailbox.LabelMessages(messageIDs); err != nil {
+		spamMailbox, err := im.storeAddress.GetMailbox("Spam")
+		if err != nil {
 			return err
 		}
-	} else {
-		if err := spamMailbox.UnlabelMessages(messageIDs); err != nil {
+		if err := spamMailbox.LabelMessages(messageIDs); err != nil {
 			return err
 		}
 	}

internal/smtp/user.go

@@ -25,7 +25,6 @@ import (
 	"io"
 	"mime"
 	"net/mail"
-	"regexp"
 	"strings"
 	"time"
 
@@ -408,9 +407,9 @@ func (su *smtpUser) handleReferencesHeader(m *pmapi.Message) (draftID, parentID
 		if !strings.Contains(reference, "@"+pmapi.InternalIDDomain) {
 			newReferences = append(newReferences, reference)
 		} else { // internalid is the parentID.
-			idMatch := regexp.MustCompile(pmapi.InternalReferenceFormat).FindStringSubmatch(reference)
-			if len(idMatch) > 0 {
-				lastID := strings.TrimSuffix(strings.Trim(idMatch[0], "<>"), "@protonmail.internalid")
+			idMatch := pmapi.RxInternalReferenceFormat.FindStringSubmatch(reference)
+			if len(idMatch) == 2 {
+				lastID := idMatch[1]
 				filter := &pmapi.MessagesFilter{ID: []string{lastID}}
 				if su.addressID != "" {
 					filter.AddressID = su.addressID

internal/store/address.go

@@ -69,7 +69,7 @@ func (storeAddress *Address) init(foldersAndLabels []*pmapi.Label) (err error) {
 			prefix := getLabelPrefix(label)
 
 			var mailbox *Mailbox
-			if mailbox, err = txNewMailbox(tx, storeAddress, label.ID, prefix, label.Name, label.Color); err != nil {
+			if mailbox, err = txNewMailbox(tx, storeAddress, label.ID, prefix, label.Path, label.Color); err != nil {
 				storeAddress.log.
 					WithError(err).
 					WithField("labelID", label.ID).

internal/store/address_mailbox.go

@@ -73,14 +73,14 @@ func (storeAddress *Address) createOrUpdateMailboxEvent(label *pmapi.Label) erro
 	prefix := getLabelPrefix(label)
 	mailbox, ok := storeAddress.mailboxes[label.ID]
 	if !ok {
-		mailbox, err := newMailbox(storeAddress, label.ID, prefix, label.Name, label.Color)
+		mailbox, err := newMailbox(storeAddress, label.ID, prefix, label.Path, label.Color)
 		if err != nil {
 			return err
 		}
 		storeAddress.mailboxes[label.ID] = mailbox
 		mailbox.store.imapMailboxCreated(storeAddress.address, mailbox.labelName)
 	} else {
-		mailbox.labelName = prefix + label.Name
+		mailbox.labelName = prefix + label.Path
 		mailbox.color = label.Color
 	}
 	return nil

internal/store/change.go

@@ -122,22 +122,10 @@ func (store *Store) imapSendUpdate(update imapBackend.Update) {
 		return
 	}
 
-	done := update.Done()
-	go func() {
-		// This timeout is to not keep running many blocked goroutines.
-		// In case nothing listens to this channel, this thread should stop.
-		select {
-		case store.imapUpdates <- update:
-		case <-time.After(1 * time.Second):
-			store.log.Warn("IMAP update could not be sent (timeout).")
-		}
-	}()
-
-	// This timeout is to not block IMAP backend by wait for IMAP client.
 	select {
-	case <-done:
 	case <-time.After(1 * time.Second):
-		store.log.Warn("IMAP update could not be delivered (timeout).")
+		store.log.Warn("IMAP update could not be sent (timeout)")
 		return
+	case store.imapUpdates <- update:
 	}
 }

internal/store/mailbox.go

@@ -142,6 +142,9 @@ func initMailboxBucket(tx *bolt.Tx, bucketName []byte) error {
 	if _, err := bucket.CreateBucketIfNotExists(apiIDsBucket); err != nil {
 		return err
 	}
+	if _, err := bucket.CreateBucketIfNotExists(deletedIDsBucket); err != nil {
+		return err
+	}
 
 	return nil
 }
@@ -240,13 +243,7 @@ func (storeMailbox *Mailbox) txGetAPIIDsBucket(tx *bolt.Tx) *bolt.Bucket {
 
 // txGetDeletedIDsBucket returns the bucket with messagesID marked as deleted
 func (storeMailbox *Mailbox) txGetDeletedIDsBucket(tx *bolt.Tx) *bolt.Bucket {
-	// There should be no error since it _...returns an error if the bucket
-	// name is blank, or if the bucket name is too long._
-	bucket, err := storeMailbox.txGetBucket(tx).CreateBucketIfNotExists(deletedIDsBucket)
-	if err != nil || bucket == nil {
-		storeMailbox.log.WithError(err).Error("Cannot create or get bucket with deleted IDs.")
-	}
-	return bucket
+	return storeMailbox.txGetBucket(tx).Bucket(deletedIDsBucket)
 }
 
 // txGetBucket returns the bucket of mailbox containing mapping buckets.

internal/store/mailbox_counts.go

@@ -125,6 +125,7 @@ func (mc *mailboxCounts) getPMLabel() *pmapi.Label {
 	return &pmapi.Label{
 		ID:        mc.LabelID,
 		Name:      mc.LabelName,
+		Path:      mc.LabelName,
 		Color:     mc.Color,
 		Order:     mc.Order,
 		Type:      pmapi.LabelTypeMailbox,
@@ -158,7 +159,7 @@ func (store *Store) createOrUpdateMailboxCountsBuckets(labels []*pmapi.Label) er
 			}
 
 			// Update mailbox info, but dont change on-API-counts.
-			mailbox.LabelName = label.Name
+			mailbox.LabelName = label.Path
 			mailbox.Color = label.Color
 			mailbox.Order = label.Order
 			mailbox.IsFolder = label.Exclusive == 1

internal/store/message.go

@@ -66,7 +66,7 @@ func (message *Message) Message() *pmapi.Message {
 // mailbox
 func (message *Message) IsMarkedDeleted() bool {
 	isMarkedAsDeleted := false
-	err := message.storeMailbox.db().Update(func(tx *bolt.Tx) error {
+	err := message.storeMailbox.db().View(func(tx *bolt.Tx) error {
 		isMarkedAsDeleted = message.storeMailbox.txGetDeletedIDsBucket(tx).Get([]byte(message.msg.ID)) != nil
 		return nil
 	})

internal/updates/updates.go

@@ -310,7 +310,9 @@ func (u *Updates) StartUpgrade(currentStatus chan<- Progress) { // nolint[funlen
 	status.UpdateDescription(InfoUpgrading)
 	switch runtime.GOOS {
 	case "windows": //nolint[goconst]
-		installerFile := strings.Split(u.winInstallerFile, "/")[1]
+		// Cannot use filepath.Base on windows it has different delimiter
+		split := strings.Split(u.winInstallerFile, "/")
+		installerFile := split[len(split)-1]
 		cmd := exec.Command("./" + installerFile) // nolint[gosec]
 		cmd.Dir = u.updateTempDir
 		status.Err = cmd.Start()

pkg/message/parser/writer.go

@@ -35,7 +35,7 @@ func newWriter(root *Part) *Writer {
 
 func (w *Writer) Write(ww io.Writer) error {
 	if !w.root.is7BitClean() {
-		w.root.Header.Add("Content-Transfer-Encoding", "base64")
+		w.root.Header.Set("Content-Transfer-Encoding", "base64")
 	}
 
 	msgWriter, err := message.CreateWriter(ww, w.root.Header)
@@ -68,7 +68,7 @@ func (w *Writer) write(writer *message.Writer, p *Part) error {
 
 func (w *Writer) writeAsChild(writer *message.Writer, p *Part) error {
 	if !p.is7BitClean() {
-		p.Header.Add("Content-Transfer-Encoding", "base64")
+		p.Header.Set("Content-Transfer-Encoding", "base64")
 	}
 
 	childWriter, err := writer.CreatePart(p.Header)

pkg/pmapi/labels.go

@@ -80,6 +80,7 @@ const (
 type Label struct {
 	ID        string
 	Name      string
+	Path      string
 	Color     string
 	Order     int `json:",omitempty"`
 	Display   int // Not used for now, leave it empty.

pkg/pmapi/messages.go

@@ -29,6 +29,7 @@ import (
 	"net/http"
 	"net/mail"
 	"net/url"
+	"regexp"
 	"strconv"
 	"strings"
 
@@ -149,8 +150,9 @@ const ConversationIDDomain = `protonmail.conversationid`
 // InternalIDDomain is used as a placeholder for reference/message ID headers to improve compatibility with various clients.
 const InternalIDDomain = `protonmail.internalid`
 
-// InternalReferenceFormat describes format of the message ID (as regex) used for parsing reference headers.
-const InternalReferenceFormat = `(?U)<.*@` + InternalIDDomain + `>`
+// RxInternalReferenceFormat is compiled regexp which describes the match for
+// a message ID used in reference headers.
+var RxInternalReferenceFormat = regexp.MustCompile(`(?U)<(.+)@` + regexp.QuoteMeta(InternalIDDomain) + `>`) //nolint[gochecknoglobals]
 
 // Message structure.
 type Message struct {

test/fakeapi/controller_control.go

@@ -83,6 +83,9 @@ func (ctl *Controller) AddUserLabel(username string, label *pmapi.Label) error {
 	}
 	label.ID = ctl.labelIDGenerator.next(prefix)
 	label.Name = labelName
+	if label.Path == "" {
+		label.Path = label.Name
+	}
 	ctl.labelsByUsername[username] = append(ctl.labelsByUsername[username], label)
 	ctl.resetUsers()
 	return nil