diff --git a/server/.golangci.yml b/server/.golangci.yml index afe6f8cfd09..9540847a7f6 100644 --- a/server/.golangci.yml +++ b/server/.golangci.yml @@ -94,7 +94,6 @@ issues: channels/app/file.go|\ channels/app/file_test.go|\ channels/app/helper_test.go|\ - channels/app/import_functions.go|\ channels/app/permissions_test.go|\ channels/app/platform/helper_test.go|\ channels/app/platform/license.go|\ diff --git a/server/channels/app/import_functions.go b/server/channels/app/import_functions.go index f5324609711..8e3ced2b9b4 100644 --- a/server/channels/app/import_functions.go +++ b/server/channels/app/import_functions.go @@ -939,32 +939,42 @@ func (a *App) importProfileImage(rctx request.CTX, userID string, data *imports. var f io.ReadCloser f, err = data.ProfileImageData.Open() if err != nil { - rctx.Logger().Warn("Unable to open the profile image data.", mlog.Err(err)) - } else { - limitedReader := io.LimitReader(f, *a.Config().FileSettings.MaxFileSize) - var b []byte - b, err = io.ReadAll(limitedReader) - if err != nil { - rctx.Logger().Warn("Unable to read all bytes from profile picture.", mlog.Err(err)) - } else { - file = bytes.NewReader(b) + return model.NewAppError("importProfileImage", "app.import.profile_image.open.app_error", map[string]any{"FileName": data.ProfileImageData.Name}, "", http.StatusInternalServerError).Wrap(err) + } + + defer func() { + if closeErr := f.Close(); closeErr != nil { + rctx.Logger().Warn("Unable to close profile image data.", mlog.String("filename", data.ProfileImageData.Name), mlog.Err(closeErr)) } - } - } else { - file, err = os.Open(*data.ProfileImage) + }() + + limitedReader := io.LimitReader(f, *a.Config().FileSettings.MaxFileSize) + var b []byte + b, err = io.ReadAll(limitedReader) if err != nil { - rctx.Logger().Warn("Unable to open the profile image.", mlog.Err(err)) - } else { - defer file.(*os.File).Close() + return model.NewAppError("importProfileImage", "app.import.profile_image.read_data.app_error", nil, "", http.StatusInternalServerError).Wrap(err) } + file = bytes.NewReader(b) + } else { + path := *data.ProfileImage + file, err = os.Open(path) + if err != nil { + return model.NewAppError("importProfileImage", "app.import.profile_image.open.app_error", nil, "", http.StatusInternalServerError).Wrap(err) + } + + defer func() { + if closeErr := file.(*os.File).Close(); closeErr != nil { + rctx.Logger().Warn("Unable to close profile image file.", mlog.String("filepath", path), mlog.Err(closeErr)) + } + }() } if file != nil { - if limitErr := checkImageLimits(file, *a.Config().FileSettings.MaxImageResolution); limitErr != nil { - return model.NewAppError("SetProfileImage", "api.user.upload_profile_user.check_image_limits.app_error", nil, "", http.StatusBadRequest).Wrap(limitErr) + if err := checkImageLimits(file, *a.Config().FileSettings.MaxImageResolution); err != nil { + return model.NewAppError("SetProfileImage", "api.user.upload_profile_user.check_image_limits.app_error", nil, "", http.StatusBadRequest).Wrap(err) } - if err := a.SetProfileImageFromFile(rctx, userID, file); err != nil { - rctx.Logger().Warn("Unable to set the profile image from a file.", mlog.Err(err)) + if appErr := a.SetProfileImageFromFile(rctx, userID, file); appErr != nil { + return appErr } } @@ -980,9 +990,9 @@ func (a *App) importUserTeams(rctx request.CTX, user *model.User, data *[]import for _, tdata := range *data { teamNames = append(teamNames, *tdata.Name) } - allTeams, err := a.getTeamsByNames(teamNames) - if err != nil { - return err + allTeams, appErr := a.getTeamsByNames(teamNames) + if appErr != nil { + return appErr } var ( @@ -1053,9 +1063,9 @@ func (a *App) importUserTeams(rctx request.CTX, user *model.User, data *[]import } if !user.IsGuest() { var userShouldBeAdmin bool - userShouldBeAdmin, err = a.UserIsInAdminRoleGroup(user.Id, team.Id, model.GroupSyncableTypeTeam) - if err != nil { - return err + userShouldBeAdmin, appErr = a.UserIsInAdminRoleGroup(user.Id, team.Id, model.GroupSyncableTypeTeam) + if appErr != nil { + return appErr } member.SchemeAdmin = userShouldBeAdmin } @@ -1078,7 +1088,6 @@ func (a *App) importUserTeams(rctx request.CTX, user *model.User, data *[]import oldMembers, nErr := a.Srv().Store().Team().UpdateMultipleMembers(oldTeamMembers) if nErr != nil { - var appErr *model.AppError switch { case errors.As(nErr, &appErr): return appErr @@ -1092,7 +1101,6 @@ func (a *App) importUserTeams(rctx request.CTX, user *model.User, data *[]import var nErr error newMembers, nErr = a.Srv().Store().Team().SaveMultipleMembers(newTeamMembers, *a.Config().TeamSettings.MaxUsersPerTeam) if nErr != nil { - var appErr *model.AppError var conflictErr *store.ErrConflict var limitExceededErr *store.ErrLimitExceeded switch { @@ -1110,12 +1118,14 @@ func (a *App) importUserTeams(rctx request.CTX, user *model.User, data *[]import for _, member := range append(newMembers, oldMembers...) { if member.ExplicitRoles != rolesByTeamID[member.TeamId] { - if _, err = a.UpdateTeamMemberRoles(rctx, member.TeamId, user.Id, rolesByTeamID[member.TeamId]); err != nil { - return err + if _, appErr = a.UpdateTeamMemberRoles(rctx, member.TeamId, user.Id, rolesByTeamID[member.TeamId]); appErr != nil { + return appErr } } - a.UpdateTeamMemberSchemeRoles(rctx, member.TeamId, user.Id, isGuestByTeamID[member.TeamId], isUserByTeamId[member.TeamId], isAdminByTeamID[member.TeamId]) + if _, appErr := a.UpdateTeamMemberSchemeRoles(rctx, member.TeamId, user.Id, isGuestByTeamID[member.TeamId], isUserByTeamId[member.TeamId], isAdminByTeamID[member.TeamId]); appErr != nil { + rctx.Logger().Warn("Error updating team member scheme roles", mlog.String("team_id", member.TeamId), mlog.String("user_id", user.Id), mlog.Err(appErr)) + } } for _, team := range allTeams { @@ -1305,7 +1315,9 @@ func (a *App) importUserChannels(rctx request.CTX, user *model.User, team *model } } - a.UpdateChannelMemberSchemeRoles(rctx, member.ChannelId, user.Id, isGuestByChannelId[member.ChannelId], isUserByChannelId[member.ChannelId], isAdminByChannelId[member.ChannelId]) + if _, appErr := a.UpdateChannelMemberSchemeRoles(rctx, member.ChannelId, user.Id, isGuestByChannelId[member.ChannelId], isUserByChannelId[member.ChannelId], isAdminByChannelId[member.ChannelId]); appErr != nil { + rctx.Logger().Warn("Error updating channel member scheme roles", mlog.String("channel_id", member.ChannelId), mlog.String("user_id", user.Id), mlog.Err(appErr)) + } } for _, channel := range allChannels { @@ -1428,7 +1440,9 @@ func (a *App) importReplies(rctx request.CTX, data []imports.ReplyImportData, po fileIDs := a.uploadAttachments(rctx, replyData.Attachments, reply, teamID, extractContent) for _, fileID := range reply.FileIds { if _, ok := fileIDs[fileID]; !ok { - a.Srv().Store().FileInfo().PermanentDelete(rctx, fileID) + if err := a.Srv().Store().FileInfo().PermanentDelete(rctx, fileID); err != nil { + rctx.Logger().Warn("Error while permanently deleting file info", mlog.String("file_id", fileID), mlog.Err(err)) + } } } reply.FileIds = make([]string, 0) @@ -1644,13 +1658,19 @@ func (a *App) importAttachment(rctx request.CTX, data *imports.AttachmentImportD } } else if data.Data != nil { rctx.Logger().Info("File is from ZIP, can't seek, opening again", mlog.String("file_name", name)) - file.Close() + if err := file.Close(); err != nil { + rctx.Logger().Warn("Error closing file", mlog.String("file_name", name), mlog.Err(err)) + } f, err := data.Data.Open() if err != nil { return nil, model.NewAppError("BulkImport", "app.import.attachment.bad_file.error", map[string]any{"FilePath": *data.Path}, "", http.StatusBadRequest).Wrap(err) } - defer f.Close() + defer func() { + if err := f.Close(); err != nil { + rctx.Logger().Warn("Error closing zip file reader", mlog.String("file_name", name), mlog.Err(err)) + } + }() file = f } @@ -1872,7 +1892,9 @@ func (a *App) importMultiplePostLines(rctx request.CTX, lines []imports.LineImpo fileIDs := a.uploadAttachments(rctx, line.Post.Attachments, post, team.Id, extractContent) for _, fileID := range post.FileIds { if _, ok := fileIDs[fileID]; !ok { - a.Srv().Store().FileInfo().PermanentDelete(rctx, fileID) + if err := a.Srv().Store().FileInfo().PermanentDelete(rctx, fileID); err != nil { + rctx.Logger().Warn("Error while permanently deleting file info", mlog.String("file_id", fileID), mlog.Err(err)) + } } } post.FileIds = make([]string, 0) @@ -2392,7 +2414,9 @@ func (a *App) importMultipleDirectPostLines(rctx request.CTX, lines []imports.Li fileIDs := a.uploadAttachments(rctx, line.DirectPost.Attachments, post, "noteam", extractContent) for _, fileID := range post.FileIds { if _, ok := fileIDs[fileID]; !ok { - a.Srv().Store().FileInfo().PermanentDelete(rctx, fileID) + if err := a.Srv().Store().FileInfo().PermanentDelete(rctx, fileID); err != nil { + rctx.Logger().Warn("Error while permanently deleting file info", mlog.String("file_id", fileID), mlog.Err(err)) + } } } post.FileIds = make([]string, 0) @@ -2566,7 +2590,11 @@ func (a *App) importEmoji(rctx request.CTX, data *imports.EmojiImportData, dryRu if err != nil { return model.NewAppError("BulkImport", "app.import.emoji.bad_file.error", map[string]any{"EmojiName": *data.Name}, "", http.StatusBadRequest).Wrap(err) } - defer file.Close() + defer func() { + if err := file.Close(); err != nil { + rctx.Logger().Warn("Error closing emoji file", mlog.String("emoji_name", *data.Name), mlog.Err(err)) + } + }() reader := utils.NewLimitedReaderWithError(file, MaxEmojiFileSize) if _, err := a.WriteFile(reader, getEmojiImagePath(emoji.Id)); err != nil { diff --git a/server/i18n/en.json b/server/i18n/en.json index 7562bda0e09..a6982b860a2 100644 --- a/server/i18n/en.json +++ b/server/i18n/en.json @@ -5546,6 +5546,14 @@ "id": "app.import.process_import_data_file_version_line.invalid_version.error", "translation": "Unable to read the version of the data import file." }, + { + "id": "app.import.profile_image.open.app_error", + "translation": "Failed to open the profile image file: {{.FileName}}" + }, + { + "id": "app.import.profile_image.read_data.app_error", + "translation": "Failed to read profile image data." + }, { "id": "app.import.validate_bot_import_data.owner_missing.error", "translation": "Bot owner is missing"