Skip to content

Commit 6936a87

Browse files
ianlancetaylorandybons
authored andcommitted
[release-branch.go1.10] 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/102778 Run-TryBot: Andrew Bonventre <[email protected]> Reviewed-by: Ian Lance Taylor <[email protected]> TryBot-Result: Gobot Gobot <[email protected]>
1 parent 678dede commit 6936a87

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
@@ -809,8 +809,8 @@ func repoRootForImportDynamic(importPath string, security web.SecurityMode) (*re
809809
}
810810
}
811811

812-
if !strings.Contains(mmi.RepoRoot, "://") {
813-
return nil, fmt.Errorf("%s: invalid repo root %q; no scheme", urlStr, mmi.RepoRoot)
812+
if err := validateRepoRootScheme(mmi.RepoRoot); err != nil {
813+
return nil, fmt.Errorf("%s: invalid repo root %q: %v", urlStr, mmi.RepoRoot, err)
814814
}
815815
rr := &repoRoot{
816816
vcs: vcsByCmd(mmi.VCS),
@@ -824,6 +824,36 @@ func repoRootForImportDynamic(importPath string, security web.SecurityMode) (*re
824824
return rr, nil
825825
}
826826

827+
// validateRepoRootScheme returns an error if repoRoot does not seem
828+
// to have a valid URL scheme. At this point we permit things that
829+
// aren't valid URLs, although later, if not using -insecure, we will
830+
// restrict repoRoots to be valid URLs. This is only because we've
831+
// historically permitted them, and people may depend on that.
832+
func validateRepoRootScheme(repoRoot string) error {
833+
end := strings.Index(repoRoot, "://")
834+
if end <= 0 {
835+
return errors.New("no scheme")
836+
}
837+
838+
// RFC 3986 section 3.1.
839+
for i := 0; i < end; i++ {
840+
c := repoRoot[i]
841+
switch {
842+
case 'a' <= c && c <= 'z' || 'A' <= c && c <= 'Z':
843+
// OK.
844+
case '0' <= c && c <= '9' || c == '+' || c == '-' || c == '.':
845+
// OK except at start.
846+
if i == 0 {
847+
return errors.New("invalid scheme")
848+
}
849+
default:
850+
return errors.New("invalid scheme")
851+
}
852+
}
853+
854+
return nil
855+
}
856+
827857
var fetchGroup singleflight.Group
828858
var (
829859
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
@@ -408,3 +408,46 @@ func TestMatchGoImport(t *testing.T) {
408408
}
409409
}
410410
}
411+
412+
func TestValidateRepoRootScheme(t *testing.T) {
413+
tests := []struct {
414+
root string
415+
err string
416+
}{
417+
{
418+
root: "",
419+
err: "no scheme",
420+
},
421+
{
422+
root: "http://",
423+
err: "",
424+
},
425+
{
426+
root: "a://",
427+
err: "",
428+
},
429+
{
430+
root: "a#://",
431+
err: "invalid scheme",
432+
},
433+
{
434+
root: "-config://",
435+
err: "invalid scheme",
436+
},
437+
}
438+
439+
for _, test := range tests {
440+
err := validateRepoRootScheme(test.root)
441+
if err == nil {
442+
if test.err != "" {
443+
t.Errorf("validateRepoRootScheme(%q) = nil, want %q", test.root, test.err)
444+
}
445+
} else if test.err == "" {
446+
if err != nil {
447+
t.Errorf("validateRepoRootScheme(%q) = %q, want nil", test.root, test.err)
448+
}
449+
} else if err.Error() != test.err {
450+
t.Errorf("validateRepoRootScheme(%q) = %q, want %q", test.root, err, test.err)
451+
}
452+
}
453+
}

0 commit comments

Comments
 (0)