From 9f8fe479ae761fba9c80d0a598a95f166f0adb50 Mon Sep 17 00:00:00 2001 From: David Svantesson Date: Thu, 31 Oct 2019 19:11:36 +0100 Subject: [PATCH 1/5] Check if file is locked on upload file commit. --- models/error.go | 17 +++++++++++++++++ modules/repofiles/upload.go | 9 +++++++++ 2 files changed, 26 insertions(+) diff --git a/models/error.go b/models/error.go index 995617e83b8ae..322e73a75fbdf 100644 --- a/models/error.go +++ b/models/error.go @@ -647,6 +647,23 @@ func (err ErrLFSLockAlreadyExist) Error() string { return fmt.Sprintf("lfs lock already exists [rid: %d, path: %s]", err.RepoID, err.Path) } +// ErrLFSFileLocked represents a "LFSFileLocked" kind of error. +type ErrLFSFileLocked struct { + RepoID int64 + Path string + UserName string +} + +// IsErrLFSFileLocked checks if an error is a ErrLFSFileLocked. +func IsErrLFSFileLocked(err error) bool { + _, ok := err.(ErrLFSFileLocked) + return ok +} + +func (err ErrLFSFileLocked) Error() string { + return fmt.Sprintf("File is lfs locked [repo: %d, locked by: %s, path: %s", err.RepoID, err.UserName, err.Path) +} + // __________ .__ __ // \______ \ ____ ______ ____ _____|__|/ |_ ___________ ___.__. // | _// __ \\____ \ / _ \/ ___/ \ __\/ _ \_ __ < | | diff --git a/modules/repofiles/upload.go b/modules/repofiles/upload.go index a2e7cc927cb8c..e00caf739fc42 100644 --- a/modules/repofiles/upload.go +++ b/modules/repofiles/upload.go @@ -72,6 +72,15 @@ func UploadRepoFiles(repo *models.Repository, doer *models.User, opts *UploadRep for i, upload := range uploads { names[i] = upload.Name infos[i] = uploadInfo{upload: upload} + + filepath := path.Join(opts.TreePath, upload.Name) + lfsLock, err := repo.GetTreePathLock(filepath) + if err != nil { + return err + } + if lfsLock != nil && lfsLock.OwnerID != doer.ID { + return models.ErrLFSFileLocked{RepoID: repo.ID, Path: filepath, UserName: doer.Name} + } } var filename2attribute2info map[string]map[string]string From c394d7f59f1250ba96af1faadd55bf9378857a5c Mon Sep 17 00:00:00 2001 From: David Svantesson Date: Thu, 31 Oct 2019 20:37:37 +0100 Subject: [PATCH 2/5] Better user message if file is locked. --- options/locale/locale_en-US.ini | 1 + routers/repo/editor.go | 6 +++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/options/locale/locale_en-US.ini b/options/locale/locale_en-US.ini index 4d1af69db53f7..794d78c6f7a95 100644 --- a/options/locale/locale_en-US.ini +++ b/options/locale/locale_en-US.ini @@ -742,6 +742,7 @@ editor.no_changes_to_show = There are no changes to show. editor.fail_to_update_file = Failed to update/create file '%s' with error: %v editor.add_subdir = Add a directory… editor.unable_to_upload_files = Failed to upload files to '%s' with error: %v +editor.upload_file_is_locked = File '%s' is locked by %s. editor.upload_files_to_dir = Upload files to '%s' editor.cannot_commit_to_protected_branch = Cannot commit to protected branch '%s'. diff --git a/routers/repo/editor.go b/routers/repo/editor.go index d4a7dab074816..d454350776528 100644 --- a/routers/repo/editor.go +++ b/routers/repo/editor.go @@ -585,7 +585,11 @@ func UploadFilePost(ctx *context.Context, form auth.UploadRepoFileForm) { Files: form.Files, }); err != nil { ctx.Data["Err_TreePath"] = true - ctx.RenderWithErr(ctx.Tr("repo.editor.unable_to_upload_files", form.TreePath, err), tplUploadFile, &form) + if IsErrLFSFileLocked(err) { + ctx.RenderWithErr(ctx.Tr("repo.editor.upload_file_is_locked", err.(models.ErrLFSFileLocked).Path), err.(models.ErrLFSFileLocked).UserName), tplUploadFile, &form) + } else { + ctx.RenderWithErr(ctx.Tr("repo.editor.unable_to_upload_files", form.TreePath, err), tplUploadFile, &form) + } return } From 45e8a7e563ad80bffa7e5c9aa861e158b0201e2f Mon Sep 17 00:00:00 2001 From: David Svantesson Date: Thu, 31 Oct 2019 19:53:59 +0000 Subject: [PATCH 3/5] Check lfs lock before creating temporary repository. fix some errors. --- models/error.go | 2 +- modules/repofiles/upload.go | 27 ++++++++++++++------------- routers/repo/editor.go | 4 ++-- 3 files changed, 17 insertions(+), 16 deletions(-) diff --git a/models/error.go b/models/error.go index 322e73a75fbdf..505df28868580 100644 --- a/models/error.go +++ b/models/error.go @@ -661,7 +661,7 @@ func IsErrLFSFileLocked(err error) bool { } func (err ErrLFSFileLocked) Error() string { - return fmt.Sprintf("File is lfs locked [repo: %d, locked by: %s, path: %s", err.RepoID, err.UserName, err.Path) + return fmt.Sprintf("File is lfs locked [repo: %d, locked by: %s, path: %s]", err.RepoID, err.UserName, err.Path) } // __________ .__ __ diff --git a/modules/repofiles/upload.go b/modules/repofiles/upload.go index e00caf739fc42..9dbe980239616 100644 --- a/modules/repofiles/upload.go +++ b/modules/repofiles/upload.go @@ -55,34 +55,35 @@ func UploadRepoFiles(repo *models.Repository, doer *models.User, opts *UploadRep return fmt.Errorf("GetUploadsByUUIDs [uuids: %v]: %v", opts.Files, err) } - t, err := NewTemporaryUploadRepository(repo) - if err != nil { - return err - } - defer t.Close() - if err := t.Clone(opts.OldBranch); err != nil { - return err - } - if err := t.SetDefaultIndex(); err != nil { - return err - } - names := make([]string, len(uploads)) infos := make([]uploadInfo, len(uploads)) for i, upload := range uploads { names[i] = upload.Name infos[i] = uploadInfo{upload: upload} + // Check file is not lfs locked filepath := path.Join(opts.TreePath, upload.Name) lfsLock, err := repo.GetTreePathLock(filepath) if err != nil { return err } if lfsLock != nil && lfsLock.OwnerID != doer.ID { - return models.ErrLFSFileLocked{RepoID: repo.ID, Path: filepath, UserName: doer.Name} + return models.ErrLFSFileLocked{RepoID: repo.ID, Path: filepath, UserName: lfsLock.Owner.Name} } } + t, err := NewTemporaryUploadRepository(repo) + if err != nil { + return err + } + defer t.Close() + if err := t.Clone(opts.OldBranch); err != nil { + return err + } + if err := t.SetDefaultIndex(); err != nil { + return err + } + var filename2attribute2info map[string]map[string]string if setting.LFS.StartServer { filename2attribute2info, err = t.CheckAttribute("filter", names...) diff --git a/routers/repo/editor.go b/routers/repo/editor.go index d454350776528..763429f8cffc8 100644 --- a/routers/repo/editor.go +++ b/routers/repo/editor.go @@ -585,8 +585,8 @@ func UploadFilePost(ctx *context.Context, form auth.UploadRepoFileForm) { Files: form.Files, }); err != nil { ctx.Data["Err_TreePath"] = true - if IsErrLFSFileLocked(err) { - ctx.RenderWithErr(ctx.Tr("repo.editor.upload_file_is_locked", err.(models.ErrLFSFileLocked).Path), err.(models.ErrLFSFileLocked).UserName), tplUploadFile, &form) + if models.IsErrLFSFileLocked(err) { + ctx.RenderWithErr(ctx.Tr("repo.editor.upload_file_is_locked", err.(models.ErrLFSFileLocked).Path, err.(models.ErrLFSFileLocked).UserName), tplUploadFile, &form) } else { ctx.RenderWithErr(ctx.Tr("repo.editor.unable_to_upload_files", form.TreePath, err), tplUploadFile, &form) } From 00efc085ffe97cd3ae0c759dacdffb57f6fa8091 Mon Sep 17 00:00:00 2001 From: David Svantesson Date: Fri, 1 Nov 2019 06:00:29 +0100 Subject: [PATCH 4/5] move lines --- modules/repofiles/upload.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/modules/repofiles/upload.go b/modules/repofiles/upload.go index 9dbe980239616..7f74bb81e2433 100644 --- a/modules/repofiles/upload.go +++ b/modules/repofiles/upload.go @@ -58,9 +58,6 @@ func UploadRepoFiles(repo *models.Repository, doer *models.User, opts *UploadRep names := make([]string, len(uploads)) infos := make([]uploadInfo, len(uploads)) for i, upload := range uploads { - names[i] = upload.Name - infos[i] = uploadInfo{upload: upload} - // Check file is not lfs locked filepath := path.Join(opts.TreePath, upload.Name) lfsLock, err := repo.GetTreePathLock(filepath) @@ -70,6 +67,9 @@ func UploadRepoFiles(repo *models.Repository, doer *models.User, opts *UploadRep if lfsLock != nil && lfsLock.OwnerID != doer.ID { return models.ErrLFSFileLocked{RepoID: repo.ID, Path: filepath, UserName: lfsLock.Owner.Name} } + + names[i] = upload.Name + infos[i] = uploadInfo{upload: upload} } t, err := NewTemporaryUploadRepository(repo) From 93346e0f6ee3489246ff29a40c67d138c8aa60a0 Mon Sep 17 00:00:00 2001 From: David Svantesson Date: Sat, 2 Nov 2019 07:48:24 +0100 Subject: [PATCH 5/5] Add comment that enabled setting is checked. --- modules/repofiles/upload.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/repofiles/upload.go b/modules/repofiles/upload.go index 7f74bb81e2433..eb1379560dfff 100644 --- a/modules/repofiles/upload.go +++ b/modules/repofiles/upload.go @@ -58,7 +58,7 @@ func UploadRepoFiles(repo *models.Repository, doer *models.User, opts *UploadRep names := make([]string, len(uploads)) infos := make([]uploadInfo, len(uploads)) for i, upload := range uploads { - // Check file is not lfs locked + // Check file is not lfs locked, will return nil if lock setting not enabled filepath := path.Join(opts.TreePath, upload.Name) lfsLock, err := repo.GetTreePathLock(filepath) if err != nil {