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: Damien Neil <dneil@google.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
diff --git a/src/os/removeall_at.go b/src/os/removeall_at.go
index fe8b1fa..7f2d592 100644
--- a/src/os/removeall_at.go
+++ b/src/os/removeall_at.go
@@ -57,8 +57,13 @@
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 @@
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 @@
}
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 @@
}
}
- // Remove the directory itself
+ // Remove the directory itself.
unlinkError := unix.Unlinkat(parentFd, path, unix.AT_REMOVEDIR)
if unlinkError == nil || IsNotExist(unlinkError) {
return nil
diff --git a/src/os/removeall_test.go b/src/os/removeall_test.go
index 0f7dce0..9dab0d4bb 100644
--- a/src/os/removeall_test.go
+++ b/src/os/removeall_test.go
@@ -292,3 +292,83 @@
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)
+ }
+ }
+ }
+}
