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 <bcmills@google.com>
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Russ Cox <rsc@golang.org>

diff --git a/src/cmd/go/internal/load/pkg.go b/src/cmd/go/internal/load/pkg.go
index 2592cf54477405fb2776dc7cb3b5fb3f5120cc3a..10a980fc65188b4b22bdfa64c53905828beda38d 100644 (file)
--- 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 0c8aaeaca1fba0f5f2cbc0d27c2262d8d43932f5..a69e171f8cec79bd9e91cb1fa8b8cfdb2bd92d55 100644 (file)
--- 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 8ea758e0a8b64bf0c8e4314660f9b79050fb4383..158fe65dc16edfc7e8f9136fcdb9b279bcc1ab74 100644 (file)
--- 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)
+               }
+       }
+}