Other: Bridge Iron 1.7.1

Changelog.md

@@ -2,17 +2,25 @@
 
 Changelog [format](http://keepachangelog.com/en/1.0.0/)
 
+## [Bridge 1.7.1] Iron
+
+### Fixed
+* GODT-1081 Properly return newlines when returning headers.
+* GODT-1150 Externally encrypted messages with missing private key would not be built with custom message.
+* GODT-1141 Attachment is named as attachment.bin in some cases.
+
+
 ## [Bridge 1.7.0] Iron
 
 ### Added
 * GODT-213 New message builder:
-    * Preserve Content-Type for undecryptable message body.
+* Preserve Content-Type for undecryptable message body.
-    * Use application/octet-stream for encrypted parts.
+* Use application/octet-stream for encrypted parts.
-    * Force no transfer encoding for embedded message/rfc822 parts.
+* Force no transfer encoding for embedded message/rfc822 parts.
-    * Remove dead code GetRelatedHeader/GetRelatedBoundary.
+* Remove dead code GetRelatedHeader/GetRelatedBoundary.
-    * Correctly expect text/plain in custom message text parts.
+* Correctly expect text/plain in custom message text parts.
-    * Force text/plain for custom message text part.
+* Force text/plain for custom message text part.
-    * Complex external encrypted tests (multipart/alternative, message/rfc822 attachment).
+* Complex external encrypted tests (multipart/alternative, message/rfc822 attachment).
 
 ### Fixed
 * GODT-1136 DB Cache header from builder and test.

Makefile

@@ -10,7 +10,7 @@ TARGET_OS?=${GOOS}
 .PHONY: build build-ie build-nogui build-ie-nogui build-launcher build-launcher-ie  versioner hasher
 
 # Keep version hardcoded so app build works also without Git repository.
-BRIDGE_APP_VERSION?=1.7.0+git
+BRIDGE_APP_VERSION?=1.7.1+git
 IE_APP_VERSION?=1.3.3+git
 APP_VERSION:=${BRIDGE_APP_VERSION}
 SRC_ICO:=logo.ico

internal/imap/mailbox_message.go

@@ -410,12 +410,13 @@ func isMessageInDraftFolder(m *pmapi.Message) bool {
 
 // This will download message (or read from cache) and pick up the section,
 // extract data (header,body, both) and trim the output if needed.
-func (im *imapMailbox) getMessageBodySection(
+func (im *imapMailbox) getMessageBodySection( //nolint[funlen]
 	storeMessage storeMessageProvider,
 	section *imap.BodySectionName,
 	msgBuildCountHistogram *msgBuildCountHistogram,
 ) (imap.Literal, error) {
 	var header textproto.MIMEHeader
+	var extraNewlineAfterHeader bool
 	var response []byte
 
 	im.log.WithField("msgID", storeMessage.ID()).Trace("Getting message body")
@@ -449,6 +450,9 @@ func (im *imapMailbox) getMessageBodySection(
 		case section.Specifier == imap.MIMESpecifier: // The MIME part specifier refers to the [MIME-IMB] header for this part.
 			fallthrough
 		case section.Specifier == imap.HeaderSpecifier:
+			if content, err := structure.GetSectionContent(bodyReader, section.Path); err == nil && content != nil {
+				extraNewlineAfterHeader = true
+			}
 			header, err = structure.GetSectionHeader(section.Path)
 		default:
 			err = errors.New("Unknown specifier " + string(section.Specifier))
@@ -461,6 +465,11 @@ func (im *imapMailbox) getMessageBodySection(
 
 	if header != nil {
 		response = filteredHeaderAsBytes(header, section)
+		// The blank line is included in all header fetches,
+		// except in the case of a message which has no body.
+		if extraNewlineAfterHeader {
+			response = append(response, []byte("\r\n")...)
+		}
 	}
 
 	// Trim any output if requested.

pkg/message/build_framework_test.go

@@ -291,7 +291,7 @@ func (matcher decryptsToMatcher) match(t *testing.T, have string) {
 	dec, err := matcher.kr.Decrypt(haveMsg, nil, crypto.GetUnixTime())
 	require.NoError(t, err)
 
-	assert.Equal(t, matcher.want, dec.GetString())
+	assert.Equal(t, matcher.want, string(dec.GetBinary()))
 }
 
 func decryptsTo(kr *crypto.KeyRing, want string) decryptsToMatcher {

pkg/message/build_rfc822.go

@@ -40,7 +40,7 @@ func buildRFC822(kr *crypto.KeyRing, msg *pmapi.Message, attData [][]byte, opts
 		return buildMultipartRFC822(kr, msg, attData, opts)
 
 	case msg.MIMEType == "multipart/mixed":
-		return buildEncryptedRFC822(kr, msg, opts)
+		return buildExternallyEncryptedRFC822(kr, msg, opts)
 
 	default:
 		return buildSimpleRFC822(kr, msg, opts)
@@ -146,25 +146,10 @@ func writeTextPart(
 			return errors.Wrap(ErrDecryptionFailed, err.Error())
 		}
 
-		/*
-			if len(msg.Attachments) > 0 {
-				return writeCustomTextPartAsAttachment(w, msg, err)
-			}
-		*/
-
 		return writeCustomTextPart(w, msg, err)
 	}
 
-	part, err := w.CreatePart(getTextPartHeader(message.Header{}, dec, msg.MIMEType))
+	return writePart(w, getTextPartHeader(message.Header{}, dec, msg.MIMEType), dec)
-	if err != nil {
-		return err
-	}
-
-	if _, err := part.Write(dec); err != nil {
-		return err
-	}
-
-	return part.Close()
 }
 
 func writeAttachmentPart(
@@ -196,16 +181,7 @@ func writeAttachmentPart(
 		return writeCustomAttachmentPart(w, att, msg, err)
 	}
 
-	part, err := w.CreatePart(getAttachmentPartHeader(att))
+	return writePart(w, getAttachmentPartHeader(att), dec.GetBinary())
-	if err != nil {
-		return err
-	}
-
-	if _, err := part.Write(dec.GetBinary()); err != nil {
-		return err
-	}
-
-	return part.Close()
 }
 
 func writeRelatedParts(
@@ -221,25 +197,31 @@ func writeRelatedParts(
 
 	hdr.SetContentType("multipart/related", map[string]string{"boundary": boundary.gen()})
 
-	rel, err := w.CreatePart(hdr)
+	return createPart(w, hdr, func(rel *message.Writer) error {
-	if err != nil {
+		if err := writeTextPart(rel, kr, msg, opts); err != nil {
-		return err
-	}
-
-	if err := writeTextPart(rel, kr, msg, opts); err != nil {
-		return err
-	}
-
-	for i, att := range atts {
-		if err := writeAttachmentPart(rel, kr, att, attData[i], opts); err != nil {
 			return err
 		}
-	}
 
-	return rel.Close()
+		for i, att := range atts {
+			if err := writeAttachmentPart(rel, kr, att, attData[i], opts); err != nil {
+				return err
+			}
+		}
+
+		return nil
+	})
 }
 
-func buildEncryptedRFC822(kr *crypto.KeyRing, msg *pmapi.Message, opts JobOptions) ([]byte, error) {
+func buildExternallyEncryptedRFC822(kr *crypto.KeyRing, msg *pmapi.Message, opts JobOptions) ([]byte, error) {
+	dec, err := msg.Decrypt(kr)
+	if err != nil {
+		if !opts.IgnoreDecryptionErrors {
+			return nil, errors.Wrap(ErrDecryptionFailed, err.Error())
+		}
+
+		return buildPGPMIMERFC822(msg, opts)
+	}
+
 	hdr := getMessageHeader(msg, opts)
 
 	hdr.SetContentType("multipart/mixed", map[string]string{"boundary": newBoundary(msg.ID).gen()})
@@ -251,31 +233,58 @@ func buildEncryptedRFC822(kr *crypto.KeyRing, msg *pmapi.Message, opts JobOption
 		return nil, err
 	}
 
-	dec, err := msg.Decrypt(kr)
-	if err != nil {
-		return nil, errors.Wrap(ErrDecryptionFailed, err.Error())
-	}
-
 	ent, err := message.Read(bytes.NewReader(dec))
 	if err != nil {
 		return nil, err
 	}
 
-	part, err := w.CreatePart(ent.Header)
-	if err != nil {
-		return nil, err
-	}
-
 	body, err := ioutil.ReadAll(ent.Body)
 	if err != nil {
 		return nil, err
 	}
 
-	if _, err := part.Write(body); err != nil {
+	if err := writePart(w, ent.Header, body); err != nil {
 		return nil, err
 	}
 
-	if err := part.Close(); err != nil {
+	if err := w.Close(); err != nil {
+		return nil, err
+	}
+
+	return buf.Bytes(), nil
+}
+
+func buildPGPMIMERFC822(msg *pmapi.Message, opts JobOptions) ([]byte, error) {
+	hdr := getMessageHeader(msg, opts)
+
+	hdr.SetContentType("multipart/encrypted", map[string]string{
+		"boundary": newBoundary(msg.ID).gen(),
+		"protocol": "application/pgp-encrypted",
+	})
+
+	buf := new(bytes.Buffer)
+
+	w, err := message.CreateWriter(buf, hdr)
+	if err != nil {
+		return nil, err
+	}
+
+	var encHdr message.Header
+
+	encHdr.SetContentType("application/pgp-encrypted", nil)
+	encHdr.Set("Content-Description", "PGP/MIME version identification")
+
+	if err := writePart(w, encHdr, []byte("Version: 1")); err != nil {
+		return nil, err
+	}
+
+	var dataHdr message.Header
+
+	dataHdr.SetContentType("application/octet-stream", map[string]string{"name": "encrypted.asc"})
+	dataHdr.SetContentDisposition("inline", map[string]string{"filename": "encrypted.asc"})
+	dataHdr.Set("Content-Description", "OpenPGP encrypted message")
+
+	if err := writePart(w, dataHdr, []byte(msg.Body)); err != nil {
 		return nil, err
 	}
 
@@ -432,3 +441,26 @@ func toAddressList(addrs []*mail.Address) string {
 
 	return strings.Join(res, ", ")
 }
+
+func createPart(w *message.Writer, hdr message.Header, fn func(*message.Writer) error) error {
+	part, err := w.CreatePart(hdr)
+	if err != nil {
+		return err
+	}
+
+	if err := fn(part); err != nil {
+		return err
+	}
+
+	return part.Close()
+}
+
+func writePart(w *message.Writer, hdr message.Header, body []byte) error {
+	return createPart(w, hdr, func(part *message.Writer) error {
+		if _, err := part.Write(body); err != nil {
+			return errors.Wrap(err, "failed to write part body")
+		}
+
+		return nil
+	})
+}

pkg/message/build_test.go

@@ -1008,6 +1008,51 @@ func TestBuildCustomMessageHTML(t *testing.T) {
 		expectTransferEncoding(isMissing())
 }
 
+func TestBuildCustomMessageEncrypted(t *testing.T) {
+	m := gomock.NewController(t)
+	defer m.Finish()
+
+	b := NewBuilder(1, 1, 1)
+	defer b.Done()
+
+	kr := tests.MakeKeyRing(t)
+
+	body := readerToString(getFileReader("pgp-mime-body-plaintext.eml"))
+
+	// Use a different keyring for encrypting the message; it won't be decryptable.
+	foreignKR := tests.MakeKeyRing(t)
+	msg := newTestMessage(t, foreignKR, "messageID", "addressID", "multipart/mixed", body, time.Date(2020, time.January, 1, 0, 0, 0, 0, time.UTC))
+
+	msg.Subject = "this is a subject to make sure we preserve subject"
+
+	// Tell the job to ignore decryption errors; a custom message will be returned instead of an error.
+	res, err := b.NewJobWithOptions(
+		context.Background(),
+		newTestFetcher(m, kr, msg),
+		msg.ID,
+		JobOptions{IgnoreDecryptionErrors: true},
+	).GetResult()
+	require.NoError(t, err)
+
+	section(t, res).
+		expectHeader(`Subject`, is(msg.Subject)).
+		expectContentType(is(`multipart/encrypted`)).
+		expectContentTypeParam(`protocol`, is(`application/pgp-encrypted`))
+
+	section(t, res, 1).
+		expectContentType(is(`application/pgp-encrypted`)).
+		expectHeader(`Content-Description`, is(`PGP/MIME version identification`)).
+		expectBody(is(`Version: 1`))
+
+	section(t, res, 2).
+		expectContentType(is(`application/octet-stream`)).
+		expectContentTypeParam(`name`, is(`encrypted.asc`)).
+		expectContentDisposition(is(`inline`)).
+		expectContentDispositionParam(`filename`, is(`encrypted.asc`)).
+		expectHeader(`Content-Description`, is(`OpenPGP encrypted message`)).
+		expectBody(decryptsTo(foreignKR, body))
+}
+
 func TestBuildCustomMessagePlainWithAttachment(t *testing.T) {
 	m := gomock.NewController(t)
 	defer m.Finish()

pkg/message/parser.go

@@ -503,12 +503,15 @@ func parseAttachment(h message.Header) (*pmapi.Attachment, error) {
 	}
 	att.Header = mimeHeader
 
-	mimeType, _, err := h.ContentType()
+	mimeType, mimeTypeParams, err := h.ContentType()
 	if err != nil {
 		return nil, err
 	}
 	att.MIMEType = mimeType
 
+	// Prefer attachment name from filename param in content disposition.
+	// If not available, try to get it from name param in content type.
+	// Otherwise fallback to attachment.bin.
 	_, dispParams, dispErr := h.ContentDisposition()
 	if dispErr != nil {
 		ext, err := mime.ExtensionsByType(att.MIMEType)
@@ -521,10 +524,12 @@ func parseAttachment(h message.Header) (*pmapi.Attachment, error) {
 		}
 	} else {
 		att.Name = dispParams["filename"]
-
+	}
-		if att.Name == "" {
+	if att.Name == "" {
-			att.Name = "attachment.bin"
+		att.Name = mimeTypeParams["name"]
-		}
+	}
+	if att.Name == "" {
+		att.Name = "attachment.bin"
 	}
 
 	// Only set ContentID if it should be inline;

pkg/message/parser_test.go

@@ -239,6 +239,24 @@ func TestParseTextPlainWithOctetAttachmentBadFilename(t *testing.T) {
 	assert.Equal(t, "attachment.bin", m.Attachments[0].Name)
 }
 
+func TestParseTextPlainWithOctetAttachmentNameInContentType(t *testing.T) {
+	f := getFileReader("text_plain_octet_attachment_name_in_contenttype.eml")
+
+	m, _, _, _, err := Parse(f) //nolint[dogsled]
+	require.NoError(t, err)
+
+	assert.Equal(t, "attachment-contenttype.txt", m.Attachments[0].Name)
+}
+
+func TestParseTextPlainWithOctetAttachmentNameConflict(t *testing.T) {
+	f := getFileReader("text_plain_octet_attachment_name_conflict.eml")
+
+	m, _, _, _, err := Parse(f) //nolint[dogsled]
+	require.NoError(t, err)
+
+	assert.Equal(t, "attachment-disposition.txt", m.Attachments[0].Name)
+}
+
 func TestParseTextPlainWithPlainAttachment(t *testing.T) {
 	f := getFileReader("text_plain_plain_attachment.eml")
 

pkg/message/testdata/text_plain_octet_attachment_name_conflict.eml

@@ -0,0 +1,14 @@
+From: Sender <sender@pm.me>
+To: Receiver <receiver@pm.me>
+Content-Type: multipart/mixed; boundary=longrandomstring
+
+--longrandomstring
+
+body
+--longrandomstring
+Content-Type: application/octet-stream; name="attachment-contenttype.txt"
+Content-Transfer-Encoding: base64
+Content-Disposition: attachment; filename="attachment-disposition.txt"
+
+aWYgeW91IGFyZSByZWFkaW5nIHRoaXMsIGhpIQ==
+--longrandomstring--

pkg/message/testdata/text_plain_octet_attachment_name_in_contenttype.eml

@@ -0,0 +1,14 @@
+From: Sender <sender@pm.me>
+To: Receiver <receiver@pm.me>
+Content-Type: multipart/mixed; boundary=longrandomstring
+
+--longrandomstring
+
+body
+--longrandomstring
+Content-Type: application/octet-stream; name="attachment-contenttype.txt"
+Content-Transfer-Encoding: base64
+Content-Disposition: attachment
+
+aWYgeW91IGFyZSByZWFkaW5nIHRoaXMsIGhpIQ==
+--longrandomstring--

release-notes/bridge_early.md

@@ -1,3 +1,18 @@
+## v1.7.0
+- 2021-04-21
+
+### New
+
+- Refactor of message builder to achieve greater RFC compliance
+- Increased the number of message fetchers to allow more parallel requests - performance improvement
+- Log changes for easier debugging (update-related)
+
+### Fixed
+
+- Removed html-wrappig of non-decriptable messages - to facilitate decryption outside Bridge and/or allow to store such messages as they are
+- Tray icon issues with multiple displays on MacOS
+
+
 ## v1.6.9
 - 2021-03-30
 

test/features/bridge/imap/message/fetch.feature

@@ -69,8 +69,9 @@ Feature: IMAP fetch messages
     And there is IMAP client selected in "Folders/mbox"
     When IMAP client sends command "FETCH 1:* rfc822"
     Then IMAP response is "OK"
-    And IMAP response contains "Date: Fri, 13 Aug 1982"
+    And IMAP response contains "\nDate: Fri, 13 Aug 1982"
-    And IMAP response contains "X-Original-Date: Sun, 20 Jul 1969"
+    And IMAP response contains "\nX-Pm-Date: Thu, 01 Jan 1970"
+    And IMAP response contains "\nX-Original-Date: Sun, 20 Jul 1969"
     # We had bug to incorectly set empty date, so let's make sure
     # there is no reference anywhere in the response.
-    And IMAP response does not contain "Date: Thu, 01 Jan 1970"
+    And IMAP response does not contain "\nDate: Thu, 01 Jan 1970"

test/store_setup_test.go

@@ -115,7 +115,7 @@ func thereAreMessagesInMailboxesForAddressOfUser(mailboxNames, bddAddressID, bdd
 			if column == "deleted" {
 				hasDeletedFlag = cell.Value == "true"
 			}
-			err := processMessageTableCell(column, cell.Value, account.Username(), message, header)
+			err := processMessageTableCell(column, cell.Value, account.Username(), message, &header)
 			if err != nil {
 				return err
 			}
@@ -151,7 +151,7 @@ func thereAreMessagesInMailboxesForAddressOfUser(mailboxNames, bddAddressID, bdd
 	return nil
 }
 
-func processMessageTableCell(column, cellValue, username string, message *pmapi.Message, header textproto.MIMEHeader) error {
+func processMessageTableCell(column, cellValue, username string, message *pmapi.Message, header *textproto.MIMEHeader) error {
 	switch column {
 	case "deleted", "id": // it is processed in the main function
 	case "from":
@@ -187,8 +187,13 @@ func processMessageTableCell(column, cellValue, username string, message *pmapi.
 		if err != nil {
 			return internalError(err, "parsing time")
 		}
-		message.Time = date.Unix()
 		header.Set("Date", date.Format(time.RFC1123Z))
+		// API will sanitize the date to not have negative timestamp
+		if date.After(time.Unix(0, 0)) {
+			message.Time = date.Unix()
+		} else {
+			message.Time = 0
+		}
 	case "n attachments":
 		numAttachments, err := strconv.Atoi(cellValue)
 		if err != nil {