Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prevent upload (overwrite) of lfs locked file #8769

Merged
merged 8 commits into from
Nov 2, 2019
17 changes: 17 additions & 0 deletions models/error.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

// __________ .__ __
// \______ \ ____ ______ ____ _____|__|/ |_ ___________ ___.__.
// | _// __ \\____ \ / _ \/ ___/ \ __\/ _ \_ __ < | |
Expand Down
24 changes: 17 additions & 7 deletions modules/repofiles/upload.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,23 @@ func UploadRepoFiles(repo *models.Repository, doer *models.User, opts *UploadRep
return fmt.Errorf("GetUploadsByUUIDs [uuids: %v]: %v", opts.Files, 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)
zeripath marked this conversation as resolved.
Show resolved Hide resolved
lfsLock, err := repo.GetTreePathLock(filepath)
davidsvantesson marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry I missed this earlier - you should only check LFS Locks if setting.LFS.Server. You need to wrap this code in a check.

Copy link
Contributor Author

@davidsvantesson davidsvantesson Nov 1, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's actually in GetTreePathLock, but I am not sure if it is a good design.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah ok just add a comment above this line to say that if LFS is off then this will return nil.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should probably be a code refactoring regarding GetTreePathLock, but I went for the easy way now and only added a code comment.

if err != nil {
return err
}
if lfsLock != nil && lfsLock.OwnerID != doer.ID {
return models.ErrLFSFileLocked{RepoID: repo.ID, Path: filepath, UserName: lfsLock.Owner.Name}
}
}

t, err := NewTemporaryUploadRepository(repo)
if err != nil {
return err
Expand All @@ -67,13 +84,6 @@ func UploadRepoFiles(repo *models.Repository, doer *models.User, opts *UploadRep
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}
}

var filename2attribute2info map[string]map[string]string
if setting.LFS.StartServer {
filename2attribute2info, err = t.CheckAttribute("filter", names...)
Expand Down
1 change: 1 addition & 0 deletions options/locale/locale_en-US.ini
Original file line number Diff line number Diff line change
Expand Up @@ -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'.

Expand Down
6 changes: 5 additions & 1 deletion routers/repo/editor.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 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)
}
return
}

Expand Down