fix: return error if parsing header fails GODT-502

.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

Changelog.md

@@ -2,7 +2,16 @@
 
 Changelog [format](http://keepachangelog.com/en/1.0.0/)
 
-## [v1.2.7] Emma - beta (2020-04-XXX)
+## [v1.2.8] Donghai-fix-append  (2020-06-XXX)
+
+### Changed
+* GODT-396 reduce number of EXISTS calls
+* GODT-143 Allow appending to Sent folder when sender matches account address
+
+### 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
@@ -24,6 +33,8 @@ Changelog [format](http://keepachangelog.com/en/1.0.0/)
 ### Fixed
 * Use correct binary name when finding location of addcert.scpt
 
+
+
 ## [v1.2.6] Donghai - beta (2020-03-31)
 
 ### Added

internal/bridge/release_notes.go

@@ -15,14 +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 18 May 2020 02:00:37 PM CEST. 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 = `• Changed directory for storing logs on Linux
+const ReleaseNotes = `
 `
 
-const ReleaseFixedBugs = `• Fixed sync errors due to localization
-• Fixed draft deletion
-• Fixed re-appearing messages in Outlook
+const ReleaseFixedBugs = `• Fixed ignored import to Sent folder
 `

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/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

@@ -290,7 +290,6 @@ func (storeMailbox *Mailbox) txCreateOrUpdateMessages(tx *bolt.Tx, msgs []*pmapi
 						seqNum,
 						msg,
 					)
-					shouldSendMailboxUpdate = true
 				}
 				continue
 			}

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)

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

release-notes/bugs.txt

@@ -1,3 +1 @@
-• Fixed sync errors due to localization
-• Fixed draft deletion
-• Fixed re-appearing messages in Outlook
+• Fixed ignored import to Sent folder

release-notes/notes.txt

@@ -1 +0,0 @@
-• Changed directory for storing logs on Linux

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