Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

os: FileInfo Mode on Windows incorrectly interpreting WSL symlinks (reparse points) as regular files always #42184

Open
kfsone opened this issue Oct 24, 2020 · 7 comments
Milestone

Comments

@kfsone
Copy link

@kfsone kfsone commented Oct 24, 2020

What version of Go are you using (go version)?

$ go version
go version go1.15.2 windows/amd64

Does this issue reproduce with the latest release?

Yes

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
set GO111MODULE=
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\oliver\AppData\Local\go-build
set GOENV=C:\Users\oliver\AppData\Roaming\go\env
set GOEXE=.exe
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=C:\Users\oliver\go\pkg\mod
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\oliver\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=C:\Go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLDIR=C:\Go\pkg\tool\windows_amd64
set GCCGO=gccgo
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fmessage-length=0 -fdebug-prefix-map=C:\Users\oliver\AppData\Local\Temp\go-build844512546=/tmp/go-build -gno-record-gcc-switches

What did you do?

Called os.FileInfo.Mode.IsRegular and got the wrong result.

This is caused by a WSL symlink:

[D:\Dev\github.com\kfsone\archive\smugl-src]
> gci compile | format-list

    Directory: D:\Dev\github.com\kfsone\archive\smugl-src

Name           : compile
Length         : 0
CreationTime   : 7/28/2019 10:36:58 PM
LastWriteTime  : 7/28/2019 10:36:58 PM
LastAccessTime : 7/28/2019 10:36:58 PM
Mode           : la---
LinkType       :
Target         :
VersionInfo    : File:             D:\Dev\github.com\kfsone\archive\smugl-src\compile
                 InternalName:
                 OriginalFilename:
                 FileVersion:
                 FileDescription:
                 Product:
                 ProductVersion:
                 Debug:            False
                 Patched:          False
                 PreRelease:       False
                 PrivateBuild:     False
                 SpecialBuild:     False
                 Language:

Running the following go code:

package main

import (
    "fmt"
    "os"
    "path/filepath"
)

func main() {
    path := "D:/dev/github.com/kfsone/archive/smugl-src/compile"
    filepath.Walk(path, func (path string, info os.FileInfo, err error) error {
        fmt.Printf("%s %s %#+v\n", path, info.Mode().IsRegular(), info)
        return nil
    })
}

produces:

D:/dev/github.com/kfsone/archive/smugl-src/compile %!s(bool=true) &os.fileStat{name:"compile", FileAttributes:0x420, CreationTime:syscall.Filetime{LowDateTime:0xa3aab077, HighDateTime:0x1d545cf}, LastAccessTime:syscall.Filetime{LowDateTime:0xa3aab077, HighDateTime:0x1d545cf}, LastWriteTime:syscall.Filetime{LowDateTime:0xa3aab077, HighDateTime:0x1d545cf}, FileSizeHigh:0x0, FileSizeLow:0x0, Reserved0:0xa000001d, filetype:0x0, Mutex:sync.Mutex{state:0, sema:0x0}, path:"", vol:0x5ef35831, idxhi:0xb0000, idxlo:0x3b47, appendNameToPath:false}

Windows is definitely telling us it's a reparse point: FileAttributes: 0x420 includes the 1024 required. The issue here is the value of Reserved0, which is not being tested for:

According to https://docs.microsoft.com/en-us/openspecs/windows_protocols/ms-fscc/c8e77b37-3909-4fe6-a4ea-2b9d423b1ee4


0xA000001D
Used by the Windows Subsystem for Linux (WSL) to represent a UNIX symbolic link. Server-side interpretation only, not meaningful over the wire.

This appears it would require a change to src/os/types_windows.go lines 104,105.

@odeke-em odeke-em changed the title os FileInfo Mode on Windows incorrectly interpreting WSL symlinks. os: FileInfo Mode on Windows incorrectly interpreting WSL symlinks (reparse points) as regular files always Oct 25, 2020
@odeke-em
Copy link
Member

@odeke-em odeke-em commented Oct 25, 2020

Thank you for filing this issue @kfsone and welcome to the Go project!

Interesting bug, and thank you for the background. Please feel free to send a change to fix this as per https://golang.org/doc/contribute.html or even just a PR on Github after signing the CLA, it would be
awesome to have your contributions!

Just for full disclosure we are about 6 days away from a code freeze for Go1.16, thus if you can, please send a change,
with a regression test, and I shall also tag some Windows experts @alexbrainman @mattn. No rush though, because even
if not meeting the deadline, in about 2 months, if/when the change is accepted, we can always backport it.

@mattn
Copy link
Member

@mattn mattn commented Oct 26, 2020

CreateFile fail to open symlink created on WSL2 without FILE_FLAG_OPEN_REPARSE_POINT. To fix this, we should add FILE_FLAG_OPEN_REPARSE_POINT to second third argument of stat.

