golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
123.99k stars 17.67k forks source link

path/filepath: Glob fails if it has a wildcard and ends with / #33617

Open BlackHole1 opened 5 years ago

BlackHole1 commented 5 years ago

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

$ go version
go version go1.12 darwin/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
GOARCH="amd64"
GOBIN=""
GOCACHE="/Users/black-hole/Library/Caches/go-build"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="darwin"
GOOS="darwin"
GOPATH="/Users/black-hole/.gvm/pkgsets/go1.12/global"
GOPROXY=""
GORACE=""
GOROOT="/Users/black-hole/.gvm/gos/go1.12"
GOTMPDIR=""
GOTOOLDIR="/Users/black-hole/.gvm/gos/go1.12/pkg/tool/darwin_amd64"
GCCGO="gccgo"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/dev/null"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/82/jkr30dwj24ncf6ng3tdv9kd40000gn/T/go-build925523187=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

https://play.golang.org/p/hOALcoEmrWz

What did you expect to see?

Accurate return path

What did you see instead?

Cannot match when there is a wildcard in the path and the last character is /

Other

image

jamdagni86 commented 5 years ago

The function first uses filepath.Split to get the parent directory of the given path. When a trailing slash is present, filepath.Split returns the path as it is as it can't split it further. The fix is to clean the path before calling filepath.Split

https://github.com/golang/go/blob/0212f0410f845815f5327a7f2e705891a9598f3d/src/path/filepath/match.go#L242-L248

odeke-em commented 5 years ago

Hello @BlackHole1, thank you for the report! @jamdagni86 would you like to send a CL as you had recommended above, and a test following up too, for Go1.14?

gopherbot commented 4 years ago

Change https://golang.org/cl/207837 mentions this issue: path/filepath: Glob fails if it has a wildcard and ends with /

ianlancetaylor commented 4 years ago

I don't understand this issue report. The docs for filepath.Glob say that it uses the rules for filepath.Match. The docs for filepath.Match do not say that a pattern with a trailing slash matches the same name without the trailing slash.

What is the actual bug here? Where do the functions not act as documented?

BlackHole1 commented 4 years ago

With or without the slash in the trailing, I think the matching result should be the same, but it is not the case.

ianlancetaylor commented 4 years ago

The documentation for Glob says that it follows the same as rules as Match. The documentation for Match does not say anything about a trailing slash. Consider https://play.golang.org/p/6j4icmyaYpp.

What do you think Glob("/usr/*/go/") should print?

BlackHole1 commented 4 years ago

I think their output should be the same. The last slash should not affect the match.

The Match design principle is to follow http://man7.org/linux/man-pages/man7/glob.7.html

Although I didn't find the relevant instructions in the above document, I tried several shells, and the results of their matching have nothing to do with the last slash.

ianlancetaylor commented 4 years ago

I have a file /home/iant/hello.go. Using bash:

> echo /home/*/hello.go
/home/iant/hello.go
> echo /home/*/hello.go/
/home/*/hello.go/

So I don't agree with you that the last slash does not affect the match. It does affect it.

BlackHole1 commented 4 years ago

It seems that if it is a file at the end, it is influential. But when the last is the directory, there is no affect.

> tree ./test
./test
├── a
│   └── x
│       └── 1.txt
└── b
    └── x
        └── 2.txt

4 directories, 2 files
> echo ./test/*/x/
./test/a/x/ ./test/b/x/

> echo ./test/*/x
./test/a/x ./test/b/x
> ls ./test/*/x/
./test/a/x/:
1.txt

./test/b/x/:
2.txt

> ls ./test/*/x
./test/a/x:
1.txt

./test/b/x:
2.txt
ianlancetaylor commented 4 years ago

So it seems that we need to decide whether filepath.Glob should permit a trailing slash to match a directory name.

BlackHole1 commented 4 years ago

I would like to confirm whether to modify filepath.Glob or filepath.Match?

ianlancetaylor commented 4 years ago

I don't see how it is possible to change filepath.Match for this, as filepath.Match is only text based.

(That is of course an argument for not changing filepath.Glob, which is currently defined in terms of filepath.Match.)