os: don't treat mount points as symbolic links

This CL changes the behavior of os.Lstat to stop setting the
os.ModeSymlink type mode bit for mount points on Windows. As a result,
filepath.EvalSymlinks no longer evaluates mount points, which was the
cause of many inconsistencies and bugs.

Additionally, os.Lstat starts setting the os.ModeIrregular type mode bit
for all reparse tags on Windows, except for those that are explicitly
supported by the os package, which, since this CL, doesn't include mount
points. This helps to identify files that need special handling outside
of the os package.

This behavior is controlled by the `winsymlink` GODEBUG setting.
For Go 1.23, it defaults to `winsymlink=1`.
Previous versions default to `winsymlink=0`.

Fixes #39786
Fixes #40176
Fixes #61893
Updates #63703
Updates #40180
Updates #63429

Cq-Include-Trybots: luci.golang.try:gotip-windows-amd64-longtest,gotip-windows-arm64
Change-Id: I2e7372ab8862f5062667d30db6958d972bce5407
Reviewed-on: https://go-review.googlesource.com/c/go/+/565136
Reviewed-by: Bryan Mills <bcmills@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Michael Knyszek <mknyszek@google.com>
This commit is contained in:
qmuntal 2024-02-28 16:06:04 +01:00 committed by Quim Muntal
parent 90796f44d5
commit 7986e26a39
9 changed files with 253 additions and 57 deletions

View file

