Merge pull request #1182 from microsoft/dev/qmuntal/apiscap1.21lp

[release-branch.go1.21] Remove long path support hack
microsoft · Apr 8, 2024 · 62ac79b · 62ac79b
2 parents 8ff847d + e471034
commit 62ac79b
Show file tree

Hide file tree

Showing 3 changed files with 726 additions and 0 deletions.
diff --git a/patches/0012-remove-long-path-support-hack.patch b/patches/0012-remove-long-path-support-hack.patch
@@ -0,0 +1,95 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: qmuntal <qmuntaldiaz@microsoft.com>
+Date: Wed, 3 Apr 2024 14:27:11 +0200
+Subject: [PATCH] remove long path support hack
+
+Upstream Go tricks Windows into enabling long path support by setting an
+undocumented flag in the PEB. The Microsoft Go fork can't use undocumented
+APIs, so this commit removes the hack.
+
+There is no documented way to enable long path support from within the
+process, so this this is a breaking change for the Microsoft Go fork.
+Note that the Go standard library makes a best effort to support long
+paths by using the `\\?\` prefix when possible, so this change should
+only affect long relative paths, which can't be used with the `\\?\`.
+---
+ src/runtime/os_windows.go | 52 +--------------------------------------
+ 1 file changed, 1 insertion(+), 51 deletions(-)
+
+diff --git a/src/runtime/os_windows.go b/src/runtime/os_windows.go
+index 766b087c18dd02..4cb57232ed4a03 100644
+--- a/src/runtime/os_windows.go
++++ b/src/runtime/os_windows.go
+@@ -134,7 +134,6 @@ var (
+ 	// links wrong printf function to cgo executable (see issue
+ 	// 12030 for details).
+ 	_NtWaitForSingleObject stdFunction
+-	_RtlGetCurrentPeb      stdFunction
+ 	_RtlGetVersion         stdFunction
+
+ 	// These are from non-kernel32.dll, so we prefer to LoadLibraryEx them.
+@@ -260,7 +259,6 @@ func loadOptionalSyscalls() {
+ 		throw("ntdll.dll not found")
+ 	}
+ 	_NtWaitForSingleObject = windowsFindfunc(n32, []byte("NtWaitForSingleObject\000"))
+-	_RtlGetCurrentPeb = windowsFindfunc(n32, []byte("RtlGetCurrentPeb\000"))
+ 	_RtlGetVersion = windowsFindfunc(n32, []byte("RtlGetVersion\000"))
+
+ 	if !haveCputicksAsm {
+@@ -460,55 +458,7 @@ var longFileName [(_MAX_PATH+1)*2 + 1]byte
+ // canUseLongPaths is set to true, and later when called, os.fixLongPath
+ // returns early without doing work.
+ func initLongPathSupport() {
+-	const (
+-		IsLongPathAwareProcess = 0x80
+-		PebBitFieldOffset      = 3
+-		OPEN_EXISTING          = 3
+-		ERROR_PATH_NOT_FOUND   = 3
+-	)
+-
+-	// Check that we're ≥ 10.0.15063.
+-	info := _OSVERSIONINFOW{}
+-	info.osVersionInfoSize = uint32(unsafe.Sizeof(info))
+-	stdcall1(_RtlGetVersion, uintptr(unsafe.Pointer(&info)))
+-	if info.majorVersion < 10 || (info.majorVersion == 10 && info.minorVersion == 0 && info.buildNumber < 15063) {
+-		return
+-	}
+-
+-	// Set the IsLongPathAwareProcess flag of the PEB's bit field.
+-	bitField := (*byte)(unsafe.Pointer(stdcall0(_RtlGetCurrentPeb) + PebBitFieldOffset))
+-	originalBitField := *bitField
+-	*bitField |= IsLongPathAwareProcess
+-
+-	// Check that this actually has an effect, by constructing a large file
+-	// path and seeing whether we get ERROR_PATH_NOT_FOUND, rather than
+-	// some other error, which would indicate the path is too long, and
+-	// hence long path support is not successful. This whole section is NOT
+-	// strictly necessary, but is a nice validity check for the near to
+-	// medium term, when this functionality is still relatively new in
+-	// Windows.
+-	getRandomData(longFileName[len(longFileName)-33 : len(longFileName)-1])
+-	start := copy(longFileName[:], sysDirectory[:sysDirectoryLen])
+-	const dig = "0123456789abcdef"
+-	for i := 0; i < 32; i++ {
+-		longFileName[start+i*2] = dig[longFileName[len(longFileName)-33+i]>>4]
+-		longFileName[start+i*2+1] = dig[longFileName[len(longFileName)-33+i]&0xf]
+-	}
+-	start += 64
+-	for i := start; i < len(longFileName)-1; i++ {
+-		longFileName[i] = 'A'
+-	}
+-	stdcall7(_CreateFileA, uintptr(unsafe.Pointer(&longFileName[0])), 0, 0, 0, OPEN_EXISTING, 0, 0)
+-	// The ERROR_PATH_NOT_FOUND error value is distinct from
+-	// ERROR_FILE_NOT_FOUND or ERROR_INVALID_NAME, the latter of which we
+-	// expect here due to the final component being too long.
+-	if getlasterror() == ERROR_PATH_NOT_FOUND {
+-		*bitField = originalBitField
+-		println("runtime: warning: IsLongPathAwareProcess failed to enable long paths; proceeding in fixup mode")
+-		return
+-	}
+-
+-	canUseLongPaths = true
++	canUseLongPaths = false
+ }
+
+ func osinit() {
diff --git a/patches/0013-os-support-UNC-paths-and-.-segments-in-fixLongPath.patch b/patches/0013-os-support-UNC-paths-and-.-segments-in-fixLongPath.patch
@@ -0,0 +1,211 @@
+From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
+From: qmuntal <qmuntaldiaz@microsoft.com>
+Date: Tue, 12 Mar 2024 12:26:53 +0100
+Subject: [PATCH] os: support UNC paths and .. segments in fixLongPath
+MIME-Version: 1.0
+Content-Type: text/plain; charset=UTF-8
+Content-Transfer-Encoding: 8bit
+
+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>
+(cherry picked from commit 2e8d84f148c69404b8eec86d9149785a3f4e3e92)
+---
+ src/os/path_windows.go      | 80 ++++++++++++++++---------------------
+ src/os/path_windows_test.go | 21 ++++++----
+ 2 files changed, 48 insertions(+), 53 deletions(-)
+
+diff --git a/src/os/path_windows.go b/src/os/path_windows.go
+index 3356908a3609ec..a8147f4e431f3b 100644
+--- a/src/os/path_windows.go
++++ b/src/os/path_windows.go
+@@ -4,6 +4,8 @@
+
+ package os
+
++import "syscall"
++
+ const (
+ 	PathSeparator     = '\\' // OS-specific path separator
+ 	PathListSeparator = ';'  // OS-specific path list separator
+@@ -134,10 +136,8 @@ var canUseLongPaths bool
+
+ // 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://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx#maxpath
+ func fixLongPath(path string) string {
+@@ -161,19 +161,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) {
+@@ -181,38 +170,39 @@ func fixLongPath(path string) string {
+ 		return path
+ 	}
+
+-	const prefix = `\\?`
+-
+-	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
++	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{'\\', '\\', '?', '\\'}
++	}
++
++	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)
+ }
+
+ // fixRootDirectory fixes a reference to a drive's root directory to
+diff --git a/src/os/path_windows_test.go b/src/os/path_windows_test.go
+index 2506b4f0d8dca7..3ed43986bba4d8 100644
+--- a/src/os/path_windows_test.go
++++ b/src/os/path_windows_test.go
+@@ -12,10 +12,14 @@ import (
+ )
+
+ func TestFixLongPath(t *testing.T) {
+-	if os.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 := os.CanUseLongPaths
++	os.CanUseLongPaths = false
++	t.Cleanup(func() {
++		os.CanUseLongPaths = old
++	})
+
+ 	// 248 is long enough to trigger the longer-than-248 checks in
+ 	// fixLongPath, but short enough not to make a path component
+@@ -34,19 +38,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)
+ 		}
+ 	}
+ }