125 lines
3.7 KiB
Diff
125 lines
3.7 KiB
Diff
From c941e27e70c3e06e1011d2dd71d72a7a06a9bcbc Mon Sep 17 00:00:00 2001
|
|
From: Ian Lance Taylor <iant@golang.org>
|
|
Date: Thu, 15 Feb 2018 15:57:13 -0800
|
|
Subject: [PATCH] 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 <bradfitz@golang.org>
|
|
---
|
|
src/cmd/go/internal/get/vcs.go | 34 +++++++++++++++++++++++++++--
|
|
src/cmd/go/internal/get/vcs_test.go | 43 +++++++++++++++++++++++++++++++++++++
|
|
2 files changed, 75 insertions(+), 2 deletions(-)
|
|
|
|
diff --git a/src/cmd/go/internal/get/vcs.go b/src/cmd/go/internal/get/vcs.go
|
|
index ee6b16a1369..dced0ed8db5 100644
|
|
--- a/src/cmd/go/internal/get/vcs.go
|
|
+++ b/src/cmd/go/internal/get/vcs.go
|
|
@@ -809,8 +809,8 @@ func repoRootForImportDynamic(importPath string, security web.SecurityMode) (*re
|
|
}
|
|
}
|
|
|
|
- if !strings.Contains(mmi.RepoRoot, "://") {
|
|
- return nil, fmt.Errorf("%s: invalid repo root %q; no scheme", urlStr, mmi.RepoRoot)
|
|
+ if err := validateRepoRootScheme(mmi.RepoRoot); err != nil {
|
|
+ return nil, fmt.Errorf("%s: invalid repo root %q: %v", urlStr, mmi.RepoRoot, err)
|
|
}
|
|
rr := &repoRoot{
|
|
vcs: vcsByCmd(mmi.VCS),
|
|
@@ -824,6 +824,36 @@ func repoRootForImportDynamic(importPath string, security web.SecurityMode) (*re
|
|
return rr, nil
|
|
}
|
|
|
|
+// validateRepoRootScheme returns an error if repoRoot does not seem
|
|
+// to have a valid URL scheme. At this point we permit things that
|
|
+// aren't valid URLs, although later, if not using -insecure, we will
|
|
+// restrict repoRoots to be valid URLs. This is only because we've
|
|
+// historically permitted them, and people may depend on that.
|
|
+func validateRepoRootScheme(repoRoot string) error {
|
|
+ end := strings.Index(repoRoot, "://")
|
|
+ if end <= 0 {
|
|
+ return errors.New("no scheme")
|
|
+ }
|
|
+
|
|
+ // RFC 3986 section 3.1.
|
|
+ for i := 0; i < end; i++ {
|
|
+ c := repoRoot[i]
|
|
+ switch {
|
|
+ case 'a' <= c && c <= 'z' || 'A' <= c && c <= 'Z':
|
|
+ // OK.
|
|
+ case '0' <= c && c <= '9' || c == '+' || c == '-' || c == '.':
|
|
+ // OK except at start.
|
|
+ if i == 0 {
|
|
+ return errors.New("invalid scheme")
|
|
+ }
|
|
+ default:
|
|
+ return errors.New("invalid scheme")
|
|
+ }
|
|
+ }
|
|
+
|
|
+ return nil
|
|
+}
|
|
+
|
|
var fetchGroup singleflight.Group
|
|
var (
|
|
fetchCacheMu sync.Mutex
|
|
diff --git a/src/cmd/go/internal/get/vcs_test.go b/src/cmd/go/internal/get/vcs_test.go
|
|
index 2cb611fabd8..ece78b563ce 100644
|
|
--- a/src/cmd/go/internal/get/vcs_test.go
|
|
+++ b/src/cmd/go/internal/get/vcs_test.go
|
|
@@ -416,3 +416,46 @@ func TestMatchGoImport(t *testing.T) {
|
|
}
|
|
}
|
|
}
|
|
+
|
|
+func TestValidateRepoRootScheme(t *testing.T) {
|
|
+ tests := []struct {
|
|
+ root string
|
|
+ err string
|
|
+ }{
|
|
+ {
|
|
+ root: "",
|
|
+ err: "no scheme",
|
|
+ },
|
|
+ {
|
|
+ root: "http://",
|
|
+ err: "",
|
|
+ },
|
|
+ {
|
|
+ root: "a://",
|
|
+ err: "",
|
|
+ },
|
|
+ {
|
|
+ root: "a#://",
|
|
+ err: "invalid scheme",
|
|
+ },
|
|
+ {
|
|
+ root: "-config://",
|
|
+ err: "invalid scheme",
|
|
+ },
|
|
+ }
|
|
+
|
|
+ for _, test := range tests {
|
|
+ err := validateRepoRootScheme(test.root)
|
|
+ if err == nil {
|
|
+ if test.err != "" {
|
|
+ t.Errorf("validateRepoRootScheme(%q) = nil, want %q", test.root, test.err)
|
|
+ }
|
|
+ } else if test.err == "" {
|
|
+ if err != nil {
|
|
+ t.Errorf("validateRepoRootScheme(%q) = %q, want nil", test.root, test.err)
|
|
+ }
|
|
+ } else if err.Error() != test.err {
|
|
+ t.Errorf("validateRepoRootScheme(%q) = %q, want %q", test.root, err, test.err)
|
|
+ }
|
|
+ }
|
|
+}
|