From 05d84a80080b1c494761dd830fc284871337f5ef Mon Sep 17 00:00:00 2001 From: Jonathan Date: Thu, 15 Feb 2018 03:02:31 -0500 Subject: Modified advanced mail implementation to properly support multiple attachments with the same file name (#8289) --- utils/mail.go | 11 +++++------ utils/mail_test.go | 41 +++++++++++++++++++++++++++++++---------- 2 files changed, 36 insertions(+), 16 deletions(-) diff --git a/utils/mail.go b/utils/mail.go index 633f97818..9023f7090 100644 --- a/utils/mail.go +++ b/utils/mail.go @@ -157,19 +157,18 @@ func sendMail(mimeTo, smtpTo string, from mail.Address, subject, htmlBody string } for _, fileInfo := range attachments { + bytes, err := fileBackend.ReadFile(fileInfo.Path) + if err != nil { + return err + } + m.Attach(fileInfo.Name, gomail.SetCopyFunc(func(writer io.Writer) error { - bytes, err := fileBackend.ReadFile(fileInfo.Path) - if err != nil { - return err - } if _, err := writer.Write(bytes); err != nil { return model.NewAppError("SendMail", "utils.mail.sendMail.attachments.write_error", nil, err.Error(), http.StatusInternalServerError) } return nil })) - } - } conn, err1 := connectToSMTPServer(config) diff --git a/utils/mail_test.go b/utils/mail_test.go index 703420441..67d108d45 100644 --- a/utils/mail_test.go +++ b/utils/mail_test.go @@ -9,6 +9,8 @@ import ( "net/mail" + "fmt" + "github.com/mattermost/mattermost-server/model" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -94,18 +96,28 @@ func TestSendMailUsingConfigAdvanced(t *testing.T) { //Delete all the messages before check the sample email DeleteMailBox(smtpTo) - // create a file that will be attached to the email fileBackend, err := NewFileBackend(&cfg.FileSettings, true) assert.Nil(t, err) - fileContents := []byte("hello world") - fileName := "file.txt" - assert.Nil(t, fileBackend.WriteFile(fileContents, fileName)) - defer fileBackend.RemoveFile(fileName) - attachments := make([]*model.FileInfo, 1) + // create two files with the same name that will both be attached to the email + fileName := "file.txt" + filePath1 := fmt.Sprintf("test1/%s", fileName) + filePath2 := fmt.Sprintf("test2/%s", fileName) + fileContents1 := []byte("hello world") + fileContents2 := []byte("foo bar") + assert.Nil(t, fileBackend.WriteFile(fileContents1, filePath1)) + assert.Nil(t, fileBackend.WriteFile(fileContents2, filePath2)) + defer fileBackend.RemoveFile(filePath1) + defer fileBackend.RemoveFile(filePath2) + + attachments := make([]*model.FileInfo, 2) attachments[0] = &model.FileInfo{ Name: fileName, - Path: fileName, + Path: filePath1, + } + attachments[1] = &model.FileInfo{ + Name: fileName, + Path: filePath2, } headers := make(map[string]string) @@ -145,10 +157,19 @@ func TestSendMailUsingConfigAdvanced(t *testing.T) { // check that the custom mime headers came through - header case seems to get mutated assert.Equal(t, "TestValue", resultsEmail.Header["Testheader"][0]) - // ensure that the attachment was successfully sent - assert.Len(t, resultsEmail.Attachments, 1) + // ensure that the attachments were successfully sent + assert.Len(t, resultsEmail.Attachments, 2) assert.Equal(t, fileName, resultsEmail.Attachments[0].Filename) - assert.Equal(t, fileContents, resultsEmail.Attachments[0].Bytes) + assert.Equal(t, fileName, resultsEmail.Attachments[1].Filename) + attachment1 := string(resultsEmail.Attachments[0].Bytes) + attachment2 := string(resultsEmail.Attachments[1].Bytes) + if attachment1 == string(fileContents1) { + assert.Equal(t, attachment2, string(fileContents2)) + } else if attachment1 == string(fileContents2) { + assert.Equal(t, attachment2, string(fileContents1)) + } else { + assert.Fail(t, "Unrecognized attachment contents") + } } } } -- cgit v1.2.3-1-g7c22