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: Readlink returns garbage when symlink target is like \\?\Volume{XXXXX}\ #30463

Closed
alexbrainman opened this issue Feb 28, 2019 · 3 comments
Closed

Comments

@alexbrainman
Copy link
Member

@alexbrainman alexbrainman commented Feb 28, 2019

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

go version devel +8ee9bca272 Wed Feb 27 08:22:03 2019 +0000 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
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\Alex\AppData\Local\go-build
set GOEXE=.exe
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOOS=windows
set GOPATH=c:\users\alex\dev
set GOPROXY=
set GORACE=
set GOROOT=c:\users\alex\dev\go
set GOTMPDIR=
set GOTOOLDIR=c:\users\alex\dev\go\pkg\tool\windows_amd64
set GCCGO=gccgo
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\Alex\AppData\Local\Temp\go-build403512930=/tmp/go-build -gno-record-gcc-switches

What did you do?

I run go test command against this main_test.go file

package main_test

import (
	"io/ioutil"
	"os"
	"os/exec"
	"path/filepath"
	"strings"
	"testing"
)

func TestReadlink(t *testing.T) {
	tmpdir, err := ioutil.TempDir("", "TestReadlink")
	if err != nil {
		t.Fatal(err)
	}
	defer os.RemoveAll(tmpdir)

	vol := filepath.VolumeName(tmpdir)
	output, err := exec.Command("cmd", "/c", "mountvol", vol, "/L").CombinedOutput()
	if err != nil {
		t.Fatalf("failed to run mountvol %v /L: %v %q", vol, err, output)
	}
	target := strings.Trim(string(output), " \n\r")

	dirlink := filepath.Join(tmpdir, "dirlink")
	output, err = exec.Command("cmd", "/c", "mklink", "/J", dirlink, target).CombinedOutput()
	if err != nil {
		t.Fatalf("failed to run mklink %v %v: %v %q", dirlink, target, err, output)
	}

	got, err := os.Readlink(dirlink)
	if err != nil {
		t.Fatal(err)
	}
	if want := vol + `\`; got != want {
		t.Errorf(`os.Readlink(%q): got %q, want %q`, dirlink, got, want)
	}
}

What did you expect to see?

I expected test to pass.

What did you see instead?

Test fails with this error:

--- FAIL: TestReadlink (0.17s)
    main_test.go:37: os.Readlink("C:\\Users\\Alex\\AppData\\Local\\Temp\\TestReadlink551162827\\dirlink"): got "Volume{ea961e77-0000-0000-0000-501f00000000}\\", want "C:\\"
FAIL

The problem here is that os.Readlink does not handle links where target looks like this \\?\Volume{XXXXX}\. Volume{ea961e77-0000-0000-0000-501f00000000}\\ that os.Readlink returns is not useful for anything.

We have to handle symlinks, like that, because, for example, windows-arm TMP directory is such link (see #29746 (comment) for details). So it will be quite common at least on windows-arm.

Alex

@gopherbot
Copy link

@gopherbot gopherbot commented Feb 28, 2019

Change https://golang.org/cl/164201 mentions this issue: os: make Readlink work with symlinks with target like \??\Volume{ABCD}\

@alexbrainman
Copy link
Member Author

@alexbrainman alexbrainman commented Feb 28, 2019

@jhowardmsft FYI, please, see https://golang.org/cl/164201 - hopefully it will be part of go1.13.

Alex

@lowenna
Copy link

@lowenna lowenna commented Feb 28, 2019

@alexbrainman Thanks for pinging me on this. @jiria FYI

@gopherbot gopherbot closed this in 2edd559 Mar 1, 2019
kolyshkin added a commit to kolyshkin/continuity that referenced this issue Feb 27, 2020
This reverts the part of commit 8100e75 (PR containerd#113)
that added the fork of Readlink() for Windows.

At that time the fork was done to work around the bug
in golang's implementation of os.Readlink() for Windows.

The above bug was never reported upstream, but fortunately
it was independently found, reported [1], and fixed [2].
The fix made its way into go-1.13 (there is no mention of
that in release notes, so I checked it in git history).

[1] golang/go#30463
[2] https://go-review.googlesource.com/c/go/+/164201

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
kolyshkin added a commit to kolyshkin/continuity that referenced this issue Feb 27, 2020
This reverts the part of commit 8100e75 (PR containerd#113)
that added the fork of Readlink() for Windows.

At that time the fork was done to work around the bug
in golang's implementation of os.Readlink() for Windows.

The above bug was never reported upstream, but fortunately
it was independently found, reported [1], and fixed [2].
The fix made its way into go-1.13 (there is no mention of
that in release notes, so I checked it in git history).

[1] golang/go#30463
[2] https://go-review.googlesource.com/c/go/+/164201

Signed-off-by: Kir Kolyshkin <kolyshkin@gmail.com>
@golang golang locked and limited conversation to collaborators Feb 29, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

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