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

path/filepath: Rel() fails for UNC volumes when trailing slash is omitted #41230

Closed
tmm1 opened this issue Sep 4, 2020 · 10 comments
Closed

path/filepath: Rel() fails for UNC volumes when trailing slash is omitted #41230

tmm1 opened this issue Sep 4, 2020 · 10 comments
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. OS-Windows
Milestone

Comments

@tmm1
Copy link
Contributor

tmm1 commented Sep 4, 2020

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

$ go version
go version go1.15.1 windows/amd64

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\micro\AppData\Local\go-build
set GOENV=C:\Users\micro\AppData\Roaming\go\env
set GOEXE=.exe
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=C:\Users\micro\go\pkg\mod
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\micro\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\micro\AppData\Local\Temp\go-build149210574=/tmp/go-build -gno-record-gcc-switches

What did you do?

package main

import (
        "fmt"
        "path/filepath"
)

func main() {
        fmt.Println(filepath.VolumeName(`\\host\share`))
        fmt.Println(filepath.VolumeName(`\\host\share\`))

        rel, err := filepath.Rel(`\\host\share`, `\\host\share\file.txt`)
        fmt.Printf("rel: %v\n", rel)
        fmt.Printf("err: %v\n", err)

        rel, err = filepath.Rel(`\\host\share\`, `\\host\share\file.txt`)
        fmt.Printf("rel: %v\n", rel)
        fmt.Printf("err: %v\n", err)
}
\\host\share
\\host\share

rel:
err: Rel: can't make \\host\share\file.txt relative to \\host\share

rel: file.txt
err: <nil>

What did you expect to see?

Expected both invocations of filepath.Rel to return the same relative path

What did you see instead?

First invocation returns an error

cc @alexbrainman

@tmm1
Copy link
Contributor Author

tmm1 commented Sep 4, 2020

I wrote a failing test here: https://go-review.googlesource.com/c/go/+/253197

PS C:\Users\micro\code\go\src> ../bin/go test -v .\path\filepath\ -run TestRel
=== RUN   TestRel
    path_test.go:1209: Rel("\\\\host\\share", "\\\\host\\share\\file.txt"): want "file.txt", got error: Rel: can't make \\host\share\file.txt relative to \\host\share
    path_test.go:1212: Rel("\\\\host\\share", "\\\\host\\share\\file.txt")="", want "file.txt"
--- FAIL: TestRel (0.00s)
FAIL
FAIL    path/filepath   0.260s
FAIL

cc @niemeyer

@alexbrainman
Copy link
Member

Rel: can't make \host\share\file.txt relative to \host\share

Thank you for your report.

I suspect we never implemented Rel bits for UNC rooted paths. I don't have time to do it now. But I will review contribution.

Generally, you should not use Rel on windows. For example, filepath.Rel is broken when we walking from one drive to another. It has no meaning for something like filepath.Rel(c:\a, d:\a\b).

Alex

@tmm1
Copy link
Contributor Author

tmm1 commented Sep 5, 2020

Thanks! I understand the limitations of filepath.Rel. I'm using it in the context of filepath.Walk to get a relative path from the base path I'm walking.

My CL only contains a failing test. I did attempt to make it pass but I wasn't sure where best to fix this case.

@andybons andybons added OS-Windows NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Sep 8, 2020
@andybons andybons added this to the Unplanned milestone Sep 8, 2020
@tmm1
Copy link
Contributor Author

tmm1 commented Mar 2, 2021

I could use some advice on the best way to make this failing test pass.

@ianlancetaylor
Copy link
Contributor

Sorry, I'm not sure quite what you are asking. It seems that you've found a bug in the path/filepath package. Are you asking how to modify the package so that your test passes?

@networkimprov
Copy link

cc @rasky @mattn

@tmm1
Copy link
Contributor Author

tmm1 commented Mar 2, 2021

Yea I'm just confused about where is best to fix this. It's not clear to me if the bug lies in filepath.Clean, filepath.VolumeName or filepath.Rel

@tmm1
Copy link
Contributor Author

tmm1 commented Mar 2, 2021

I updated my CL with the dumbest possible thing I could do to make the test pass. It still feels like I'm missing a simpler solution, but maybe a special case is the best.

https://go-review.googlesource.com/c/go/+/253197

If someone wants to experiment, here's a playground link with all the windows-specific code paths: https://play.golang.org/p/GhDba2BSex9

@ianlancetaylor
Copy link
Contributor

I think the right place to fix this is filepath.Rel.

I commented on your CL. Thanks.

@gopherbot
Copy link

Change https://golang.org/cl/253197 mentions this issue: path/filepath: make Rel handle Windows UNC share

@dmitshur dmitshur modified the milestones: Unplanned, Go1.17 Mar 25, 2021
@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Mar 25, 2021
@golang golang locked and limited conversation to collaborators Mar 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done. OS-Windows
Projects
None yet
Development

No branches or pull requests

7 participants