diff --git a/app/import_functions.go b/app/import_functions.go index 00e3f2a422..3078ddf37e 100644 --- a/app/import_functions.go +++ b/app/import_functions.go @@ -5,10 +5,12 @@ package app import ( "bytes" + "crypto/sha1" "fmt" "io" "net/http" "os" + "path" "strings" "github.com/mattermost/mattermost-server/mlog" @@ -916,8 +918,32 @@ func (a *App) ImportAttachment(data *AttachmentImportData, post *model.Post, tea if file != nil { timestamp := utils.TimeFromMillis(post.CreateAt) buf := bytes.NewBuffer(nil) - io.Copy(buf, file) + _, _ = io.Copy(buf, file) + // Go over existing files in the post and see if there already exists a file with the same name, size and hash. If so - skip it + if post.Id != "" { + if oldFiles, err := a.GetFileInfosForPost(post.Id, true); err == nil { + for _, oldFile := range oldFiles { + if oldFile.Name != path.Base(file.Name()) || oldFile.Size != int64(buf.Len()) { + continue + } + // check md5 + newHash := sha1.Sum(buf.Bytes()) + oldFileData, err := a.GetFile(oldFile.Id) + if err != nil { + return nil, model.NewAppError("BulkImport", "app.import.attachment.file_upload.error", map[string]interface{}{"FilePath": *data.Path}, "", http.StatusBadRequest) + } + oldHash := sha1.Sum(oldFileData) + if bytes.Equal(oldHash[:], newHash[:]) { + mlog.Info(fmt.Sprintf("Skipping uploading of file with name %s, already exists", file.Name())) + return nil, nil + } + + } + } else { + return nil, model.NewAppError("BulkImport", "app.import.attachment.file_upload.error", map[string]interface{}{"FilePath": *data.Path}, "", http.StatusBadRequest) + } + } fileInfo, err := a.DoUploadFile(timestamp, teamId, post.ChannelId, post.UserId, file.Name(), buf.Bytes()) if err != nil { @@ -1055,7 +1081,9 @@ func (a *App) uploadAttachments(attachments *[]AttachmentImportData, post *model if err != nil { return nil, err } - fileIds = append(fileIds, fileInfo.Id) + if fileInfo != nil { // nil is returned when the file was skipped due to duplication + fileIds = append(fileIds, fileInfo.Id) + } } return fileIds, nil } diff --git a/app/import_functions_test.go b/app/import_functions_test.go index 0103c76195..d01d837136 100644 --- a/app/import_functions_test.go +++ b/app/import_functions_test.go @@ -5,6 +5,8 @@ package app import ( "fmt" + "io/ioutil" + "os" "path/filepath" "strings" "testing" @@ -2843,6 +2845,14 @@ func TestImportDirectPostWithAttachments(t *testing.T) { testsDir, _ := fileutils.FindDir("tests") testImage := filepath.Join(testsDir, "test.png") + testImage2 := filepath.Join(testsDir, "test.svg") + // create a temp file with same name as original but with a different first byte + tmpFolder, _ := ioutil.TempDir("", "imgFake") + testImageFake := filepath.Join(tmpFolder, "test.png") + fakeFileData, _ := ioutil.ReadFile(testImage) + fakeFileData[0] = 0 + _ = ioutil.WriteFile(testImageFake, fakeFileData, 0644) + defer os.RemoveAll(tmpFolder) // Create a user. username := model.NewId() @@ -2877,12 +2887,63 @@ func TestImportDirectPostWithAttachments(t *testing.T) { Attachments: &[]AttachmentImportData{{Path: &testImage}}, } - if err := th.App.ImportDirectPost(directImportData, false); err != nil { - t.Fatalf("Expected success.") - } + t.Run("Regular import of attachment", func(t *testing.T) { + if err := th.App.ImportDirectPost(directImportData, false); err != nil { + t.Fatalf("Expected success.") + } - attachments := GetAttachments(user1.Id, th, t) - assert.Equal(t, len(attachments), 1) - assert.Contains(t, attachments[0].Path, "noteam") - AssertFileIdsInPost(attachments, th, t) + attachments := GetAttachments(user1.Id, th, t) + assert.Equal(t, len(attachments), 1) + assert.Contains(t, attachments[0].Path, "noteam") + AssertFileIdsInPost(attachments, th, t) + }) + + t.Run("Attempt to import again with same file entirely, should NOT add an attachment", func(t *testing.T) { + if err := th.App.ImportDirectPost(directImportData, false); err != nil { + t.Fatalf("Expected success.") + } + + attachments := GetAttachments(user1.Id, th, t) + assert.Equal(t, len(attachments), 1) + }) + + t.Run("Attempt to import again with same name and size but different content, SHOULD add an attachment", func(t *testing.T) { + directImportDataFake := &DirectPostImportData{ + ChannelMembers: &[]string{ + user1.Username, + user2.Username, + }, + User: &user1.Username, + Message: ptrStr("Direct message"), + CreateAt: ptrInt64(model.GetMillis()), + Attachments: &[]AttachmentImportData{{Path: &testImageFake}}, + } + + if err := th.App.ImportDirectPost(directImportDataFake, false); err != nil { + t.Fatalf("Expected success.") + } + + attachments := GetAttachments(user1.Id, th, t) + assert.Equal(t, len(attachments), 2) + }) + + t.Run("Attempt to import again with same data, SHOULD add an attachment, since it's different name", func(t *testing.T) { + directImportData2 := &DirectPostImportData{ + ChannelMembers: &[]string{ + user1.Username, + user2.Username, + }, + User: &user1.Username, + Message: ptrStr("Direct message"), + CreateAt: ptrInt64(model.GetMillis()), + Attachments: &[]AttachmentImportData{{Path: &testImage2}}, + } + + if err := th.App.ImportDirectPost(directImportData2, false); err != nil { + t.Fatalf("Expected success.") + } + + attachments := GetAttachments(user1.Id, th, t) + assert.Equal(t, len(attachments), 3) + }) }