From 5bf6c97e76c721242a9b064950cd901c33f6f0b9 Mon Sep 17 00:00:00 2001 From: tenkoh Date: Wed, 30 Mar 2022 15:30:19 +0900 Subject: [PATCH] cmd/go: open correct path when loading embeds from root directory The existing implementation of `load.resolveEmbed` uses an expression like `path[len(pkgdir)+1:]`. Though the `+1` is intended to remove a prefix slash, the expression returns an incorrect path when `pkgdir` is "/". (ex.: when removing "/" from "/foo", want "foo", but got "oo") It seems that `str.TrimFilePathPrefix` would solve the problem, but the function contains the same bug. So, this commit fixes `str.TrimFilePathPrefix` then applies it to `load.resolveEmbed` to solve the issue. The fix is quite simple. First, remove prefix. Then check whether the remained first letter is equal to `filepath.Separator`. If so, remove it then return. Fixed #49570 Change-Id: I26ab727ee4dfcbf51ed9bd0a573957ced2154515 Reviewed-on: https://go-review.googlesource.com/c/go/+/396694 Reviewed-by: Bryan Mills Run-TryBot: Bryan Mills Auto-Submit: Bryan Mills TryBot-Result: Gopher Robot Reviewed-by: Russ Cox --- src/cmd/go/internal/load/pkg.go | 5 +- src/cmd/go/internal/str/path.go | 9 ++-- src/cmd/go/internal/str/str_test.go | 71 +++++++++++++++++++++++++++++ 3 files changed, 80 insertions(+), 5 deletions(-) diff --git a/src/cmd/go/internal/load/pkg.go b/src/cmd/go/internal/load/pkg.go index 2592cf5447..10a980fc65 100644 --- a/src/cmd/go/internal/load/pkg.go +++ b/src/cmd/go/internal/load/pkg.go @@ -2056,7 +2056,8 @@ func resolveEmbed(pkgdir string, patterns []string) (files []string, pmap map[st // then there may be other things lying around, like symbolic links or .git directories.) var list []string for _, file := range match { - rel := filepath.ToSlash(file[len(pkgdir)+1:]) // file, relative to p.Dir + // relative path to p.Dir which begins without prefix slash + rel := filepath.ToSlash(str.TrimFilePathPrefix(file, pkgdir)) what := "file" info, err := fsys.Lstat(file) @@ -2106,7 +2107,7 @@ func resolveEmbed(pkgdir string, patterns []string) (files []string, pmap map[st if err != nil { return err } - rel := filepath.ToSlash(path[len(pkgdir)+1:]) + rel := filepath.ToSlash(str.TrimFilePathPrefix(path, pkgdir)) name := info.Name() if path != file && (isBadEmbedName(name) || ((name[0] == '.' || name[0] == '_') && !all)) { // Ignore bad names, assuming they won't go into modules. diff --git a/src/cmd/go/internal/str/path.go b/src/cmd/go/internal/str/path.go index 0c8aaeaca1..a69e171f8c 100644 --- a/src/cmd/go/internal/str/path.go +++ b/src/cmd/go/internal/str/path.go @@ -58,8 +58,11 @@ func TrimFilePathPrefix(s, prefix string) string { if !HasFilePathPrefix(s, prefix) { return s } - if len(s) == len(prefix) { - return "" + 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 } - return s[len(prefix)+1:] + return trimmed[1:] } diff --git a/src/cmd/go/internal/str/str_test.go b/src/cmd/go/internal/str/str_test.go index 8ea758e0a8..158fe65dc1 100644 --- a/src/cmd/go/internal/str/str_test.go +++ b/src/cmd/go/internal/str/str_test.go @@ -5,6 +5,8 @@ package str import ( + "os" + "runtime" "testing" ) @@ -27,3 +29,72 @@ func TestFoldDup(t *testing.T) { } } } + +type trimFilePathPrefixTest struct { + s, prefix, want string +} + +func TestTrimFilePathPrefixSlash(t *testing.T) { + if os.PathSeparator != '/' { + t.Skipf("test requires slash-separated file paths") + } + tests := []trimFilePathPrefixTest{ + {"/foo", "", "foo"}, + {"/foo", "/", "foo"}, + {"/foo", "/foo", ""}, + {"/foo/bar", "/foo", "bar"}, + {"/foo/bar", "/foo/", "bar"}, + // 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 { + t.Errorf("TrimFilePathPrefix(%q, %q) = %q, want %q", tt.s, tt.prefix, got, tt.want) + } + } +} + +func TestTrimFilePathPrefixWindows(t *testing.T) { + if runtime.GOOS != "windows" { + t.Skipf("test requires Windows file paths") + } + tests := []trimFilePathPrefixTest{ + {`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`}, + // if volumes are different, return s + {`C:\foo`, ``, `C:\foo`}, + {`C:\foo`, `\foo`, `C:\foo`}, + {`C:\foo`, `D:\foo`, `C:\foo`}, + + //UNC path + {`\\host\share\foo`, `\\host\share`, `foo`}, + {`\\host\share\foo`, `\\host\share\`, `foo`}, + {`\\host\share\foo`, `\\host\share\foo`, ``}, + {`\\host\share\foo\bar`, `\\host\share\foo`, `bar`}, + {`\\host\share\foo\bar`, `\\host\share\foo\`, `bar`}, + // if prefix is not s's prefix, return s + {`\\host\share\foo`, `\\host\share\bar`, `\\host\share\foo`}, + {`\\host\share\foo`, `\\host\share\foo\bar`, `\\host\share\foo`}, + // if either host or share name is different, return s + {`\\host\share\foo`, ``, `\\host\share\foo`}, + {`\\host\share\foo`, `\foo`, `\\host\share\foo`}, + {`\\host\share\foo`, `\\host\other\`, `\\host\share\foo`}, + {`\\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) + } + } +} -- 2.50.0