os: handle trailing slashes in os.RemoveDir on Windows

CL 661575 inadvertently caused os.RemoveDir on Windows to
fail when given a path with a trailing / or \, due to the
splitPath function not correctly stripping trailing
separators.

Fixes #73317

Change-Id: I21977b94bb08ff1e563de6f5f16a4bdf5024a15e
Reviewed-on: https://go-review.googlesource.com/c/go/+/664715
Auto-Submit: Damien Neil <dneil@google.com>
TryBot-Bypass: Dmitri Shuralyov <dmitshur@golang.org>
Reviewed-by: Alan Donovan <adonovan@google.com>

diff --git a/src/os/export_windows_test.go b/src/os/export_windows_test.go
index aefbe4033e3738d14a47c8db5e5afd20305113b6..5b939b4c25f307b97859369952931e9002e4c2ce 100644 (file)
--- a/src/os/export_windows_test.go
+++ b/src/os/export_windows_test.go
@@ -11,4 +11,5 @@ var (
        NewConsoleFile     = newConsoleFile
        CommandLineToArgv  = commandLineToArgv
        AllowReadDirFileID = &allowReadDirFileID
+       SplitPath          = splitPath
 )

diff --git a/src/os/os_windows_test.go b/src/os/os_windows_test.go
index 15f1b616e6bb32989897de2461bc2e5c4679b9ce..d78080ccc4198d46e89a12b3d386af10c664b038 100644 (file)
--- a/src/os/os_windows_test.go
+++ b/src/os/os_windows_test.go
@@ -2074,3 +2074,39 @@ func TestFileAssociatedWithExternalIOCP(t *testing.T) {
                t.Error(err)
        }
 }
+
+func TestSplitPath(t *testing.T) {
+       t.Parallel()
+       for _, tt := range []struct{ path, wantDir, wantBase string }{
+               {`a`, `.`, `a`},
+               {`a\`, `.`, `a`},
+               {`a\\`, `.`, `a`},
+               {`a\b`, `a`, `b`},
+               {`a\\b`, `a`, `b`},
+               {`a\b\`, `a`, `b`},
+               {`a\b\c`, `a\b`, `c`},
+               {`\a`, `\`, `a`},
+               {`\a\`, `\`, `a`},
+               {`\a\b`, `\a`, `b`},
+               {`\a\b\`, `\a`, `b`},
+               {`\a\b\c`, `\a\b`, `c`},
+               {`\\a`, `\\a`, `.`},
+               {`\\a\`, `\\a\`, `.`},
+               {`\\\a`, `\\\a`, `.`},
+               {`\\\a\`, `\\\a`, `.`},
+               {`\\a\b\c`, `\\a\b`, `c`},
+               {`c:`, `c:`, `.`},
+               {`c:\`, `c:\`, `.`},
+               {`c:\a`, `c:\`, `a`},
+               {`c:a`, `c:`, `a`},
+               {`c:a\b\`, `c:a`, `b`},
+               {`c:base`, `c:`, `base`},
+               {`a/b/c`, `a/b`, `c`},
+               {`a/b/c/`, `a/b`, `c`},
+               {`\\?\c:\a`, `\\?\c:\`, `a`},
+       } {
+               if dir, base := os.SplitPath(tt.path); dir != tt.wantDir || base != tt.wantBase {
+                       t.Errorf("splitPath(%q) = %q, %q, want %q, %q", tt.path, dir, base, tt.wantDir, tt.wantBase)
+               }
+       }
+}

diff --git a/src/os/path_windows.go b/src/os/path_windows.go
index 03c5231b54735f5407d1ff9991605ef2fed1f4e8..8273d8b04727704763c7b2805c8537111220210f 100644 (file)
--- a/src/os/path_windows.go
+++ b/src/os/path_windows.go
@@ -23,11 +23,51 @@ func IsPathSeparator(c uint8) bool {
 
 // splitPath returns the base name and parent directory.
 func splitPath(path string) (string, string) {
-       dirname, basename := filepathlite.Split(path)
-       volnamelen := filepathlite.VolumeNameLen(dirname)
-       for len(dirname) > volnamelen && IsPathSeparator(dirname[len(dirname)-1]) {
-               dirname = dirname[:len(dirname)-1]
+       if path == "" {
+               return ".", "."
        }
+
+       // The first prefixlen bytes are part of the parent directory.
+       // The prefix consists of the volume name (if any) and the first \ (if significant).
+       prefixlen := filepathlite.VolumeNameLen(path)
+       if len(path) > prefixlen && IsPathSeparator(path[prefixlen]) {
+               if prefixlen == 0 {
+                       // This is a path relative to the current volume, like \foo.
+                       // Include the initial \ in the prefix.
+                       prefixlen = 1
+               } else if path[prefixlen-1] == ':' {
+                       // This is an absolute path on a named drive, like c:\foo.
+                       // Include the initial \ in the prefix.
+                       prefixlen++
+               }
+       }
+
+       i := len(path) - 1
+
+       // Remove trailing slashes.
+       for i >= prefixlen && IsPathSeparator(path[i]) {
+               i--
+       }
+       path = path[:i+1]
+
+       // Find the last path separator. The basename is what follows.
+       for i >= prefixlen && !IsPathSeparator(path[i]) {
+               i--
+       }
+       basename := path[i+1:]
+       if basename == "" {
+               basename = "."
+       }
+
+       // Remove trailing slashes. The remainder is dirname.
+       for i >= prefixlen && IsPathSeparator(path[i]) {
+               i--
+       }
+       dirname := path[:i+1]
+       if dirname == "" {
+               dirname = "."
+       }
+
        return dirname, basename
 }
 

diff --git a/src/os/removeall_test.go b/src/os/removeall_test.go
index 474db2cac83b6ef514f36b50540e51f77a42371f..bc439e4a5eb762952390999bea1463c7e884ead1 100644 (file)
--- a/src/os/removeall_test.go
+++ b/src/os/removeall_test.go
@@ -471,6 +471,27 @@ func TestRemoveAllNoFcntl(t *testing.T) {
        }
 }
 
+func TestRemoveAllTrailingSlash(t *testing.T) {
+       slashes := []string{"/"}
+       if runtime.GOOS == "windows" {
+               slashes = append(slashes, `\`)
+       }
+       for _, slash := range slashes {
+               dir := makefs(t, []string{
+                       "dir/a/file1",
+                       "dir/a/file2",
+                       "dir/file3",
+               })
+               path := dir + "/dir"
+               if err := RemoveAll(path + slash); err != nil {
+                       t.Fatal(err)
+               }
+               if _, err := Stat(path); !IsNotExist(err) {
+                       t.Errorf("after RemoveAll(%q), directory still exists", path+slash)
+               }
+       }
+}
+
 func BenchmarkRemoveAll(b *testing.B) {
        tmpDir := filepath.Join(b.TempDir(), "target")
        b.ReportAllocs()