From 6955dba06e88ee450013472fd93efa60db93fc2e Mon Sep 17 00:00:00 2001 From: Romain LE JEUNE Date: Mon, 23 Oct 2023 11:53:11 +0200 Subject: [PATCH] test(GODT-3036): Also sort attachment for sent message + add unitary test. --- server/backend/api.go | 35 +++++---- server/server_test.go | 119 +++++++++++++++++++++++++++++++ testdata/MultipleAttachments.eml | 68 ++++++++++++++++++ 3 files changed, 210 insertions(+), 12 deletions(-) create mode 100644 testdata/MultipleAttachments.eml diff --git a/server/backend/api.go b/server/backend/api.go index 9d431ea..902c4a9 100644 --- a/server/backend/api.go +++ b/server/backend/api.go @@ -728,10 +728,7 @@ func (b *Backend) SendMessage(userID, messageID string, packages []*proton.Messa newMsg.unread = true messages[newMsg.messageID] = newMsg - // collect attachment with contentID - attIDs := sortAttachment(atts, msg.attIDs) - - for _, attID := range attIDs { + for _, attID := range msg.attIDs { attKey, err := base64.StdEncoding.DecodeString(recipient.AttachmentKeyPackets[attID]) if err != nil { return err @@ -749,7 +746,11 @@ func (b *Backend) SendMessage(userID, messageID string, packages []*proton.Messa atts[att.attachID] = att messages[newMsg.messageID].attIDs = append(messages[newMsg.messageID].attIDs, att.attachID) } + // Sort Message attachments + messages[newMsg.messageID].attIDs = sortAttachment(atts, messages[newMsg.messageID].attIDs) + msg.attIDs = sortAttachment(atts, msg.attIDs) + // Send the update event updateID, err := b.newUpdate(&messageCreated{messageID: newMsg.messageID}) if err != nil { return err @@ -780,28 +781,35 @@ func sortAttachment(atts map[string]*attachment, attIDs []string) []string { attContentId[atts[attID].contentID] = attID } } - // sort contentID contentIDs := make([]string, 0, len(attContentId)) for k := range attContentId { contentIDs = append(contentIDs, k) } sort.Strings(contentIDs) - // set contentID attachment first - attachments := make([]string, len(attIDs)) + attachments := make([]string, 0, len(attIDs)) for _, id := range contentIDs { attachments = append(attachments, attContentId[id]) } - // follow with attachment without contentID + // treat the rest by filename + attNames := make(map[string]string, len(attIDs)) for _, attID := range attIDs { - if atts[attID].contentID != "" { - continue + if atts[attID].contentID == "" { + attNames[atts[attID].filename] = attID } - attachments = append(attachments, attContentId[attID]) } - + // sort name + names := make([]string, 0, len(attNames)) + for k := range attNames { + names = append(names, k) + } + sort.Strings(names) + // Append by alphabetical order + for _, name := range names { + attachments = append(attachments, attNames[name]) + } return attachments } @@ -838,7 +846,10 @@ func (b *Backend) CreateAttachment( atts[att.attachID] = att + // append attachment messages[messageID].attIDs = append(messages[messageID].attIDs, att.attachID) + // sort the attachment list + messages[messageID].attIDs = sortAttachment(atts, messages[messageID].attIDs) updateID, err := b.newUpdate(&messageUpdated{messageID: messageID}) if err != nil { diff --git a/server/server_test.go b/server/server_test.go index 45bd061..e74eab4 100644 --- a/server/server_test.go +++ b/server/server_test.go @@ -6,6 +6,7 @@ import ( "encoding/json" "errors" "fmt" + "github.com/ProtonMail/go-proton-api/server/backend" "net/http" "net/mail" "net/url" @@ -818,6 +819,124 @@ func TestServer_SendMessage(t *testing.T) { }) } +func TestServer_SendMessageAttachmentSort(t *testing.T) { + withServer(t, func(ctx context.Context, s *Server, m *proton.Manager) { + withUser(ctx, t, s, m, "user", "pass", func(c *proton.Client) { + user, err := c.GetUser(ctx) + require.NoError(t, err) + + addr, err := c.GetAddresses(ctx) + require.NoError(t, err) + + salt, err := c.GetSalts(ctx) + require.NoError(t, err) + + pass, err := salt.SaltForKey([]byte("pass"), user.Keys.Primary().ID) + require.NoError(t, err) + + _, addrKRs, err := proton.Unlock(user, addr, pass, async.NoopPanicHandler{}) + require.NoError(t, err) + + body, err := os.ReadFile("../testdata/MultipleAttachments.eml") + require.NoError(t, err) + + // create the draft + draft, err := c.CreateDraft(ctx, addrKRs[addr[0].ID], proton.CreateDraftReq{ + Message: proton.DraftTemplate{ + Subject: "My subject", + Sender: &mail.Address{Address: addr[0].Email}, + ToList: []*mail.Address{{Address: "user@proton.local"}}, + Body: string(body), + }, + }) + require.NoError(t, err) + + // upload attachments in mixed order + { + _, err = c.UploadAttachment(ctx, addrKRs[addr[0].ID], proton.CreateAttachmentReq{ + MessageID: draft.ID, + Filename: "inlinepart2.png", + MIMEType: "image/png", + Disposition: proton.InlineDisposition, + ContentID: "part2.erfefw", + }) + require.NoError(t, err) + + _, err = c.UploadAttachment(ctx, addrKRs[addr[0].ID], proton.CreateAttachmentReq{ + MessageID: draft.ID, + Filename: "alphabeticalZZZZ.png", + MIMEType: "image/png", + Disposition: proton.AttachmentDisposition, + }) + require.NoError(t, err) + + _, err = c.UploadAttachment(ctx, addrKRs[addr[0].ID], proton.CreateAttachmentReq{ + MessageID: draft.ID, + Filename: "inlinepart1.png", + MIMEType: "image/png", + Disposition: proton.InlineDisposition, + ContentID: "part1.erfefw", + }) + require.NoError(t, err) + + _, err = c.UploadAttachment(ctx, addrKRs[addr[0].ID], proton.CreateAttachmentReq{ + MessageID: draft.ID, + Filename: "alphabeticalAAA.png", + MIMEType: "image/png", + Disposition: proton.AttachmentDisposition, + }) + require.NoError(t, err) + } + + // prepare the package + var req proton.SendDraftReq + attkeys := make(map[string]*crypto.SessionKey) + err = req.AddTextPackage(addrKRs[addr[0].ID], string(body), "text/html", map[string]proton.SendPreferences{"user@proton.local": { + Encrypt: true, + PubKey: addrKRs[addr[0].ID], + SignatureType: proton.DetachedSignature, + EncryptionScheme: proton.InternalScheme, + MIMEType: rfc822.TextHTML, + }}, attkeys) + require.NoError(t, err) + + // send the draft + sent, err := c.SendDraft(ctx, draft.ID, req) + require.NoError(t, err) + + // Check attachment order + require.Equal(t, 4, len(sent.Attachments)) + require.Equal(t, "inlinepart1.png", sent.Attachments[0].Name) + require.Equal(t, "inlinepart2.png", sent.Attachments[1].Name) + require.Equal(t, "alphabeticalAAA.png", sent.Attachments[2].Name) + require.Equal(t, "alphabeticalZZZZ.png", sent.Attachments[3].Name) + + // catch the message created event from the receiver + rawEventID, err := c.GetLatestEventID(ctx) + require.NoError(t, err) + var eventID backend.ID + err = eventID.FromString(rawEventID) + require.NoError(t, err) + eventID = eventID - 1 + events, _, err := c.GetEvent(ctx, eventID.String()) + require.NoError(t, err) + require.Equal(t, 1, len(events)) + require.Equal(t, 1, len(events[0].Messages)) + + // get the message from receiver inbox + rcv, err := c.GetMessage(ctx, events[0].Messages[0].Message.ID) + require.NoError(t, err) + + // Check attachment order + require.Equal(t, 4, len(rcv.Attachments)) + require.Equal(t, "inlinepart1.png", rcv.Attachments[0].Name) + require.Equal(t, "inlinepart2.png", rcv.Attachments[1].Name) + require.Equal(t, "alphabeticalAAA.png", sent.Attachments[2].Name) + require.Equal(t, "alphabeticalZZZZ.png", sent.Attachments[3].Name) + }) + }) +} + func TestServer_AuthDelete(t *testing.T) { withServer(t, func(ctx context.Context, s *Server, m *proton.Manager) { withUser(ctx, t, s, m, "user", "pass", func(c *proton.Client) { diff --git a/testdata/MultipleAttachments.eml b/testdata/MultipleAttachments.eml new file mode 100644 index 0000000..89384e2 --- /dev/null +++ b/testdata/MultipleAttachments.eml @@ -0,0 +1,68 @@ +Content-Type: multipart/related;boundary="------------pXWj190lQsd0d77xbCjkhoss" +User-Agent: Mozilla Thunderbird +Content-Language: en-GB +To: <[user:to]@[domain]> +From: <[user:user]@[domain]> +Subject: HTML message with multiple inline images + +--------------pXWj190lQsd0d77xbCjkhoss +Content-Type: text/html; charset=UTF-8 +Content-Transfer-Encoding: 7bit + + + + + + + + +

Inline image 1

+

+

Inline image 2

+

+

End
+

+
+ + +--------------pXWj190lQsd0d77xbCjkhoss +Content-Type: image/png; name="inlinepart2.png" +Content-Disposition: inline; filename="inlinepart2.png" +Content-Id: +Content-Transfer-Encoding: base64 + +iVBORw0KGgoAAAANSUhEUgAAAOEAAADhCAMAAAAJbSJIAAAAh1BMVEX///8AAAABAQGbm5sP +Qm1ljU7fCDEb0EoXQHyzccFedRPEVPtOY91tb9BVtzebhEdH0R1oLSwO3QvtoFl8YgQXV7eL +BYPFOEzNxiFmI0jD8eL1M3D/A1pNGGh1w+TvAAAAAElFTkSuQmCC + +--------------pXWj190lQsd0d77xbCjkhoss +Content-Type: image/png; name="alphabeticalZZZZ.png" +Content-Disposition: attachment; filename="alphabeticalZZZZ.png" +Content-Transfer-Encoding: base64 + +iVBORw0KGgoAAAANSUhEUgAAAOEAAADhCAMAAAAJbSJIAAAAh1BMVEX///8AAAABAQGbm5sP +Qm1ljU7fCDEb0EoXQHyzccFedRPEVPtOY91tb9BVtzebhEdH0R1oLSwO3QvtoFl8YgQXV7eL +BYPFOEzNxiFmI0jD8eL1M3D/A1pNGGh1w+TvAAAAAElFTkSuQmCC + +--------------pXWj190lQsd0d77xbCjkhoss +Content-Type: image/png; name="inlinepart1.png" +Content-Disposition: inline; filename="inlinepart1.png" +Content-Id: +Content-Transfer-Encoding: base64 + +iVBORw0KGgoAAAANSUhEUgAAAOEAAADhCAMAAAAJbSJIAAAAh1BMVEX///8AAAABAQGbm5sP +Qm1ljU7fCDEb0EoXQHyzccFedRPEVPtOY91tb9BVtzebhEdH0R1oLSwO3QvtoFl8YgQXV7eL +BYPFOEzNxiFmI0jD8eL1M3D/A1pNGGh1w+TvAAAAAElFTkSuQmCC + +--------------pXWj190lQsd0d77xbCjkhoss +Content-Type: image/png; name="alphabeticalAAA.png" +Content-Disposition: attachment; filename="alphabeticalAAA.png" +Content-Transfer-Encoding: base64 + +iVBORw0KGgoAAAANSUhEUgAAAOEAAADhCAMAAAAJbSJIAAAAh1BMVEX///8AAAABAQGbm5sP +Qm1ljU7fCDEb0EoXQHyzccFedRPEVPtOY91tb9BVtzebhEdH0R1oLSwO3QvtoFl8YgQXV7eL +BYPFOEzNxiFmI0jD8eL1M3D/A1pNGGh1w+TvAAAAAElFTkSuQmCC + +--------------pXWj190lQsd0d77xbCjkhoss-- \ No newline at end of file