]> Cypherpunks repositories - gostls13.git/commitdiff
os: use FILE_FLAG_OPEN_REPARSE_POINT in SameFile
authorAlex Brainman <alex.brainman@gmail.com>
Sat, 8 Sep 2018 06:05:29 +0000 (16:05 +1000)
committerAlex Brainman <alex.brainman@gmail.com>
Sat, 29 Sep 2018 04:02:38 +0000 (04:02 +0000)
SameFile opens file to discover identifier and volume serial
number that uniquely identify the file. SameFile uses Windows
CreateFile API to open the file, and that works well for files
and directories. But CreateFile always follows symlinks, so
SameFile always opens symlink target instead of symlink itself.

This CL uses FILE_FLAG_OPEN_REPARSE_POINT flag to adjust
CreateFile behavior when handling symlinks.

As per https://docs.microsoft.com/en-us/windows/desktop/FileIO/symbolic-link-effects-on-file-systems-functions#createfile-and-createfiletransacted

"... If FILE_FLAG_OPEN_REPARSE_POINT is specified and:

If an existing file is opened and it is a symbolic link, the handle
returned is a handle to the symbolic link. ...".

I also added new tests for both issue #21854 and #27225.
Issue #27225 is still to be fixed, so skipping the test on
windows for the moment.

Fixes #21854
Updates #27225

Change-Id: I8aaa13ad66ce3b4074991bb50994d2aeeeaa7c95
Reviewed-on: https://go-review.googlesource.com/134195
Run-TryBot: Alex Brainman <alex.brainman@gmail.com>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@golang.org>
src/os/os_windows_test.go
src/os/stat_test.go [new file with mode: 0644]
src/os/types_windows.go

index 8984dd2c6623dba179ec56c0975660f3d6cc0807..c5553694880eefb4a1d2a7612fb0bbf190de4d0f 100644 (file)
@@ -895,16 +895,6 @@ func main() {
        }
 }
 
