From 78e8f627066d4559038710b417dddaaff5707e24 Mon Sep 17 00:00:00 2001 From: 6543 <6543@obermui.de> Date: Sat, 20 Mar 2021 17:30:29 +0100 Subject: [PATCH] [Refactor] remove possible resource leak (#15067) * move "copy uploaded lfs files 2 repo" to own function for "defer file.Close()" * rm type overload * Update modules/repofiles/upload.go Co-authored-by: zeripath --- modules/repofiles/upload.go | 73 +++++++++++++++++++------------------ 1 file changed, 38 insertions(+), 35 deletions(-) diff --git a/modules/repofiles/upload.go b/modules/repofiles/upload.go index 13c89f55b6..8716e1c8f1 100644 --- a/modules/repofiles/upload.go +++ b/modules/repofiles/upload.go @@ -99,38 +99,8 @@ func UploadRepoFiles(repo *models.Repository, doer *models.User, opts *UploadRep } // Copy uploaded files into repository. - for i, uploadInfo := range infos { - file, err := os.Open(uploadInfo.upload.LocalPath()) - if err != nil { - return err - } - defer file.Close() - - var objectHash string - if setting.LFS.StartServer && filename2attribute2info[uploadInfo.upload.Name] != nil && filename2attribute2info[uploadInfo.upload.Name]["filter"] == "lfs" { - // Handle LFS - // FIXME: Inefficient! this should probably happen in models.Upload - oid, err := models.GenerateLFSOid(file) - if err != nil { - return err - } - fileInfo, err := file.Stat() - if err != nil { - return err - } - - uploadInfo.lfsMetaObject = &models.LFSMetaObject{Oid: oid, Size: fileInfo.Size(), RepositoryID: t.repo.ID} - - if objectHash, err = t.HashObject(strings.NewReader(uploadInfo.lfsMetaObject.Pointer())); err != nil { - return err - } - infos[i] = uploadInfo - } else if objectHash, err = t.HashObject(file); err != nil { - return err - } - - // Add the object to the index - if err := t.AddObjectToIndex("100644", objectHash, path.Join(opts.TreePath, uploadInfo.upload.Name)); err != nil { + for i := range infos { + if err := copyUploadedLFSFileIntoRepository(&infos[i], filename2attribute2info, t, opts.TreePath); err != nil { return err } } @@ -152,11 +122,11 @@ func UploadRepoFiles(repo *models.Repository, doer *models.User, opts *UploadRep } // Now deal with LFS objects - for _, uploadInfo := range infos { - if uploadInfo.lfsMetaObject == nil { + for i := range infos { + if infos[i].lfsMetaObject == nil { continue } - uploadInfo.lfsMetaObject, err = models.NewLFSMetaObject(uploadInfo.lfsMetaObject) + infos[i].lfsMetaObject, err = models.NewLFSMetaObject(infos[i].lfsMetaObject) if err != nil { // OK Now we need to cleanup return cleanUpAfterFailure(&infos, t, err) @@ -182,6 +152,39 @@ func UploadRepoFiles(repo *models.Repository, doer *models.User, opts *UploadRep return models.DeleteUploads(uploads...) } +func copyUploadedLFSFileIntoRepository(info *uploadInfo, filename2attribute2info map[string]map[string]string, t *TemporaryUploadRepository, treePath string) error { + file, err := os.Open(info.upload.LocalPath()) + if err != nil { + return err + } + defer file.Close() + + var objectHash string + if setting.LFS.StartServer && filename2attribute2info[info.upload.Name] != nil && filename2attribute2info[info.upload.Name]["filter"] == "lfs" { + // Handle LFS + // FIXME: Inefficient! this should probably happen in models.Upload + oid, err := models.GenerateLFSOid(file) + if err != nil { + return err + } + fileInfo, err := file.Stat() + if err != nil { + return err + } + + info.lfsMetaObject = &models.LFSMetaObject{Oid: oid, Size: fileInfo.Size(), RepositoryID: t.repo.ID} + + if objectHash, err = t.HashObject(strings.NewReader(info.lfsMetaObject.Pointer())); err != nil { + return err + } + } else if objectHash, err = t.HashObject(file); err != nil { + return err + } + + // Add the object to the index + return t.AddObjectToIndex("100644", objectHash, path.Join(treePath, info.upload.Name)) +} + func uploadToLFSContentStore(info uploadInfo, contentStore *lfs.ContentStore) error { if info.lfsMetaObject == nil { return nil