Commit 56c9f8e8 authored by Ian Lance Taylor's avatar Ian Lance Taylor

os: treat EACCES as a permission error in RemoveAll

Fixes #29983

Change-Id: I24077bde991e621c23d00973b2a77bb3a18e4ae7
Reviewed-on: https://go-review.googlesource.com/c/160180
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: default avatarDamien Neil <dneil@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
parent ea27cd35
......@@ -57,8 +57,13 @@ func removeAllFrom(parent *File, path string) error {
return nil
}
// If not a "is directory" error, we have a problem
if err != syscall.EISDIR && err != syscall.EPERM {
// EISDIR means that we have a directory, and we need to
// remove its contents.
// EPERM or EACCES means that we don't have write permission on
// the parent directory, but this entry might still be a directory
// whose contents need to be removed.
// Otherwise just return the error.
if err != syscall.EISDIR && err != syscall.EPERM && err != syscall.EACCES {
return err
}
......@@ -69,11 +74,11 @@ func removeAllFrom(parent *File, path string) error {
return statErr
}
if statInfo.Mode&syscall.S_IFMT != syscall.S_IFDIR {
// Not a directory; return the error from the Remove
// Not a directory; return the error from the Remove.
return err
}
// Remove the directory's entries
// Remove the directory's entries.
var recurseErr error
for {
const request = 1024
......@@ -88,7 +93,7 @@ func removeAllFrom(parent *File, path string) error {
}
names, readErr := file.Readdirnames(request)
// Errors other than EOF should stop us from continuing
// Errors other than EOF should stop us from continuing.
if readErr != nil && readErr != io.EOF {
file.Close()
if IsNotExist(readErr) {
......@@ -117,7 +122,7 @@ func removeAllFrom(parent *File, path string) error {
}
}
// Remove the directory itself
// Remove the directory itself.
unlinkError := unix.Unlinkat(parentFd, path, unix.AT_REMOVEDIR)
if unlinkError == nil || IsNotExist(unlinkError) {
return nil
......
......@@ -292,3 +292,83 @@ func TestRemoveReadOnlyDir(t *testing.T) {
t.Error("subdirectory was not removed")
}
}
// Issue #29983.
func TestRemoveAllButReadOnly(t *testing.T) {
switch runtime.GOOS {
case "nacl", "js", "windows":
t.Skipf("skipping test on %s", runtime.GOOS)
}
if Getuid() == 0 {
t.Skip("skipping test when running as root")
}
t.Parallel()
tempDir, err := ioutil.TempDir("", "TestRemoveAllButReadOnly-")
if err != nil {
t.Fatal(err)
}
defer RemoveAll(tempDir)
dirs := []string{
"a",
"a/x",
"a/x/1",
"b",
"b/y",
"b/y/2",
"c",
"c/z",
"c/z/3",
}
readonly := []string{
"b",
}
inReadonly := func(d string) bool {
for _, ro := range readonly {
if d == ro {
return true
}
dd, _ := filepath.Split(d)
if filepath.Clean(dd) == ro {
return true
}
}
return false
}
for _, dir := range dirs {
if err := Mkdir(filepath.Join(tempDir, dir), 0777); err != nil {
t.Fatal(err)
}
}
for _, dir := range readonly {
d := filepath.Join(tempDir, dir)
if err := Chmod(d, 0555); err != nil {
t.Fatal(err)
}
// Defer changing the mode back so that the deferred
// RemoveAll(tempDir) can succeed.
defer Chmod(d, 0777)
}
if err := RemoveAll(tempDir); err == nil {
t.Fatal("RemoveAll succeeded unexpectedly")
}
for _, dir := range dirs {
_, err := Stat(filepath.Join(tempDir, dir))
if inReadonly(dir) {
if err != nil {
t.Errorf("file %q was deleted but should still exist", dir)
}
} else {
if err == nil {
t.Errorf("file %q still exists but should have been deleted", dir)
}
}
}
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment