os: use FILE_FLAG_OPEN_REPARSE_POINT in SameFile

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

@@ -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) {

src/os/stat_test.go

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

src/os/types_windows.go

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