Skip to content

Commit aa359df

Browse files
committed
Replace uses of filepath.HasPrefix with a path-aware function
internal.HasFilepathPrefix determines whether a given path is contained in another, being careful to take into account that "/foo" doesn't contain "/foobar" and that there are case-sensitive and case-insensitive fileystems out there. This fixes issue golang#296. Signed-off-by: Marcelo E. Magallon <[email protected]>
1 parent a28d05c commit aa359df

File tree

5 files changed

+252
-18
lines changed

5 files changed

+252
-18
lines changed

.travis.yml

-3
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,6 @@ before_script:
2121
script:
2222
- go build -v ./cmd/dep
2323
- go vet $PKGS
24-
# Ignore the deprecation warning about filepath.HasPrefix (SA1019). This flag
25-
# can be removed when issue #296 is resolved.
26-
- staticcheck -ignore='github.com/golang/dep/context.go:SA1019 github.com/golang/dep/cmd/dep/init.go:SA1019' $PKGS
2724
- gosimple $PKGS
2825
- test -z "$(gofmt -s -l . 2>&1 | grep -v vendor/ | tee /dev/stderr)"
2926
- go test -race $PKGS

context.go

+4-3
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111
"strings"
1212

1313
"github.com/Masterminds/vcs"
14+
"github.com/golang/dep/internal"
1415
"github.com/pkg/errors"
1516
"github.com/sdboyer/gps"
1617
)
@@ -37,7 +38,7 @@ func NewContext() (*Ctx, error) {
3738
for _, gp := range filepath.SplitList(buildContext.GOPATH) {
3839
gp = filepath.FromSlash(gp)
3940

40-
if filepath.HasPrefix(wd, gp) {
41+
if internal.HasFilepathPrefix(wd, gp) {
4142
ctx.GOPATH = gp
4243
}
4344

@@ -164,7 +165,7 @@ func (c *Ctx) resolveProjectRoot(path string) (string, error) {
164165
// Determine if the symlink is within any of the GOPATHs, in which case we're not
165166
// sure how to resolve it.
166167
for _, gp := range c.GOPATHS {
167-
if filepath.HasPrefix(path, gp) {
168+
if internal.HasFilepathPrefix(path, gp) {
168169
return "", errors.Errorf("'%s' is linked to another path within a GOPATH (%s)", path, gp)
169170
}
170171
}
@@ -179,7 +180,7 @@ func (c *Ctx) resolveProjectRoot(path string) (string, error) {
179180
// The second returned string indicates which GOPATH value was used.
180181
func (c *Ctx) SplitAbsoluteProjectRoot(path string) (string, error) {
181182
srcprefix := filepath.Join(c.GOPATH, "src") + string(filepath.Separator)
182-
if filepath.HasPrefix(path, srcprefix) {
183+
if internal.HasFilepathPrefix(path, srcprefix) {
183184
// filepath.ToSlash because we're dealing with an import path now,
184185
// not an fs path
185186
return filepath.ToSlash(path[len(srcprefix):]), nil

fs.go

+2-12
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import (
1212
"runtime"
1313
"syscall"
1414

15+
"github.com/golang/dep/internal"
1516
"github.com/pelletier/go-toml"
1617
"github.com/pkg/errors"
1718
)
@@ -32,18 +33,7 @@ func IsRegular(name string) (bool, error) {
3233
}
3334

3435
func IsDir(name string) (bool, error) {
35-
// TODO: lstat?
36-
fi, err := os.Stat(name)
37-
if os.IsNotExist(err) {
38-
return false, nil
39-
}
40-
if err != nil {
41-
return false, err
42-
}
43-
if !fi.IsDir() {
44-
return false, errors.Errorf("%q is not a directory", name)
45-
}
46-
return true, nil
36+
return internal.IsDir(name)
4737
}
4838

4939
func IsNonEmptyDir(name string) (bool, error) {

internal/fs.go

+154
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
package internal
2+
3+
import (
4+
"os"
5+
"path/filepath"
6+
"strings"
7+
"unicode"
8+
9+
"github.com/pkg/errors"
10+
)
11+
12+
func IsDir(name string) (bool, error) {
13+
// TODO: lstat?
14+
fi, err := os.Stat(name)
15+
if os.IsNotExist(err) {
16+
return false, nil
17+
}
18+
if err != nil {
19+
return false, err
20+
}
21+
if !fi.IsDir() {
22+
return false, errors.Errorf("%q is not a directory", name)
23+
}
24+
return true, nil
25+
}
26+
27+
// HasFilepathPrefix will determine if "path" starts with "prefix" from
28+
// the point of view of a filesystem.
29+
//
30+
// Unlike filepath.HasPrefix, this function is path-aware, meaning that
31+
// it knows that two directories /foo and /foobar are not the same
32+
// thing, and therefore HasFilepathPrefix("/foobar", "/foo") will return
33+
// false.
34+
//
35+
// This function also handles the case where the involved filesystems
36+
// are case-insensitive, meaning /foo/bar and /Foo/Bar correspond to the
37+
// same file. In that situation HasFilepathPrefix("/Foo/Bar", "/foo")
38+
// will return true. The implementation is *not* OS-specific, so a FAT32
39+
// filesystem mounted on Linux will be handled correctly.
40+
func HasFilepathPrefix(path, prefix string) bool {
41+
if filepath.VolumeName(path) != filepath.VolumeName(prefix) {
42+
return false
43+
}
44+
45+
var dn string
46+
47+
if isDir, err := IsDir(path); err != nil {
48+
return false
49+
} else if isDir {
50+
dn = path
51+
} else {
52+
dn = filepath.Dir(path)
53+
}
54+
55+
dn = strings.TrimSuffix(dn, string(os.PathSeparator))
56+
prefix = strings.TrimSuffix(prefix, string(os.PathSeparator))
57+
58+
dirs := strings.Split(dn, string(os.PathSeparator))[1:]
59+
prefixes := strings.Split(prefix, string(os.PathSeparator))[1:]
60+
61+
if len(prefixes) > len(dirs) {
62+
return false
63+
}
64+
65+
var d, p string
66+
67+
for i := range prefixes {
68+
// need to test each component of the path for
69+
// case-sensitiveness because on Unix we could have
70+
// something like ext4 filesystem mounted on FAT
71+
// mountpoint, mounted on ext4 filesystem, i.e. the
72+
// problematic filesystem is not the last one.
73+
if isCaseSensitiveFilesystem(filepath.Join(d, dirs[i])) {
74+
d = filepath.Join(d, dirs[i])
75+
p = filepath.Join(p, prefixes[i])
76+
} else {
77+
d = filepath.Join(d, strings.ToLower(dirs[i]))
78+
p = filepath.Join(p, strings.ToLower(prefixes[i]))
79+
}
80+
81+
if p != d {
82+
return false
83+
}
84+
}
85+
86+
return true
87+
}
88+
89+
// genTestFilename returns a string with at most one rune case-flipped.
90+
//
91+
// The transformation is applied only to the first rune that can be
92+
// reversibly case-flipped, meaning:
93+
//
94+
// * A lowercase rune for which it's true that lower(upper(r)) == r
95+
// * An uppercase rune for which it's true that upper(lower(r)) == r
96+
//
97+
// All the other runes are left intact.
98+
func genTestFilename(str string) string {
99+
flip := true
100+
return strings.Map(func(r rune) rune {
101+
if flip {
102+
if unicode.IsLower(r) {
103+
u := unicode.ToUpper(r)
104+
if unicode.ToLower(u) == r {
105+
r = u
106+
flip = false
107+
}
108+
} else if unicode.IsUpper(r) {
109+
l := unicode.ToLower(r)
110+
if unicode.ToUpper(l) == r {
111+
r = l
112+
flip = false
113+
}
114+
}
115+
}
116+
return r
117+
}, str)
118+
}
119+
120+
// isCaseSensitiveFilesystem determines if the filesystem where dir
121+
// exists is case sensitive or not.
122+
//
123+
// CAVEAT: this function works by taking the last component of the given
124+
// path and flipping the case of the first letter for which case
125+
// flipping is a reversible operation (/foo/Bar → /foo/bar), then
126+
// testing for the existence of the new filename. There are two
127+
// possibilities:
128+
//
129+
// 1. The alternate filename does not exist. We can conclude that the
130+
// filesystem is case sensitive.
131+
//
132+
// 2. The filename happens to exist. We have to test if the two files
133+
// are the same file (case insensitive file system) or different ones
134+
// (case sensitive filesystem).
135+
//
136+
// If the input directory is such that the last component is composed
137+
// exclusively of case-less codepoints (e.g. numbers), this function will
138+
// return false.
139+
func isCaseSensitiveFilesystem(dir string) bool {
140+
alt := filepath.Join(filepath.Dir(dir),
141+
genTestFilename(filepath.Base(dir)))
142+
143+
dInfo, err := os.Stat(dir)
144+
if err != nil {
145+
return true
146+
}
147+
148+
aInfo, err := os.Stat(alt)
149+
if err != nil {
150+
return true
151+
}
152+
153+
return !os.SameFile(dInfo, aInfo)
154+
}

internal/fs_test.go

+92
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
package internal
2+
3+
import (
4+
"io/ioutil"
5+
"os"
6+
"path/filepath"
7+
"testing"
8+
)
9+
10+
func TestHasFilepathPrefix(t *testing.T) {
11+
dir, err := ioutil.TempDir("", "dep")
12+
if err != nil {
13+
t.Fatal(err)
14+
}
15+
defer os.RemoveAll(dir)
16+
17+
cases := []struct {
18+
dir string
19+
prefix string
20+
want bool
21+
}{
22+
{filepath.Join(dir, "a", "b"), filepath.Join(dir), true},
23+
{filepath.Join(dir, "a", "b"), filepath.Join(dir, "a"), true},
24+
{filepath.Join(dir, "a", "b"), filepath.Join(dir, "a", "b"), true},
25+
{filepath.Join(dir, "a", "b"), filepath.Join(dir, "c"), false},
26+
{filepath.Join(dir, "a", "b"), filepath.Join(dir, "a", "d", "b"), false},
27+
{filepath.Join(dir, "a", "b"), filepath.Join(dir, "a", "b2"), false},
28+
{filepath.Join(dir, "ab"), filepath.Join(dir, "a", "b"), false},
29+
{filepath.Join(dir, "ab"), filepath.Join(dir, "a"), false},
30+
{filepath.Join(dir, "123"), filepath.Join(dir, "123"), true},
31+
{filepath.Join(dir, "123"), filepath.Join(dir, "1"), false},
32+
{filepath.Join(dir, "⌘"), filepath.Join(dir, "⌘"), true},
33+
{filepath.Join(dir, "a"), filepath.Join(dir, "⌘"), false},
34+
{filepath.Join(dir, "⌘"), filepath.Join(dir, "a"), false},
35+
}
36+
37+
for _, c := range cases {
38+
err := os.MkdirAll(c.dir, 0755)
39+
if err != nil {
40+
t.Fatal(err)
41+
}
42+
43+
err = os.MkdirAll(c.prefix, 0755)
44+
if err != nil {
45+
t.Fatal(err)
46+
}
47+
48+
got := HasFilepathPrefix(c.dir, c.prefix)
49+
if c.want != got {
50+
t.Fatalf("dir: %q, prefix: %q, expected: %v, got: %v", c.dir, c.prefix, c.want, got)
51+
}
52+
}
53+
}
54+
55+
func TestGenTestFilename(t *testing.T) {
56+
cases := []struct {
57+
str string
58+
want string
59+
}{
60+
{"abc", "Abc"},
61+
{"ABC", "aBC"},
62+
{"AbC", "abC"},
63+
{"αβγ", "Αβγ"},
64+
{"123", "123"},
65+
{"1a2", "1A2"},
66+
{"12a", "12A"},
67+
{"⌘", "⌘"},
68+
}
69+
70+
for _, c := range cases {
71+
got := genTestFilename(c.str)
72+
if c.want != got {
73+
t.Fatalf("str: %q, expected: %q, got: %q", c.str, c.want, got)
74+
}
75+
}
76+
}
77+
78+
func BenchmarkGenTestFilename(b *testing.B) {
79+
cases := []string{
80+
"aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
81+
"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA",
82+
"αααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααααα",
83+
"11111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111111",
84+
"⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘⌘",
85+
}
86+
87+
for i := 0; i < b.N; i++ {
88+
for _, str := range cases {
89+
genTestFilename(str)
90+
}
91+
}
92+
}

0 commit comments

Comments
 (0)