From: qmuntal Date: Tue, 12 Mar 2024 11:26:53 +0000 (+0100) Subject: os: support UNC paths and .. segments in fixLongPath X-Git-Tag: go1.23rc1~850 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=2e8d84f148c69404b8eec86d9149785a3f4e3e92;p=gostls13.git os: support UNC paths and .. segments in fixLongPath This CL reimplements fixLongPath using syscall.GetFullPathName instead of a custom implementation that was not handling UNC paths and .. segments correctly. It also fixes a bug here multiple trailing \ were removed instead of replaced by a single one. The new implementation is slower than the previous one, as it does a syscall and needs to convert UTF-8 to UTF-16 (and back), but it is correct and should be fast enough for most use cases. goos: windows goarch: amd64 pkg: os cpu: Intel(R) Core(TM) i7-10850H CPU @ 2.70GHz │ old.txt │ new.txt │ │ sec/op │ sec/op vs base │ LongPath-12 1.007µ ± 53% 4.093µ ± 109% +306.41% (p=0.000 n=10) │ old.txt │ new.txt │ │ B/op │ B/op vs base │ LongPath-12 576.0 ± 0% 1376.0 ± 0% +138.89% (p=0.000 n=10) │ old.txt │ new.txt │ │ allocs/op │ allocs/op vs base │ LongPath-12 2.000 ± 0% 3.000 ± 0% +50.00% (p=0.000 n=10) Fixes #41734. Change-Id: Iced5cf47f56f6ab0ca74a6e2374c31a75100902d Reviewed-on: https://go-review.googlesource.com/c/go/+/570995 Reviewed-by: Damien Neil TryBot-Result: Gopher Robot Reviewed-by: David Chase LUCI-TryBot-Result: Go LUCI --- diff --git a/src/os/path_windows.go b/src/os/path_windows.go index 98139679d4..29766843c0 100644 --- a/src/os/path_windows.go +++ b/src/os/path_windows.go @@ -4,7 +4,10 @@ package os -import "internal/syscall/windows" +import ( + "internal/syscall/windows" + "syscall" +) const ( PathSeparator = '\\' // OS-specific path separator @@ -132,10 +135,8 @@ 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 path is not easily converted to -// the extended-length form (for example, if path is a relative path -// or contains .. elements), or is short enough, fixLongPath returns -// path unmodified. +// path limit imposed by Windows. If the path is short enough or is relative, +// fixLongPath returns path unmodified. // // See https://learn.microsoft.com/en-us/windows/win32/fileio/naming-a-file#maximum-path-length-limitation func fixLongPath(path string) string { @@ -159,19 +160,8 @@ func fixLongPath(path string) string { return path } - // The extended form begins with \\?\, as in - // \\?\c:\windows\foo.txt or \\?\UNC\server\share\foo.txt. - // The extended form disables evaluation of . and .. path - // elements and disables the interpretation of / as equivalent - // to \. The conversion here rewrites / to \ and elides - // . elements as well as trailing or duplicate separators. For - // simplicity it avoids the conversion entirely for relative - // paths or paths containing .. elements. For now, - // \\server\share paths are not converted to - // \\?\UNC\server\share paths because the rules for doing so - // are less well-specified. - if len(path) >= 2 && path[:2] == `\\` { - // Don't canonicalize UNC paths. + if prefix := path[:4]; prefix == `\\.\` || prefix == `\\?\` || prefix == `\??\` { + // Don't fix. Device path or extended path form. return path } if !isAbs(path) { @@ -179,36 +169,37 @@ func fixLongPath(path string) string { return path } - const prefix = `\\?` + var prefix []uint16 + var isUNC bool + if path[:2] == `\\` { + // UNC path, prepend the \\?\UNC\ prefix. + prefix = []uint16{'\\', '\\', '?', '\\', 'U', 'N', 'C', '\\'} + isUNC = true + } else { + prefix = []uint16{'\\', '\\', '?', '\\'} + } - pathbuf := make([]byte, len(prefix)+len(path)+len(`\`)) - copy(pathbuf, prefix) - n := len(path) - r, w := 0, len(prefix) - for r < n { - switch { - case IsPathSeparator(path[r]): - // empty block - r++ - case path[r] == '.' && (r+1 == n || IsPathSeparator(path[r+1])): - // /./ - r++ - case r+1 < n && path[r] == '.' && path[r+1] == '.' && (r+2 == n || IsPathSeparator(path[r+2])): - // /../ is currently unhandled + p, err := syscall.UTF16FromString(path) + if err != nil { + return path + } + n := uint32(len(p)) + var buf []uint16 + for { + buf = make([]uint16, n+uint32(len(prefix))) + n, err = syscall.GetFullPathName(&p[0], n, &buf[len(prefix)], nil) + if err != nil { return path - default: - pathbuf[w] = '\\' - w++ - for ; r < n && !IsPathSeparator(path[r]); r++ { - pathbuf[w] = path[r] - w++ - } + } + if n <= uint32(len(buf)-len(prefix)) { + buf = buf[:n+uint32(len(prefix))] + break } } - // A drive's root directory needs a trailing \ - if w == len(`\\?\c:`) { - pathbuf[w] = '\\' - w++ + if isUNC { + // Remove leading \\. + buf = buf[2:] } - return string(pathbuf[:w]) + copy(buf, prefix) + return syscall.UTF16ToString(buf) } diff --git a/src/os/path_windows_test.go b/src/os/path_windows_test.go index 6fa864a98d..fef9f62e55 100644 --- a/src/os/path_windows_test.go +++ b/src/os/path_windows_test.go @@ -16,10 +16,14 @@ import ( ) func TestFixLongPath(t *testing.T) { - if windows.CanUseLongPaths { - return - } - t.Parallel() + // 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 @@ -38,19 +42,20 @@ func TestFixLongPath(t *testing.T) { // cases below where it doesn't. {`C:\long\foo.txt`, `\\?\C:\long\foo.txt`}, {`C:/long/foo.txt`, `\\?\C:\long\foo.txt`}, - {`C:\long\foo\\bar\.\baz\\`, `\\?\C:\long\foo\bar\baz`}, - {`\\unc\path`, `\\unc\path`}, + {`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:\long\..\bar\baz`}, + {`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`}, } { in := strings.ReplaceAll(test.in, "long", veryLong) want := strings.ReplaceAll(test.want, "long", veryLong) if got := os.FixLongPath(in); got != want { got = strings.ReplaceAll(got, veryLong, "long") - t.Errorf("fixLongPath(%q) = %q; want %q", test.in, got, test.want) + t.Errorf("fixLongPath(%#q) = %#q; want %#q", test.in, got, test.want) } } } @@ -155,3 +160,11 @@ func TestMkdirAllVolumeNameAtRoot(t *testing.T) { volName := syscall.UTF16ToString(buf[:]) testMkdirAllAtRoot(t, volName) } + +func BenchmarkLongPath(b *testing.B) { + veryLong := `C:\l` + strings.Repeat("o", 248) + "ng" + b.ReportAllocs() + for i := 0; i < b.N; i++ { + os.FixLongPath(veryLong) + } +}