Skip to content

Commit

Permalink
Fix #1020 os.Chtimes invalid arg
Browse files Browse the repository at this point in the history
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
  • Loading branch information
cvgw committed Feb 24, 2020
1 parent 27aeb89 commit 6130e90
Show file tree
Hide file tree
Showing 2 changed files with 83 additions and 3 deletions.
32 changes: 29 additions & 3 deletions pkg/util/fs_util.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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) {
Expand Down
54 changes: 54 additions & 0 deletions pkg/util/fs_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
})
}
}

0 comments on commit 6130e90

Please sign in to comment.