From 2925427a47f41622f28f84889ad7aade27581144 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Tue, 8 Nov 2016 01:06:06 +0000 Subject: os: on Windows, don't fix long paths that aren't long Notably, don't allocate. Follow-up to https://golang.org/cl/32451 which added long path cleaning. Updates #3358 Change-Id: I89c59cbd660d0a030f31b6acd070fa9f3250683b Reviewed-on: https://go-review.googlesource.com/32886 Run-TryBot: Brad Fitzpatrick Reviewed-by: Russ Cox --- src/os/path_windows.go | 32 +++++++++++++++++++++++--------- src/os/path_windows_test.go | 37 +++++++++++++++++++++++++++---------- 2 files changed, 50 insertions(+), 19 deletions(-) diff --git a/src/os/path_windows.go b/src/os/path_windows.go index 1a4223deab..ccac1c0b64 100644 --- a/src/os/path_windows.go +++ b/src/os/path_windows.go @@ -129,26 +129,40 @@ func dirname(path string) string { } // fixLongPath returns the extended-length (\\?\-prefixed) form of -// path if possible, in order to avoid the default 260 character file -// path limit imposed by Windows. If path is not easily converted to +// 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), fixLongPath returns path unmodified. +// or contains .. elements), or is short enough, 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 { + // 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 + // matches what the docs above say: + // "When using an API to create a directory, the specified + // path cannot be so long that you cannot append an 8.3 file + // name (that is, the directory name cannot exceed MAX_PATH + // minus 12)." Since MAX_PATH is 260, 260 - 12 = 248. + if len(path) <= 248 { + // Don't fix. (This is how Go 1.7 and earlier worked, + // not automatically generating the \\?\ form) + 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 + // 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, + // 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. - // - // For details of \\?\ paths, see: - // https://msdn.microsoft.com/en-us/library/windows/desktop/aa365247(v=vs.85).aspx#maxpath - if len(path) == 0 || (len(path) >= 2 && path[:2] == `\\`) { + if len(path) >= 2 && path[:2] == `\\` { // Don't canonicalize UNC paths. return path } diff --git a/src/os/path_windows_test.go b/src/os/path_windows_test.go index 8fd515728e..cce0bdd522 100644 --- a/src/os/path_windows_test.go +++ b/src/os/path_windows_test.go @@ -6,23 +6,40 @@ package os_test import ( "os" + "strings" "testing" ) func TestFixLongPath(t *testing.T) { + // 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" for _, test := range []struct{ in, want string }{ - {`C:\foo.txt`, `\\?\C:\foo.txt`}, - {`C:/foo.txt`, `\\?\C:\foo.txt`}, - {`C:\foo\\bar\.\baz\\`, `\\?\C:\foo\bar\baz`}, - {`C:\`, `\\?\C:\`}, // drives must have a trailing slash + // Short; unchanged: + {`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`}, + {`C:\long\foo\\bar\.\baz\\`, `\\?\C:\long\foo\bar\baz`}, {`\\unc\path`, `\\unc\path`}, - {`foo.txt`, `foo.txt`}, - {`C:foo.txt`, `C:foo.txt`}, - {`c:\foo\..\bar\baz`, `c:\foo\..\bar\baz`}, - {`\\?\c:\windows\foo.txt`, `\\?\c:\windows\foo.txt`}, - {`\\?\c:\windows/foo.txt`, `\\?\c:\windows/foo.txt`}, + {`long.txt`, `long.txt`}, + {`C:long.txt`, `C:long.txt`}, + {`c:\long\..\bar\baz`, `c:\long\..\bar\baz`}, + {`\\?\c:\long\foo.txt`, `\\?\c:\long\foo.txt`}, + {`\\?\c:\long/foo.txt`, `\\?\c:\long/foo.txt`}, } { - if got := os.FixLongPath(test.in); got != test.want { + in := strings.Replace(test.in, "long", veryLong, -1) + want := strings.Replace(test.want, "long", veryLong, -1) + if got := os.FixLongPath(in); got != want { + got = strings.Replace(got, veryLong, "long", -1) t.Errorf("fixLongPath(%q) = %q; want %q", test.in, got, test.want) } } -- cgit v1.2.3-54-g00ecf