From: Bryan C. Mills Date: Fri, 20 Jan 2023 19:40:09 +0000 (-0500) Subject: cmd/go/internal/str: fix PathPrefix functions for root directories X-Git-Tag: go1.21rc1~1799 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=77cd3a46a86dca22348a313912093503f3cded66;p=gostls13.git cmd/go/internal/str: fix PathPrefix functions for root directories For #51506. For #50807. Change-Id: I4c0ae85a2103ac4f07351a4f01ce24fa02f03104 Reviewed-on: https://go-review.googlesource.com/c/go/+/463176 Run-TryBot: Bryan Mills TryBot-Result: Gopher Robot Auto-Submit: Bryan Mills Reviewed-by: Russ Cox --- diff --git a/src/cmd/go/internal/search/search.go b/src/cmd/go/internal/search/search.go index 60953fdee3..7ea6493d4a 100644 --- a/src/cmd/go/internal/search/search.go +++ b/src/cmd/go/internal/search/search.go @@ -8,6 +8,7 @@ import ( "cmd/go/internal/base" "cmd/go/internal/cfg" "cmd/go/internal/fsys" + "cmd/go/internal/str" "cmd/internal/pkgpattern" "fmt" "go/build" @@ -255,7 +256,7 @@ func (m *Match) MatchDirs(modRoots []string) { } var found bool for _, modRoot := range modRoots { - if modRoot != "" && hasFilepathPrefix(abs, modRoot) { + if modRoot != "" && str.HasFilePathPrefix(abs, modRoot) { found = true } } @@ -428,22 +429,6 @@ func CleanPatterns(patterns []string) []string { return out } -// hasFilepathPrefix reports whether the path s begins with the -// elements in prefix. -func hasFilepathPrefix(s, prefix string) bool { - switch { - default: - return false - case len(s) == len(prefix): - return s == prefix - case len(s) > len(prefix): - if prefix != "" && prefix[len(prefix)-1] == filepath.Separator { - return strings.HasPrefix(s, prefix) - } - return s[len(prefix)] == filepath.Separator && s[:len(prefix)] == prefix - } -} - // IsStandardImportPath reports whether $GOROOT/src/path should be considered // part of the standard distribution. For historical reasons we allow people to add // their own code to $GOROOT instead of using $GOPATH, but we assume that @@ -475,67 +460,44 @@ func IsRelativePath(pattern string) bool { // If not, InDir returns an empty string. // InDir makes some effort to succeed even in the presence of symbolic links. func InDir(path, dir string) string { - if rel := inDirLex(path, dir); rel != "" { + // inDirLex reports whether path is lexically in dir, + // without considering symbolic or hard links. + inDirLex := func(path, dir string) (string, bool) { + if dir == "" { + return path, true + } + rel := str.TrimFilePathPrefix(path, dir) + if rel == path { + return "", false + } + if rel == "" { + return ".", true + } + return rel, true + } + + if rel, ok := inDirLex(path, dir); ok { return rel } xpath, err := filepath.EvalSymlinks(path) if err != nil || xpath == path { xpath = "" } else { - if rel := inDirLex(xpath, dir); rel != "" { + if rel, ok := inDirLex(xpath, dir); ok { return rel } } xdir, err := filepath.EvalSymlinks(dir) if err == nil && xdir != dir { - if rel := inDirLex(path, xdir); rel != "" { + if rel, ok := inDirLex(path, xdir); ok { return rel } if xpath != "" { - if rel := inDirLex(xpath, xdir); rel != "" { + if rel, ok := inDirLex(xpath, xdir); ok { return rel } } } return "" } - -// inDirLex is like inDir but only checks the lexical form of the file names. -// It does not consider symbolic links. -// TODO(rsc): This is a copy of str.HasFilePathPrefix, modified to -// return the suffix. Most uses of str.HasFilePathPrefix should probably -// be calling InDir instead. -func inDirLex(path, dir string) string { - pv := strings.ToUpper(filepath.VolumeName(path)) - dv := strings.ToUpper(filepath.VolumeName(dir)) - path = path[len(pv):] - dir = dir[len(dv):] - switch { - default: - return "" - case pv != dv: - return "" - case len(path) == len(dir): - if path == dir { - return "." - } - return "" - case dir == "": - return path - case len(path) > len(dir): - if dir[len(dir)-1] == filepath.Separator { - if path[:len(dir)] == dir { - return path[len(dir):] - } - return "" - } - if path[len(dir)] == filepath.Separator && path[:len(dir)] == dir { - if len(path) == len(dir)+1 { - return "." - } - return path[len(dir)+1:] - } - return "" - } -} diff --git a/src/cmd/go/internal/str/path.go b/src/cmd/go/internal/str/path.go index c165b91785..0c8f47988e 100644 --- a/src/cmd/go/internal/str/path.go +++ b/src/cmd/go/internal/str/path.go @@ -5,7 +5,9 @@ package str import ( + "os" "path/filepath" + "runtime" "strings" ) @@ -28,11 +30,32 @@ func HasPathPrefix(s, prefix string) bool { // HasFilePathPrefix reports whether the filesystem path s // begins with the elements in prefix. +// +// HasFilePathPrefix is case-sensitive (except for volume names) even if the +// filesystem is not, does not apply Unicode normalization even if the +// filesystem does, and assumes that all path separators are canonicalized to +// filepath.Separator (as returned by filepath.Clean). func HasFilePathPrefix(s, prefix string) bool { - sv := strings.ToUpper(filepath.VolumeName(s)) - pv := strings.ToUpper(filepath.VolumeName(prefix)) + sv := filepath.VolumeName(s) + pv := filepath.VolumeName(prefix) + + // Strip the volume from both paths before canonicalizing sv and pv: + // it's unlikely that strings.ToUpper will change the length of the string, + // but doesn't seem impossible. s = s[len(sv):] prefix = prefix[len(pv):] + + // Always treat Windows volume names as case-insensitive, even though + // we don't treat the rest of the path as such. + // + // TODO(bcmills): Why do we care about case only for the volume name? It's + // been this way since https://go.dev/cl/11316, but I don't understand why + // that problem doesn't apply to case differences in the entire path. + if sv != pv { + sv = strings.ToUpper(sv) + pv = strings.ToUpper(pv) + } + switch { default: return false @@ -50,21 +73,36 @@ func HasFilePathPrefix(s, prefix string) bool { } } -// TrimFilePathPrefix returns s without the leading path elements in prefix. +// TrimFilePathPrefix returns s without the leading path elements in prefix, +// such that joining the string to prefix produces s. +// // If s does not start with prefix (HasFilePathPrefix with the same arguments // returns false), TrimFilePathPrefix returns s. If s equals prefix, // TrimFilePathPrefix returns "". func TrimFilePathPrefix(s, prefix string) string { + if prefix == "" { + // Trimming the empty string from a path should join to produce that path. + // (Trim("/tmp/foo", "") should give "/tmp/foo", not "tmp/foo".) + return s + } if !HasFilePathPrefix(s, prefix) { return s } + trimmed := s[len(prefix):] - if len(trimmed) == 0 || trimmed[0] != filepath.Separator { - // Prefix either is equal to s, or ends with a separator - // (for example, if it is exactly "/"). - return trimmed + if len(trimmed) > 0 && os.IsPathSeparator(trimmed[0]) { + if runtime.GOOS == "windows" && prefix == filepath.VolumeName(prefix) && len(prefix) == 2 && prefix[1] == ':' { + // Joining a relative path to a bare Windows drive letter produces a path + // relative to the working directory on that drive, but the original path + // was absolute, not relative. Keep the leading path separator so that it + // remains absolute when joined to prefix. + } else { + // Prefix ends in a regular path element, so strip the path separator that + // follows it. + trimmed = trimmed[1:] + } } - return trimmed[1:] + return trimmed } // QuoteGlob returns s with all Glob metacharacters quoted. diff --git a/src/cmd/go/internal/str/str_test.go b/src/cmd/go/internal/str/str_test.go index 158fe65dc1..7c19877666 100644 --- a/src/cmd/go/internal/str/str_test.go +++ b/src/cmd/go/internal/str/str_test.go @@ -6,7 +6,9 @@ package str import ( "os" + "path/filepath" "runtime" + "strings" "testing" ) @@ -30,29 +32,75 @@ func TestFoldDup(t *testing.T) { } } -type trimFilePathPrefixTest struct { - s, prefix, want string +func TestHasPathPrefix(t *testing.T) { + type testCase struct { + s, prefix string + want bool + } + for _, tt := range []testCase{ + {"", "", true}, + {"", "/", false}, + {"foo", "", true}, + {"foo", "/", false}, + {"foo", "foo", true}, + {"foo", "foo/", false}, + {"foo", "/foo", false}, + {"foo/bar", "", true}, + {"foo/bar", "foo", true}, + {"foo/bar", "foo/", true}, + {"foo/bar", "/foo", false}, + {"foo/bar", "foo/bar", true}, + {"foo/bar", "foo/bar/", false}, + {"foo/bar", "/foo/bar", false}, + } { + got := HasPathPrefix(tt.s, tt.prefix) + if got != tt.want { + t.Errorf("HasPathPrefix(%q, %q) = %v; want %v", tt.s, tt.prefix, got, tt.want) + } + } } func TestTrimFilePathPrefixSlash(t *testing.T) { if os.PathSeparator != '/' { t.Skipf("test requires slash-separated file paths") } - tests := []trimFilePathPrefixTest{ - {"/foo", "", "foo"}, + + type testCase struct { + s, prefix, want string + } + for _, tt := range []testCase{ + {"/", "", "/"}, + {"/", "/", ""}, + {"/foo", "", "/foo"}, {"/foo", "/", "foo"}, {"/foo", "/foo", ""}, {"/foo/bar", "/foo", "bar"}, {"/foo/bar", "/foo/", "bar"}, + {"/foo/", "/", "foo/"}, + {"/foo/", "/foo", ""}, + {"/foo/", "/foo/", ""}, + // if prefix is not s's prefix, return s + {"", "/", ""}, {"/foo", "/bar", "/foo"}, {"/foo", "/foo/bar", "/foo"}, - } - - for _, tt := range tests { - if got := TrimFilePathPrefix(tt.s, tt.prefix); got != tt.want { + {"foo", "/foo", "foo"}, + {"/foo", "foo", "/foo"}, + {"/foo", "/foo/", "/foo"}, + } { + got := TrimFilePathPrefix(tt.s, tt.prefix) + if got == tt.want { + t.Logf("TrimFilePathPrefix(%q, %q) = %q", tt.s, tt.prefix, got) + } else { t.Errorf("TrimFilePathPrefix(%q, %q) = %q, want %q", tt.s, tt.prefix, got, tt.want) } + + if HasFilePathPrefix(tt.s, tt.prefix) { + joined := filepath.Join(tt.prefix, got) + if clean := filepath.Clean(tt.s); joined != clean { + t.Errorf("filepath.Join(%q, %q) = %q, want %q", tt.prefix, got, joined, clean) + } + } } } @@ -60,16 +108,29 @@ func TestTrimFilePathPrefixWindows(t *testing.T) { if runtime.GOOS != "windows" { t.Skipf("test requires Windows file paths") } - tests := []trimFilePathPrefixTest{ - {`C:\foo`, `C:`, `foo`}, + type testCase struct { + s, prefix, want string + } + for _, tt := range []testCase{ + {`\`, ``, `\`}, + {`\`, `\`, ``}, + {`C:`, `C:`, ``}, + {`C:\`, `C:`, `\`}, + {`C:\`, `C:\`, ``}, + {`C:\foo`, ``, `C:\foo`}, + {`C:\foo`, `C:`, `\foo`}, {`C:\foo`, `C:\`, `foo`}, {`C:\foo`, `C:\foo`, ``}, + {`C:\foo\`, `C:\foo`, ``}, {`C:\foo\bar`, `C:\foo`, `bar`}, {`C:\foo\bar`, `C:\foo\`, `bar`}, // if prefix is not s's prefix, return s {`C:\foo`, `C:\bar`, `C:\foo`}, {`C:\foo`, `C:\foo\bar`, `C:\foo`}, + {`C:`, `C:\`, `C:`}, // if volumes are different, return s + {`C:`, ``, `C:`}, + {`C:\`, ``, `C:\`}, {`C:\foo`, ``, `C:\foo`}, {`C:\foo`, `\foo`, `C:\foo`}, {`C:\foo`, `D:\foo`, `C:\foo`}, @@ -90,11 +151,35 @@ func TestTrimFilePathPrefixWindows(t *testing.T) { {`\\host\share\foo`, `\\other\share\`, `\\host\share\foo`}, {`\\host\share\foo`, `\\host\`, `\\host\share\foo`}, {`\\host\share\foo`, `\share\`, `\\host\share\foo`}, - } - for _, tt := range tests { - if got := TrimFilePathPrefix(tt.s, tt.prefix); got != tt.want { - t.Errorf("TrimFilePathPrefix(%q, %q) = %q, want %q", tt.s, tt.prefix, got, tt.want) + // only volume names are case-insensitive + {`C:\foo`, `c:`, `\foo`}, + {`C:\foo`, `c:\foo`, ``}, + {`c:\foo`, `C:`, `\foo`}, + {`c:\foo`, `C:\foo`, ``}, + {`C:\foo`, `C:\Foo`, `C:\foo`}, + {`\\Host\Share\foo`, `\\host\share`, `foo`}, + {`\\Host\Share\foo`, `\\host\share\foo`, ``}, + {`\\host\share\foo`, `\\Host\Share`, `foo`}, + {`\\host\share\foo`, `\\Host\Share\foo`, ``}, + {`\\Host\Share\foo`, `\\Host\Share\Foo`, `\\Host\Share\foo`}, + } { + got := TrimFilePathPrefix(tt.s, tt.prefix) + if got == tt.want { + t.Logf("TrimFilePathPrefix(%#q, %#q) = %#q", tt.s, tt.prefix, got) + } else { + t.Errorf("TrimFilePathPrefix(%#q, %#q) = %#q, want %#q", tt.s, tt.prefix, got, tt.want) + } + + if HasFilePathPrefix(tt.s, tt.prefix) { + // Although TrimFilePathPrefix is only case-insensitive in the volume name, + // what we care about in testing Join is that absolute paths remain + // absolute and relative paths remaining relative — there is no harm in + // over-normalizing letters in the comparison, so we use EqualFold. + joined := filepath.Join(tt.prefix, got) + if clean := filepath.Clean(tt.s); !strings.EqualFold(joined, clean) { + t.Errorf("filepath.Join(%#q, %#q) = %#q, want %#q", tt.prefix, got, joined, clean) + } } } }