]> Cypherpunks repositories - gostls13.git/commitdiff
os: support UNC paths and .. segments in fixLongPath
authorqmuntal <quimmuntal@gmail.com>
Tue, 12 Mar 2024 11:26:53 +0000 (12:26 +0100)
committerQuim Muntal <quimmuntal@gmail.com>
Mon, 18 Mar 2024 18:31:42 +0000 (18:31 +0000)
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 <dneil@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: David Chase <drchase@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

src/os/path_windows.go
src/os/path_windows_test.go

index 98139679d452b31f3b94393ce772c75a6c44ff5f..29766843c091b9d819823e504a411460ee23d766 100644 (file)
@@ -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)
 }
index 6fa864a98ddb464207ef2f7472e91125f7e87f86..fef9f62e554fef738dd444d207ccd4852a00365b 100644 (file)
@@ -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)
+       }
+}