-func testIsDir(t *testing.T, path string, fi os.FileInfo) {
-       t.Helper()
-       if !fi.IsDir() {
-               t.Errorf("%q should be a directory", path)
-       }
-       if fi.Mode()&os.ModeSymlink != 0 {
-               t.Errorf("%q should not be a symlink", path)
-       }
-}
-
 func findOneDriveDir() (string, error) {
        // as per https://stackoverflow.com/questions/42519624/how-to-determine-location-of-onedrive-on-windows-7-and-8-in-c
        const onedrivekey = `SOFTWARE\Microsoft\OneDrive`
@@ -927,57 +917,7 @@ func TestOneDrive(t *testing.T) {
        if err != nil {
                t.Skipf("Skipping, because we did not find OneDrive directory: %v", err)
        }
-
-       // test os.Stat
-       fi, err := os.Stat(dir)
-       if err != nil {
-               t.Fatal(err)
-       }
-       testIsDir(t, dir, fi)
-
-       // test os.Lstat
-       fi, err = os.Lstat(dir)
-       if err != nil {
-               t.Fatal(err)
-       }
-       testIsDir(t, dir, fi)
-
-       // test os.File.Stat
-       f, err := os.Open(dir)
-       if err != nil {
-               t.Fatal(err)
-       }
-       defer f.Close()
-
-       fi, err = f.Stat()
-       if err != nil {
-               t.Fatal(err)
-       }
-       testIsDir(t, dir, fi)
-
-       // test os.FileInfo returned by os.Readdir
-       parent, err := os.Open(filepath.Dir(dir))
-       if err != nil {
-               t.Fatal(err)
-       }
-       defer parent.Close()
-
-       fis, err := parent.Readdir(-1)
-       if err != nil {
-               t.Fatal(err)
-       }
-       fi = nil
-       base := filepath.Base(dir)
-       for _, fi2 := range fis {
-               if fi2.Name() == base {
-                       fi = fi2
-                       break
-               }
-       }
-       if fi == nil {
-               t.Errorf("failed to find %q in its parent", dir)
-       }
-       testIsDir(t, dir, fi)
+       testDirStats(t, dir)
 }
 
 func TestWindowsDevNullFile(t *testing.T) {
diff --git a/src/os/stat_test.go b/src/os/stat_test.go
new file mode 100644 (file)
index 0000000..d59edeb
--- /dev/null
@@ -0,0 +1,276 @@
+// Copyright 2018 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+package os_test
+
+import (
+       "internal/testenv"
+       "io/ioutil"
+       "os"
+       "path/filepath"
+       "runtime"
+       "testing"
+)
+
+// testStatAndLstat verifies that all os.Stat, os.Lstat os.File.Stat and os.Readdir work.
+func testStatAndLstat(t *testing.T, path string, isLink bool, statCheck, lstatCheck func(*testing.T, string, os.FileInfo)) {
+       // test os.Stat
+       sfi, err := os.Stat(path)
+       if err != nil {
+               t.Error(err)
+               return
+       }
+       statCheck(t, path, sfi)
+
+       // test os.Lstat
+       lsfi, err := os.Lstat(path)
+       if err != nil {
+               t.Error(err)
+               return
+       }
+       lstatCheck(t, path, lsfi)
+
+       if isLink {
+               if os.SameFile(sfi, lsfi) {
+                       t.Errorf("stat and lstat of %q should not be the same", path)
+               }
+       } else {
+               if !os.SameFile(sfi, lsfi) {
+                       t.Errorf("stat and lstat of %q should be the same", path)
+               }
+       }
+
+       // test os.File.Stat
+       f, err := os.Open(path)
+       if err != nil {
+               t.Error(err)
+               return
+       }
+       defer f.Close()
+
+       sfi2, err := f.Stat()
+       if err != nil {
+               t.Error(err)
+               return
+       }
+       statCheck(t, path, sfi2)
+
+       if !os.SameFile(sfi, sfi2) {
+               t.Errorf("stat of open %q file and stat of %q should be the same", path, path)
+       }
+
+       if isLink {
+               if os.SameFile(sfi2, lsfi) {
+                       t.Errorf("stat of opened %q file and lstat of %q should not be the same", path, path)
+               }
+       } else {
+               if !os.SameFile(sfi2, lsfi) {
+                       t.Errorf("stat of opened %q file and lstat of %q should be the same", path, path)
+               }
+       }
+
+       // test os.FileInfo returned by os.Readdir
+       if len(path) > 0 && os.IsPathSeparator(path[len(path)-1]) {
+               // skip os.Readdir test of directories with slash at the end
+               return
+       }
+       parentdir := filepath.Dir(path)
+       parent, err := os.Open(parentdir)
+       if err != nil {
+               t.Error(err)
+               return
+       }
+       defer parent.Close()
+
+       fis, err := parent.Readdir(-1)
+       if err != nil {
+               t.Error(err)
+               return
+       }
+       var lsfi2 os.FileInfo
+       base := filepath.Base(path)
+       for _, fi2 := range fis {
+               if fi2.Name() == base {
+                       lsfi2 = fi2
+                       break
+               }
+       }
+       if lsfi2 == nil {
+               t.Errorf("failed to find %q in its parent", path)
+               return
+       }
+       lstatCheck(t, path, lsfi2)
+
+       if !os.SameFile(lsfi, lsfi2) {
+               t.Errorf("lstat of %q file in %q directory and %q should be the same", lsfi2.Name(), parentdir, path)
+       }
+}
+
+// testIsDir verifies that fi refers to directory.
+func testIsDir(t *testing.T, path string, fi os.FileInfo) {
+       t.Helper()
+       if !fi.IsDir() {
+               t.Errorf("%q should be a directory", path)
+       }
+       if fi.Mode()&os.ModeSymlink != 0 {
+               t.Errorf("%q should not be a symlink", path)
+       }
+}
+
+// testIsSymlink verifies that fi refers to symlink.
+func testIsSymlink(t *testing.T, path string, fi os.FileInfo) {
+       t.Helper()
+       if fi.IsDir() {
+               t.Errorf("%q should not be a directory", path)
+       }
+       if fi.Mode()&os.ModeSymlink == 0 {
+               t.Errorf("%q should be a symlink", path)
+       }
+}
+
+// testIsFile verifies that fi refers to file.
+func testIsFile(t *testing.T, path string, fi os.FileInfo) {
+       t.Helper()
+       if fi.IsDir() {
+               t.Errorf("%q should not be a directory", path)
+       }
+       if fi.Mode()&os.ModeSymlink != 0 {
+               t.Errorf("%q should not be a symlink", path)
+       }
+}
+
+func testDirStats(t *testing.T, path string) {
+       testStatAndLstat(t, path, false, testIsDir, testIsDir)
+}
+
+func testFileStats(t *testing.T, path string) {
+       testStatAndLstat(t, path, false, testIsFile, testIsFile)
+}
+
+func testSymlinkStats(t *testing.T, path string, isdir bool) {
+       if isdir {
+               testStatAndLstat(t, path, true, testIsDir, testIsSymlink)
+       } else {
+               testStatAndLstat(t, path, true, testIsFile, testIsSymlink)
+       }
+}
+
+func testSymlinkSameFile(t *testing.T, path, link string) {
+       pathfi, err := os.Stat(path)
+       if err != nil {
+               t.Error(err)
+               return
+       }
+
+       linkfi, err := os.Stat(link)
+       if err != nil {
+               t.Error(err)
+               return
+       }
+       if !os.SameFile(pathfi, linkfi) {
+               t.Errorf("os.Stat(%q) and os.Stat(%q) are not the same file", path, link)
+       }
+
+       linkfi, err = os.Lstat(link)
+       if err != nil {
+               t.Error(err)
+               return
+       }
+       if os.SameFile(pathfi, linkfi) {
+               t.Errorf("os.Stat(%q) and os.Lstat(%q) are the same file", path, link)
+       }
+}
+
+func TestDirAndSymlinkStats(t *testing.T) {
+       testenv.MustHaveSymlink(t)
+
+       tmpdir, err := ioutil.TempDir("", "TestDirAndSymlinkStats")
+       if err != nil {
+               t.Fatal(err)
+       }
+       defer os.RemoveAll(tmpdir)
+
+       dir := filepath.Join(tmpdir, "dir")
+       err = os.Mkdir(dir, 0777)
+       if err != nil {
+               t.Fatal(err)
+       }
+       testDirStats(t, dir)
+
+       dirlink := filepath.Join(tmpdir, "link")
+       err = os.Symlink(dir, dirlink)
+       if err != nil {
+               t.Fatal(err)
+       }
+       testSymlinkStats(t, dirlink, true)
+       testSymlinkSameFile(t, dir, dirlink)
+}
+
+func TestFileAndSymlinkStats(t *testing.T) {
+       testenv.MustHaveSymlink(t)
+
+       tmpdir, err := ioutil.TempDir("", "TestFileAndSymlinkStats")
+       if err != nil {
+               t.Fatal(err)
+       }
+       defer os.RemoveAll(tmpdir)
+
+       file := filepath.Join(tmpdir, "file")
+       err = ioutil.WriteFile(file, []byte(""), 0644)
+       if err != nil {
+               t.Fatal(err)
+       }
+       testFileStats(t, file)
+
+       filelink := filepath.Join(tmpdir, "link")
+       err = os.Symlink(file, filelink)
+       if err != nil {
+               t.Fatal(err)
+       }
+       testSymlinkStats(t, filelink, false)
+       testSymlinkSameFile(t, file, filelink)
+}
+
+// see issue 27225 for details
+func TestSymlinkWithTrailingSlash(t *testing.T) {
+       if runtime.GOOS == "windows" {
+               t.Skip("skipping on windows; issue 27225")
+       }
+
+       testenv.MustHaveSymlink(t)
+
+       tmpdir, err := ioutil.TempDir("", "TestSymlinkWithTrailingSlash")
+       if err != nil {
+               t.Fatal(err)
+       }
+       defer os.RemoveAll(tmpdir)
+
+       dir := filepath.Join(tmpdir, "dir")
+       err = os.Mkdir(dir, 0777)
+       if err != nil {
+               t.Fatal(err)
+       }
+       dirlink := filepath.Join(tmpdir, "link")
+       err = os.Symlink(dir, dirlink)
+       if err != nil {
+               t.Fatal(err)
+       }
+       dirlinkWithSlash := dirlink + string(os.PathSeparator)
+
+       testDirStats(t, dirlinkWithSlash)
+
+       fi1, err := os.Stat(dir)
+       if err != nil {
+               t.Error(err)
+               return
+       }
+       fi2, err := os.Stat(dirlinkWithSlash)
+       if err != nil {
+               t.Error(err)
+               return
+       }
+       if !os.SameFile(fi1, fi2) {
+               t.Errorf("os.Stat(%q) and os.Stat(%q) are not the same file", dir, dirlinkWithSlash)
+       }
+}
index f3297c033856a8bdc9e1fce24117936b74605d7f..7ebeec50eff4a522efeedd7a38f7a1e39162d9e5 100644 (file)
@@ -211,7 +211,13 @@ func (fs *fileStat) loadFileId() error {
        if err != nil {
                return err
        }
-       h, err := syscall.CreateFile(pathp, 0, 0, nil, syscall.OPEN_EXISTING, syscall.FILE_FLAG_BACKUP_SEMANTICS, 0)
+       attrs := uint32(syscall.FILE_FLAG_BACKUP_SEMANTICS)
+       if fs.isSymlink() {
+               // Use FILE_FLAG_OPEN_REPARSE_POINT, otherwise CreateFile will follow symlink.
+               // See https://docs.microsoft.com/en-us/windows/desktop/FileIO/symbolic-link-effects-on-file-systems-functions#createfile-and-createfiletransacted
+               attrs |= syscall.FILE_FLAG_OPEN_REPARSE_POINT
+       }
+       h, err := syscall.CreateFile(pathp, 0, 0, nil, syscall.OPEN_EXISTING, attrs, 0)
        if err != nil {
                return err
        }