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 <[email protected]>
Reviewed-by: Damien Neil <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>

Author: ianlancetaylor

src/os/removeall_at.go

@@ -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

src/os/removeall_test.go

@@ -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)
+			}
+		}
+	}
+}