diff --git a/e2e-tests/cypress/tests/integration/channels/enterprise/system_console/compliance/compliance_export_ui_spec.js b/e2e-tests/cypress/tests/integration/channels/enterprise/system_console/compliance/compliance_export_ui_spec.js index 0023614a45..7d5f565dc2 100644 --- a/e2e-tests/cypress/tests/integration/channels/enterprise/system_console/compliance/compliance_export_ui_spec.js +++ b/e2e-tests/cypress/tests/integration/channels/enterprise/system_console/compliance/compliance_export_ui_spec.js @@ -44,7 +44,7 @@ describe('Compliance Export', () => { }); }); - it.skip('MM-60115 - MM-T3435 - Download Compliance Export Files - CSV Format', () => { + it('MM-T3435 - Download Compliance Export Files - CSV Format', () => { // # Navigate to a team and post an attachment cy.visit(`/${teamName}/channels/town-square`); gotoTeamAndPostImage(); @@ -65,7 +65,7 @@ describe('Compliance Export', () => { }); }); - it.skip('MM-60115 - MM-T3438 - Download Compliance Export Files when 0 messages exported', () => { + it('MM-T3438 - Download Compliance Export Files when 0 messages exported', () => { // # Navigate to a team and post an attachment cy.visit(`/${teamName}/channels/town-square`); gotoTeamAndPostImage(); @@ -94,7 +94,7 @@ describe('Compliance Export', () => { }); }); - it.skip('MM-60115 - MM-T1168 - Compliance Export - Run Now, entry appears in job table', () => { + it('MM-T1168 - Compliance Export - Run Now, entry appears in job table', () => { // # Navigate to a team and post an attachment cy.visit(`/${teamName}/channels/town-square`); gotoTeamAndPostImage(); diff --git a/e2e-tests/cypress/tests/integration/channels/enterprise/system_console/compliance/download_bot_compliance_export_file_spec.js b/e2e-tests/cypress/tests/integration/channels/enterprise/system_console/compliance/download_bot_compliance_export_file_spec.js index eaa17ee2ab..fbb2a499a2 100644 --- a/e2e-tests/cypress/tests/integration/channels/enterprise/system_console/compliance/download_bot_compliance_export_file_spec.js +++ b/e2e-tests/cypress/tests/integration/channels/enterprise/system_console/compliance/download_bot_compliance_export_file_spec.js @@ -60,7 +60,7 @@ describe('Compliance Export', () => { cy.shellRm('-rf', downloadsFolder); }); - it.skip('MM-60115 - MM-T1175_1 - UserType identifies that the message is posted by a bot', () => { + it('MM-T1175_1 - UserType identifies that the message is posted by a bot', () => { const message = `This is CSV bot message from ${botName} at ${Date.now()}`; // # Post bot message @@ -81,7 +81,7 @@ describe('Compliance Export', () => { ); }); - it.skip('MM-60115 - MM-T1175_2 - UserType identifies that the message is posted by a bot', () => { + it('MM-T1175_2 - UserType identifies that the message is posted by a bot', () => { const message = `This is XML bot message from ${botName} at ${Date.now()}`; // # Post bot message diff --git a/e2e-tests/cypress/tests/integration/channels/enterprise/system_console/compliance/download_compliance_export_file_spec.js b/e2e-tests/cypress/tests/integration/channels/enterprise/system_console/compliance/download_compliance_export_file_spec.js index 2f7d00a60e..5418ff4c74 100644 --- a/e2e-tests/cypress/tests/integration/channels/enterprise/system_console/compliance/download_compliance_export_file_spec.js +++ b/e2e-tests/cypress/tests/integration/channels/enterprise/system_console/compliance/download_compliance_export_file_spec.js @@ -52,7 +52,7 @@ describe('Compliance Export', () => { cy.shellRm('-rf', downloadsFolder); }); - it.skip('MM-60115 - MM-T1172 - Compliance Export - Deleted file is indicated in CSV File Export', () => { + it('MM-T1172 - Compliance Export - Deleted file is indicated in CSV File Export', () => { // # Go to compliance page and enable export cy.uiGoToCompliancePage(); cy.uiEnableComplianceExport(); @@ -84,7 +84,7 @@ describe('Compliance Export', () => { ); }); - it.skip('MM-60115 - MM-T1173 - Compliance Export - Deleted file is indicated in Actiance XML File Export', () => { + it('MM-T1173 - Compliance Export - Deleted file is indicated in Actiance XML File Export', () => { // # Go to compliance page and enable export cy.uiGoToCompliancePage(); cy.uiEnableComplianceExport(ExportFormatActiance); @@ -121,7 +121,7 @@ describe('Compliance Export', () => { }); }); - it.skip('MM-60115 - MM-T1176 - Compliance export should include updated post after editing', () => { + it('MM-T1176 - Compliance export should include updated post after editing', () => { // # Go to compliance page and enable export cy.uiGoToCompliancePage(); cy.uiEnableComplianceExport(ExportFormatActiance); @@ -154,7 +154,7 @@ describe('Compliance Export', () => { ); }); - it.skip('MM-60115 - MM-T3305 - Verify Deactivated users are displayed properly in Compliance Exports', () => { + it('MM-T3305 - Verify Deactivated users are displayed properly in Compliance Exports', () => { // # Post a message by Admin cy.postMessageAs({ sender: adminUser, diff --git a/e2e-tests/cypress/tests/integration/channels/enterprise/system_console/compliance/helpers.js b/e2e-tests/cypress/tests/integration/channels/enterprise/system_console/compliance/helpers.js index b41a129352..5823e73de5 100644 --- a/e2e-tests/cypress/tests/integration/channels/enterprise/system_console/compliance/helpers.js +++ b/e2e-tests/cypress/tests/integration/channels/enterprise/system_console/compliance/helpers.js @@ -34,12 +34,9 @@ export function verifyPostsCSVFile(targetFolder, type, match) { } export function verifyActianceXMLFile(targetFolder, type, match) { - cy.shellFind(targetFolder, /actiance_export.xml/). - then((files) => { - cy.readFile(files[files.length - 1]). - should('exist'). - and(type, match); - }); + cy.readFile(`${targetFolder}/actiance_export.xml`). + should('exist'). + and(type, match); } export function verifyExportedMessagesCount(expectedNumber) { diff --git a/server/channels/api4/job.go b/server/channels/api4/job.go index a794b1874f..ca9776c3ea 100644 --- a/server/channels/api4/job.go +++ b/server/channels/api4/job.go @@ -6,10 +6,13 @@ package api4 import ( "encoding/json" "net/http" + "path" "path/filepath" "strconv" "time" + "github.com/mattermost/mattermost/server/v8/platform/shared/filestore" + "github.com/mattermost/mattermost/server/public/model" "github.com/mattermost/mattermost/server/public/shared/mlog" "github.com/mattermost/mattermost/server/v8/channels/audit" @@ -55,7 +58,7 @@ func getJob(c *Context, w http.ResponseWriter, r *http.Request) { func downloadJob(c *Context, w http.ResponseWriter, r *http.Request) { config := c.App.Config() - const FilePath = "export" + const oldFilePath = "export" const FileMime = "application/zip" c.RequireJobId() @@ -90,19 +93,53 @@ func downloadJob(c *Context, w http.ResponseWriter, r *http.Request) { return } - fileName := job.Id + ".zip" - filePath := filepath.Join(FilePath, fileName) - fileReader, err := c.App.FileReader(filePath) - if err != nil { - c.Err = err - c.Err.StatusCode = http.StatusNotFound + exportDir, ok := job.Data["export_dir"] + fileName := path.Base(exportDir) + if !ok || exportDir == "" || fileName == "/" || fileName == "." { + // Could be a pre-overhaul job. Try the old method: + fileName = job.Id + ".zip" + filePath := filepath.Join(oldFilePath, fileName) + var fileReader filestore.ReadCloseSeeker + fileReader, err = c.App.FileReader(filePath) + if err != nil { + c.Err = model.NewAppError("unableToDownloadJob", "api.job.unable_to_download_job", nil, + "job.Data did not include export_dir, export_dir was malformed, or jobId.zip wasn't found", + http.StatusNotFound).Wrap(err) + return + } + defer fileReader.Close() + + // We are able to pass 0 for content size due to the fact that Golang's serveContent (https://golang.org/src/net/http/fs.go) + // already sets that for us + web.WriteFileResponse(fileName, FileMime, 0, time.UnixMilli(job.LastActivityAt), *c.App.Config().ServiceSettings.WebserverMode, fileReader, true, w, r) return } - defer fileReader.Close() - // We are able to pass 0 for content size due to the fact that Golang's serveContent (https://golang.org/src/net/http/fs.go) - // already sets that for us - web.WriteFileResponse(fileName, FileMime, 0, time.Unix(0, job.LastActivityAt*int64(1000*1000)), *c.App.Config().ServiceSettings.WebserverMode, fileReader, true, w, r) + // We have a base directory, we're using that as the exported filename: + fileName += ".zip" + + cleanedExportDir := filepath.Clean(exportDir) + if !filepath.IsLocal(cleanedExportDir) { + c.Err = model.NewAppError("unableToDownloadJob", "api.job.unable_to_download_job", nil, + "job.Data did not include export_dir, export_dir was malformed, or jobId.zip wasn't found", + http.StatusNotFound).Wrap(err) + return + } + + zipReader, err := c.App.ZipReader(cleanedExportDir, false) + if err != nil { + c.Err = model.NewAppError("unableToDownloadJob", "api.job.unable_to_download_job", nil, + "error creating zip reader", http.StatusNotFound).Wrap(err) + return + } + defer zipReader.Close() + + if err := web.WriteStreamResponse(w, zipReader, fileName, FileMime, true); err != nil { + c.Err = model.NewAppError("unableToDownloadJob", "api.job.unable_to_download_job", nil, + "failure to WriteStreamResponse", http.StatusInternalServerError). + Wrap(err) + return + } } func createJob(c *Context, w http.ResponseWriter, r *http.Request) { diff --git a/server/channels/app/app_iface.go b/server/channels/app/app_iface.go index ab484313b7..8f877dce8a 100644 --- a/server/channels/app/app_iface.go +++ b/server/channels/app/app_iface.go @@ -54,10 +54,6 @@ type AppIface interface { AddPublicKey(name string, key io.Reader) *model.AppError // AddUserToChannel adds a user to a given channel. AddUserToChannel(c request.CTX, user *model.User, channel *model.Channel, skipTeamMemberIntegrityCheck bool) (*model.ChannelMember, *model.AppError) - // Caller must close the first return value - ExportFileReader(path string) (filestore.ReadCloseSeeker, *model.AppError) - // Caller must close the first return value - FileReader(path string) (filestore.ReadCloseSeeker, *model.AppError) // ChannelMembersMinusGroupMembers returns the set of users in the given channel minus the set of users in the given // groups. // @@ -105,8 +101,6 @@ type AppIface interface { // CreateUser creates a user and sets several fields of the returned User struct to // their zero values. CreateUser(c request.CTX, user *model.User) (*model.User, *model.AppError) - // Creates and stores FileInfos for a post created before the FileInfos table existed. - MigrateFilenamesToFileInfos(rctx request.CTX, post *model.Post) []*model.FileInfo // DefaultChannelNames returns the list of system-wide default channel names. // // By default the list will be (not necessarily in this order): @@ -153,10 +147,18 @@ type AppIface interface { // attributes of the attachment structure. The Slack attachment structure is // documented here: https://api.slack.com/docs/attachments ProcessSlackAttachments(attachments []*model.SlackAttachment) []*model.SlackAttachment + // ExportFileReader returns a ReadCloseSeeker for path from the ExportFileBackend. + // + // The caller is responsible for closing the returned ReadCloseSeeker. + ExportFileReader(path string) (filestore.ReadCloseSeeker, *model.AppError) // ExtendSessionExpiryIfNeeded extends Session.ExpiresAt based on session lengths in config. // A new ExpiresAt is only written if enough time has elapsed since last update. // Returns true only if the session was extended. ExtendSessionExpiryIfNeeded(rctx request.CTX, session *model.Session) bool + // FileReader returns a ReadCloseSeeker for path from the FileBackend. + // + // The caller is responsible for closing the returned ReadCloseSeeker. + FileReader(path string) (filestore.ReadCloseSeeker, *model.AppError) // FillInPostProps should be invoked before saving posts to fill in properties such as // channel_mentions. // @@ -270,6 +272,8 @@ type AppIface interface { // MentionsToTeamMembers returns all the @ mentions found in message that // belong to users in the specified team, linking them to their users MentionsToTeamMembers(c request.CTX, message, teamID string) model.UserMentionMap + // MigrateFilenamesToFileInfos creates and stores FileInfos for a post created before the FileInfos table existed. + MigrateFilenamesToFileInfos(rctx request.CTX, post *model.Post) []*model.FileInfo // MoveChannel method is prone to data races if someone joins to channel during the move process. However this // function is only exposed to sysadmins and the possibility of this edge case is relatively small. MoveChannel(c request.CTX, team *model.Team, channel *model.Channel, user *model.User) *model.AppError @@ -422,6 +426,10 @@ type AppIface interface { ValidateUserPermissionsOnChannels(c request.CTX, userId string, channelIds []string) []string // VerifyPlugin checks that the given signature corresponds to the given plugin and matches a trusted certificate. VerifyPlugin(plugin, signature io.ReadSeeker) *model.AppError + // ZipReader returns a ReadCloser for path. If deflate is true, the zip will use compression. + // + // The caller is responsible for closing the returned ReadCloser. + ZipReader(path string, deflate bool) (io.ReadCloser, *model.AppError) // validateMoveOrCopy performs validation on a provided post list to determine // if all permissions are in place to allow the for the posts to be moved or // copied. diff --git a/server/channels/app/file.go b/server/channels/app/file.go index 6583dac115..049c0437fa 100644 --- a/server/channels/app/file.go +++ b/server/channels/app/file.go @@ -124,20 +124,43 @@ func fileReader(backend filestore.FileBackend, path string) (filestore.ReadClose return result, nil } +func zipReader(backend filestore.FileBackend, path string, deflate bool) (io.ReadCloser, *model.AppError) { + result, err := backend.ZipReader(path, deflate) + if err != nil { + return nil, model.NewAppError("ZipReader", "api.file.zip_file_reader.app_error", nil, "", http.StatusInternalServerError).Wrap(err) + } + return result, nil +} + func (s *Server) fileReader(path string) (filestore.ReadCloseSeeker, *model.AppError) { return fileReader(s.FileBackend(), path) } +func (s *Server) zipReader(path string, deflate bool) (io.ReadCloser, *model.AppError) { + return zipReader(s.FileBackend(), path, deflate) +} + func (s *Server) exportFileReader(path string) (filestore.ReadCloseSeeker, *model.AppError) { return fileReader(s.ExportFileBackend(), path) } -// Caller must close the first return value +// FileReader returns a ReadCloseSeeker for path from the FileBackend. +// +// The caller is responsible for closing the returned ReadCloseSeeker. func (a *App) FileReader(path string) (filestore.ReadCloseSeeker, *model.AppError) { return a.Srv().fileReader(path) } -// Caller must close the first return value +// ZipReader returns a ReadCloser for path. If deflate is true, the zip will use compression. +// +// The caller is responsible for closing the returned ReadCloser. +func (a *App) ZipReader(path string, deflate bool) (io.ReadCloser, *model.AppError) { + return a.Srv().zipReader(path, deflate) +} + +// ExportFileReader returns a ReadCloseSeeker for path from the ExportFileBackend. +// +// The caller is responsible for closing the returned ReadCloseSeeker. func (a *App) ExportFileReader(path string) (filestore.ReadCloseSeeker, *model.AppError) { return a.Srv().exportFileReader(path) } @@ -424,7 +447,7 @@ func parseOldFilenames(rctx request.CTX, filenames []string, channelID, userID s return parsed } -// Creates and stores FileInfos for a post created before the FileInfos table existed. +// MigrateFilenamesToFileInfos creates and stores FileInfos for a post created before the FileInfos table existed. func (a *App) MigrateFilenamesToFileInfos(rctx request.CTX, post *model.Post) []*model.FileInfo { if len(post.Filenames) == 0 { rctx.Logger().Warn("Unable to migrate post to use FileInfos with an empty Filenames field", mlog.String("post_id", post.Id)) diff --git a/server/channels/app/opentracing/opentracing_layer.go b/server/channels/app/opentracing/opentracing_layer.go index 188d9a4d31..cbfa356826 100644 --- a/server/channels/app/opentracing/opentracing_layer.go +++ b/server/channels/app/opentracing/opentracing_layer.go @@ -20061,6 +20061,28 @@ func (a *OpenTracingAppLayer) WriteFileContext(ctx context.Context, fr io.Reader return resultVar0, resultVar1 } +func (a *OpenTracingAppLayer) ZipReader(path string, deflate bool) (io.ReadCloser, *model.AppError) { + origCtx := a.ctx + span, newCtx := tracing.StartSpanWithParentByContext(a.ctx, "app.ZipReader") + + a.ctx = newCtx + a.app.Srv().Store().SetContext(newCtx) + defer func() { + a.app.Srv().Store().SetContext(origCtx) + a.ctx = origCtx + }() + + defer span.Finish() + resultVar0, resultVar1 := a.app.ZipReader(path, deflate) + + if resultVar1 != nil { + span.LogFields(spanlog.Error(resultVar1)) + ext.Error.Set(span, true) + } + + return resultVar0, resultVar1 +} + func NewOpenTracingAppLayer(childApp app.AppIface, ctx context.Context) *OpenTracingAppLayer { newApp := OpenTracingAppLayer{ app: childApp, diff --git a/server/enterprise/message_export/actiance_export/actiance_export.go b/server/enterprise/message_export/actiance_export/actiance_export.go index 06d7a13617..e29e0b6a27 100644 --- a/server/enterprise/message_export/actiance_export/actiance_export.go +++ b/server/enterprise/message_export/actiance_export/actiance_export.go @@ -302,8 +302,11 @@ func writeExport(rctx request.CTX, export *RootNode, uploadedFiles []*model.File var attachmentReader io.ReadCloser attachmentReader, err = fileAttachmentBackend.Reader(fileInfo.Path) if err != nil { - missingFiles = append(missingFiles, "Warning:"+shared.MissingFileMessage+" - "+fileInfo.Path) - rctx.Logger().Warn(shared.MissingFileMessage, mlog.String("filename", fileInfo.Path)) + missingFiles = append(missingFiles, "Warning:"+shared.MissingFileMessageDuringBackendRead+" - "+fileInfo.Path) + rctx.Logger().Warn(shared.MissingFileMessageDuringBackendRead, + mlog.String("filename", fileInfo.Path), + mlog.Err(err), + ) continue } @@ -316,14 +319,21 @@ func writeExport(rctx request.CTX, export *RootNode, uploadedFiles []*model.File return err } - // CopyBuffer works with dirty buffers, no need to clear it. if _, err = io.CopyBuffer(zipWriter, attachmentReader, buf); err != nil { return err } return nil }(); err != nil { - return res, fmt.Errorf("unable to write into the zipFile created with the batch temporary file: %w", err) + // s3 only errors _here_ if the object key wasn't found. So to handle that: if there is a read + // error (even for local), let's add a warning instead of failing the export. + // Failing the export would fail the entire export run, and every future run would also fail on + // this non-existent file -- not good. + missingFiles = append(missingFiles, "Warning:"+shared.MissingFileMessageDuringCopy+" - "+fileInfo.Path) + rctx.Logger().Warn(shared.MissingFileMessageDuringCopy, + mlog.String("filename", fileInfo.Path), + mlog.Err(err), + ) } } diff --git a/server/enterprise/message_export/actiance_export/actiance_export_test.go b/server/enterprise/message_export/actiance_export/actiance_export_test.go index 392508ed00..86a4ee9928 100644 --- a/server/enterprise/message_export/actiance_export/actiance_export_test.go +++ b/server/enterprise/message_export/actiance_export/actiance_export_test.go @@ -891,7 +891,7 @@ func runTestActianceExport(t *testing.T, exportBackend filestore.FileBackend, at expectedFiles: 3, }, { - name: "joins and leaves after last post, one batch", + name: "joins and leaves after last post, one batch, and post from bot", jobEndTime: 500, cmhs: map[string][]*model.ChannelMemberHistoryResult{ "channel-id": { @@ -899,7 +899,7 @@ func runTestActianceExport(t *testing.T, exportBackend filestore.FileBackend, at {JoinTime: 8, ChannelId: "channel-id", UserId: "test2", UserEmail: "test2@email", Username: "test2name", LeaveTime: model.NewPointer(int64(80))}, {JoinTime: 400, ChannelId: "channel-id", UserId: "test3", UserEmail: "test3@email", Username: "test3name"}, {JoinTime: 450, ChannelId: "channel-id", UserId: "test4", UserEmail: "test4@email", Username: "test4name", LeaveTime: model.NewPointer(int64(460))}, - {JoinTime: 10, ChannelId: "channel-id", UserId: "test_bot", UserEmail: "test_bot@email", Username: "test_botname", IsBot: true, LeaveTime: model.NewPointer(int64(20))}, + {JoinTime: 10, ChannelId: "channel-id", UserId: "test-bot", UserEmail: "test-bot@email", Username: "test-botname", IsBot: true, LeaveTime: model.NewPointer(int64(20))}, }, }, activity: []string{"channel-id"}, @@ -965,6 +965,24 @@ func runTestActianceExport(t *testing.T, exportBackend filestore.FileBackend, at PostFileIds: []string{}, PostProps: model.NewPointer("{\"deleteBy\":\"fy8j97gwii84bk4zxprbpc9d9w\"}"), }, + { + PostId: model.NewPointer("post-id5"), + TeamId: model.NewPointer("team-id"), + TeamName: model.NewPointer("team-name"), + TeamDisplayName: model.NewPointer("team-display-name"), + ChannelId: model.NewPointer("channel-id"), + ChannelName: model.NewPointer("channel-name"), + ChannelDisplayName: model.NewPointer("channel-display-name"), + PostCreateAt: model.NewPointer(int64(20)), + PostUpdateAt: model.NewPointer(int64(20)), + PostMessage: model.NewPointer("message"), + UserEmail: model.NewPointer("test-bot@email"), + UserId: model.NewPointer("test-bot"), + Username: model.NewPointer("test-botname"), + IsBot: true, + ChannelType: &chanTypeDirect, + PostFileIds: []string{}, + }, { PostId: model.NewPointer("post-id4"), PostRootId: model.NewPointer("post-root-id"), @@ -1010,10 +1028,10 @@ func runTestActianceExport(t *testing.T, exportBackend filestore.FileBackend, at " test2@email\n", " \n", " \n", - " test_bot@email\n", + " test-bot@email\n", " bot\n", " 10\n", - " test_bot@email\n", + " test-bot@email\n", " \n", " \n", " test3@email\n", @@ -1060,6 +1078,13 @@ func runTestActianceExport(t *testing.T, exportBackend filestore.FileBackend, at " delete message\n", " \n", " \n", + " post-id5\n", + " test-bot@email\n", + " bot\n", + " 20\n", + " message\n", + " \n", + " \n", " post-id4\n", " test@test.com\n", " user\n", @@ -1067,10 +1092,10 @@ func runTestActianceExport(t *testing.T, exportBackend filestore.FileBackend, at " message\n", " \n", " \n", - " test_bot@email\n", + " test-bot@email\n", " bot\n", " 20\n", - " test_bot@email\n", + " test-bot@email\n", " \n", " \n", " test2@email\n", diff --git a/server/enterprise/message_export/csv_export/csv_export.go b/server/enterprise/message_export/csv_export/csv_export.go index 74b38b2b55..bc20b31313 100644 --- a/server/enterprise/message_export/csv_export/csv_export.go +++ b/server/enterprise/message_export/csv_export/csv_export.go @@ -150,10 +150,14 @@ func CsvExport(rctx request.CTX, p shared.ExportParams) (shared.RunExportResults for _, attachment := range attachments { var r io.ReadCloser - r, nErr := p.FileAttachmentBackend.Reader(attachment.Path) - if nErr != nil { - missingFiles = append(missingFiles, "Warning:"+shared.MissingFileMessage+" - Post: "+*post.PostId+" - "+attachment.Path) - rctx.Logger().Warn(shared.MissingFileMessage, mlog.String("post_id", *post.PostId), mlog.String("filename", attachment.Path)) + r, err = p.FileAttachmentBackend.Reader(attachment.Path) + if err != nil { + missingFiles = append(missingFiles, "Warning:"+shared.MissingFileMessageDuringBackendRead+" - Post: "+*post.PostId+" - "+attachment.Path) + rctx.Logger().Warn(shared.MissingFileMessageDuringBackendRead, + mlog.String("post_id", *post.PostId), + mlog.String("filename", attachment.Path), + mlog.Err(err), + ) continue } @@ -173,7 +177,16 @@ func CsvExport(rctx request.CTX, p shared.ExportParams) (shared.RunExportResults return nil }(); err != nil { - return results, fmt.Errorf("unable to copy the attachment into the zip file: %w", err) + // s3 only errors _here_ if the object key wasn't found. So to handle that: if there is a read + // error (even for local), let's add a warning instead of failing the export. + // Failing the export would fail the entire export run, and every future run would also fail on + // this non-existent file -- not good. + missingFiles = append(missingFiles, "Warning:"+shared.MissingFileMessageDuringCopy+" - Post: "+*post.PostId+" - "+attachment.Path) + rctx.Logger().Warn(shared.MissingFileMessageDuringCopy, + mlog.String("post_id", *post.PostId), + mlog.String("filename", attachment.Path), + mlog.Err(err), + ) } } } diff --git a/server/enterprise/message_export/csv_export/csv_export_test.go b/server/enterprise/message_export/csv_export/csv_export_test.go index 3de297f00f..0136544044 100644 --- a/server/enterprise/message_export/csv_export/csv_export_test.go +++ b/server/enterprise/message_export/csv_export/csv_export_test.go @@ -906,7 +906,7 @@ func TestWriteExportWarnings(t *testing.T) { warnings := string(data) expectedWarnings := fmt.Sprintf("Warning:%[1]s - Post: post-id-1 - test1\nWarning:%[1]s - Post: post-id-3 - test2\n", - shared.MissingFileMessage) + shared.MissingFileMessageDuringBackendRead) assert.Equal(t, expectedWarnings, warnings) } diff --git a/server/enterprise/message_export/shared/shared.go b/server/enterprise/message_export/shared/shared.go index ef2de996c2..f588a9dad3 100644 --- a/server/enterprise/message_export/shared/shared.go +++ b/server/enterprise/message_export/shared/shared.go @@ -21,7 +21,8 @@ import ( ) const ( - MissingFileMessage = "File missing for post; cannot copy file to archive" + MissingFileMessageDuringBackendRead = "File backend read: File missing for post; cannot copy file to archive" + MissingFileMessageDuringCopy = "Copy buffer: File missing for post; cannot copy file to archive" EstimatedPostCount = 10_000_000 diff --git a/server/enterprise/message_export/worker.go b/server/enterprise/message_export/worker.go index 82b5c04668..95e1f40ffa 100644 --- a/server/enterprise/message_export/worker.go +++ b/server/enterprise/message_export/worker.go @@ -251,7 +251,7 @@ func (w *MessageExportWorker) finishExport(rctx request.CTX, logger *mlog.Logger // we've exported everything up to the current time logger.Debug("FormatExport complete") - job.Data[shared.JobDataIsDownloadable] = "false" + job.Data[shared.JobDataIsDownloadable] = "true" if totalWarningCount > 0 { w.setJobWarning(logger, job) diff --git a/server/i18n/en.json b/server/i18n/en.json index 42fd0d0dc9..bd858af413 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -2136,6 +2136,10 @@ "id": "api.file.write_file.app_error", "translation": "Unable to write the file." }, + { + "id": "api.file.zip_file_reader.app_error", + "translation": "Unable to get a zip file reader." + }, { "id": "api.filter_config_error", "translation": "Unable to filter the configuration." diff --git a/server/platform/shared/filestore/filesstore.go b/server/platform/shared/filestore/filesstore.go index b6038fa651..46116579a4 100644 --- a/server/platform/shared/filestore/filesstore.go +++ b/server/platform/shared/filestore/filesstore.go @@ -40,6 +40,7 @@ type FileBackend interface { ListDirectory(path string) ([]string, error) ListDirectoryRecursively(path string) ([]string, error) RemoveDirectory(path string) error + ZipReader(path string, deflate bool) (io.ReadCloser, error) } type FileBackendWithLinkGenerator interface { diff --git a/server/platform/shared/filestore/filesstore_test.go b/server/platform/shared/filestore/filesstore_test.go index ed12b1cc2c..130620202e 100644 --- a/server/platform/shared/filestore/filesstore_test.go +++ b/server/platform/shared/filestore/filesstore_test.go @@ -4,6 +4,7 @@ package filestore import ( + "archive/zip" "bytes" "context" "fmt" @@ -11,15 +12,17 @@ import ( "math" "math/rand" "os" + "path/filepath" "strings" "testing" "time" + "github.com/mattermost/mattermost/server/public/model" + "github.com/stretchr/testify/require" "github.com/stretchr/testify/suite" "github.com/xtgo/uuid" - "github.com/mattermost/mattermost/server/public/model" "github.com/mattermost/mattermost/server/public/shared/mlog" ) @@ -114,12 +117,12 @@ func (s *FileBackendTestSuite) TestReadWriteFile() { path := "tests/" + randomString() written, err := s.backend.WriteFile(bytes.NewReader(b), path) - s.Nil(err) + s.NoError(err) s.EqualValues(len(b), written, "expected given number of bytes to have been written") defer s.backend.RemoveFile(path) read, err := s.backend.ReadFile(path) - s.Nil(err) + s.NoError(err) readString := string(read) s.EqualValues(readString, "test") @@ -215,12 +218,12 @@ func (s *FileBackendTestSuite) TestReadWriteFileImage() { path := "tests/" + randomString() + ".png" written, err := s.backend.WriteFile(bytes.NewReader(b), path) - s.Nil(err) + s.NoError(err) s.EqualValues(len(b), written, "expected given number of bytes to have been written") defer s.backend.RemoveFile(path) read, err := s.backend.ReadFile(path) - s.Nil(err) + s.NoError(err) readString := string(read) s.EqualValues(readString, "testimage") @@ -231,15 +234,15 @@ func (s *FileBackendTestSuite) TestFileExists() { path := "tests/" + randomString() + ".png" _, err := s.backend.WriteFile(bytes.NewReader(b), path) - s.Nil(err) + s.NoError(err) defer s.backend.RemoveFile(path) res, err := s.backend.FileExists(path) - s.Nil(err) + s.NoError(err) s.True(res) res, err = s.backend.FileExists("tests/idontexist.png") - s.Nil(err) + s.NoError(err) s.False(res) } @@ -249,19 +252,19 @@ func (s *FileBackendTestSuite) TestCopyFile() { path2 := "tests/" + randomString() written, err := s.backend.WriteFile(bytes.NewReader(b), path1) - s.Nil(err) + s.NoError(err) s.EqualValues(len(b), written, "expected given number of bytes to have been written") defer s.backend.RemoveFile(path1) err = s.backend.CopyFile(path1, path2) - s.Nil(err) + s.NoError(err) defer s.backend.RemoveFile(path2) data1, err := s.backend.ReadFile(path1) - s.Nil(err) + s.NoError(err) data2, err := s.backend.ReadFile(path2) - s.Nil(err) + s.NoError(err) s.Equal(b, data1) s.Equal(b, data2) @@ -273,19 +276,19 @@ func (s *FileBackendTestSuite) TestCopyFileToDirectoryThatDoesntExist() { path2 := "tests/newdirectory/" + randomString() written, err := s.backend.WriteFile(bytes.NewReader(b), path1) - s.Nil(err) + s.NoError(err) s.EqualValues(len(b), written, "expected given number of bytes to have been written") defer s.backend.RemoveFile(path1) err = s.backend.CopyFile(path1, path2) - s.Nil(err) + s.NoError(err) defer s.backend.RemoveFile(path2) _, err = s.backend.ReadFile(path1) - s.Nil(err) + s.NoError(err) _, err = s.backend.ReadFile(path2) - s.Nil(err) + s.NoError(err) } func (s *FileBackendTestSuite) TestMoveFile() { @@ -294,7 +297,7 @@ func (s *FileBackendTestSuite) TestMoveFile() { path2 := "tests/" + randomString() written, err := s.backend.WriteFile(bytes.NewReader(b), path1) - s.Nil(err) + s.NoError(err) s.EqualValues(len(b), written, "expected given number of bytes to have been written") defer s.backend.RemoveFile(path1) @@ -305,7 +308,7 @@ func (s *FileBackendTestSuite) TestMoveFile() { s.Error(err) data, err := s.backend.ReadFile(path2) - s.Nil(err) + s.NoError(err) s.Equal(b, data) } @@ -315,7 +318,7 @@ func (s *FileBackendTestSuite) TestRemoveFile() { path := "tests/" + randomString() written, err := s.backend.WriteFile(bytes.NewReader(b), path) - s.Nil(err) + s.NoError(err) s.EqualValues(len(b), written, "expected given number of bytes to have been written") s.Nil(s.backend.RemoveFile(path)) @@ -323,15 +326,15 @@ func (s *FileBackendTestSuite) TestRemoveFile() { s.Error(err) written, err = s.backend.WriteFile(bytes.NewReader(b), "tests2/foo") - s.Nil(err) + s.NoError(err) s.EqualValues(len(b), written, "expected given number of bytes to have been written") written, err = s.backend.WriteFile(bytes.NewReader(b), "tests2/bar") - s.Nil(err) + s.NoError(err) s.EqualValues(len(b), written, "expected given number of bytes to have been written") written, err = s.backend.WriteFile(bytes.NewReader(b), "tests2/asdf") - s.Nil(err) + s.NoError(err) s.EqualValues(len(b), written, "expected given number of bytes to have been written") s.Nil(s.backend.RemoveDirectory("tests2")) @@ -343,35 +346,35 @@ func (s *FileBackendTestSuite) TestListDirectory() { path2 := "19800101/" + randomString() paths, err := s.backend.ListDirectory("19700101") - s.Nil(err) + s.NoError(err) s.Len(paths, 0) written, err := s.backend.WriteFile(bytes.NewReader(b), path1) - s.Nil(err) + s.NoError(err) s.EqualValues(len(b), written, "expected given number of bytes to have been written") written, err = s.backend.WriteFile(bytes.NewReader(b), path2) - s.Nil(err) + s.NoError(err) s.EqualValues(len(b), written, "expected given number of bytes to have been written") paths, err = s.backend.ListDirectory("19700101") - s.Nil(err) + s.NoError(err) s.Len(paths, 1) s.Equal(path1, (paths)[0]) paths, err = s.backend.ListDirectory("19800101/") - s.Nil(err) + s.NoError(err) s.Len(paths, 1) s.Equal(path2, (paths)[0]) if s.settings.DriverName == driverLocal { paths, err = s.backend.ListDirectory("19800102") - s.Nil(err) + s.NoError(err) s.Len(paths, 0) } paths, err = s.backend.ListDirectory("") - s.Nil(err) + s.NoError(err) found1 := false found2 := false for _, path := range paths { @@ -395,39 +398,39 @@ func (s *FileBackendTestSuite) TestListDirectoryRecursively() { longPath := "19800102" + strings.Repeat("/toomuch", MaxRecursionDepth+1) + randomString() paths, err := s.backend.ListDirectoryRecursively("19700101") - s.Nil(err) + s.NoError(err) s.Len(paths, 0) written, err := s.backend.WriteFile(bytes.NewReader(b), path1) - s.Nil(err) + s.NoError(err) s.EqualValues(len(b), written, "expected given number of bytes to have been written") written, err = s.backend.WriteFile(bytes.NewReader(b), path2) - s.Nil(err) + s.NoError(err) s.EqualValues(len(b), written, "expected given number of bytes to have been written") written, err = s.backend.WriteFile(bytes.NewReader(b), longPath) - s.Nil(err) + s.NoError(err) s.EqualValues(len(b), written, "expected given number of bytes to have been written") paths, err = s.backend.ListDirectoryRecursively("19700101") - s.Nil(err) + s.NoError(err) s.Len(paths, 1) s.Equal(path1, (paths)[0]) paths, err = s.backend.ListDirectoryRecursively("19800101/") - s.Nil(err) + s.NoError(err) s.Len(paths, 1) s.Equal(path2, (paths)[0]) if s.settings.DriverName == driverLocal { paths, err = s.backend.ListDirectory("19800102") - s.Nil(err) + s.NoError(err) s.Len(paths, 1) } paths, err = s.backend.ListDirectoryRecursively("") - s.Nil(err) + s.NoError(err) found1 := false found2 := false found3 := false @@ -455,15 +458,15 @@ func (s *FileBackendTestSuite) TestRemoveDirectory() { b := []byte("test") written, err := s.backend.WriteFile(bytes.NewReader(b), "tests2/foo") - s.Nil(err) + s.NoError(err) s.EqualValues(len(b), written, "expected given number of bytes to have been written") written, err = s.backend.WriteFile(bytes.NewReader(b), "tests2/bar") - s.Nil(err) + s.NoError(err) s.EqualValues(len(b), written, "expected given number of bytes to have been written") written, err = s.backend.WriteFile(bytes.NewReader(b), "tests2/aaa") - s.Nil(err) + s.NoError(err) s.EqualValues(len(b), written, "expected given number of bytes to have been written") s.Nil(s.backend.RemoveDirectory("tests2")) @@ -492,7 +495,7 @@ func (s *FileBackendTestSuite) TestAppendFile() { path := "tests/" + randomString() written, err := s.backend.WriteFile(bytes.NewReader(b), path) - s.Nil(err) + s.NoError(err) s.EqualValues(len(b), written) defer s.backend.RemoveFile(path) @@ -502,11 +505,11 @@ func (s *FileBackendTestSuite) TestAppendFile() { } written, err = s.backend.AppendFile(bytes.NewReader(b2), path) - s.Nil(err) + s.NoError(err) s.EqualValues(int64(len(b2)), written) read, err := s.backend.ReadFile(path) - s.Nil(err) + s.NoError(err) s.EqualValues(len(b)+len(b2), len(read)) s.True(bytes.Equal(append(b, b2...), read)) @@ -516,11 +519,11 @@ func (s *FileBackendTestSuite) TestAppendFile() { } written, err = s.backend.AppendFile(bytes.NewReader(b3), path) - s.Nil(err) + s.NoError(err) s.EqualValues(int64(len(b3)), written) read, err = s.backend.ReadFile(path) - s.Nil(err) + s.NoError(err) s.EqualValues(len(b)+len(b2)+len(b3), len(read)) s.True(bytes.Equal(append(append(b, b2...), b3...), read)) }) @@ -538,12 +541,12 @@ func (s *FileBackendTestSuite) TestFileSize() { path := "tests/" + randomString() written, err := s.backend.WriteFile(bytes.NewReader(data), path) - s.Nil(err) + s.NoError(err) s.EqualValues(len(data), written) defer s.backend.RemoveFile(path) size, err := s.backend.FileSize(path) - s.Nil(err) + s.NoError(err) s.Equal(int64(len(data)), size) }) } @@ -560,12 +563,12 @@ func (s *FileBackendTestSuite) TestFileModTime() { data := []byte("some data") written, err := s.backend.WriteFile(bytes.NewReader(data), path) - s.Nil(err) + s.NoError(err) s.EqualValues(len(data), written) defer s.backend.RemoveFile(path) modTime, err := s.backend.FileModTime(path) - s.Nil(err) + s.NoError(err) s.NotEmpty(modTime) // We wait 1 second so that the times will differ enough to be testable. @@ -573,12 +576,12 @@ func (s *FileBackendTestSuite) TestFileModTime() { path2 := "tests/" + randomString() written, err = s.backend.WriteFile(bytes.NewReader(data), path2) - s.Nil(err) + s.NoError(err) s.EqualValues(len(data), written) defer s.backend.RemoveFile(path2) modTime2, err := s.backend.FileModTime(path2) - s.Nil(err) + s.NoError(err) s.NotEmpty(modTime2) s.True(modTime2.After(modTime)) }) @@ -809,3 +812,268 @@ func TestNewExportFileBackendSettingsFromConfig(t *testing.T) { require.Equal(t, expected, actual) }) } + +func (s *FileBackendTestSuite) TestZipReaderSingleFile() { + // Test zipping a single file (but not its neighbours) + b := []byte("testdata") + path := "tests/" + randomString() + ".txt" + + written, err := s.backend.WriteFile(bytes.NewReader(b), path) + s.NoError(err) + s.EqualValues(len(b), written) + defer s.backend.RemoveFile(path) + + // neighbour, not included + b2 := []byte("testdata2") + path2 := "tests/" + randomString() + ".txt" + + written, err = s.backend.WriteFile(bytes.NewReader(b2), path2) + s.NoError(err) + s.EqualValues(len(b2), written) + defer s.backend.RemoveFile(path2) + + // Test without compression + reader, err := s.backend.ZipReader(path, false) + s.NoError(err) + defer reader.Close() + + // Read the zip file + zipBytes, err := io.ReadAll(reader) + s.NoError(err) + + zipReader, err := zip.NewReader(bytes.NewReader(zipBytes), int64(len(zipBytes))) + s.NoError(err) + s.Len(zipReader.File, 1) + + // Verify file contents + zf := zipReader.File[0] + s.Equal(filepath.Base(path), zf.Name) + s.Equal(zip.Store, zf.Method) + + rc, err := zf.Open() + s.NoError(err) + defer rc.Close() + + content, err := io.ReadAll(rc) + s.NoError(err) + s.Equal(b, content) +} + +func (s *FileBackendTestSuite) TestZipReaderSingleFileCompressed() { + // Test zipping a single file (but not its neighbours) with compression + b := []byte("testdata") + path := "tests/" + randomString() + ".txt" + + written, err := s.backend.WriteFile(bytes.NewReader(b), path) + s.NoError(err) + s.EqualValues(len(b), written) + defer s.backend.RemoveFile(path) + + // neighbour, not included + b2 := []byte("testdata2") + path2 := "tests/" + randomString() + ".txt" + + written, err = s.backend.WriteFile(bytes.NewReader(b2), path2) + s.NoError(err) + s.EqualValues(len(b2), written) + defer s.backend.RemoveFile(path2) + + reader, err := s.backend.ZipReader(path, true) + s.NoError(err) + defer reader.Close() + + zipBytes, err := io.ReadAll(reader) + s.NoError(err) + + zipReader, err := zip.NewReader(bytes.NewReader(zipBytes), int64(len(zipBytes))) + s.NoError(err) + s.Len(zipReader.File, 1) + + zf := zipReader.File[0] + s.Equal(filepath.Base(path), zf.Name) + s.Equal(zip.Deflate, zf.Method) + + rc, err := zf.Open() + s.NoError(err) + defer rc.Close() + + content, err := io.ReadAll(rc) + s.NoError(err) + s.Equal(b, content) +} + +func (s *FileBackendTestSuite) TestZipReaderDirectory() { + // Create test directory structure + dirPath := "tests/zip_test_" + randomString() + files := map[string][]byte{ + "file1.txt": []byte("file1 content"), + "file2.png": []byte("file2 content"), + "subdir/file3.txt": []byte("file3 content"), + "subdir2/file4.json": []byte("file4 content"), + } + + for path, content := range files { + fullPath := filepath.Join(dirPath, path) + written, err := s.backend.WriteFile(bytes.NewReader(content), fullPath) + s.NoError(err) + s.EqualValues(len(content), written) + defer s.backend.RemoveFile(fullPath) + } + + // Test without compression + reader, err := s.backend.ZipReader(dirPath, false) + s.NoError(err) + defer reader.Close() + + // Read and verify zip contents + zipBytes, err := io.ReadAll(reader) + s.NoError(err) + + zipReader, err := zip.NewReader(bytes.NewReader(zipBytes), int64(len(zipBytes))) + s.NoError(err) + + // Verify each file + foundFiles := make(map[string]bool) + for _, zf := range zipReader.File { + s.Contains(files, zf.Name) + expectedContent := files[zf.Name] + delete(files, zf.Name) + + rc, err := zf.Open() + s.NoError(err) + + content, err := io.ReadAll(rc) + s.NoError(err) + rc.Close() + + s.Equal(expectedContent, content) + foundFiles[zf.Name] = true + } + + // Verify we found all files + s.Len(foundFiles, 4) + s.Empty(files) +} + +func (s *FileBackendTestSuite) TestZipReaderDirectoryCompressed() { + // Create test directory structure + dirPath := "tests/zip_test_" + randomString() + files := map[string][]byte{ + "file1.txt": []byte("file1 content"), + "file2.png": []byte("file2 content"), + "subdir/file3.txt": []byte("file3 content"), + "subdir2/file4.json": []byte("file4 content"), + } + + for path, content := range files { + fullPath := filepath.Join(dirPath, path) + written, err := s.backend.WriteFile(bytes.NewReader(content), fullPath) + s.NoError(err) + s.EqualValues(len(content), written) + defer s.backend.RemoveFile(fullPath) + } + + // Test with compression + reader, err := s.backend.ZipReader(dirPath, true) + s.NoError(err) + defer reader.Close() + + // Read and verify zip contents + zipBytes, err := io.ReadAll(reader) + s.NoError(err) + + zipReader, err := zip.NewReader(bytes.NewReader(zipBytes), int64(len(zipBytes))) + s.NoError(err) + + // Verify each file + foundFiles := make(map[string]bool) + for _, zf := range zipReader.File { + s.Contains(files, zf.Name) + s.Equal(zip.Deflate, zf.Method) + expectedContent := files[zf.Name] + delete(files, zf.Name) + + rc, err := zf.Open() + s.NoError(err) + + content, err := io.ReadAll(rc) + s.NoError(err) + rc.Close() + + s.Equal(expectedContent, content) + foundFiles[zf.Name] = true + } + + // Verify we found all files + s.Len(foundFiles, 4) + s.Empty(files) +} + +func (s *FileBackendTestSuite) TestZipReaderErrors() { + // Test non-existent path + reader, err := s.backend.ZipReader("path/to/nonexistent.txt", false) + if s.settings.DriverName == driverLocal { + // Only local will return the error immediately. + s.Error(err) + s.Nil(reader) + } else { + s.NoError(err) + defer reader.Close() + var content []byte + content, err = io.ReadAll(reader) + s.NoError(err) + s.assertEmptyZip(content) + } + + // Test empty directory + emptyDir := "tests/empty_" + randomString() + err = os.MkdirAll(filepath.Join(s.settings.Directory, emptyDir), 0750) + s.NoError(err) + defer os.RemoveAll(filepath.Join(s.settings.Directory, emptyDir)) + + reader, err = s.backend.ZipReader(emptyDir, false) + s.NoError(err) + defer reader.Close() + content, err := io.ReadAll(reader) + s.NoError(err) + s.assertEmptyZip(content) +} + +func (s *FileBackendTestSuite) TestZipReaderErrorsCompressed() { + // Test non-existent path with compression + reader, err := s.backend.ZipReader("path/to/nonexistent.txt", true) + if s.settings.DriverName == driverLocal { + // Only local will return the error immediately. + s.Error(err) + s.Nil(reader) + } else { + s.NoError(err) + defer reader.Close() + var content []byte + content, err = io.ReadAll(reader) + s.NoError(err) + s.assertEmptyZip(content) + } + + // Test empty directory with compression + emptyDir := "tests/empty_" + randomString() + err = os.MkdirAll(filepath.Join(s.settings.Directory, emptyDir), 0750) + s.NoError(err) + defer os.RemoveAll(filepath.Join(s.settings.Directory, emptyDir)) + + reader, err = s.backend.ZipReader(emptyDir, true) + s.NoError(err) + defer reader.Close() + content, err := io.ReadAll(reader) + s.NoError(err) + s.assertEmptyZip(content) +} + +func (s *FileBackendTestSuite) assertEmptyZip(content []byte) { + s.NotNil(content) + + // Verify it's a valid but empty zip + zipReader, err := zip.NewReader(bytes.NewReader(content), int64(len(content))) + s.NoError(err) + s.Len(zipReader.File, 0) +} diff --git a/server/platform/shared/filestore/localstore.go b/server/platform/shared/filestore/localstore.go index 5d4e514f4c..54c97665fd 100644 --- a/server/platform/shared/filestore/localstore.go +++ b/server/platform/shared/filestore/localstore.go @@ -4,6 +4,7 @@ package filestore import ( + "archive/zip" "bytes" "io" "os" @@ -257,3 +258,90 @@ func (b *LocalFileBackend) RemoveDirectory(path string) error { } return nil } + +// ZipReader will create a zip of path. If path is a single file, it will zip the single file. +// If deflate is true, the contents will be compressed. It will stream the zip to io.ReadCloser. +func (b *LocalFileBackend) ZipReader(path string, deflate bool) (io.ReadCloser, error) { + deflateMethod := zip.Store + if deflate { + deflateMethod = zip.Deflate + } + + fullPath := filepath.Join(b.directory, path) + baseInfo, err := os.Stat(fullPath) + if err != nil { + return nil, errors.Wrapf(err, "unable to stat path %s", path) + } + + pr, pw := io.Pipe() + + go func() { + defer pw.Close() + + zipWriter := zip.NewWriter(pw) + defer zipWriter.Close() + + err = filepath.Walk(fullPath, func(filePath string, info os.FileInfo, err error) error { + if err != nil { + return err + } + + // Handle single file case + baseDir := fullPath + if !baseInfo.IsDir() { + baseDir = filepath.Dir(baseDir) + } + + // Get the relative path from the base directory + relPath, err := filepath.Rel(baseDir, filePath) + if err != nil { + return errors.Wrapf(err, "unable to get relative path for %s", filePath) + } + + // Skip the root directory itself + if relPath == "." { + return nil + } + + // Create zip header + header, err := zip.FileInfoHeader(info) + if err != nil { + return errors.Wrapf(err, "unable to create zip header for %s", relPath) + } + + // Ensure consistent forward slashes in paths + header.Name = filepath.ToSlash(relPath) + + // Skip directories - we don't need to create entries for them + if info.IsDir() { + return nil + } + + // Create file entry + header.Method = deflateMethod + header.SetMode(0644) // rw-r--r-- permissions + writer, err := zipWriter.CreateHeader(header) + if err != nil { + return errors.Wrapf(err, "unable to create zip entry for %s", relPath) + } + + file, err := os.Open(filePath) + if err != nil { + return errors.Wrapf(err, "unable to open file %s", filePath) + } + defer file.Close() + + if _, err := io.Copy(writer, file); err != nil { + return errors.Wrapf(err, "unable to copy file content for %s", relPath) + } + + return nil + }) + + if err != nil { + pw.CloseWithError(errors.Wrap(err, "error walking directory")) + } + }() + + return pr, nil +} diff --git a/server/platform/shared/filestore/mocks/FileBackend.go b/server/platform/shared/filestore/mocks/FileBackend.go index dd294916e7..6c71de2e8e 100644 --- a/server/platform/shared/filestore/mocks/FileBackend.go +++ b/server/platform/shared/filestore/mocks/FileBackend.go @@ -387,6 +387,36 @@ func (_m *FileBackend) WriteFile(fr io.Reader, path string) (int64, error) { return r0, r1 } +// ZipReader provides a mock function with given fields: path, deflate +func (_m *FileBackend) ZipReader(path string, deflate bool) (io.ReadCloser, error) { + ret := _m.Called(path, deflate) + + if len(ret) == 0 { + panic("no return value specified for ZipReader") + } + + var r0 io.ReadCloser + var r1 error + if rf, ok := ret.Get(0).(func(string, bool) (io.ReadCloser, error)); ok { + return rf(path, deflate) + } + if rf, ok := ret.Get(0).(func(string, bool) io.ReadCloser); ok { + r0 = rf(path, deflate) + } else { + if ret.Get(0) != nil { + r0 = ret.Get(0).(io.ReadCloser) + } + } + + if rf, ok := ret.Get(1).(func(string, bool) error); ok { + r1 = rf(path, deflate) + } else { + r1 = ret.Error(1) + } + + return r0, r1 +} + // NewFileBackend creates a new instance of FileBackend. It also registers a testing interface on the mock and a cleanup function to assert the mocks expectations. // The first argument is typically a *testing.T value. func NewFileBackend(t interface { diff --git a/server/platform/shared/filestore/s3store.go b/server/platform/shared/filestore/s3store.go index ba496dd5f4..a1136767fd 100644 --- a/server/platform/shared/filestore/s3store.go +++ b/server/platform/shared/filestore/s3store.go @@ -4,6 +4,7 @@ package filestore import ( + "archive/zip" "bytes" "context" "crypto/tls" @@ -705,6 +706,103 @@ func (b *S3FileBackend) RemoveDirectory(path string) error { return nil } +// ZipReader will create a zip of path. If path is a single file, it will zip the single file. +// If deflate is true, the contents will be compressed. It will stream the zip to io.ReadCloser. +func (b *S3FileBackend) ZipReader(path string, deflate bool) (io.ReadCloser, error) { + deflateMethod := zip.Store + if deflate { + deflateMethod = zip.Deflate + } + + path, err := b.prefixedPath(path) + if err != nil { + return nil, err + } + + pr, pw := io.Pipe() + + go func() { + defer pw.Close() + + zipWriter := zip.NewWriter(pw) + defer zipWriter.Close() + + ctx, cancel := context.WithTimeout(context.Background(), b.timeout) + defer cancel() + + // Is path a single file? + object, err := b.client.StatObject(ctx, b.bucket, path, s3.StatObjectOptions{}) + if err == nil { + // We want the zipped file to be at the root of the zip. E.g., given a path of + // "path/to/file.sh" we want the zip to have one file: "file.sh", not "path/to/file.sh". + stripPath := filepath.Dir(path) + if stripPath != "" { + stripPath += "/" + } + if err = b._copyObjectToZipWriter(zipWriter, object, stripPath, deflateMethod); err != nil { + pw.CloseWithError(err) + } + return + } + + // Is path a directory? + path = path + "/" + opts := s3.ListObjectsOptions{ + Prefix: path, + Recursive: true, + } + ctx2, cancel2 := context.WithTimeout(context.Background(), b.timeout) + defer cancel2() + + for object := range b.client.ListObjects(ctx2, b.bucket, opts) { + if object.Err != nil { + pw.CloseWithError(errors.Wrapf(object.Err, "unable to list the directory %s", path)) + return + } + + if err = b._copyObjectToZipWriter(zipWriter, object, path, deflateMethod); err != nil { + pw.CloseWithError(err) + return + } + } + }() + + return pr, nil +} + +func (b *S3FileBackend) _copyObjectToZipWriter(zipWriter *zip.Writer, object s3.ObjectInfo, stripPath string, deflateMethod uint16) error { + // We strip the path prefix that gets applied, + // so that it remains transparent to the application. + object.Key = strings.TrimPrefix(object.Key, b.pathPrefix) + + // We strip the path prefix + path so the zip file is relative to the root of the requested path + relPath := strings.TrimPrefix(object.Key, stripPath) + header := &zip.FileHeader{ + Name: relPath, + Method: deflateMethod, + Modified: object.LastModified, + } + header.SetMode(0644) // rw-r--r-- permissions + + writer, err := zipWriter.CreateHeader(header) + if err != nil { + return errors.Wrapf(err, "unable to create zip entry for %s", object.Key) + } + + reader, err := b.Reader(object.Key) + if err != nil { + return errors.Wrapf(err, "unable to create reader for %s", object.Key) + } + defer reader.Close() + + _, err = io.Copy(writer, reader) + if err != nil { + return errors.Wrapf(err, "unable to copy content for %s", object.Key) + } + + return nil +} + func (b *S3FileBackend) GeneratePublicLink(path string) (string, time.Duration, error) { path, err := b.prefixedPath(path) if err != nil { diff --git a/server/platform/shared/web/files.go b/server/platform/shared/web/files.go index e93c1b2ada..4b652401af 100644 --- a/server/platform/shared/web/files.go +++ b/server/platform/shared/web/files.go @@ -10,6 +10,8 @@ import ( "strconv" "strings" "time" + + "github.com/pkg/errors" ) var UnsafeContentTypes = [...]string{ @@ -35,9 +37,10 @@ var MediaContentTypes = [...]string{ "audio/wav", } +// WriteFileResponse copies the io.ReadSeeker `fileReader` to the ResponseWriter `w`. Use this when you have a +// ReadSeeker. func WriteFileResponse(filename string, contentType string, contentSize int64, lastModification time.Time, webserverMode string, fileReader io.ReadSeeker, forceDownload bool, w http.ResponseWriter, r *http.Request) { - w.Header().Set("Cache-Control", "private, max-age=86400") - w.Header().Set("X-Content-Type-Options", "nosniff") + setHeaders(w, contentType, forceDownload, filename) if contentSize > 0 { contentSizeStr := strconv.Itoa(int(contentSize)) @@ -48,6 +51,25 @@ func WriteFileResponse(filename string, contentType string, contentSize int64, l } } + http.ServeContent(w, r, filename, lastModification, fileReader) +} + +// WriteStreamResponse copies the ReadCloser `r` to the ResponseWriter `w`. Use this when you need to stream a response +// to the client that will appear as a file `filename` of type `contentType`. +func WriteStreamResponse(w http.ResponseWriter, r io.ReadCloser, filename string, contentType string, forceDownload bool) error { + setHeaders(w, contentType, forceDownload, filename) + + if _, err := io.Copy(w, r); err != nil { + return errors.Wrap(err, "error streaming ReadCloser") + } + + return nil +} + +func setHeaders(w http.ResponseWriter, contentType string, forceDownload bool, filename string) { + w.Header().Set("Cache-Control", "private, max-age=86400") + w.Header().Set("X-Content-Type-Options", "nosniff") + if contentType == "" { contentType = "application/octet-stream" } else { @@ -66,14 +88,12 @@ func WriteFileResponse(filename string, contentType string, contentSize int64, l toDownload = true } else { isMediaType := false - for _, mediaContentType := range MediaContentTypes { if strings.HasPrefix(contentType, mediaContentType) { isMediaType = true break } } - toDownload = !isMediaType } @@ -88,6 +108,4 @@ func WriteFileResponse(filename string, contentType string, contentSize int64, l // prevent file links from being embedded in iframes w.Header().Set("X-Frame-Options", "DENY") w.Header().Set("Content-Security-Policy", "Frame-ancestors 'none'") - - http.ServeContent(w, r, filename, lastModification, fileReader) }