@ -126,6 +126,19 @@ for example,
see the [runtime documentation](/pkg/runtime#hdr-Environment_Variables)
and the [go command documentation](/cmd/go#hdr-Build_and_test_caching).
### Go 1.23
Go 1.23 changed the mode bits reported by [`os.Lstat`](/pkg/os#Lstat) and [`os.Stat`](/pkg/os#Stat)
for reparse points, which can be controlled with the `winsymlink` setting.
As of Go 1.23 (`winsymlink=1`), mount points no longer have [`os.ModeSymlink`](/pkg/os#ModeSymlink)
set, and reparse points that are not symlinks, Unix sockets, or dedup files now
always have [`os.ModeIrregular`](/pkg/os#ModeIrregular) set. As a result of these changes,
[`filepath.EvalSymlinks`](/pkg/path/filepath#EvalSymlinks) no longer evaluates
mount points, which was a source of many inconsistencies and bugs.
At previous versions (`winsymlink=0`), mount points are treated as symlinks,
and other reparse points with non-default [`os.ModeType`](/pkg/os#ModeType) bits
(such as [`os.ModeDir`](/pkg/os#ModeDir)) do not have the `ModeIrregular` bit set.
### Go 1.22
Go 1.22 adds a configurable limit to control the maximum acceptable RSA key size

View file

@ -0,0 +1,7 @@
On Windows, the mode bits reported by [`os.Lstat`](/pkg/os#Lstat) and [`os.Stat`](/pkg/os#Stat)
for reparse points changed. Mount points no longer have [`os.ModeSymlink`](/pkg/os#ModeSymlink) set,
and reparse points that are not symlinks, Unix sockets, or dedup files now
always have [`os.ModeIrregular`](/pkg/os#ModeIrregular) set.
This behavior is controlled by the `winsymlink` setting.
For Go 1.23, it defaults to `winsymlink=1`.
Previous versions default to `winsymlink=0`.

View file

@ -0,0 +1,5 @@
On Windows, [`filepath.EvalSymlinks`](/pkg/path/filepath#EvalSymlinks) no longer evaluates
mount points, which was a source of many inconsistencies and bugs.
This behavior is controlled by the `winsymlink` setting.
For Go 1.23, it defaults to `winsymlink=1`.
Previous versions default to `winsymlink=0`.

View file

@ -49,6 +49,7 @@ var All = []Info{
{Name: "tlsmaxrsasize", Package: "crypto/tls"},
{Name: "tlsrsakex", Package: "crypto/tls", Changed: 22, Old: "1"},
{Name: "tlsunsafeekm", Package: "crypto/tls", Changed: 22, Old: "1"},
{Name: "winsymlink", Package: "os", Changed: 22, Old: "0"},
{Name: "x509sha1", Package: "crypto/x509"},
{Name: "x509usefallbackroots", Package: "crypto/x509"},
{Name: "x509usepolicies", Package: "crypto/x509"},

View file

@ -7,6 +7,7 @@ package os_test
import (
"errors"
"fmt"
"internal/godebug"
"internal/poll"
"internal/syscall/windows"
"internal/syscall/windows/registry"
@ -27,6 +28,8 @@ import (
"unsafe"
)
var winsymlink = godebug.New("winsymlink")
// For TestRawConnReadWrite.
type syscallDescriptor = syscall.Handle
@ -90,9 +93,10 @@ func TestSameWindowsFile(t *testing.T) {
}
type dirLinkTest struct {
name string
mklink func(link, target string) error
issueNo int // correspondent issue number (for broken tests)
name string
mklink func(link, target string) error
issueNo int // correspondent issue number (for broken tests)
isMountPoint bool
}
func testDirLinks(t *testing.T, tests []dirLinkTest) {
@ -140,8 +144,8 @@ func testDirLinks(t *testing.T, tests []dirLinkTest) {
t.Errorf("failed to stat link %v: %v", link, err)
continue
}
if !fi1.IsDir() {
t.Errorf("%q should be a directory", link)
if tp := fi1.Mode().Type(); tp != fs.ModeDir {
t.Errorf("Stat(%q) is type %v; want %v", link, tp, fs.ModeDir)
continue
}
if fi1.Name() != filepath.Base(link) {
@ -158,13 +162,16 @@ func testDirLinks(t *testing.T, tests []dirLinkTest) {
t.Errorf("failed to lstat link %v: %v", link, err)
continue
}
if m := fi2.Mode(); m&fs.ModeSymlink == 0 {
t.Errorf("%q should be a link, but is not (mode=0x%x)", link, uint32(m))
continue
var wantType fs.FileMode
if test.isMountPoint && winsymlink.Value() != "0" {
// Mount points are reparse points, and we no longer treat them as symlinks.
wantType = fs.ModeIrregular
} else {
// This is either a real symlink, or a mount point treated as a symlink.
wantType = fs.ModeSymlink
}
if m := fi2.Mode(); m&fs.ModeDir != 0 {
t.Errorf("%q should be a link, not a directory (mode=0x%x)", link, uint32(m))
continue
if tp := fi2.Mode().Type(); tp != wantType {
t.Errorf("Lstat(%q) is type %v; want %v", link, tp, fs.ModeDir)
}
}
}
@ -272,7 +279,8 @@ func TestDirectoryJunction(t *testing.T) {
var tests = []dirLinkTest{
{
// Create link similar to what mklink does, by inserting \??\ at the front of absolute target.
name: "standard",
name: "standard",
isMountPoint: true,
mklink: func(link, target string) error {
var t reparseData
t.addSubstituteName(`\??\` + target)
@ -282,7 +290,8 @@ func TestDirectoryJunction(t *testing.T) {
},
{
// Do as junction utility https://learn.microsoft.com/en-us/sysinternals/downloads/junction does - set PrintNameLength to 0.
name: "have_blank_print_name",
name: "have_blank_print_name",
isMountPoint: true,
mklink: func(link, target string) error {
var t reparseData
t.addSubstituteName(`\??\` + target)
@ -296,7 +305,8 @@ func TestDirectoryJunction(t *testing.T) {
if mklinkSupportsJunctionLinks {
tests = append(tests,
dirLinkTest{
name: "use_mklink_cmd",
name: "use_mklink_cmd",
isMountPoint: true,
mklink: func(link, target string) error {
output, err := testenv.Command(t, "cmd", "/c", "mklink", "/J", link, target).CombinedOutput()
if err != nil {
@ -1414,16 +1424,10 @@ func TestAppExecLinkStat(t *testing.T) {
if lfi.Name() != pythonExeName {
t.Errorf("Stat %s: got %q, but wanted %q", pythonPath, lfi.Name(), pythonExeName)
}
if m := lfi.Mode(); m&fs.ModeSymlink != 0 {
t.Errorf("%q should be a file, not a link (mode=0x%x)", pythonPath, uint32(m))
}
if m := lfi.Mode(); m&fs.ModeDir != 0 {
t.Errorf("%q should be a file, not a directory (mode=0x%x)", pythonPath, uint32(m))
}
if m := lfi.Mode(); m&fs.ModeIrregular == 0 {
if tp := lfi.Mode().Type(); tp != fs.ModeIrregular {
// A reparse point is not a regular file, but we don't have a more appropriate
// ModeType bit for it, so it should be marked as irregular.
t.Errorf("%q should not be a regular file (mode=0x%x)", pythonPath, uint32(m))
t.Errorf("%q should not be a an irregular file (mode=0x%x)", pythonPath, uint32(tp))
}
if sfi.Name() != pythonExeName {

View file

@ -5,6 +5,7 @@
package os
import (
"internal/godebug"
"internal/syscall/windows"
"sync"
"syscall"
@ -151,37 +152,86 @@ func newFileStatFromWin32finddata(d *syscall.Win32finddata) *fileStat {
// and https://learn.microsoft.com/en-us/windows/win32/fileio/reparse-point-tags.
func (fs *fileStat) isReparseTagNameSurrogate() bool {
// True for IO_REPARSE_TAG_SYMLINK and IO_REPARSE_TAG_MOUNT_POINT.
return fs.ReparseTag&0x20000000 != 0
}
func (fs *fileStat) isSymlink() bool {
// As of https://go.dev/cl/86556, we treat MOUNT_POINT reparse points as
// symlinks because otherwise certain directory junction tests in the
// path/filepath package would fail.
//
// However,
// https://learn.microsoft.com/en-us/windows/win32/fileio/hard-links-and-junctions
// seems to suggest that directory junctions should be treated like hard
// links, not symlinks.
//
// TODO(bcmills): Get more input from Microsoft on what the behavior ought to
// be for MOUNT_POINT reparse points.
return fs.ReparseTag == syscall.IO_REPARSE_TAG_SYMLINK ||
fs.ReparseTag == windows.IO_REPARSE_TAG_MOUNT_POINT
return fs.FileAttributes&syscall.FILE_ATTRIBUTE_REPARSE_POINT != 0 && fs.ReparseTag&0x20000000 != 0
}
func (fs *fileStat) Size() int64 {
return int64(fs.FileSizeHigh)<<32 + int64(fs.FileSizeLow)
}
var winsymlink = godebug.New("winsymlink")
func (fs *fileStat) Mode() (m FileMode) {
if winsymlink.Value() == "0" {
return fs.modePreGo1_23()
}
if fs.FileAttributes&syscall.FILE_ATTRIBUTE_READONLY != 0 {
m |= 0444
} else {
m |= 0666
}
if fs.isSymlink() {
// Windows reports the FILE_ATTRIBUTE_DIRECTORY bit for reparse points
// that refer to directories, such as symlinks and mount points.
// However, we follow symlink POSIX semantics and do not set the mode bits.
// This allows users to walk directories without following links
// by just calling "fi, err := os.Lstat(name); err == nil && fi.IsDir()".
// Note that POSIX only defines the semantics for symlinks, not for
// mount points or other surrogate reparse points, but we treat them
// the same way for consistency. Also, mount points can contain infinite
// loops, so it is not safe to walk them without special handling.
if !fs.isReparseTagNameSurrogate() {
if fs.FileAttributes&syscall.FILE_ATTRIBUTE_DIRECTORY != 0 {
m |= ModeDir | 0111
}
switch fs.filetype {
case syscall.FILE_TYPE_PIPE:
m |= ModeNamedPipe
case syscall.FILE_TYPE_CHAR:
m |= ModeDevice | ModeCharDevice
}
}
if fs.FileAttributes&syscall.FILE_ATTRIBUTE_REPARSE_POINT != 0 {
switch fs.ReparseTag {
case syscall.IO_REPARSE_TAG_SYMLINK:
m |= ModeSymlink
case windows.IO_REPARSE_TAG_AF_UNIX:
m |= ModeSocket
case windows.IO_REPARSE_TAG_DEDUP:
// If the Data Deduplication service is enabled on Windows Server, its
// Optimization job may convert regular files to IO_REPARSE_TAG_DEDUP
// whenever that job runs.
//
// However, DEDUP reparse points remain similar in most respects to
// regular files: they continue to support random-access reads and writes
// of persistent data, and they shouldn't add unexpected latency or
// unavailability in the way that a network filesystem might.
//
// Go programs may use ModeIrregular to filter out unusual files (such as
// raw device files on Linux, POSIX FIFO special files, and so on), so
// to avoid files changing unpredictably from regular to irregular we will
// consider DEDUP files to be close enough to regular to treat as such.
default:
m |= ModeIrregular
}
}
return
}
// modePreGo1_23 returns the FileMode for the fileStat, using the pre-Go 1.23
// logic for determining the file mode.
// The logic is subtle and not well-documented, so it is better to keep it
// separate from the new logic.
func (fs *fileStat) modePreGo1_23() (m FileMode) {
if fs.FileAttributes&syscall.FILE_ATTRIBUTE_READONLY != 0 {
m |= 0444
} else {
m |= 0666
}
if fs.ReparseTag == syscall.IO_REPARSE_TAG_SYMLINK ||
fs.ReparseTag == windows.IO_REPARSE_TAG_MOUNT_POINT {
return m | ModeSymlink
}
if fs.FileAttributes&syscall.FILE_ATTRIBUTE_DIRECTORY != 0 {
@ -199,19 +249,7 @@ func (fs *fileStat) Mode() (m FileMode) {
}
if m&ModeType == 0 {
if fs.ReparseTag == windows.IO_REPARSE_TAG_DEDUP {
// If the Data Deduplication service is enabled on Windows Server, its
// Optimization job may convert regular files to IO_REPARSE_TAG_DEDUP
// whenever that job runs.
//
// However, DEDUP reparse points remain similar in most respects to
// regular files: they continue to support random-access reads and writes
// of persistent data, and they shouldn't add unexpected latency or
// unavailability in the way that a network filesystem might.
//
// Go programs may use ModeIrregular to filter out unusual files (such as
// raw device files on Linux, POSIX FIFO special files, and so on), so
// to avoid files changing unpredictably from regular to irregular we will
// consider DEDUP files to be close enough to regular to treat as such.
// See comment in fs.Mode.
} else {
m |= ModeIrregular
}

View file

@ -1980,3 +1980,16 @@ func TestEscaping(t *testing.T) {
}
}
}
func TestEvalSymlinksTooManyLinks(t *testing.T) {
testenv.MustHaveSymlink(t)
dir := filepath.Join(t.TempDir(), "dir")
err := os.Symlink(dir, dir)
if err != nil {
t.Fatal(err)
}
_, err = filepath.EvalSymlinks(dir)
if err == nil {
t.Fatal("expected error, got nil")
}
}

View file

@ -7,6 +7,7 @@ package filepath_test
import (
"flag"
"fmt"
"internal/godebug"
"internal/testenv"
"io/fs"
"os"
@ -486,6 +487,109 @@ func TestWalkDirectorySymlink(t *testing.T) {
testWalkMklink(t, "D")
}
func createMountPartition(t *testing.T, vhd string, args string) []byte {
testenv.MustHaveExecPath(t, "powershell")
t.Cleanup(func() {
cmd := testenv.Command(t, "powershell", "-Command", fmt.Sprintf("Dismount-VHD %q", vhd))
out, err := cmd.CombinedOutput()
if err != nil {
if t.Skipped() {
// Probably failed to dismount because we never mounted it in
// the first place. Log the error, but ignore it.
t.Logf("%v: %v (skipped)\n%s", cmd, err, out)
} else {
// Something went wrong, and we don't want to leave dangling VHDs.
// Better to fail the test than to just log the error and continue.
t.Errorf("%v: %v\n%s", cmd, err, out)
}
}
})
script := filepath.Join(t.TempDir(), "test.ps1")
cmd := strings.Join([]string{
"$ErrorActionPreference = \"Stop\"",
fmt.Sprintf("$vhd = New-VHD -Path %q -SizeBytes 3MB -Fixed", vhd),
"$vhd | Mount-VHD",
fmt.Sprintf("$vhd = Get-VHD %q", vhd),
"$vhd | Get-Disk | Initialize-Disk -PartitionStyle GPT",
"$part = $vhd | Get-Disk | New-Partition -UseMaximumSize -AssignDriveLetter:$false",
"$vol = $part | Format-Volume -FileSystem NTFS",
args,
}, "\n")
err := os.WriteFile(script, []byte(cmd), 0666)
if err != nil {
t.Fatal(err)
}
output, err := testenv.Command(t, "powershell", "-File", script).CombinedOutput()
if err != nil {
// This can happen if Hyper-V is not installed or enabled.
t.Skip("skipping test because failed to create VHD: ", err, string(output))
}
return output
}
var winsymlink = godebug.New("winsymlink")
func TestEvalSymlinksJunctionToVolumeID(t *testing.T) {
// Test that EvalSymlinks resolves a directory junction which
// is mapped to volumeID (instead of drive letter). See go.dev/issue/39786.
if winsymlink.Value() == "0" {
t.Skip("skipping test because winsymlink is not enabled")
}
t.Parallel()
output, _ := exec.Command("cmd", "/c", "mklink", "/?").Output()
if !strings.Contains(string(output), " /J ") {
t.Skip("skipping test because mklink command does not support junctions")
}
tmpdir := tempDirCanonical(t)
vhd := filepath.Join(tmpdir, "Test.vhdx")
output = createMountPartition(t, vhd, "Write-Host $vol.Path -NoNewline")
vol := string(output)
dirlink := filepath.Join(tmpdir, "dirlink")
output, err := testenv.Command(t, "cmd", "/c", "mklink", "/J", dirlink, vol).CombinedOutput()
if err != nil {
t.Fatalf("failed to run mklink %v %v: %v %q", dirlink, vol, err, output)
}
got, err := filepath.EvalSymlinks(dirlink)
if err != nil {
t.Fatal(err)
}
if got != dirlink {
t.Errorf(`EvalSymlinks(%q): got %q, want %q`, dirlink, got, dirlink)
}
}
func TestEvalSymlinksMountPointRecursion(t *testing.T) {
// Test that EvalSymlinks doesn't follow recursive mount points.
// See go.dev/issue/40176.
if winsymlink.Value() == "0" {
t.Skip("skipping test because winsymlink is not enabled")
}
t.Parallel()
tmpdir := tempDirCanonical(t)
dirlink := filepath.Join(tmpdir, "dirlink")
err := os.Mkdir(dirlink, 0755)
if err != nil {
t.Fatal(err)
}
vhd := filepath.Join(tmpdir, "Test.vhdx")
createMountPartition(t, vhd, fmt.Sprintf("$part | Add-PartitionAccessPath -AccessPath %q\n", dirlink))
got, err := filepath.EvalSymlinks(dirlink)
if err != nil {
t.Fatal(err)
}
if got != dirlink {
t.Errorf(`EvalSymlinks(%q): got %q, want %q`, dirlink, got, dirlink)
}
}
func TestNTNamespaceSymlink(t *testing.T) {
output, _ := exec.Command("cmd", "/c", "mklink", "/?").Output()
if !strings.Contains(string(output), " /J ") {
@ -511,7 +615,13 @@ func TestNTNamespaceSymlink(t *testing.T) {
if err != nil {
t.Fatal(err)
}
if want := vol + `\`; got != want {
var want string
if winsymlink.Value() == "0" {
want = vol + `\`
} else {
want = dirlink
}
if got != want {
t.Errorf(`EvalSymlinks(%q): got %q, want %q`, dirlink, got, want)
}
@ -524,7 +634,7 @@ func TestNTNamespaceSymlink(t *testing.T) {
t.Fatal(err)
}
target += file[len(filepath.VolumeName(file)):]
target = filepath.Join(target, file[len(filepath.VolumeName(file)):])
filelink := filepath.Join(tmpdir, "filelink")
output, err = exec.Command("cmd", "/c", "mklink", filelink, target).CombinedOutput()
@ -536,7 +646,8 @@ func TestNTNamespaceSymlink(t *testing.T) {
if err != nil {
t.Fatal(err)
}
if want := file; got != want {
want = file
if got != want {
t.Errorf(`EvalSymlinks(%q): got %q, want %q`, filelink, got, want)
}
}

View file

@ -319,6 +319,10 @@ Below is the full list of supported metrics, ordered lexicographically.
The number of non-default behaviors executed by the crypto/tls
package due to a non-default GODEBUG=tlsunsafeekm=... setting.
/godebug/non-default-behavior/winsymlink:events
The number of non-default behaviors executed by the os package
due to a non-default GODEBUG=winsymlink=... setting.
/godebug/non-default-behavior/x509sha1:events
The number of non-default behaviors executed by the crypto/x509
package due to a non-default GODEBUG=x509sha1=... setting.