From 60b1c4d8f7611ec15e8a6c5029f9015cc3f72d47 Mon Sep 17 00:00:00 2001
From: James Houlahan <james.houlahan@proton.ch>
Date: Tue, 24 Jan 2023 16:09:01 +0100
Subject: [PATCH] GODT-2177: Use correct attachment disposition when content ID
 is set

---
 internal/user/smtp.go      | 28 +++++++++++++++++++---------
 pkg/message/parser.go      |  4 ++--
 pkg/message/parser_test.go |  5 ++++-
 3 files changed, 25 insertions(+), 12 deletions(-)

diff --git a/internal/user/smtp.go b/internal/user/smtp.go
index 702dfd24..8defe540 100644
--- a/internal/user/smtp.go
+++ b/internal/user/smtp.go
@@ -385,22 +385,32 @@ func createAttachments(
 			"mime-type":   att.MIMEType,
 		}).Debug("Uploading attachment")
 
-		// Some client might have leave empty the content disposition or use unsupported values.
-		if att.Disposition != string(proton.InlineDisposition) && att.Disposition != string(proton.AttachmentDisposition) {
-			att.Disposition = string(proton.AttachmentDisposition)
-		}
+		switch att.Disposition {
+		case proton.InlineDisposition:
+			// Some clients use inline disposition but don't set a content ID. Our API doesn't support this.
+			// We could generate our own content ID, but for simplicity, we just set the disposition to attachment.
+			if att.ContentID == "" {
+				att.Disposition = proton.AttachmentDisposition
+			}
 
-		// Some clients use inline disposition but don't set a content ID. Our API doesn't support this.
-		// We could generate our own content ID, but for simplicity, we just set the disposition to attachment.
-		if att.Disposition == string(proton.InlineDisposition) && att.ContentID == "" {
-			att.Disposition = string(proton.AttachmentDisposition)
+		case proton.AttachmentDisposition:
+			// Nothing to do.
+
+		default:
+			// Some clients leave the content disposition empty or use unsupported values.
+			// We default to inline disposition if a content ID is set, and to attachment disposition otherwise.
+			if att.ContentID != "" {
+				att.Disposition = proton.InlineDisposition
+			} else {
+				att.Disposition = proton.AttachmentDisposition
+			}
 		}
 
 		attachment, err := client.UploadAttachment(ctx, addrKR, proton.CreateAttachmentReq{
 			Filename:    att.Name,
 			MessageID:   draftID,
 			MIMEType:    rfc822.MIMEType(att.MIMEType),
-			Disposition: proton.Disposition(att.Disposition),
+			Disposition: att.Disposition,
 			ContentID:   att.ContentID,
 			Body:        att.Data,
 		})
diff --git a/pkg/message/parser.go b/pkg/message/parser.go
index f12155a1..e0aa9445 100644
--- a/pkg/message/parser.go
+++ b/pkg/message/parser.go
@@ -66,7 +66,7 @@ type Attachment struct {
 	Name        string
 	ContentID   string
 	MIMEType    string
-	Disposition string
+	Disposition proton.Disposition
 	Data        []byte
 }
 
@@ -528,7 +528,7 @@ func parseAttachment(h message.Header, body []byte) (Attachment, error) {
 	// If not available, try to get it from name param in content type.
 	// Otherwise fallback to attachment.bin.
 	if disp, dispParams, err := h.ContentDisposition(); err == nil {
-		att.Disposition = disp
+		att.Disposition = proton.Disposition(disp)
 
 		if filename, ok := dispParams["filename"]; ok {
 			att.Name = filename
diff --git a/pkg/message/parser_test.go b/pkg/message/parser_test.go
index 78b2dcf3..6942deee 100644
--- a/pkg/message/parser_test.go
+++ b/pkg/message/parser_test.go
@@ -25,6 +25,7 @@ import (
 	"path/filepath"
 	"testing"
 
+	"github.com/ProtonMail/go-proton-api"
 	"github.com/ProtonMail/proton-bridge/v3/pkg/message/parser"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -631,15 +632,17 @@ func TestParseIcsAttachment(t *testing.T) {
 	assert.Equal(t, m.Attachments[0].MIMEType, "text/calendar")
 	assert.Equal(t, m.Attachments[0].Name, "invite.ics")
 	assert.Equal(t, m.Attachments[0].ContentID, "")
-	assert.Equal(t, m.Attachments[0].Disposition, "attachment")
+	assert.Equal(t, m.Attachments[0].Disposition, proton.Disposition("attachment"))
 	assert.Equal(t, string(m.Attachments[0].Data), "This is an ics calendar invite")
 }
 
 func TestParsePanic(t *testing.T) {
 	var err error
+
 	require.NotPanics(t, func() {
 		_, err = Parse(&panicReader{})
 	})
+
 	require.Error(t, err)
 }