Skip to content

Commit 15474dc

Browse files
ianlancetaylorandybons
authored andcommitted
[release-branch.go1.9] cmd/go: restrict meta imports to valid schemes
Before this change, when using -insecure, we permitted any meta import repo root as long as it contained "://". When not using -insecure, we restrict meta import repo roots to be valid URLs. People may depend on that somehow, so permit meta import repo roots to be invalid URLs, but require them to have valid schemes per RFC 3986. Fixes #23867 Change-Id: Iac666dfc75ac321bf8639dda5b0dba7c8840922d Reviewed-on: https://go-review.googlesource.com/94603 Reviewed-by: Brad Fitzpatrick <[email protected]> Reviewed-on: https://go-review.googlesource.com/102776 Run-TryBot: Andrew Bonventre <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]>
1 parent 7467b00 commit 15474dc

File tree

2 files changed

+75
-2
lines changed

2 files changed

+75
-2
lines changed

src/cmd/go/internal/get/vcs.go

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -767,8 +767,8 @@ func repoRootForImportDynamic(importPath string, security web.SecurityMode) (*re
767767
}
768768
}
769769

770-
if !strings.Contains(mmi.RepoRoot, "://") {
771-
return nil, fmt.Errorf("%s: invalid repo root %q; no scheme", urlStr, mmi.RepoRoot)
770+
if err := validateRepoRootScheme(mmi.RepoRoot); err != nil {
771+
return nil, fmt.Errorf("%s: invalid repo root %q: %v", urlStr, mmi.RepoRoot, err)
772772
}
773773
rr := &repoRoot{
774774
vcs: vcsByCmd(mmi.VCS),
@@ -782,6 +782,36 @@ func repoRootForImportDynamic(importPath string, security web.SecurityMode) (*re
782782
return rr, nil
783783
}
784784

785+
// validateRepoRootScheme returns an error if repoRoot does not seem
786+
// to have a valid URL scheme. At this point we permit things that
787+
// aren't valid URLs, although later, if not using -insecure, we will
788+
// restrict repoRoots to be valid URLs. This is only because we've
789+
// historically permitted them, and people may depend on that.
790+
func validateRepoRootScheme(repoRoot string) error {
791+
end := strings.Index(repoRoot, "://")
792+
if end <= 0 {
793+
return errors.New("no scheme")
794+
}
795+
796+
// RFC 3986 section 3.1.
797+
for i := 0; i < end; i++ {
798+
c := repoRoot[i]
799+
switch {
800+
case 'a' <= c && c <= 'z' || 'A' <= c && c <= 'Z':
801+
// OK.
802+
case '0' <= c && c <= '9' || c == '+' || c == '-' || c == '.':
803+
// OK except at start.
804+
if i == 0 {
805+
return errors.New("invalid scheme")
806+
}
807+
default:
808+
return errors.New("invalid scheme")
809+
}
810+
}
811+
812+
return nil
813+
}
814+
785815
var fetchGroup singleflight.Group
786816
var (
787817
fetchCacheMu sync.Mutex

src/cmd/go/internal/get/vcs_test.go

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -390,3 +390,46 @@ func TestMatchGoImport(t *testing.T) {
390390
}
391391
}
392392
}
393+
394+
func TestValidateRepoRootScheme(t *testing.T) {
395+
tests := []struct {
396+
root string
397+
err string
398+
}{
399+
{
400+
root: "",
401+
err: "no scheme",
402+
},
403+
{
404+
root: "http://",
405+
err: "",
406+
},
407+
{
408+
root: "a://",
409+
err: "",
410+
},
411+
{
412+
root: "a#://",
413+
err: "invalid scheme",
414+
},
415+
{
416+
root: "-config://",
417+
err: "invalid scheme",
418+
},
419+
}
420+
421+
for _, test := range tests {
422+
err := validateRepoRootScheme(test.root)
423+
if err == nil {
424+
if test.err != "" {
425+
t.Errorf("validateRepoRootScheme(%q) = nil, want %q", test.root, test.err)
426+
}
427+
} else if test.err == "" {
428+
if err != nil {
429+
t.Errorf("validateRepoRootScheme(%q) = %q, want nil", test.root, test.err)
430+
}
431+
} else if err.Error() != test.err {
432+
t.Errorf("validateRepoRootScheme(%q) = %q, want %q", test.root, err, test.err)
433+
}
434+
}
435+
}

0 commit comments

Comments
 (0)