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>

diff --git a/src/os/os_test.go b/src/os/os_test.go
index b1e20b78398191004e14bfd22bc74b74cbb6aec6..b7300cd38c468243959ff271b65a8c9222c07d2d 100644 (file)
--- a/src/os/os_test.go
+++ b/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)
+                               }
+                       }
+               })
        }
 }
 

diff --git a/src/os/path_windows.go b/src/os/path_windows.go
index ccac1c0b64b6ab87cbe4f100a37ac56c026fad49..101b026dc9e1b89bf09cfcab8a8e377f65a35103 100644 (file)
--- a/src/os/path_windows.go
+++ b/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