Permalink
Browse files

cmd/go: match go-import package prefixes by slash

The existing implementation for path collision resolution would
incorrectly determine that:

    example.org/aa

collides with:

    example.org/a

This change splits by slash rather than comparing on a byte-by-byte
basis.

Fixes: #15947

Change-Id: I18b3aaafbc787c81253203cf1328bb3c4420a0c4
Reviewed-on: https://go-review.googlesource.com/23732
Reviewed-by: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
  • Loading branch information...
smcquay authored and ianlancetaylor committed Jun 3, 2016
1 parent 4b64c53 commit adf261b7d6ed17bdefadc29ff71ddaba667b1a99
Showing with 111 additions and 1 deletion.
  1. +17 −1 src/cmd/go/vcs.go
  2. +94 −0 src/cmd/go/vcs_test.go
View
@@ -779,15 +779,31 @@ type metaImport struct {
// errNoMatch is returned from matchGoImport when there's no applicable match.
var errNoMatch = errors.New("no import match")
func splitPathHasPrefix(path, prefix []string) bool {
if len(path) < len(prefix) {
return false
}
for i, p := range prefix {
if path[i] != p {
return false
}
}
return true
}
// matchGoImport returns the metaImport from imports matching importPath.
// An error is returned if there are multiple matches.
// errNoMatch is returned if none match.
func matchGoImport(imports []metaImport, importPath string) (_ metaImport, err error) {
match := -1
imp := strings.Split(importPath, "/")
for i, im := range imports {
if !strings.HasPrefix(importPath, im.Prefix) {
pre := strings.Split(im.Prefix, "/")
if !splitPathHasPrefix(imp, pre) {
continue
}
if match != -1 {
err = fmt.Errorf("multiple meta tags match import path %q", importPath)
return
View
@@ -5,6 +5,7 @@
package main
import (
"errors"
"internal/testenv"
"io/ioutil"
"os"
@@ -227,3 +228,96 @@ func TestIsSecure(t *testing.T) {
}
}
}
func TestMatchGoImport(t *testing.T) {
tests := []struct {
imports []metaImport
path string
mi metaImport
err error
}{
{
imports: []metaImport{
{Prefix: "example.com/user/foo", VCS: "git", RepoRoot: "https://example.com/repo/target"},
},
path: "example.com/user/foo",
mi: metaImport{Prefix: "example.com/user/foo", VCS: "git", RepoRoot: "https://example.com/repo/target"},
},
{
imports: []metaImport{
{Prefix: "example.com/user/foo", VCS: "git", RepoRoot: "https://example.com/repo/target"},
},
path: "example.com/user/foo/",
mi: metaImport{Prefix: "example.com/user/foo", VCS: "git", RepoRoot: "https://example.com/repo/target"},
},
{
imports: []metaImport{
{Prefix: "example.com/user/foo", VCS: "git", RepoRoot: "https://example.com/repo/target"},
{Prefix: "example.com/user/fooa", VCS: "git", RepoRoot: "https://example.com/repo/target"},
},
path: "example.com/user/foo",
mi: metaImport{Prefix: "example.com/user/foo", VCS: "git", RepoRoot: "https://example.com/repo/target"},
},
{
imports: []metaImport{
{Prefix: "example.com/user/foo", VCS: "git", RepoRoot: "https://example.com/repo/target"},
{Prefix: "example.com/user/fooa", VCS: "git", RepoRoot: "https://example.com/repo/target"},
},
path: "example.com/user/fooa",
mi: metaImport{Prefix: "example.com/user/fooa", VCS: "git", RepoRoot: "https://example.com/repo/target"},
},
{
imports: []metaImport{
{Prefix: "example.com/user/foo", VCS: "git", RepoRoot: "https://example.com/repo/target"},
{Prefix: "example.com/user/foo/bar", VCS: "git", RepoRoot: "https://example.com/repo/target"},
},
path: "example.com/user/foo/bar",
err: errors.New("should not be allowed to create nested repo"),
},
{
imports: []metaImport{
{Prefix: "example.com/user/foo", VCS: "git", RepoRoot: "https://example.com/repo/target"},
{Prefix: "example.com/user/foo/bar", VCS: "git", RepoRoot: "https://example.com/repo/target"},
},
path: "example.com/user/foo/bar/baz",
err: errors.New("should not be allowed to create nested repo"),
},
{
imports: []metaImport{
{Prefix: "example.com/user/foo", VCS: "git", RepoRoot: "https://example.com/repo/target"},
{Prefix: "example.com/user/foo/bar", VCS: "git", RepoRoot: "https://example.com/repo/target"},
},
path: "example.com/user/foo/bar/baz/qux",
err: errors.New("should not be allowed to create nested repo"),
},
{
imports: []metaImport{
{Prefix: "example.com/user/foo", VCS: "git", RepoRoot: "https://example.com/repo/target"},
{Prefix: "example.com/user/foo/bar", VCS: "git", RepoRoot: "https://example.com/repo/target"},
},
path: "example.com/user/foo/bar/baz/",
err: errors.New("should not be allowed to create nested repo"),
},
{
imports: []metaImport{
{Prefix: "example.com/user/foo", VCS: "git", RepoRoot: "https://example.com/repo/target"},
{Prefix: "example.com/user/foo/bar", VCS: "git", RepoRoot: "https://example.com/repo/target"},
},
path: "example.com",
err: errors.New("pathologically short path"),
},
}
for _, test := range tests {
mi, err := matchGoImport(test.imports, test.path)
if mi != test.mi {
t.Errorf("unexpected metaImport; got %v, want %v", mi, test.mi)
}
got := err
want := test.err
if (got == nil) != (want == nil) {
t.Errorf("unexpected error; got %v, want %v", got, want)
}
}
}

0 comments on commit adf261b

Please sign in to comment.