os: must fixup path when 248 bytes long

Fixes #18283

Change-Id: Ic044d2d0657579e8e7786d7264fda2037ddc5ffb
Reviewed-on: https://go-review.googlesource.com/34280
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>

src/os/os_test.go

@@ -1708,51 +1708,62 @@ func TestLongPath(t *testing.T) {
 			t.Fatalf("RemoveAll failed: %v", err)
 		}
 	}(tmpdir)
+
+	// Test the boundary of 247 and fewer bytes (normal) and 248 and more bytes (adjusted).
+	sizes := []int{247, 248, 249, 400}
 	for len(tmpdir) < 400 {
 		tmpdir += "/dir3456789"
 	}
-	if err := MkdirAll(tmpdir, 0755); err != nil {
-		t.Fatalf("MkdirAll failed: %v", err)
-	}
-	data := []byte("hello world\n")
-	if err := ioutil.WriteFile(tmpdir+"/foo.txt", data, 0644); err != nil {
-		t.Fatalf("ioutil.WriteFile() failed: %v", err)
-	}
-	if err := Rename(tmpdir+"/foo.txt", tmpdir+"/bar.txt"); err != nil {
-		t.Fatalf("Rename failed: %v", err)
-	}
-	mtime := time.Now().Truncate(time.Minute)
-	if err := Chtimes(tmpdir+"/bar.txt", mtime, mtime); err != nil {
-		t.Fatalf("Chtimes failed: %v", err)
-	}
-	names := []string{"bar.txt"}
-	if testenv.HasSymlink() {
-		if err := Symlink(tmpdir+"/bar.txt", tmpdir+"/symlink.txt"); err != nil {
-			t.Fatalf("Symlink failed: %v", err)
-		}
-		names = append(names, "symlink.txt")
-	}
-	if testenv.HasLink() {
-		if err := Link(tmpdir+"/bar.txt", tmpdir+"/link.txt"); err != nil {
-			t.Fatalf("Link failed: %v", err)
-		}
-		names = append(names, "link.txt")
-	}
-	for _, wantSize := range []int64{int64(len(data)), 0} {
-		for _, name := range names {
-			path := tmpdir + "/" + name
-			dir, err := Stat(path)
-			if err != nil {
-				t.Fatalf("Stat(%q) failed: %v", path, err)
+	for _, sz := range sizes {
+		t.Run(fmt.Sprintf("length=%d", sz), func(t *testing.T) {
+			sizedTempDir := tmpdir[:sz-1] + "x" // Ensure it does not end with a slash.
+
+			// The various sized runs are for this call to trigger the boundary
+			// condition.
+			if err := MkdirAll(sizedTempDir, 0755); err != nil {
+				t.Fatalf("MkdirAll failed: %v", err)
 			}
-			filesize := size(path, t)
-			if dir.Size() != filesize || filesize != wantSize {
-				t.Errorf("Size(%q) is %d, len(ReadFile()) is %d, want %d", path, dir.Size(), filesize, wantSize)
+			data := []byte("hello world\n")
+			if err := ioutil.WriteFile(sizedTempDir+"/foo.txt", data, 0644); err != nil {
+				t.Fatalf("ioutil.WriteFile() failed: %v", err)
 			}
-		}
-		if err := Truncate(tmpdir+"/bar.txt", 0); err != nil {
-			t.Fatalf("Truncate failed: %v", err)
-		}
+			if err := Rename(sizedTempDir+"/foo.txt", sizedTempDir+"/bar.txt"); err != nil {
+				t.Fatalf("Rename failed: %v", err)
+			}
+			mtime := time.Now().Truncate(time.Minute)
+			if err := Chtimes(sizedTempDir+"/bar.txt", mtime, mtime); err != nil {
+				t.Fatalf("Chtimes failed: %v", err)
+			}
+			names := []string{"bar.txt"}
+			if testenv.HasSymlink() {
+				if err := Symlink(sizedTempDir+"/bar.txt", sizedTempDir+"/symlink.txt"); err != nil {
+					t.Fatalf("Symlink failed: %v", err)
+				}
+				names = append(names, "symlink.txt")
+			}
+			if testenv.HasLink() {
+				if err := Link(sizedTempDir+"/bar.txt", sizedTempDir+"/link.txt"); err != nil {
+					t.Fatalf("Link failed: %v", err)
+				}
+				names = append(names, "link.txt")
+			}
+			for _, wantSize := range []int64{int64(len(data)), 0} {
+				for _, name := range names {
+					path := sizedTempDir + "/" + name
+					dir, err := Stat(path)
+					if err != nil {
+						t.Fatalf("Stat(%q) failed: %v", path, err)
+					}
+					filesize := size(path, t)
+					if dir.Size() != filesize || filesize != wantSize {
+						t.Errorf("Size(%q) is %d, len(ReadFile()) is %d, want %d", path, dir.Size(), filesize, wantSize)
+					}
+				}
+				if err := Truncate(sizedTempDir+"/bar.txt", 0); err != nil {
+					t.Fatalf("Truncate failed: %v", err)
+				}
+			}
+		})
 	}
 }
 

src/os/path_windows.go

@@ -139,13 +139,16 @@ func dirname(path string) string {
 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
+	// 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 {
+	//
+	// The MSDN docs appear to say that a normal path that is 248 bytes long
+	// will work; empirically the path must be less then 248 bytes long.
+	if len(path) < 248 {
 		// Don't fix. (This is how Go 1.7 and earlier worked,
 		// not automatically generating the \\?\ form)
 		return path