diff options
author | Jacob Vosmaer (GitLab) <jacob@gitlab.com> | 2018-04-04 14:49:29 +0300 |
---|---|---|
committer | Ahmad Sherif <ahmad.m.sherif@gmail.com> | 2018-04-04 14:49:29 +0300 |
commit | b6dfef3c3ea302faf7a15272e8d93e811ec93da4 (patch) | |
tree | 7631979f70185a4a901d669d0773110ac48d463e /internal/tempdir | |
parent | 31b0ec1d1b815a744d49075fb1fd7ef968c464a4 (diff) |
Fix directory permission walker for Go 1.10
Diffstat (limited to 'internal/tempdir')
-rw-r--r-- | internal/tempdir/tempdir.go | 47 | ||||
-rw-r--r-- | internal/tempdir/tempdir_test.go | 23 |
2 files changed, 28 insertions, 42 deletions
diff --git a/internal/tempdir/tempdir.go b/internal/tempdir/tempdir.go index 6cff3f8ba..69565c590 100644 --- a/internal/tempdir/tempdir.go +++ b/internal/tempdir/tempdir.go @@ -2,17 +2,16 @@ package tempdir import ( "context" - "fmt" "io/ioutil" "os" "path" - "path/filepath" "strings" "time" pb "gitlab.com/gitlab-org/gitaly-proto/go" "gitlab.com/gitlab-org/gitaly/internal/config" "gitlab.com/gitlab-org/gitaly/internal/helper" + "gitlab.com/gitlab-org/gitaly/internal/helper/housekeeping" log "github.com/sirupsen/logrus" ) @@ -22,6 +21,8 @@ const ( // directory name that could be provided by a user. The '+' character is // not allowed in GitLab namespaces or repositories. tmpRootPrefix = "+gitaly/tmp" + + maxAge = 7 * 24 * time.Hour ) // New returns the path of a new temporary directory for use with the @@ -76,6 +77,8 @@ func StartCleaning() { } } +type invalidCleanRoot string + func clean(dir string) error { // If we start "cleaning up" the wrong directory we may delete user data // which is Really Bad. @@ -84,37 +87,27 @@ func clean(dir string) error { panic(invalidCleanRoot("invalid tempdir clean root: panicking to prevent data loss")) } - return filepath.Walk(dir, cleanFunc) -} - -const ( - maxAge = 7 * 24 * time.Hour -) - -type invalidCleanRoot string - -func cleanFunc(path string, info os.FileInfo, errIncoming error) error { - if errIncoming != nil && !os.IsNotExist(errIncoming) { - return fmt.Errorf("incoming %q: %v", path, errIncoming) - } - - if info == nil { + entries, err := ioutil.ReadDir(dir) + if os.IsNotExist(err) { return nil } + if err != nil { + return err + } - if perm := info.Mode().Perm(); info.IsDir() && perm&0700 < 0700 { - // Fix directory read permissions - if err := os.Chmod(path, perm|0700); err != nil { - return err + for _, info := range entries { + if time.Since(info.ModTime()) < maxAge { + continue } - } - if time.Since(info.ModTime()) < maxAge { - return nil - } + fullPath := path.Join(dir, info.Name()) + if err := housekeeping.FixDirectoryPermissions(fullPath); err != nil { + return err + } - if err := os.Remove(path); err != nil && !info.IsDir() { - return err + if err := os.RemoveAll(fullPath); err != nil { + return err + } } return nil diff --git a/internal/tempdir/tempdir_test.go b/internal/tempdir/tempdir_test.go index 833292abb..e4dcc3e74 100644 --- a/internal/tempdir/tempdir_test.go +++ b/internal/tempdir/tempdir_test.go @@ -74,39 +74,32 @@ func TestCleanSuccess(t *testing.T) { recent := time.Now() makeDir(t, "a", old) + makeDir(t, "a/b", recent) // Messes up mtime of "a", we fix that below makeDir(t, "c", recent) makeDir(t, "f", old) - makeFile(t, "a/b", old) + makeFile(t, "a/b/g", old) makeFile(t, "c/d", old) makeFile(t, "e", recent) // This is really evil and even breaks 'rm -rf' + require.NoError(t, chmod("a/b", 0), "apply evil permissions to 'a/b'") require.NoError(t, chmod("a", 0), "apply evil permissions to 'a'") + require.NoError(t, chtimes("a", old), "reset mtime of 'a'") + assertEntries(t, "a", "c", "e", "f") require.NoError(t, clean(cleanRoot), "walk first pass") - // 'a' won't get removed because it's mtime is bumped when 'a/b' is deleted - assertEntries(t, "a", "c", "e") - - info, err := stat("a") - require.NoError(t, err) - require.Equal(t, os.FileMode(0700), info.Mode().Perm(), "permissions of 'a' should have been fixed") - - _, err = stat("a/b") - require.True(t, os.IsNotExist(err), "entry 'a/b' should be gone") - - require.NoError(t, clean(cleanRoot), "walk second pass") - assertEntries(t, "a", "c", "e") + assertEntries(t, "c", "e") } func chmod(p string, mode os.FileMode) error { return os.Chmod(path.Join(cleanRoot, p), mode) } -func stat(p string) (os.FileInfo, error) { - return os.Stat(path.Join(cleanRoot, p)) +func chtimes(p string, t time.Time) error { + return os.Chtimes(path.Join(cleanRoot, p), t, t) } func assertEntries(t *testing.T, entries ...string) { |