diff options
author | zeripath <art27@cantab.net> | 2020-08-11 21:05:34 +0100 |
---|---|---|
committer | GitHub <noreply@github.com> | 2020-08-11 21:05:34 +0100 |
commit | 74bd9691c685942798f2761607731697498ceeae (patch) | |
tree | 531d661263b839ccf8aa6af73bfb6710984f0dd9 | |
parent | faa676cc8b4419ac56fbf9d009ea8c6b79834024 (diff) | |
download | gitea-74bd9691c685942798f2761607731697498ceeae.tar.gz gitea-74bd9691c685942798f2761607731697498ceeae.zip |
Re-attempt to delete temporary upload if the file is locked by another process (#12447)
Replace all calls to os.Remove/os.RemoveAll by retrying util.Remove/util.RemoveAll and remove circular dependencies from util.
Fix #12339
Signed-off-by: Andrew Thornton <art27@cantab.net>
Co-authored-by: silverwind <me@silverwind.io>
60 files changed, 304 insertions, 202 deletions
diff --git a/build/generate-gitignores.go b/build/generate-gitignores.go index 0f56ff3a89..f341c1ec48 100644 --- a/build/generate-gitignores.go +++ b/build/generate-gitignores.go @@ -15,6 +15,8 @@ import ( "path" "path/filepath" "strings" + + "code.gitea.io/gitea/modules/util" ) func main() { @@ -33,7 +35,7 @@ func main() { log.Fatalf("Failed to create temp file. %s", err) } - defer os.Remove(file.Name()) + defer util.Remove(file.Name()) resp, err := http.Get(url) diff --git a/build/generate-licenses.go b/build/generate-licenses.go index 15db19e70a..53623e4193 100644 --- a/build/generate-licenses.go +++ b/build/generate-licenses.go @@ -15,6 +15,8 @@ import ( "path" "path/filepath" "strings" + + "code.gitea.io/gitea/modules/util" ) func main() { @@ -33,7 +35,7 @@ func main() { log.Fatalf("Failed to create temp file. %s", err) } - defer os.Remove(file.Name()) + defer util.Remove(file.Name()) resp, err := http.Get(url) diff --git a/cmd/doctor.go b/cmd/doctor.go index 48fd3158a4..20c1904afa 100644 --- a/cmd/doctor.go +++ b/cmd/doctor.go @@ -24,6 +24,7 @@ import ( "code.gitea.io/gitea/modules/options" "code.gitea.io/gitea/modules/repository" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" "xorm.io/builder" "github.com/urfave/cli" @@ -312,7 +313,7 @@ func runDoctorWritableDir(path string) error { if err != nil { return err } - if err := os.Remove(tmpFile.Name()); err != nil { + if err := util.Remove(tmpFile.Name()); err != nil { fmt.Printf("Warning: can't remove temporary file: '%s'\n", tmpFile.Name()) } tmpFile.Close() diff --git a/cmd/dump.go b/cmd/dump.go index dd259575fa..c647341221 100644 --- a/cmd/dump.go +++ b/cmd/dump.go @@ -18,6 +18,7 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" "gitea.com/macaron/session" archiver "github.com/mholt/archiver/v3" @@ -243,7 +244,11 @@ func runDump(ctx *cli.Context) error { if err != nil { fatal("Failed to create tmp file: %v", err) } - defer os.Remove(dbDump.Name()) + defer func() { + if err := util.Remove(dbDump.Name()); err != nil { + log.Warn("Unable to remove temporary file: %s: Error: %v", dbDump.Name(), err) + } + }() targetDBType := ctx.String("database") if len(targetDBType) > 0 && targetDBType != setting.Database.Type { @@ -313,7 +318,7 @@ func runDump(ctx *cli.Context) error { if fileName != "-" { if err = w.Close(); err != nil { - _ = os.Remove(fileName) + _ = util.Remove(fileName) fatal("Failed to save %s: %v", fileName, err) } diff --git a/contrib/pr/checkout.go b/contrib/pr/checkout.go index b4cf80ed4c..8fe1aa4206 100644 --- a/contrib/pr/checkout.go +++ b/contrib/pr/checkout.go @@ -29,6 +29,7 @@ import ( "code.gitea.io/gitea/modules/markup" "code.gitea.io/gitea/modules/markup/external" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/routers" "code.gitea.io/gitea/routers/routes" @@ -108,8 +109,8 @@ func runPR() { os.Exit(1) } models.LoadFixtures() - os.RemoveAll(setting.RepoRootPath) - os.RemoveAll(models.LocalCopyPath()) + util.RemoveAll(setting.RepoRootPath) + util.RemoveAll(models.LocalCopyPath()) com.CopyDir(path.Join(curDir, "integrations/gitea-repositories-meta"), setting.RepoRootPath) log.Printf("[PR] Setting up router\n") @@ -141,20 +142,20 @@ func runPR() { log.Printf("[PR] Cleaning up ...\n") /* - if err = os.RemoveAll(setting.Indexer.IssuePath); err != nil { - fmt.Printf("os.RemoveAll: %v\n", err) + if err = util.RemoveAll(setting.Indexer.IssuePath); err != nil { + fmt.Printf("util.RemoveAll: %v\n", err) os.Exit(1) } - if err = os.RemoveAll(setting.Indexer.RepoPath); err != nil { + if err = util.RemoveAll(setting.Indexer.RepoPath); err != nil { fmt.Printf("Unable to remove repo indexer: %v\n", err) os.Exit(1) } */ - if err = os.RemoveAll(setting.RepoRootPath); err != nil { - log.Fatalf("os.RemoveAll: %v\n", err) + if err = util.RemoveAll(setting.RepoRootPath); err != nil { + log.Fatalf("util.RemoveAll: %v\n", err) } - if err = os.RemoveAll(setting.AppDataPath); err != nil { - log.Fatalf("os.RemoveAll: %v\n", err) + if err = util.RemoveAll(setting.AppDataPath); err != nil { + log.Fatalf("util.RemoveAll: %v\n", err) } } diff --git a/integrations/api_repo_test.go b/integrations/api_repo_test.go index f0f43ac32c..0d0d4a117b 100644 --- a/integrations/api_repo_test.go +++ b/integrations/api_repo_test.go @@ -9,12 +9,12 @@ import ( "io/ioutil" "net/http" "net/url" - "os" "testing" "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/modules/util" "github.com/stretchr/testify/assert" ) @@ -341,7 +341,7 @@ func testAPIRepoMigrateConflict(t *testing.T, u *url.URL) { httpContext.Reponame = "repo-tmp-17" dstPath, err := ioutil.TempDir("", httpContext.Reponame) assert.NoError(t, err) - defer os.RemoveAll(dstPath) + defer util.RemoveAll(dstPath) t.Run("CreateRepo", doAPICreateRepository(httpContext, false)) user, err := models.GetUserByName(httpContext.Username) @@ -404,7 +404,7 @@ func testAPIRepoCreateConflict(t *testing.T, u *url.URL) { httpContext.Reponame = "repo-tmp-17" dstPath, err := ioutil.TempDir("", httpContext.Reponame) assert.NoError(t, err) - defer os.RemoveAll(dstPath) + defer util.RemoveAll(dstPath) t.Run("CreateRepo", doAPICreateRepository(httpContext, false)) req := NewRequestWithJSON(t, "POST", "/api/v1/user/repos?token="+httpContext.Token, diff --git a/integrations/create_no_session_test.go b/integrations/create_no_session_test.go index 8d5b6f2f7c..5380d17509 100644 --- a/integrations/create_no_session_test.go +++ b/integrations/create_no_session_test.go @@ -14,6 +14,7 @@ import ( "testing" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/routers/routes" "gitea.com/macaron/session" @@ -72,7 +73,7 @@ func TestSessionFileCreation(t *testing.T) { assert.NoError(t, err) defer func() { if _, err := os.Stat(tmpDir); !os.IsNotExist(err) { - _ = os.RemoveAll(tmpDir) + _ = util.RemoveAll(tmpDir) } }() config.ProviderConfig = tmpDir diff --git a/integrations/git_helper_for_declarative_test.go b/integrations/git_helper_for_declarative_test.go index 13b3e92c14..5823ce38dc 100644 --- a/integrations/git_helper_for_declarative_test.go +++ b/integrations/git_helper_for_declarative_test.go @@ -21,6 +21,7 @@ import ( "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/ssh" + "code.gitea.io/gitea/modules/util" "github.com/stretchr/testify/assert" "github.com/unknwon/com" @@ -30,7 +31,7 @@ func withKeyFile(t *testing.T, keyname string, callback func(string)) { tmpDir, err := ioutil.TempDir("", "key-file") assert.NoError(t, err) - defer os.RemoveAll(tmpDir) + defer util.RemoveAll(tmpDir) err = os.Chmod(tmpDir, 0700) assert.NoError(t, err) @@ -119,7 +120,7 @@ func doGitCloneFail(u *url.URL) func(*testing.T) { return func(t *testing.T) { tmpDir, err := ioutil.TempDir("", "doGitCloneFail") assert.NoError(t, err) - defer os.RemoveAll(tmpDir) + defer util.RemoveAll(tmpDir) assert.Error(t, git.Clone(u.String(), tmpDir, git.CloneRepoOptions{})) assert.False(t, com.IsExist(filepath.Join(tmpDir, "README.md"))) } diff --git a/integrations/git_test.go b/integrations/git_test.go index 7dd0bc056f..c3c1126829 100644 --- a/integrations/git_test.go +++ b/integrations/git_test.go @@ -10,7 +10,6 @@ import ( "math/rand" "net/http" "net/url" - "os" "path" "path/filepath" "strconv" @@ -21,6 +20,7 @@ import ( "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/modules/util" "github.com/stretchr/testify/assert" ) @@ -51,7 +51,7 @@ func testGit(t *testing.T, u *url.URL) { dstPath, err := ioutil.TempDir("", httpContext.Reponame) assert.NoError(t, err) - defer os.RemoveAll(dstPath) + defer util.RemoveAll(dstPath) t.Run("CreateRepoInDifferentUser", doAPICreateRepository(forkedUserCtx, false)) t.Run("AddUserAsCollaborator", doAPIAddCollaborator(forkedUserCtx, httpContext.Username, models.AccessModeRead)) @@ -99,7 +99,7 @@ func testGit(t *testing.T, u *url.URL) { //Setup clone folder dstPath, err := ioutil.TempDir("", sshContext.Reponame) assert.NoError(t, err) - defer os.RemoveAll(dstPath) + defer util.RemoveAll(dstPath) t.Run("Clone", doGitClone(dstPath, sshURL)) @@ -124,7 +124,7 @@ func testGit(t *testing.T, u *url.URL) { func ensureAnonymousClone(t *testing.T, u *url.URL) { dstLocalPath, err := ioutil.TempDir("", "repo1") assert.NoError(t, err) - defer os.RemoveAll(dstLocalPath) + defer util.RemoveAll(dstLocalPath) t.Run("CloneAnonymous", doGitClone(dstLocalPath, u)) } @@ -498,7 +498,7 @@ func doPushCreate(ctx APITestContext, u *url.URL) func(t *testing.T) { // Create a temporary directory tmpDir, err := ioutil.TempDir("", ctx.Reponame) assert.NoError(t, err) - defer os.RemoveAll(tmpDir) + defer util.RemoveAll(tmpDir) // Now create local repository to push as our test and set its origin t.Run("InitTestRepository", doGitInitTestRepository(tmpDir)) diff --git a/integrations/gpg_git_test.go b/integrations/gpg_git_test.go index e01f0f31c1..8c78faf7d2 100644 --- a/integrations/gpg_git_test.go +++ b/integrations/gpg_git_test.go @@ -16,6 +16,7 @@ import ( "code.gitea.io/gitea/modules/process" "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/modules/util" "github.com/stretchr/testify/assert" "golang.org/x/crypto/openpgp" "golang.org/x/crypto/openpgp/armor" @@ -28,7 +29,7 @@ func TestGPGGit(t *testing.T) { // OK Set a new GPG home tmpDir, err := ioutil.TempDir("", "temp-gpg") assert.NoError(t, err) - defer os.RemoveAll(tmpDir) + defer util.RemoveAll(tmpDir) err = os.Chmod(tmpDir, 0700) assert.NoError(t, err) diff --git a/integrations/integration_test.go b/integrations/integration_test.go index e99264beb8..36e0682099 100644 --- a/integrations/integration_test.go +++ b/integrations/integration_test.go @@ -29,6 +29,7 @@ import ( "code.gitea.io/gitea/modules/graceful" "code.gitea.io/gitea/modules/queue" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" "code.gitea.io/gitea/routers" "code.gitea.io/gitea/routers/routes" @@ -98,11 +99,11 @@ func TestMain(m *testing.M) { writerCloser.t = nil - if err = os.RemoveAll(setting.Indexer.IssuePath); err != nil { - fmt.Printf("os.RemoveAll: %v\n", err) + if err = util.RemoveAll(setting.Indexer.IssuePath); err != nil { + fmt.Printf("util.RemoveAll: %v\n", err) os.Exit(1) } - if err = os.RemoveAll(setting.Indexer.RepoPath); err != nil { + if err = util.RemoveAll(setting.Indexer.RepoPath); err != nil { fmt.Printf("Unable to remove repo indexer: %v\n", err) os.Exit(1) } @@ -138,7 +139,7 @@ func initIntegrationTest() { setting.SetCustomPathAndConf("", "", "") setting.NewContext() - os.RemoveAll(models.LocalCopyPath()) + util.RemoveAll(models.LocalCopyPath()) setting.CheckLFSVersion() setting.InitDBConfig() @@ -230,7 +231,7 @@ func prepareTestEnv(t testing.TB, skip ...int) func() { } deferFn := PrintCurrentTest(t, ourSkip) assert.NoError(t, models.LoadFixtures()) - assert.NoError(t, os.RemoveAll(setting.RepoRootPath)) + assert.NoError(t, util.RemoveAll(setting.RepoRootPath)) assert.NoError(t, com.CopyDir(path.Join(filepath.Dir(setting.AppPath), "integrations/gitea-repositories-meta"), setting.RepoRootPath)) @@ -473,7 +474,7 @@ func GetCSRF(t testing.TB, session *TestSession, urlStr string) string { func resetFixtures(t *testing.T) { assert.NoError(t, queue.GetManager().FlushAll(context.Background(), -1)) assert.NoError(t, models.LoadFixtures()) - assert.NoError(t, os.RemoveAll(setting.RepoRootPath)) + assert.NoError(t, util.RemoveAll(setting.RepoRootPath)) assert.NoError(t, com.CopyDir(path.Join(filepath.Dir(setting.AppPath), "integrations/gitea-repositories-meta"), setting.RepoRootPath)) } diff --git a/integrations/migration-test/migration_test.go b/integrations/migration-test/migration_test.go index 4ee045db38..976a59a579 100644 --- a/integrations/migration-test/migration_test.go +++ b/integrations/migration-test/migration_test.go @@ -24,6 +24,7 @@ import ( "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/charset" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" "github.com/stretchr/testify/assert" "github.com/unknwon/com" @@ -58,7 +59,7 @@ func initMigrationTest(t *testing.T) func() { setting.NewContext() assert.True(t, len(setting.RepoRootPath) != 0) - assert.NoError(t, os.RemoveAll(setting.RepoRootPath)) + assert.NoError(t, util.RemoveAll(setting.RepoRootPath)) assert.NoError(t, com.CopyDir(path.Join(filepath.Dir(setting.AppPath), "integrations/gitea-repositories-meta"), setting.RepoRootPath)) setting.CheckLFSVersion() @@ -129,7 +130,7 @@ func restoreOldDB(t *testing.T, version string) bool { switch { case setting.Database.UseSQLite3: - os.Remove(setting.Database.Path) + util.Remove(setting.Database.Path) err := os.MkdirAll(path.Dir(setting.Database.Path), os.ModePerm) assert.NoError(t, err) diff --git a/integrations/ssh_key_test.go b/integrations/ssh_key_test.go index d445c7f9e5..91774f6335 100644 --- a/integrations/ssh_key_test.go +++ b/integrations/ssh_key_test.go @@ -9,13 +9,13 @@ import ( "io/ioutil" "net/http" "net/url" - "os" "path/filepath" "testing" "time" "code.gitea.io/gitea/modules/git" api "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/modules/util" "github.com/stretchr/testify/assert" ) @@ -63,7 +63,7 @@ func testPushDeployKeyOnEmptyRepo(t *testing.T, u *url.URL) { // Setup the testing repository dstPath, err := ioutil.TempDir("", "repo-tmp-deploy-key-empty-repo-1") assert.NoError(t, err) - defer os.RemoveAll(dstPath) + defer util.RemoveAll(dstPath) t.Run("InitTestRepository", doGitInitTestRepository(dstPath)) @@ -109,7 +109,7 @@ func testKeyOnlyOneType(t *testing.T, u *url.URL) { t.Run("KeyCanOnlyBeUser", func(t *testing.T) { dstPath, err := ioutil.TempDir("", ctx.Reponame) assert.NoError(t, err) - defer os.RemoveAll(dstPath) + defer util.RemoveAll(dstPath) sshURL := createSSHUrl(ctx.GitPath(), u) @@ -135,7 +135,7 @@ func testKeyOnlyOneType(t *testing.T, u *url.URL) { t.Run("KeyCanBeAnyDeployButNotUserAswell", func(t *testing.T) { dstPath, err := ioutil.TempDir("", ctx.Reponame) assert.NoError(t, err) - defer os.RemoveAll(dstPath) + defer util.RemoveAll(dstPath) sshURL := createSSHUrl(ctx.GitPath(), u) @@ -153,7 +153,7 @@ func testKeyOnlyOneType(t *testing.T, u *url.URL) { otherSSHURL := createSSHUrl(otherCtx.GitPath(), u) dstOtherPath, err := ioutil.TempDir("", otherCtx.Reponame) assert.NoError(t, err) - defer os.RemoveAll(dstOtherPath) + defer util.RemoveAll(dstOtherPath) t.Run("AddWriterDeployKeyToOther", doAPICreateDeployKey(otherCtx, keyname, keyFile, false)) @@ -170,7 +170,7 @@ func testKeyOnlyOneType(t *testing.T, u *url.URL) { otherSSHURL := createSSHUrl(otherCtx.GitPath(), u) dstOtherPath, err := ioutil.TempDir("", otherCtx.Reponame) assert.NoError(t, err) - defer os.RemoveAll(dstOtherPath) + defer util.RemoveAll(dstOtherPath) t.Run("DeleteRepository", doAPIDeleteRepository(ctx)) @@ -192,7 +192,7 @@ func testKeyOnlyOneType(t *testing.T, u *url.URL) { dstPath, err := ioutil.TempDir("", ctx.Reponame) assert.NoError(t, err) - defer os.RemoveAll(dstPath) + defer util.RemoveAll(dstPath) sshURL := createSSHUrl(ctx.GitPath(), u) diff --git a/models/admin.go b/models/admin.go index 01f0ba3486..ebd01419a8 100644 --- a/models/admin.go +++ b/models/admin.go @@ -6,10 +6,10 @@ package models import ( "fmt" - "os" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/timeutil" + "code.gitea.io/gitea/modules/util" "github.com/unknwon/com" ) @@ -66,7 +66,7 @@ func RemoveAllWithNotice(title, path string) { } func removeAllWithNotice(e Engine, title, path string) { - if err := os.RemoveAll(path); err != nil { + if err := util.RemoveAll(path); err != nil { desc := fmt.Sprintf("%s [%s]: %v", title, path, err) log.Warn(title+" [%s]: %v", path, err) if err = createNotice(e, NoticeRepository, desc); err != nil { diff --git a/models/attachment.go b/models/attachment.go index 6c3996ff01..6215de95b5 100644 --- a/models/attachment.go +++ b/models/attachment.go @@ -13,6 +13,7 @@ import ( "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/timeutil" + "code.gitea.io/gitea/modules/util" gouuid "github.com/google/uuid" "xorm.io/xorm" @@ -237,7 +238,7 @@ func DeleteAttachments(attachments []*Attachment, remove bool) (int, error) { if remove { for i, a := range attachments { - if err := os.Remove(a.LocalPath()); err != nil { + if err := util.Remove(a.LocalPath()); err != nil { return i, err } } diff --git a/models/helper_directory.go b/models/helper_directory.go index 8119ec91a3..aed2dbcf9b 100644 --- a/models/helper_directory.go +++ b/models/helper_directory.go @@ -13,6 +13,7 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" ) // LocalCopyPath returns the local repository temporary copy path. @@ -41,7 +42,7 @@ func CreateTemporaryPath(prefix string) (string, error) { // RemoveTemporaryPath removes the temporary path func RemoveTemporaryPath(basePath string) error { if _, err := os.Stat(basePath); !os.IsNotExist(err) { - return os.RemoveAll(basePath) + return util.RemoveAll(basePath) } return nil } diff --git a/models/migrations/v112.go b/models/migrations/v112.go index 76d9933689..7e80037700 100644 --- a/models/migrations/v112.go +++ b/models/migrations/v112.go @@ -6,10 +6,10 @@ package migrations import ( "fmt" - "os" "path" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" "xorm.io/builder" "xorm.io/xorm" @@ -31,7 +31,7 @@ func removeAttachmentMissedRepo(x *xorm.Engine) error { for i := 0; i < len(attachments); i++ { uuid := attachments[i].UUID - if err = os.RemoveAll(path.Join(setting.AttachmentPath, uuid[0:1], uuid[1:2], uuid)); err != nil { + if err = util.RemoveAll(path.Join(setting.AttachmentPath, uuid[0:1], uuid[1:2], uuid)); err != nil { fmt.Printf("Error: %v", err) } } diff --git a/models/migrations/v115.go b/models/migrations/v115.go index 9b5c70a7ac..fe3b086119 100644 --- a/models/migrations/v115.go +++ b/models/migrations/v115.go @@ -15,6 +15,7 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" "xorm.io/xorm" ) @@ -110,8 +111,8 @@ func renameExistingUserAvatarName(x *xorm.Engine) error { log.Info("Deleting %d old avatars ...", deleteCount) i := 0 for file := range deleteList { - if err := os.Remove(file); err != nil { - log.Warn("os.Remove: %v", err) + if err := util.Remove(file); err != nil { + log.Warn("util.Remove: %v", err) } i++ select { diff --git a/models/migrations/v96.go b/models/migrations/v96.go index 9840248f61..5decb832cd 100644 --- a/models/migrations/v96.go +++ b/models/migrations/v96.go @@ -5,10 +5,10 @@ package migrations import ( - "os" "path" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" "xorm.io/xorm" ) @@ -58,7 +58,7 @@ func deleteOrphanedAttachments(x *xorm.Engine) error { } for _, attachment := range attachements { - if err := os.RemoveAll(AttachmentLocalPath(attachment.UUID)); err != nil { + if err := util.RemoveAll(AttachmentLocalPath(attachment.UUID)); err != nil { return err } } diff --git a/models/org.go b/models/org.go index 9fa213e17e..0915e7fd56 100644 --- a/models/org.go +++ b/models/org.go @@ -7,12 +7,12 @@ package models import ( "fmt" - "os" "strings" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/structs" + "code.gitea.io/gitea/modules/util" "github.com/unknwon/com" "xorm.io/builder" @@ -305,14 +305,14 @@ func deleteOrg(e *xorm.Session, u *User) error { // so just keep error logs of those operations. path := UserPath(u.Name) - if err := os.RemoveAll(path); err != nil { + if err := util.RemoveAll(path); err != nil { return fmt.Errorf("Failed to RemoveAll %s: %v", path, err) } if len(u.Avatar) > 0 { avatarPath := u.CustomAvatarPath() if com.IsExist(avatarPath) { - if err := os.Remove(avatarPath); err != nil { + if err := util.Remove(avatarPath); err != nil { return fmt.Errorf("Failed to remove %s: %v", avatarPath, err) } } diff --git a/models/repo.go b/models/repo.go index b2b6e1a26f..9f7ce8af1e 100644 --- a/models/repo.go +++ b/models/repo.go @@ -1440,7 +1440,7 @@ func updateRepository(e Engine, repo *Repository, visibilityChanged bool) (err e // Create/Remove git-daemon-export-ok for git-daemon... daemonExportFile := path.Join(repo.RepoPath(), `git-daemon-export-ok`) if repo.IsPrivate && com.IsExist(daemonExportFile) { - if err = os.Remove(daemonExportFile); err != nil { + if err = util.Remove(daemonExportFile); err != nil { log.Error("Failed to remove %s: %v", daemonExportFile, err) } } else if !repo.IsPrivate && !com.IsExist(daemonExportFile) { @@ -1708,7 +1708,7 @@ func DeleteRepository(doer *User, uid, repoID int64) error { if len(repo.Avatar) > 0 { avatarPath := repo.CustomAvatarPath() if com.IsExist(avatarPath) { - if err := os.Remove(avatarPath); err != nil { + if err := util.Remove(avatarPath); err != nil { return fmt.Errorf("Failed to remove %s: %v", avatarPath, err) } } @@ -1852,7 +1852,7 @@ func DeleteRepositoryArchives(ctx context.Context) error { return ErrCancelledf("before deleting repository archives for %s", repo.FullName()) default: } - return os.RemoveAll(filepath.Join(repo.RepoPath(), "archives")) + return util.RemoveAll(filepath.Join(repo.RepoPath(), "archives")) }) } @@ -1911,7 +1911,7 @@ func deleteOldRepositoryArchives(ctx context.Context, olderThan time.Duration, i } toDelete := filepath.Join(path, info.Name()) // This is a best-effort purge, so we do not check error codes to confirm removal. - if err = os.Remove(toDelete); err != nil { + if err = util.Remove(toDelete); err != nil { log.Trace("Unable to delete %s, but proceeding: %v", toDelete, err) } } @@ -2280,7 +2280,7 @@ func (repo *Repository) UploadAvatar(data []byte) error { } if len(oldAvatarPath) > 0 && oldAvatarPath != repo.CustomAvatarPath() { - if err := os.Remove(oldAvatarPath); err != nil { + if err := util.Remove(oldAvatarPath); err != nil { return fmt.Errorf("UploadAvatar: Failed to remove old repo avatar %s: %v", oldAvatarPath, err) } } @@ -2311,7 +2311,7 @@ func (repo *Repository) DeleteAvatar() error { } if _, err := os.Stat(avatarPath); err == nil { - if err := os.Remove(avatarPath); err != nil { + if err := util.Remove(avatarPath); err != nil { return fmt.Errorf("DeleteAvatar: Failed to remove %s: %v", avatarPath, err) } } else { diff --git a/models/ssh_key.go b/models/ssh_key.go index 64b859fa02..753ad57934 100644 --- a/models/ssh_key.go +++ b/models/ssh_key.go @@ -28,6 +28,7 @@ import ( "code.gitea.io/gitea/modules/process" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" + "code.gitea.io/gitea/modules/util" "github.com/unknwon/com" "golang.org/x/crypto/ssh" @@ -227,7 +228,11 @@ func SSHKeyGenParsePublicKey(key string) (string, int, error) { if err != nil { return "", 0, fmt.Errorf("writeTmpKeyFile: %v", err) } - defer os.Remove(tmpName) + defer func() { + if err := util.Remove(tmpName); err != nil { + log.Warn("Unable to remove temporary key file: %s: Error: %v", tmpName, err) + } + }() stdout, stderr, err := process.GetManager().Exec("SSHKeyGenParsePublicKey", setting.SSH.KeygenPath, "-lf", tmpName) if err != nil { @@ -423,7 +428,11 @@ func calcFingerprintSSHKeygen(publicKeyContent string) (string, error) { if err != nil { return "", err } - defer os.Remove(tmpPath) + defer func() { + if err := util.Remove(tmpPath); err != nil { + log.Warn("Unable to remove temporary key file: %s: Error: %v", tmpPath, err) + } + }() stdout, stderr, err := process.GetManager().Exec("AddPublicKey", "ssh-keygen", "-lf", tmpPath) if err != nil { if strings.Contains(stderr, "is not a public key file") { @@ -692,7 +701,9 @@ func rewriteAllPublicKeys(e Engine) error { } defer func() { t.Close() - os.Remove(tmpPath) + if err := util.Remove(tmpPath); err != nil { + log.Warn("Unable to remove temporary authorized keys file: %s: Error: %v", tmpPath, err) + } }() if setting.SSH.AuthorizedKeysBackup && com.IsExist(fPath) { diff --git a/models/unit_tests.go b/models/unit_tests.go index 9266b3a658..dca363ffac 100644 --- a/models/unit_tests.go +++ b/models/unit_tests.go @@ -12,10 +12,10 @@ import ( "os" "path/filepath" "testing" - "time" "code.gitea.io/gitea/modules/base" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" "github.com/stretchr/testify/assert" "github.com/unknwon/com" @@ -67,19 +67,19 @@ func MainTest(m *testing.M, pathToGiteaRoot string) { fatalTestError("url.Parse: %v\n", err) } - if err = removeAllWithRetry(setting.RepoRootPath); err != nil { - fatalTestError("os.RemoveAll: %v\n", err) + if err = util.RemoveAll(setting.RepoRootPath); err != nil { + fatalTestError("util.RemoveAll: %v\n", err) } if err = com.CopyDir(filepath.Join(pathToGiteaRoot, "integrations", "gitea-repositories-meta"), setting.RepoRootPath); err != nil { fatalTestError("com.CopyDir: %v\n", err) } exitStatus := m.Run() - if err = removeAllWithRetry(setting.RepoRootPath); err != nil { - fatalTestError("os.RemoveAll: %v\n", err) + if err = util.RemoveAll(setting.RepoRootPath); err != nil { + fatalTestError("util.RemoveAll: %v\n", err) } - if err = removeAllWithRetry(setting.AppDataPath); err != nil { - fatalTestError("os.RemoveAll: %v\n", err) + if err = util.RemoveAll(setting.AppDataPath); err != nil { + fatalTestError("util.RemoveAll: %v\n", err) } os.Exit(exitStatus) } @@ -103,18 +103,6 @@ func CreateTestEngine(fixturesDir string) error { return InitFixtures(fixturesDir) } -func removeAllWithRetry(dir string) error { - var err error - for i := 0; i < 20; i++ { - err = os.RemoveAll(dir) - if err == nil { - break - } - time.Sleep(100 * time.Millisecond) - } - return err -} - // PrepareTestDatabase load test fixtures into test database func PrepareTestDatabase() error { return LoadFixtures() @@ -124,7 +112,7 @@ func PrepareTestDatabase() error { // by tests that use the above MainTest(..) function. func PrepareTestEnv(t testing.TB) { assert.NoError(t, PrepareTestDatabase()) - assert.NoError(t, removeAllWithRetry(setting.RepoRootPath)) + assert.NoError(t, util.RemoveAll(setting.RepoRootPath)) metaPath := filepath.Join(giteaRoot, "integrations", "gitea-repositories-meta") assert.NoError(t, com.CopyDir(metaPath, setting.RepoRootPath)) base.SetupGiteaRoot() // Makes sure GITEA_ROOT is set diff --git a/models/upload.go b/models/upload.go index 21c73a43f7..3ad1129c25 100644 --- a/models/upload.go +++ b/models/upload.go @@ -13,6 +13,7 @@ import ( "path" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" gouuid "github.com/google/uuid" "github.com/unknwon/com" @@ -129,7 +130,7 @@ func DeleteUploads(uploads ...*Upload) (err error) { continue } - if err := os.Remove(localPath); err != nil { + if err := util.Remove(localPath); err != nil { return fmt.Errorf("remove upload: %v", err) } } diff --git a/models/user.go b/models/user.go index 608bf219cf..6bd9d18b07 100644 --- a/models/user.go +++ b/models/user.go @@ -582,7 +582,7 @@ func (u *User) UploadAvatar(data []byte) error { func (u *User) DeleteAvatar() error { log.Trace("DeleteAvatar[%d]: %s", u.ID, u.CustomAvatarPath()) if len(u.Avatar) > 0 { - if err := os.Remove(u.CustomAvatarPath()); err != nil { + if err := util.Remove(u.CustomAvatarPath()); err != nil { return fmt.Errorf("Failed to remove %s: %v", u.CustomAvatarPath(), err) } } @@ -1282,14 +1282,14 @@ func deleteUser(e *xorm.Session, u *User) error { // so just keep error logs of those operations. path := UserPath(u.Name) - if err := os.RemoveAll(path); err != nil { + if err := util.RemoveAll(path); err != nil { return fmt.Errorf("Failed to RemoveAll %s: %v", path, err) } if len(u.Avatar) > 0 { avatarPath := u.CustomAvatarPath() if com.IsExist(avatarPath) { - if err := os.Remove(avatarPath); err != nil { + if err := util.Remove(avatarPath); err != nil { return fmt.Errorf("Failed to remove %s: %v", avatarPath, err) } } diff --git a/modules/git/commit_info_test.go b/modules/git/commit_info_test.go index ef40e4a3ab..8bdf1a769b 100644 --- a/modules/git/commit_info_test.go +++ b/modules/git/commit_info_test.go @@ -10,6 +10,7 @@ import ( "testing" "time" + "code.gitea.io/gitea/modules/util" "github.com/stretchr/testify/assert" ) @@ -87,7 +88,7 @@ func TestEntries_GetCommitsInfo(t *testing.T) { clonedPath, err := cloneRepo(bareRepo1Path, testReposDir, "repo1_TestEntries_GetCommitsInfo") assert.NoError(t, err) - defer os.RemoveAll(clonedPath) + defer util.RemoveAll(clonedPath) clonedRepo1, err := OpenRepository(clonedPath) assert.NoError(t, err) defer clonedRepo1.Close() diff --git a/modules/git/hook.go b/modules/git/hook.go index 245c6b92ce..7b75405901 100644 --- a/modules/git/hook.go +++ b/modules/git/hook.go @@ -12,6 +12,7 @@ import ( "path/filepath" "strings" + "code.gitea.io/gitea/modules/util" "github.com/unknwon/com" ) @@ -82,7 +83,7 @@ func (h *Hook) Name() string { func (h *Hook) Update() error { if len(strings.TrimSpace(h.Content)) == 0 { if isExist(h.path) { - err := os.Remove(h.path) + err := util.Remove(h.path) if err != nil { return err } @@ -129,7 +130,7 @@ func SetUpdateHook(repoPath, content string) (err error) { log("Setting update hook: %s", repoPath) hookPath := path.Join(repoPath, HookPathUpdate) if com.IsExist(hookPath) { - err = os.Remove(hookPath) + err = util.Remove(hookPath) } else { err = os.MkdirAll(path.Dir(hookPath), os.ModePerm) } diff --git a/modules/git/repo_compare_test.go b/modules/git/repo_compare_test.go index bf4631d853..f0e20838f4 100644 --- a/modules/git/repo_compare_test.go +++ b/modules/git/repo_compare_test.go @@ -7,10 +7,10 @@ package git import ( "bytes" "io/ioutil" - "os" "path/filepath" "testing" + "code.gitea.io/gitea/modules/util" "github.com/stretchr/testify/assert" ) @@ -18,7 +18,7 @@ func TestGetFormatPatch(t *testing.T) { bareRepo1Path := filepath.Join(testReposDir, "repo1_bare") clonedPath, err := cloneRepo(bareRepo1Path, testReposDir, "repo1_TestGetFormatPatch") assert.NoError(t, err) - defer os.RemoveAll(clonedPath) + defer util.RemoveAll(clonedPath) repo, err := OpenRepository(clonedPath) assert.NoError(t, err) defer repo.Close() diff --git a/modules/git/repo_tag_test.go b/modules/git/repo_tag_test.go index 7a644e39db..1dee29ba57 100644 --- a/modules/git/repo_tag_test.go +++ b/modules/git/repo_tag_test.go @@ -5,10 +5,10 @@ package git import ( - "os" "path/filepath" "testing" + "code.gitea.io/gitea/modules/util" "github.com/stretchr/testify/assert" ) @@ -31,7 +31,7 @@ func TestRepository_GetTag(t *testing.T) { clonedPath, err := cloneRepo(bareRepo1Path, testReposDir, "repo1_TestRepository_GetTag") assert.NoError(t, err) - defer os.RemoveAll(clonedPath) + defer util.RemoveAll(clonedPath) bareRepo1, err := OpenRepository(clonedPath) assert.NoError(t, err) @@ -81,7 +81,7 @@ func TestRepository_GetAnnotatedTag(t *testing.T) { clonedPath, err := cloneRepo(bareRepo1Path, testReposDir, "repo1_TestRepository_GetTag") assert.NoError(t, err) - defer os.RemoveAll(clonedPath) + defer util.RemoveAll(clonedPath) bareRepo1, err := OpenRepository(clonedPath) assert.NoError(t, err) diff --git a/modules/graceful/net_unix.go b/modules/graceful/net_unix.go index 1e496e9d91..2dc714955e 100644 --- a/modules/graceful/net_unix.go +++ b/modules/graceful/net_unix.go @@ -17,6 +17,7 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" ) const ( @@ -173,7 +174,7 @@ func GetListenerUnix(network string, address *net.UnixAddr) (*net.UnixListener, } // make a fresh listener - if err := os.Remove(address.Name); err != nil && !os.IsNotExist(err) { + if err := util.Remove(address.Name); err != nil && !os.IsNotExist(err) { return nil, fmt.Errorf("Failed to remove unix socket %s: %v", address.Name, err) } diff --git a/modules/indexer/code/bleve.go b/modules/indexer/code/bleve.go index 576cc3e5e4..6502259ba4 100644 --- a/modules/indexer/code/bleve.go +++ b/modules/indexer/code/bleve.go @@ -19,6 +19,7 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" + "code.gitea.io/gitea/modules/util" "github.com/blevesearch/bleve" analyzer_custom "github.com/blevesearch/bleve/analysis/analyzer/custom" @@ -75,14 +76,14 @@ func openIndexer(path string, latestVersion int) (bleve.Index, error) { if metadata.Version < latestVersion { // the indexer is using a previous version, so we should delete it and // re-populate - return nil, os.RemoveAll(path) + return nil, util.RemoveAll(path) } index, err := bleve.Open(path) if err != nil && err == upsidedown.IncompatibleVersion { // the indexer was built with a previous version of bleve, so we should // delete it and re-populate - return nil, os.RemoveAll(path) + return nil, util.RemoveAll(path) } else if err != nil { return nil, err } diff --git a/modules/indexer/code/bleve_test.go b/modules/indexer/code/bleve_test.go index 89cfceea2d..2b3128ac88 100644 --- a/modules/indexer/code/bleve_test.go +++ b/modules/indexer/code/bleve_test.go @@ -6,12 +6,12 @@ package code import ( "io/ioutil" - "os" "path/filepath" "testing" "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" "github.com/stretchr/testify/assert" ) @@ -29,7 +29,7 @@ func TestIndexAndSearch(t *testing.T) { assert.Fail(t, "Unable to create temporary directory") return } - defer os.RemoveAll(dir) + defer util.RemoveAll(dir) setting.Indexer.RepoIndexerEnabled = true idx, _, err := NewBleveIndexer(dir) diff --git a/modules/indexer/issues/bleve.go b/modules/indexer/issues/bleve.go index 655bc7dd17..a1f51dba50 100644 --- a/modules/indexer/issues/bleve.go +++ b/modules/indexer/issues/bleve.go @@ -10,6 +10,7 @@ import ( "strconv" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/util" "github.com/blevesearch/bleve" "github.com/blevesearch/bleve/analysis/analyzer/custom" "github.com/blevesearch/bleve/analysis/token/lowercase" @@ -86,14 +87,14 @@ func openIndexer(path string, latestVersion int) (bleve.Index, error) { if metadata.Version < latestVersion { // the indexer is using a previous version, so we should delete it and // re-populate - return nil, os.RemoveAll(path) + return nil, util.RemoveAll(path) } index, err := bleve.Open(path) if err != nil && err == upsidedown.IncompatibleVersion { // the indexer was built with a previous version of bleve, so we should // delete it and re-populate - return nil, os.RemoveAll(path) + return nil, util.RemoveAll(path) } else if err != nil { return nil, err } diff --git a/modules/indexer/issues/bleve_test.go b/modules/indexer/issues/bleve_test.go index 234da076cd..a8b6e8138b 100644 --- a/modules/indexer/issues/bleve_test.go +++ b/modules/indexer/issues/bleve_test.go @@ -6,9 +6,9 @@ package issues import ( "io/ioutil" - "os" "testing" + "code.gitea.io/gitea/modules/util" "github.com/stretchr/testify/assert" ) @@ -19,7 +19,7 @@ func TestBleveIndexAndSearch(t *testing.T) { assert.Fail(t, "Unable to create temporary directory") return } - defer os.RemoveAll(dir) + defer util.RemoveAll(dir) indexer := NewBleveIndexer(dir) defer indexer.Close() diff --git a/modules/indexer/issues/indexer_test.go b/modules/indexer/issues/indexer_test.go index 8a54c200ff..95007d8faf 100644 --- a/modules/indexer/issues/indexer_test.go +++ b/modules/indexer/issues/indexer_test.go @@ -6,7 +6,6 @@ package issues import ( "io/ioutil" - "os" "path" "path/filepath" "testing" @@ -14,6 +13,7 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" "gopkg.in/ini.v1" @@ -40,7 +40,7 @@ func TestBleveSearchIssues(t *testing.T) { defer func() { setting.Indexer.IssueQueueDir = oldQueueDir setting.Indexer.IssuePath = oldIssuePath - os.RemoveAll(tmpIndexerDir) + util.RemoveAll(tmpIndexerDir) }() setting.Indexer.IssueType = "bleve" diff --git a/modules/lfs/content_store.go b/modules/lfs/content_store.go index 6ad565178c..b0fa77e255 100644 --- a/modules/lfs/content_store.go +++ b/modules/lfs/content_store.go @@ -14,6 +14,7 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/util" ) var ( @@ -61,7 +62,11 @@ func (s *ContentStore) Put(meta *models.LFSMetaObject, r io.Reader) error { log.Error("Whilst putting LFS OID[%s]: Unable to open temporary file for writing: %s Error: %v", tmpPath, err) return err } - defer os.Remove(tmpPath) + defer func() { + if err := util.Remove(tmpPath); err != nil { + log.Warn("Unable to remove temporary path: %s: Error: %v", tmpPath, err) + } + }() hash := sha256.New() hw := io.MultiWriter(hash, file) diff --git a/modules/log/file.go b/modules/log/file.go index 925d83f2b7..c9b5d47c0b 100644 --- a/modules/log/file.go +++ b/modules/log/file.go @@ -15,6 +15,8 @@ import ( "strings" "sync" "time" + + "code.gitea.io/gitea/modules/util" ) // FileLogger implements LoggerProvider. @@ -214,11 +216,11 @@ func compressOldLogFile(fname string, compressionLevel int) error { if err != nil { zw.Close() fw.Close() - os.Remove(fname + ".gz") + util.Remove(fname + ".gz") return err } reader.Close() - return os.Remove(fname) + return util.Remove(fname) } func (log *FileLogger) deleteOldLog() { @@ -233,7 +235,7 @@ func (log *FileLogger) deleteOldLog() { if !info.IsDir() && info.ModTime().Unix() < (time.Now().Unix()-60*60*24*log.Maxdays) { if strings.HasPrefix(filepath.Base(path), filepath.Base(log.Filename)) { - if err := os.Remove(path); err != nil { + if err := util.Remove(path); err != nil { returnErr = fmt.Errorf("Failed to remove %s: %v", path, err) } } diff --git a/modules/log/file_test.go b/modules/log/file_test.go index 38279315ab..af6fbcb29d 100644 --- a/modules/log/file_test.go +++ b/modules/log/file_test.go @@ -14,13 +14,14 @@ import ( "testing" "time" + "code.gitea.io/gitea/modules/util" "github.com/stretchr/testify/assert" ) func TestFileLoggerFails(t *testing.T) { tmpDir, err := ioutil.TempDir("", "TestFileLogger") assert.NoError(t, err) - defer os.RemoveAll(tmpDir) + defer util.RemoveAll(tmpDir) prefix := "TestPrefix " level := INFO @@ -48,7 +49,7 @@ func TestFileLoggerFails(t *testing.T) { func TestFileLogger(t *testing.T) { tmpDir, err := ioutil.TempDir("", "TestFileLogger") assert.NoError(t, err) - defer os.RemoveAll(tmpDir) + defer util.RemoveAll(tmpDir) prefix := "TestPrefix " level := INFO @@ -151,7 +152,7 @@ func TestFileLogger(t *testing.T) { func TestCompressFileLogger(t *testing.T) { tmpDir, err := ioutil.TempDir("", "TestFileLogger") assert.NoError(t, err) - defer os.RemoveAll(tmpDir) + defer util.RemoveAll(tmpDir) prefix := "TestPrefix " level := INFO @@ -211,7 +212,7 @@ func TestCompressFileLogger(t *testing.T) { func TestCompressOldFile(t *testing.T) { tmpDir, err := ioutil.TempDir("", "TestFileLogger") assert.NoError(t, err) - defer os.RemoveAll(tmpDir) + defer util.RemoveAll(tmpDir) fname := filepath.Join(tmpDir, "test") nonGzip := filepath.Join(tmpDir, "test-nonGzip") diff --git a/modules/markup/external/external.go b/modules/markup/external/external.go index 02cf61be16..7acf936171 100644 --- a/modules/markup/external/external.go +++ b/modules/markup/external/external.go @@ -16,6 +16,7 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/markup" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" ) // RegisterParsers registers all supported third part parsers according settings @@ -70,7 +71,12 @@ func (p *Parser) Render(rawBytes []byte, urlPrefix string, metas map[string]stri log.Error("%s create temp file when rendering %s failed: %v", p.Name(), p.Command, err) return []byte("") } - defer os.Remove(f.Name()) + tmpPath := f.Name() + defer func() { + if err := util.Remove(tmpPath); err != nil { + log.Warn("Unable to remove temporary file: %s: Error: %v", tmpPath, err) + } + }() _, err = io.Copy(f, rd) if err != nil { diff --git a/modules/queue/queue_disk_channel_test.go b/modules/queue/queue_disk_channel_test.go index 4ef68961c6..5049fb58d0 100644 --- a/modules/queue/queue_disk_channel_test.go +++ b/modules/queue/queue_disk_channel_test.go @@ -7,10 +7,10 @@ package queue import ( "context" "io/ioutil" - "os" "testing" "time" + "code.gitea.io/gitea/modules/util" "github.com/stretchr/testify/assert" ) @@ -29,7 +29,7 @@ func TestPersistableChannelQueue(t *testing.T) { tmpDir, err := ioutil.TempDir("", "persistable-channel-queue-test-data") assert.NoError(t, err) - defer os.RemoveAll(tmpDir) + defer util.RemoveAll(tmpDir) queue, err := NewPersistableChannelQueue(handle, PersistableChannelQueueConfiguration{ DataDir: tmpDir, diff --git a/modules/queue/queue_disk_test.go b/modules/queue/queue_disk_test.go index c7d3eb160b..789b6c3e64 100644 --- a/modules/queue/queue_disk_test.go +++ b/modules/queue/queue_disk_test.go @@ -7,11 +7,11 @@ package queue import ( "context" "io/ioutil" - "os" "sync" "testing" "time" + "code.gitea.io/gitea/modules/util" "github.com/stretchr/testify/assert" ) @@ -31,7 +31,7 @@ func TestLevelQueue(t *testing.T) { tmpDir, err := ioutil.TempDir("", "level-queue-test-data") assert.NoError(t, err) - defer os.RemoveAll(tmpDir) + defer util.RemoveAll(tmpDir) queue, err := NewLevelQueue(handle, LevelQueueConfiguration{ ByteFIFOQueueConfiguration: ByteFIFOQueueConfiguration{ diff --git a/modules/repository/create.go b/modules/repository/create.go index 2f7d10f0d1..945f4a8cea 100644 --- a/modules/repository/create.go +++ b/modules/repository/create.go @@ -6,13 +6,13 @@ package repository import ( "fmt" - "os" "strings" "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" ) // CreateRepository creates a repository for the user/organization. @@ -48,7 +48,7 @@ func CreateRepository(doer, u *models.User, opts models.CreateRepoOptions) (_ *m if !opts.IsMirror { repoPath := models.RepoPath(u.Name, repo.Name) if err = initRepository(ctx, repoPath, doer, repo, opts); err != nil { - if err2 := os.RemoveAll(repoPath); err2 != nil { + if err2 := util.RemoveAll(repoPath); err2 != nil { log.Error("initRepository: %v", err) return fmt.Errorf( "delete repo directory %s/%s failed(2): %v", u.Name, repo.Name, err2) diff --git a/modules/repository/generate.go b/modules/repository/generate.go index 6d80488de7..c5fb0af383 100644 --- a/modules/repository/generate.go +++ b/modules/repository/generate.go @@ -16,6 +16,7 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/util" "github.com/huandu/xstrings" ) @@ -119,7 +120,7 @@ func generateRepoCommit(repo, templateRepo, generateRepo *models.Repository, tmp return fmt.Errorf("git clone: %v", err) } - if err := os.RemoveAll(path.Join(tmpDir, ".git")); err != nil { + if err := util.RemoveAll(path.Join(tmpDir, ".git")); err != nil { return fmt.Errorf("remove git dir: %v", err) } @@ -130,7 +131,7 @@ func generateRepoCommit(repo, templateRepo, generateRepo *models.Repository, tmp } if gt != nil { - if err := os.Remove(gt.Path); err != nil { + if err := util.Remove(gt.Path); err != nil { return fmt.Errorf("remove .giteatemplate: %v", err) } @@ -191,7 +192,7 @@ func generateGitContent(ctx models.DBContext, repo, templateRepo, generateRepo * } defer func() { - if err := os.RemoveAll(tmpDir); err != nil { + if err := util.RemoveAll(tmpDir); err != nil { log.Error("RemoveAll: %v", err) } }() diff --git a/modules/repository/hooks.go b/modules/repository/hooks.go index 6050f21f7f..2cefd56069 100644 --- a/modules/repository/hooks.go +++ b/modules/repository/hooks.go @@ -14,6 +14,7 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" "github.com/unknwon/com" "xorm.io/builder" @@ -53,7 +54,7 @@ func createDelegateHooks(repoPath string) (err error) { } // WARNING: This will override all old server-side hooks - if err = os.Remove(oldHookPath); err != nil && !os.IsNotExist(err) { + if err = util.Remove(oldHookPath); err != nil && !os.IsNotExist(err) { return fmt.Errorf("unable to pre-remove old hook file '%s' prior to rewriting: %v ", oldHookPath, err) } if err = ioutil.WriteFile(oldHookPath, []byte(hookTpls[i]), 0777); err != nil { @@ -64,7 +65,7 @@ func createDelegateHooks(repoPath string) (err error) { return fmt.Errorf("Unable to set %s executable. Error %v", oldHookPath, err) } - if err = os.Remove(newHookPath); err != nil && !os.IsNotExist(err) { + if err = util.Remove(newHookPath); err != nil && !os.IsNotExist(err) { return fmt.Errorf("unable to pre-remove new hook file '%s' prior to rewriting: %v", newHookPath, err) } if err = ioutil.WriteFile(newHookPath, []byte(giteaHookTpls[i]), 0777); err != nil { diff --git a/modules/repository/init.go b/modules/repository/init.go index 5bdfa7490d..8c8827c511 100644 --- a/modules/repository/init.go +++ b/modules/repository/init.go @@ -17,6 +17,7 @@ import ( "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" "github.com/mcuadros/go-version" "github.com/unknwon/com" @@ -188,8 +189,11 @@ func initRepository(ctx models.DBContext, repoPath string, u *models.User, repo if err != nil { return fmt.Errorf("Failed to create temp dir for repository %s: %v", repo.RepoPath(), err) } - - defer os.RemoveAll(tmpDir) + defer func() { + if err := util.RemoveAll(tmpDir); err != nil { + log.Warn("Unable to remove temporary directory: %s: Error: %v", tmpDir, err) + } + }() if err = prepareRepoCommit(ctx, repo, tmpDir, repoPath, opts); err != nil { return fmt.Errorf("prepareRepoCommit: %v", err) diff --git a/modules/repository/repo.go b/modules/repository/repo.go index 01d4701c71..2d5551d987 100644 --- a/modules/repository/repo.go +++ b/modules/repository/repo.go @@ -6,7 +6,6 @@ package repository import ( "fmt" - "os" "path" "strings" "time" @@ -17,6 +16,7 @@ import ( "code.gitea.io/gitea/modules/setting" api "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/timeutil" + "code.gitea.io/gitea/modules/util" "gopkg.in/ini.v1" ) @@ -57,7 +57,7 @@ func MigrateRepositoryGitData(doer, u *models.User, repo *models.Repository, opt migrateTimeout := time.Duration(setting.Git.Timeout.Migrate) * time.Second var err error - if err = os.RemoveAll(repoPath); err != nil { + if err = util.RemoveAll(repoPath); err != nil { return repo, fmt.Errorf("Failed to remove %s: %v", repoPath, err) } @@ -73,7 +73,7 @@ func MigrateRepositoryGitData(doer, u *models.User, repo *models.Repository, opt wikiPath := models.WikiPath(u.Name, opts.RepoName) wikiRemotePath := WikiRemoteURL(opts.CloneAddr) if len(wikiRemotePath) > 0 { - if err := os.RemoveAll(wikiPath); err != nil { + if err := util.RemoveAll(wikiPath); err != nil { return repo, fmt.Errorf("Failed to remove %s: %v", wikiPath, err) } @@ -84,7 +84,7 @@ func MigrateRepositoryGitData(doer, u *models.User, repo *models.Repository, opt Branch: "master", }); err != nil { log.Warn("Clone wiki: %v", err) - if err := os.RemoveAll(wikiPath); err != nil { + if err := util.RemoveAll(wikiPath); err != nil { return repo, fmt.Errorf("Failed to remove %s: %v", wikiPath, err) } } diff --git a/modules/util/remove.go b/modules/util/remove.go new file mode 100644 index 0000000000..f2bbbc30b9 --- /dev/null +++ b/modules/util/remove.go @@ -0,0 +1,57 @@ +// Copyright 2020 The Gitea Authors. All rights reserved. +// Use of this source code is governed by a MIT-style +// license that can be found in the LICENSE file. + +package util + +import ( + "os" + "syscall" + "time" +) + +// Remove removes the named file or (empty) directory with at most 5 attempts. +func Remove(name string) error { + var err error + for i := 0; i < 5; i++ { + err = os.Remove(name) + if err == nil { + break + } + unwrapped := err.(*os.PathError).Err + if unwrapped == syscall.EBUSY || unwrapped == syscall.ENOTEMPTY || unwrapped == syscall.EPERM || unwrapped == syscall.EMFILE || unwrapped == syscall.ENFILE { + // try again + <-time.After(100 * time.Millisecond) + continue + } + + if unwrapped == syscall.ENOENT { + // it's already gone + return nil + } + } + return err +} + +// RemoveAll removes the named file or (empty) directory with at most 5 attempts.Remove +func RemoveAll(name string) error { + var err error + for i := 0; i < 5; i++ { + err = os.RemoveAll(name) + if err == nil { + break + } + unwrapped := err.(*os.PathError).Err + if unwrapped == syscall.EBUSY || unwrapped == syscall.ENOTEMPTY || unwrapped == syscall.EPERM || unwrapped == syscall.EMFILE || unwrapped == syscall.ENFILE { + // try again + <-time.After(100 * time.Millisecond) + continue + } + + if unwrapped == syscall.ENOENT { + // it's already gone + return nil + } + } + return err +} diff --git a/modules/util/sanitize.go b/modules/util/sanitize.go index d04e1dee77..b1c17b29cf 100644 --- a/modules/util/sanitize.go +++ b/modules/util/sanitize.go @@ -7,8 +7,6 @@ package util import ( "net/url" "strings" - - "code.gitea.io/gitea/modules/log" ) // urlSafeError wraps an error whose message may contain a sensitive URL @@ -38,7 +36,6 @@ func SanitizeMessage(message, unsanitizedURL string) string { func SanitizeURLCredentials(unsanitizedURL string, usePlaceholder bool) string { u, err := url.Parse(unsanitizedURL) if err != nil { - log.Error("parse url %s failed: %v", unsanitizedURL, err) // don't log the error, since it might contain unsanitized URL. return "(unparsable url)" } diff --git a/modules/util/url.go b/modules/util/url.go index 263255fcd3..96c66df92b 100644 --- a/modules/util/url.go +++ b/modules/util/url.go @@ -8,9 +8,6 @@ import ( "net/url" "path" "strings" - - "code.gitea.io/gitea/modules/log" - "code.gitea.io/gitea/modules/setting" ) // PathEscapeSegments escapes segments of a path while not escaping forward slash @@ -30,13 +27,11 @@ func URLJoin(base string, elems ...string) string { } baseURL, err := url.Parse(base) if err != nil { - log.Error("URLJoin: Invalid base URL %s", base) return "" } joinedPath := path.Join(elems...) argURL, err := url.Parse(joinedPath) if err != nil { - log.Error("URLJoin: Invalid arg %s", joinedPath) return "" } joinedURL := baseURL.ResolveReference(argURL).String() @@ -45,16 +40,3 @@ func URLJoin(base string, elems ...string) string { } return joinedURL } - -// IsExternalURL checks if rawURL points to an external URL like http://example.com -func IsExternalURL(rawURL string) bool { - parsed, err := url.Parse(rawURL) - if err != nil { - return true - } - appURL, _ := url.Parse(setting.AppURL) - if len(parsed.Host) != 0 && strings.Replace(parsed.Host, "www.", "", 1) != strings.Replace(appURL.Host, "www.", "", 1) { - return true - } - return false -} diff --git a/modules/util/util_test.go b/modules/util/util_test.go index 04ab42f292..1d4f23de90 100644 --- a/modules/util/util_test.go +++ b/modules/util/util_test.go @@ -8,8 +8,6 @@ import ( "strings" "testing" - "code.gitea.io/gitea/modules/setting" - "github.com/stretchr/testify/assert" ) @@ -46,39 +44,6 @@ func TestURLJoin(t *testing.T) { } } -func TestIsExternalURL(t *testing.T) { - setting.AppURL = "https://try.gitea.io" - type test struct { - Expected bool - RawURL string - } - newTest := func(expected bool, rawURL string) test { - return test{Expected: expected, RawURL: rawURL} - } - for _, test := range []test{ - newTest(false, - "https://try.gitea.io"), - newTest(true, - "https://example.com/"), - newTest(true, - "//example.com"), - newTest(true, - "http://example.com"), - newTest(false, - "a/"), - newTest(false, - "https://try.gitea.io/test?param=false"), - newTest(false, - "test?param=false"), - newTest(false, - "//try.gitea.io/test?param=false"), - newTest(false, - "/hey/hey/hey#3244"), - } { - assert.Equal(t, test.Expected, IsExternalURL(test.RawURL)) - } -} - func TestIsEmptyString(t *testing.T) { cases := []struct { diff --git a/routers/repo/http.go b/routers/repo/http.go index 565ee5eff4..bc3b81f511 100644 --- a/routers/repo/http.go +++ b/routers/repo/http.go @@ -31,6 +31,7 @@ import ( "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/structs" "code.gitea.io/gitea/modules/timeutil" + "code.gitea.io/gitea/modules/util" repo_service "code.gitea.io/gitea/services/repository" ) @@ -391,7 +392,7 @@ func dummyInfoRefs(ctx *context.Context) { } defer func() { - if err := os.RemoveAll(tmpDir); err != nil { + if err := util.RemoveAll(tmpDir); err != nil { log.Error("RemoveAll: %v", err) } }() diff --git a/routers/repo/lfs.go b/routers/repo/lfs.go index d8f1ce62a9..8aff89dd6a 100644 --- a/routers/repo/lfs.go +++ b/routers/repo/lfs.go @@ -11,7 +11,6 @@ import ( gotemplate "html/template" "io" "io/ioutil" - "os" "path" "path/filepath" "sort" @@ -29,6 +28,7 @@ import ( "code.gitea.io/gitea/modules/lfs" "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" gogit "github.com/go-git/go-git/v5" "github.com/go-git/go-git/v5/plumbing" @@ -356,7 +356,7 @@ func LFSDelete(ctx *context.Context) { // Please note a similar condition happens in models/repo.go DeleteRepository if count == 0 { oidPath := filepath.Join(oid[0:2], oid[2:4], oid[4:]) - err = os.Remove(filepath.Join(setting.LFS.ContentPath, oidPath)) + err = util.Remove(filepath.Join(setting.LFS.ContentPath, oidPath)) if err != nil { ctx.ServerError("LFSDelete", err) return diff --git a/routers/repo/settings_test.go b/routers/repo/settings_test.go index 2325c0255c..97c3fb0471 100644 --- a/routers/repo/settings_test.go +++ b/routers/repo/settings_test.go @@ -7,7 +7,6 @@ package repo import ( "io/ioutil" "net/http" - "os" "testing" "code.gitea.io/gitea/models" @@ -15,6 +14,7 @@ import ( "code.gitea.io/gitea/modules/context" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/test" + "code.gitea.io/gitea/modules/util" "github.com/stretchr/testify/assert" ) @@ -31,7 +31,7 @@ func createSSHAuthorizedKeysTmpPath(t *testing.T) func() { return func() { setting.SSH.RootPath = oldPath - os.RemoveAll(tmpDir) + util.RemoveAll(tmpDir) } } diff --git a/routers/user/auth.go b/routers/user/auth.go index a253e6cdbb..4e6ac9c87f 100644 --- a/routers/user/auth.go +++ b/routers/user/auth.go @@ -22,7 +22,7 @@ import ( "code.gitea.io/gitea/modules/recaptcha" "code.gitea.io/gitea/modules/setting" "code.gitea.io/gitea/modules/timeutil" - "code.gitea.io/gitea/modules/util" + "code.gitea.io/gitea/routers/utils" "code.gitea.io/gitea/services/externalaccount" "code.gitea.io/gitea/services/mailer" @@ -537,7 +537,7 @@ func handleSignInFull(ctx *context.Context, u *models.User, remember bool, obeyR return setting.AppSubURL + "/" } - if redirectTo := ctx.GetCookie("redirect_to"); len(redirectTo) > 0 && !util.IsExternalURL(redirectTo) { + if redirectTo := ctx.GetCookie("redirect_to"); len(redirectTo) > 0 && !utils.IsExternalURL(redirectTo) { ctx.SetCookie("redirect_to", "", -1, setting.AppSubURL, "", setting.SessionConfig.Secure, true) if obeyRedirect { ctx.RedirectToFirst(redirectTo) @@ -1540,7 +1540,7 @@ func MustChangePasswordPost(ctx *context.Context, cpt *captcha.Captcha, form aut log.Trace("User updated password: %s", u.Name) - if redirectTo := ctx.GetCookie("redirect_to"); len(redirectTo) > 0 && !util.IsExternalURL(redirectTo) { + if redirectTo := ctx.GetCookie("redirect_to"); len(redirectTo) > 0 && !utils.IsExternalURL(redirectTo) { ctx.SetCookie("redirect_to", "", -1, setting.AppSubURL) ctx.RedirectToFirst(redirectTo) return diff --git a/routers/utils/utils.go b/routers/utils/utils.go index 64b132ff3e..7c845f8763 100644 --- a/routers/utils/utils.go +++ b/routers/utils/utils.go @@ -6,7 +6,10 @@ package utils import ( "html" + "net/url" "strings" + + "code.gitea.io/gitea/modules/setting" ) // RemoveUsernameParameterSuffix returns the username parameter without the (fullname) suffix - leaving just the username @@ -46,3 +49,16 @@ func SanitizeFlashErrorString(x string) string { return strings.Replace(html.EscapeString(x), "\n", "<br>", -1) } + +// IsExternalURL checks if rawURL points to an external URL like http://example.com +func IsExternalURL(rawURL string) bool { + parsed, err := url.Parse(rawURL) + if err != nil { + return true + } + appURL, _ := url.Parse(setting.AppURL) + if len(parsed.Host) != 0 && strings.Replace(parsed.Host, "www.", "", 1) != strings.Replace(appURL.Host, "www.", "", 1) { + return true + } + return false +} diff --git a/routers/utils/utils_test.go b/routers/utils/utils_test.go index d96e1d7d26..ec5e69862a 100644 --- a/routers/utils/utils_test.go +++ b/routers/utils/utils_test.go @@ -7,6 +7,7 @@ package utils import ( "testing" + "code.gitea.io/gitea/modules/setting" "github.com/stretchr/testify/assert" ) @@ -32,3 +33,36 @@ func TestIsValidSlackChannel(t *testing.T) { assert.Equal(t, v.expected, IsValidSlackChannel(v.channelName)) } } + +func TestIsExternalURL(t *testing.T) { + setting.AppURL = "https://try.gitea.io" + type test struct { + Expected bool + RawURL string + } + newTest := func(expected bool, rawURL string) test { + return test{Expected: expected, RawURL: rawURL} + } + for _, test := range []test{ + newTest(false, + "https://try.gitea.io"), + newTest(true, + "https://example.com/"), + newTest(true, + "//example.com"), + newTest(true, + "http://example.com"), + newTest(false, + "a/"), + newTest(false, + "https://try.gitea.io/test?param=false"), + newTest(false, + "test?param=false"), + newTest(false, + "//try.gitea.io/test?param=false"), + newTest(false, + "/hey/hey/hey#3244"), + } { + assert.Equal(t, test.Expected, IsExternalURL(test.RawURL)) + } +} diff --git a/services/pull/check.go b/services/pull/check.go index ea91f3a862..d6817bc81b 100644 --- a/services/pull/check.go +++ b/services/pull/check.go @@ -20,6 +20,7 @@ import ( "code.gitea.io/gitea/modules/notification" "code.gitea.io/gitea/modules/queue" "code.gitea.io/gitea/modules/timeutil" + "code.gitea.io/gitea/modules/util" "github.com/unknwon/com" ) @@ -82,7 +83,11 @@ func getMergeCommit(pr *models.PullRequest) (*git.Commit, error) { if err != nil { return nil, fmt.Errorf("Failed to create temp dir for repository %s: %v", pr.BaseRepo.RepoPath(), err) } - defer os.RemoveAll(indexTmpPath) + defer func() { + if err := util.RemoveAll(indexTmpPath); err != nil { + log.Warn("Unable to remove temporary index path: %s: Error: %v", indexTmpPath, err) + } + }() headFile := pr.GetGitRefName() diff --git a/services/pull/patch.go b/services/pull/patch.go index 776bfdb751..9b894b781c 100644 --- a/services/pull/patch.go +++ b/services/pull/patch.go @@ -17,6 +17,7 @@ import ( "code.gitea.io/gitea/models" "code.gitea.io/gitea/modules/git" "code.gitea.io/gitea/modules/log" + "code.gitea.io/gitea/modules/util" ) // DownloadDiffOrPatch will write the patch for the pr to the writer @@ -80,7 +81,7 @@ func TestPatch(pr *models.PullRequest) error { return fmt.Errorf("Unable to create temporary patch file! Error: %v", err) } defer func() { - _ = os.Remove(tmpPatchFile.Name()) + _ = util.Remove(tmpPatchFile.Name()) }() if err := gitRepo.GetDiff(pr.MergeBase, "tracking", tmpPatchFile); err != nil { diff --git a/services/pull/pull.go b/services/pull/pull.go index b94a46639a..e624b182aa 100644 --- a/services/pull/pull.go +++ b/services/pull/pull.go @@ -10,7 +10,6 @@ import ( "context" "encoding/json" "fmt" - "os" "path" "strings" "time" @@ -21,6 +20,7 @@ import ( "code.gitea.io/gitea/modules/log" "code.gitea.io/gitea/modules/notification" "code.gitea.io/gitea/modules/setting" + "code.gitea.io/gitea/modules/util" issue_service "code.gitea.io/gitea/services/issue" "github.com/unknwon/com" @@ -436,7 +436,7 @@ func PushToBaseRepo(pr *models.PullRequest) (err error) { // Remove head in case there is a conflict. file := path.Join(pr.BaseRepo.RepoPath(), headFile) - _ = os.Remove(file) + _ = util.Remove(file) if err = pr.LoadIssue(); err != nil { return fmt.Errorf("unable to load issue %d for pr %d: %v", pr.IssueID, pr.ID, err) diff --git a/services/release/release.go b/services/release/release.go index 2fc3cb199b..c308c19f24 100644 --- a/services/release/release.go +++ b/services/release/release.go @@ -6,7 +6,6 @@ package release import ( "fmt" - "os" "strings" "code.gitea.io/gitea/models" @@ -15,6 +14,7 @@ import ( "code.gitea.io/gitea/modules/notification" "code.gitea.io/gitea/modules/repository" "code.gitea.io/gitea/modules/timeutil" + "code.gitea.io/gitea/modules/util" ) func createTag(gitRepo *git.Repository, rel *models.Release) error { @@ -168,7 +168,7 @@ func DeleteReleaseByID(id int64, doer *models.User, delTag bool) error { for i := range rel.Attachments { attachment := rel.Attachments[i] - if err := os.RemoveAll(attachment.LocalPath()); err != nil { + if err := util.RemoveAll(attachment.LocalPath()); err != nil { log.Error("Delete attachment %s of release %s failed: %v", attachment.UUID, rel.ID, err) } } |