diff --git a/src/os/stat_windows.go b/src/os/stat_windows.go
index da4c49090e..43f176376c 100644
--- a/src/os/stat_windows.go
+++ b/src/os/stat_windows.go
@@ -103,7 +103,7 @@ func stat(funcname, name string, createFileAttrs uint32) (FileInfo, error) {
 
 // statNolog implements Stat for Windows.
 func statNolog(name string) (FileInfo, error) {
-	return stat("Stat", name, syscall.FILE_FLAG_BACKUP_SEMANTICS)
+	return stat("Stat", name, syscall.FILE_FLAG_OPEN_REPARSE_POINT|syscall.FILE_FLAG_BACKUP_SEMANTICS)
 }
 
 // lstatNolog implements Lstat for Windows.
@mattn
Copy link
Member

@mattn mattn commented Oct 26, 2020

Hmm, TestDirectoryJunction fail if adding FILE_FLAG_OPEN_REPARSE_POINT. So it should do fallback if CreateFile fail.

@gopherbot
Copy link

@gopherbot gopherbot commented Oct 26, 2020

Change https://golang.org/cl/265037 mentions this issue: os: fallback to open symlinks

@cagedmantis cagedmantis added this to the Backlog milestone Oct 26, 2020
@kfsone
Copy link
Author

@kfsone kfsone commented Oct 28, 2020

CreateFile fail to open symlink created on WSL2 without FILE_FLAG_OPEN_REPARSE_POINT. To fix this, we should add FILE_FLAG_OPEN_REPARSE_POINT to second third argument of stat.

diff --git a/src/os/stat_windows.go b/src/os/stat_windows.go
index da4c49090e..43f176376c 100644
--- a/src/os/stat_windows.go
+++ b/src/os/stat_windows.go
@@ -103,7 +103,7 @@ func stat(funcname, name string, createFileAttrs uint32) (FileInfo, error) {
 
 // statNolog implements Stat for Windows.
 func statNolog(name string) (FileInfo, error) {
-	return stat("Stat", name, syscall.FILE_FLAG_BACKUP_SEMANTICS)
+	return stat("Stat", name, syscall.FILE_FLAG_OPEN_REPARSE_POINT|syscall.FILE_FLAG_BACKUP_SEMANTICS)
 }
 
 // lstatNolog implements Lstat for Windows.

Note the distinction between IO_REPARSE_TAG_SYMLINK and io_reparse_tag_LX_symlink in IO_REPARSE_TAG_LX_SYMLINK (lx for 'linux' maybe)

@kfsone
Copy link
Author

@kfsone kfsone commented Oct 28, 2020

Notes from looking at implementing handling for this:

  • Microsoft themselves have not fully supported this yet:
> cd $env:TEMP ; rm -recurs -force symlinks ; cd (mkdir symlinks)
[C:\Users\oliver\AppData\Local\Temp\symlinks]
> wsl ln -s target.txt relative.lnk
[C:\Users\oliver\AppData\Local\Temp\symlinks]
> echo this is target >target.txt
[C:\Users\oliver\AppData\Local\Temp\symlinks]
> get-content target.txt
this
is
target
[C:\Users\oliver\AppData\Local\Temp\symlinks]
> get-content relative.lnk
Get-Content: The file cannot be accessed by the system. : 'C:\Users\oliver\AppData\Local\Temp\symlinks\relative.lnk'
[C:\Users\oliver\AppData\Local\Temp\symlinks]
> wsl cat relative.lnk
this
is
target
[C:\Users\oliver\AppData\Local\Temp\symlinks]
> fsutil reparsePoint query relative.lnk
Reparse Tag Value : 0xa000001d
Tag value: Microsoft
Tag value: Name Surrogate

Reparse Data Length: 0xe
Reparse Data:
0000:  02 00 00 00 74 61 72 67  65 74 2e 74 78 74        ....target.txt

Perhaps for now, rather than appearing as a symlink, these entities should at least appear as not being regular files?

@kfsone
Copy link
Author

@kfsone kfsone commented Oct 28, 2020

@odeke-em Taking a look; previously signed the CLA for grpc in 2014, setup a password. The windows tests appear to be failing tho as a result of #40604 but I also experience a seemingly obvious failure of TestLookPath:

    lp_windows_test.go:147: test={rootDir:C:\Users\oliver\AppData\Local\Temp\TestLookPath336921111\d16 PATH:p1;p2 PATHEXT:.COM;.EXE;.BAT files:[p1\a.bat p1\a.exe p2\a.bat p2\a.exe] searchFor:a fails:false} failed: expected to find "p1\\a.bat", but found "p1\\a.exe"```

I'll ticket that separately, but the error looks backwards.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.