From 6130e9053755729980602947991974738ac1bbce Mon Sep 17 00:00:00 2001 From: cvgw <cgwippern@google.com> Date: Sun, 23 Feb 2020 13:47:17 -0800 Subject: [PATCH] Fix #1020 os.Chtimes invalid arg The zero value of time.Time is not a valid argument to os.Chtimes because of the syscall that os.Chtimes calls. Instead we update the zero value of time.Time to the zero value of Unix Epoch --- pkg/util/fs_util.go | 32 +++++++++++++++++++++--- pkg/util/fs_util_test.go | 54 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+), 3 deletions(-) diff --git a/pkg/util/fs_util.go b/pkg/util/fs_util.go index afd20e65b8..be26f10ba0 100644 --- a/pkg/util/fs_util.go +++ b/pkg/util/fs_util.go @@ -298,9 +298,7 @@ func ExtractFile(dest string, hdr *tar.Header, tr io.Reader) error { return err } - // We set AccessTime because its a required arg but we only care about - // ModTime. The file will get accessed again so AccessTime will change. - if err := os.Chtimes(path, hdr.AccessTime, hdr.ModTime); err != nil { + if err = setFileTimes(path, hdr.AccessTime, hdr.ModTime); err != nil { return err } @@ -737,6 +735,34 @@ func setFilePermissions(path string, mode os.FileMode, uid, gid int) error { return os.Chmod(path, mode) } +func setFileTimes(path string, aTime, mTime time.Time) error { + // The zero value of time.Time is not a valid argument to os.Chtimes as it cannot be + // converted into a valid argument to the syscall that os.Chtimes uses. If mTime or + // aTime are zero we convert them to the zero value for Unix Epoch. + if mTime.IsZero() { + logrus.Tracef("mod time for %s is zero, converting to zero for epoch", path) + mTime = time.Unix(0) + } + + if aTime.IsZero() { + logrus.Tracef("access time for %s is zero, converting to zero for epoch", path) + aTime = time.Unix(0) + } + + // We set AccessTime because its a required arg but we only care about + // ModTime. The file will get accessed again so AccessTime will change. + if err := os.Chtimes(path, aTime, mTime); err != nil { + return errors.Wrapf( + err, + "couldn't modify times: atime %v mtime %v", + aTime, + mTime, + ) + } + + return nil +} + // CreateTargetTarfile creates target tar file for downloading the context file. // Make directory if directory does not exist func CreateTargetTarfile(tarpath string) (*os.File, error) { diff --git a/pkg/util/fs_util_test.go b/pkg/util/fs_util_test.go index dcb17771e1..b8eb6329e9 100644 --- a/pkg/util/fs_util_test.go +++ b/pkg/util/fs_util_test.go @@ -1314,3 +1314,57 @@ func TestUpdateWhitelist(t *testing.T) { }) } } + +func Test_setFileTimes(t *testing.T) { + testDir, err := ioutil.TempDir("", "") + if err != nil { + t.Fatal(err) + } + + defer os.RemoveAll(testDir) + + p := filepath.Join(testDir, "foo.txt") + + if err := ioutil.WriteFile(p, []byte("meow"), 0777); err != nil { + t.Fatal(err) + } + + type testcase struct { + desc string + path string + aTime time.Time + mTime time.Time + } + + testCases := []testcase{ + { + desc: "zero for mod and access", + path: p, + }, + { + desc: "zero for mod", + path: p, + aTime: time.Now(), + }, + { + desc: "zero for access", + path: p, + mTime: time.Now(), + }, + { + desc: "both non-zero", + path: p, + mTime: time.Now(), + aTime: time.Now(), + }, + } + + for _, tc := range testCases { + t.Run(tc.desc, func(t *testing.T) { + err := setFileTimes(tc.path, tc.aTime, tc.mTime) + if err != nil { + t.Errorf("expected err to be nil not %s", err) + } + }) + } +}