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 <bradfitz@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>

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)
 		}
 	}