diff --git a/src/os/export_windows_test.go b/src/os/export_windows_test.go index 2e5904b3f5..aefbe4033e 100644 --- a/src/os/export_windows_test.go +++ b/src/os/export_windows_test.go @@ -7,7 +7,7 @@ package os // Export for testing. var ( - FixLongPath = fixLongPath + AddExtendedPrefix = addExtendedPrefix NewConsoleFile = newConsoleFile CommandLineToArgv = commandLineToArgv AllowReadDirFileID = &allowReadDirFileID diff --git a/src/os/file.go b/src/os/file.go index fae7bf1039..a41aac9bb3 100644 --- a/src/os/file.go +++ b/src/os/file.go @@ -337,6 +337,11 @@ func Chdir(dir string) error { testlog.Open(dir) // observe likely non-existent directory return &PathError{Op: "chdir", Path: dir, Err: e} } + if runtime.GOOS == "windows" { + getwdCache.Lock() + getwdCache.dir = dir + getwdCache.Unlock() + } if log := testlog.Logger(); log != nil { wd, err := Getwd() if err == nil { diff --git a/src/os/file_windows.go b/src/os/file_windows.go index a304a5e4a7..6ee15eb993 100644 --- a/src/os/file_windows.go +++ b/src/os/file_windows.go @@ -313,7 +313,18 @@ func Symlink(oldname, newname string) error { if err != nil { return &LinkError{"symlink", oldname, newname, err} } - o, err := syscall.UTF16PtrFromString(fixLongPath(oldname)) + var o *uint16 + if isAbs(oldname) { + o, err = syscall.UTF16PtrFromString(fixLongPath(oldname)) + } else { + // Do not use fixLongPath on oldname for relative symlinks, + // as it would turn the name into an absolute path thus making + // an absolute symlink instead. + // Notice that CreateSymbolicLinkW does not fail for relative + // symlinks beyond MAX_PATH, so this does not prevent the + // creation of an arbitrary long path name. + o, err = syscall.UTF16PtrFromString(oldname) + } if err != nil { return &LinkError{"symlink", oldname, newname, err} } diff --git a/src/os/path_windows.go b/src/os/path_windows.go index 29766843c0..e908d3ddf5 100644 --- a/src/os/path_windows.go +++ b/src/os/path_windows.go @@ -135,14 +135,33 @@ func dirname(path string) string { // fixLongPath returns the extended-length (\\?\-prefixed) form of // path when needed, in order to avoid the default 260 character file -// path limit imposed by Windows. If the path is short enough or is relative, -// fixLongPath returns path unmodified. +// path limit imposed by Windows. If the path is short enough or already +// has the extended-length prefix, fixLongPath returns path unmodified. +// If the path is relative and joining it with the current working +// directory results in a path that is too long, fixLongPath returns +// the absolute path with the extended-length prefix. // // See https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file#maximum-path-length-limitation func fixLongPath(path string) string { if windows.CanUseLongPaths { return path } + return addExtendedPrefix(path) +} + +// addExtendedPrefix adds the extended path prefix (\\?\) to path. +func addExtendedPrefix(path string) string { + if len(path) >= 4 { + if path[:4] == `\??\` { + // Already extended with \??\ + return path + } + if IsPathSeparator(path[0]) && IsPathSeparator(path[1]) && path[2] == '?' && IsPathSeparator(path[3]) { + // Already extended with \\?\ or any combination of directory separators. + return path + } + } + // Do nothing (and don't allocate) if the path is "short". // Empirically (at least on the Windows Server 2013 builder), // the kernel is arbitrarily okay with < 248 bytes. That @@ -154,27 +173,47 @@ func fixLongPath(path string) string { // // The MSDN docs appear to say that a normal path that is 248 bytes long // will work; empirically the path must be less then 248 bytes long. - if len(path) < 248 { + pathLength := len(path) + if !isAbs(path) { + // If the path is relative, we need to prepend the working directory + // plus a separator to the path before we can determine if it's too long. + // We don't want to call syscall.Getwd here, as that call is expensive to do + // every time fixLongPath is called with a relative path, so we use a cache. + // Note that getwdCache might be outdated if the working directory has been + // changed without using os.Chdir, i.e. using syscall.Chdir directly or cgo. + // This is fine, as the worst that can happen is that we fail to fix the path. + getwdCache.Lock() + if getwdCache.dir == "" { + // Init the working directory cache. + getwdCache.dir, _ = syscall.Getwd() + } + pathLength += len(getwdCache.dir) + 1 + getwdCache.Unlock() + } + + if pathLength < 248 { // Don't fix. (This is how Go 1.7 and earlier worked, // not automatically generating the \\?\ form) return path } - if prefix := path[:4]; prefix == `\\.\` || prefix == `\\?\` || prefix == `\??\` { - // Don't fix. Device path or extended path form. - return path + var isUNC, isDevice bool + if len(path) >= 2 && IsPathSeparator(path[0]) && IsPathSeparator(path[1]) { + if len(path) >= 4 && path[2] == '.' && IsPathSeparator(path[3]) { + // Starts with //./ + isDevice = true + } else { + // Starts with // + isUNC = true + } } - if !isAbs(path) { - // Relative path - return path - } - var prefix []uint16 - var isUNC bool - if path[:2] == `\\` { + if isUNC { // UNC path, prepend the \\?\UNC\ prefix. prefix = []uint16{'\\', '\\', '?', '\\', 'U', 'N', 'C', '\\'} - isUNC = true + } else if isDevice { + // Don't add the extended prefix to device paths, as it would + // change its meaning. } else { prefix = []uint16{'\\', '\\', '?', '\\'} } @@ -183,7 +222,10 @@ func fixLongPath(path string) string { if err != nil { return path } - n := uint32(len(p)) + // Estimate the required buffer size using the path length plus the null terminator. + // pathLength includes the working directory. This should be accurate unless + // the working directory has changed without using os.Chdir. + n := uint32(pathLength) + 1 var buf []uint16 for { buf = make([]uint16, n+uint32(len(prefix))) @@ -194,6 +236,8 @@ func fixLongPath(path string) string { if n <= uint32(len(buf)-len(prefix)) { buf = buf[:n+uint32(len(prefix))] break + } else { + continue } } if isUNC { diff --git a/src/os/path_windows_test.go b/src/os/path_windows_test.go index fef9f62e55..b37cae52b3 100644 --- a/src/os/path_windows_test.go +++ b/src/os/path_windows_test.go @@ -15,47 +15,106 @@ import ( "testing" ) -func TestFixLongPath(t *testing.T) { - // Test fixLongPath even if long path are supported by the system, - // else the function might not be tested at all when the test builders - // support long paths. - old := windows.CanUseLongPaths - windows.CanUseLongPaths = false - t.Cleanup(func() { - windows.CanUseLongPaths = old - }) - - // 248 is long enough to trigger the longer-than-248 checks in - // fixLongPath, but short enough not to make a path component - // longer than 255, which is illegal on Windows. (which - // doesn't really matter anyway, since this is purely a string - // function we're testing, and it's not actually being used to - // do a system call) - veryLong := "l" + strings.Repeat("o", 248) + "ng" +func TestAddExtendedPrefix(t *testing.T) { + // Test addExtendedPrefix instead of fixLongPath so the path manipulation code + // is exercised even if long path are supported by the system, else the + // function might not be tested at all if/when all test builders support long paths. + cwd, err := os.Getwd() + if err != nil { + t.Fatal("cannot get cwd") + } + drive := strings.ToLower(filepath.VolumeName(cwd)) + cwd = strings.ToLower(cwd[len(drive)+1:]) + // Build a very long pathname. Paths in Go are supposed to be arbitrarily long, + // so let's make a long path which is comfortably bigger than MAX_PATH on Windows + // (256) and thus requires fixLongPath to be correctly interpreted in I/O syscalls. + veryLong := "l" + strings.Repeat("o", 500) + "ng" for _, test := range []struct{ in, want string }{ - // Short; unchanged: + // Testcases use word subsitutions: + // * "long" is replaced with a very long pathname + // * "c:" or "C:" are replaced with the drive of the current directory (preserving case) + // * "cwd" is replaced with the current directory + + // Drive Absolute + {`C:\long\foo.txt`, `\\?\C:\long\foo.txt`}, + {`C:/long/foo.txt`, `\\?\C:\long\foo.txt`}, + {`C:\\\long///foo.txt`, `\\?\C:\long\foo.txt`}, + {`C:\long\.\foo.txt`, `\\?\C:\long\foo.txt`}, + {`C:\long\..\foo.txt`, `\\?\C:\foo.txt`}, + {`C:\long\..\..\foo.txt`, `\\?\C:\foo.txt`}, + + // Drive Relative + {`C:long\foo.txt`, `\\?\C:\cwd\long\foo.txt`}, + {`C:long/foo.txt`, `\\?\C:\cwd\long\foo.txt`}, + {`C:long///foo.txt`, `\\?\C:\cwd\long\foo.txt`}, + {`C:long\.\foo.txt`, `\\?\C:\cwd\long\foo.txt`}, + {`C:long\..\foo.txt`, `\\?\C:\cwd\foo.txt`}, + + // Rooted + {`\long\foo.txt`, `\\?\C:\long\foo.txt`}, + {`/long/foo.txt`, `\\?\C:\long\foo.txt`}, + {`\long///foo.txt`, `\\?\C:\long\foo.txt`}, + {`\long\.\foo.txt`, `\\?\C:\long\foo.txt`}, + {`\long\..\foo.txt`, `\\?\C:\foo.txt`}, + + // Relative + {`long\foo.txt`, `\\?\C:\cwd\long\foo.txt`}, + {`long/foo.txt`, `\\?\C:\cwd\long\foo.txt`}, + {`long///foo.txt`, `\\?\C:\cwd\long\foo.txt`}, + {`long\.\foo.txt`, `\\?\C:\cwd\long\foo.txt`}, + {`long\..\foo.txt`, `\\?\C:\cwd\foo.txt`}, + {`.\long\foo.txt`, `\\?\C:\cwd\long\foo.txt`}, + + // UNC Absolute + {`\\srv\share\long`, `\\?\UNC\srv\share\long`}, + {`//srv/share/long`, `\\?\UNC\srv\share\long`}, + {`/\srv/share/long`, `\\?\UNC\srv\share\long`}, + {`\\srv\share\long\`, `\\?\UNC\srv\share\long\`}, + {`\\srv\share\bar\.\long`, `\\?\UNC\srv\share\bar\long`}, + {`\\srv\share\bar\..\long`, `\\?\UNC\srv\share\long`}, + {`\\srv\share\bar\..\..\long`, `\\?\UNC\srv\share\long`}, // share name is not removed by ".." + + // Local Device + {`\\.\C:\long\foo.txt`, `\\.\C:\long\foo.txt`}, + {`//./C:/long/foo.txt`, `\\.\C:\long\foo.txt`}, + {`/\./C:/long/foo.txt`, `\\.\C:\long\foo.txt`}, + {`\\.\C:\long///foo.txt`, `\\.\C:\long\foo.txt`}, + {`\\.\C:\long\.\foo.txt`, `\\.\C:\long\foo.txt`}, + {`\\.\C:\long\..\foo.txt`, `\\.\C:\foo.txt`}, + + // Misc tests {`C:\short.txt`, `C:\short.txt`}, {`C:\`, `C:\`}, {`C:`, `C:`}, - // The "long" substring is replaced by a looooooong - // string which triggers the rewriting. Except in the - // cases below where it doesn't. - {`C:\long\foo.txt`, `\\?\C:\long\foo.txt`}, - {`C:/long/foo.txt`, `\\?\C:\long\foo.txt`}, + {`\\srv\path`, `\\srv\path`}, + {`long.txt`, `\\?\C:\cwd\long.txt`}, + {`C:long.txt`, `\\?\C:\cwd\long.txt`}, + {`C:\long\.\bar\baz`, `\\?\C:\long\bar\baz`}, + {`C:long\.\bar\baz`, `\\?\C:\cwd\long\bar\baz`}, + {`C:\long\..\bar\baz`, `\\?\C:\bar\baz`}, + {`C:long\..\bar\baz`, `\\?\C:\cwd\bar\baz`}, {`C:\long\foo\\bar\.\baz\\`, `\\?\C:\long\foo\bar\baz\`}, - {`\\server\path\long`, `\\?\UNC\server\path\long`}, - {`long.txt`, `long.txt`}, - {`C:long.txt`, `C:long.txt`}, - {`c:\long\..\bar\baz`, `\\?\c:\bar\baz`}, - {`\\?\c:\long\foo.txt`, `\\?\c:\long\foo.txt`}, - {`\\?\c:\long/foo.txt`, `\\?\c:\long/foo.txt`}, - {`\??\c:\long/foo.txt`, `\??\c:\long/foo.txt`}, + {`C:\long\..`, `\\?\C:\`}, + {`C:\.\long\..\.`, `\\?\C:\`}, + {`\\?\C:\long\foo.txt`, `\\?\C:\long\foo.txt`}, + {`\\?\C:\long/foo.txt`, `\\?\C:\long/foo.txt`}, } { in := strings.ReplaceAll(test.in, "long", veryLong) + in = strings.ToLower(in) + in = strings.ReplaceAll(in, "c:", drive) + want := strings.ReplaceAll(test.want, "long", veryLong) - if got := os.FixLongPath(in); got != want { + want = strings.ToLower(want) + want = strings.ReplaceAll(want, "c:", drive) + want = strings.ReplaceAll(want, "cwd", cwd) + + got := os.AddExtendedPrefix(in) + got = strings.ToLower(got) + if got != want { + in = strings.ReplaceAll(in, veryLong, "long") got = strings.ReplaceAll(got, veryLong, "long") - t.Errorf("fixLongPath(%#q) = %#q; want %#q", test.in, got, test.want) + want = strings.ReplaceAll(want, veryLong, "long") + t.Errorf("addExtendedPrefix(%#q) = %#q; want %#q", in, got, want) } } } @@ -161,10 +220,61 @@ func TestMkdirAllVolumeNameAtRoot(t *testing.T) { testMkdirAllAtRoot(t, volName) } -func BenchmarkLongPath(b *testing.B) { +func TestRemoveAllLongPathRelative(t *testing.T) { + // Test that RemoveAll doesn't hang with long relative paths. + // See go.dev/issue/36375. + tmp := t.TempDir() + chdir(t, tmp) + dir := filepath.Join(tmp, "foo", "bar", strings.Repeat("a", 150), strings.Repeat("b", 150)) + err := os.MkdirAll(dir, 0755) + if err != nil { + t.Fatal(err) + } + err = os.RemoveAll("foo") + if err != nil { + t.Fatal(err) + } +} + +func testLongPathAbs(t *testing.T, target string) { + t.Helper() + testWalkFn := func(path string, info os.FileInfo, err error) error { + if err != nil { + t.Error(err) + } + return err + } + if err := os.MkdirAll(target, 0777); err != nil { + t.Fatal(err) + } + // Test that Walk doesn't fail with long paths. + // See go.dev/issue/21782. + filepath.Walk(target, testWalkFn) + // Test that RemoveAll doesn't hang with long paths. + // See go.dev/issue/36375. + if err := os.RemoveAll(target); err != nil { + t.Error(err) + } +} + +func TestLongPathAbs(t *testing.T) { + t.Parallel() + + target := t.TempDir() + "\\" + strings.Repeat("a\\", 300) + testLongPathAbs(t, target) +} + +func TestLongPathRel(t *testing.T) { + chdir(t, t.TempDir()) + + target := strings.Repeat("b\\", 300) + testLongPathAbs(t, target) +} + +func BenchmarkAddExtendedPrefix(b *testing.B) { veryLong := `C:\l` + strings.Repeat("o", 248) + "ng" b.ReportAllocs() for i := 0; i < b.N; i++ { - os.FixLongPath(veryLong) + os.AddExtendedPrefix(veryLong) } }