From 4c1a7ab49c4c68907bc7f7f7f776edd9116584a5 Mon Sep 17 00:00:00 2001 From: Baokun Lee Date: Mon, 18 Jan 2021 14:41:20 +0800 Subject: [PATCH] cmd/go: reject relative paths in GOMODCACHE environment Go already rejects relative paths in a couple environment variables, It should reject relative paths in GOMODCACHE. Fixes #43715 Change-Id: Id1ceff839c7ab21c00cf4ace45ce48324733a526 Reviewed-on: https://go-review.googlesource.com/c/go/+/284432 Run-TryBot: Baokun Lee TryBot-Result: Go Bot Reviewed-by: Jay Conrod Reviewed-by: Bryan C. Mills Trust: Jay Conrod Trust: Baokun Lee --- src/cmd/go/internal/envcmd/env.go | 2 +- src/cmd/go/internal/modfetch/cache.go | 31 ++++++++++++-------- src/cmd/go/internal/modfetch/fetch.go | 6 ++-- src/cmd/go/testdata/script/env_write.txt | 6 ++++ src/cmd/go/testdata/script/mod_cache_dir.txt | 11 +++++++ 5 files changed, 39 insertions(+), 17 deletions(-) create mode 100644 src/cmd/go/testdata/script/mod_cache_dir.txt diff --git a/src/cmd/go/internal/envcmd/env.go b/src/cmd/go/internal/envcmd/env.go index 6937187522..aad5d704e5 100644 --- a/src/cmd/go/internal/envcmd/env.go +++ b/src/cmd/go/internal/envcmd/env.go @@ -428,7 +428,7 @@ func checkEnvWrite(key, val string) error { return fmt.Errorf("GOPATH entry is relative; must be absolute path: %q", val) } // Make sure CC and CXX are absolute paths - case "CC", "CXX": + case "CC", "CXX", "GOMODCACHE": if !filepath.IsAbs(val) && val != "" && val != filepath.Base(val) { return fmt.Errorf("%s entry is relative; must be absolute path: %q", key, val) } diff --git a/src/cmd/go/internal/modfetch/cache.go b/src/cmd/go/internal/modfetch/cache.go index 3a2ff63721..9e751931a0 100644 --- a/src/cmd/go/internal/modfetch/cache.go +++ b/src/cmd/go/internal/modfetch/cache.go @@ -28,10 +28,8 @@ import ( ) func cacheDir(path string) (string, error) { - if cfg.GOMODCACHE == "" { - // modload.Init exits if GOPATH[0] is empty, and cfg.GOMODCACHE - // is set to GOPATH[0]/pkg/mod if GOMODCACHE is empty, so this should never happen. - return "", fmt.Errorf("internal error: cfg.GOMODCACHE not set") + if err := checkCacheDir(); err != nil { + return "", err } enc, err := module.EscapePath(path) if err != nil { @@ -64,10 +62,8 @@ func CachePath(m module.Version, suffix string) (string, error) { // along with the directory if the directory does not exist or if the directory // is not completely populated. func DownloadDir(m module.Version) (string, error) { - if cfg.GOMODCACHE == "" { - // modload.Init exits if GOPATH[0] is empty, and cfg.GOMODCACHE - // is set to GOPATH[0]/pkg/mod if GOMODCACHE is empty, so this should never happen. - return "", fmt.Errorf("internal error: cfg.GOMODCACHE not set") + if err := checkCacheDir(); err != nil { + return "", err } enc, err := module.EscapePath(m.Path) if err != nil { @@ -134,10 +130,8 @@ func lockVersion(mod module.Version) (unlock func(), err error) { // user's working directory. // If err is nil, the caller MUST eventually call the unlock function. func SideLock() (unlock func(), err error) { - if cfg.GOMODCACHE == "" { - // modload.Init exits if GOPATH[0] is empty, and cfg.GOMODCACHE - // is set to GOPATH[0]/pkg/mod if GOMODCACHE is empty, so this should never happen. - base.Fatalf("go: internal error: cfg.GOMODCACHE not set") + if err := checkCacheDir(); err != nil { + base.Fatalf("go: %v", err) } path := filepath.Join(cfg.GOMODCACHE, "cache", "lock") @@ -633,3 +627,16 @@ func rewriteVersionList(dir string) { base.Fatalf("go: failed to write version list: %v", err) } } + +func checkCacheDir() error { + if cfg.GOMODCACHE == "" { + // modload.Init exits if GOPATH[0] is empty, and cfg.GOMODCACHE + // is set to GOPATH[0]/pkg/mod if GOMODCACHE is empty, so this should never happen. + return fmt.Errorf("internal error: cfg.GOMODCACHE not set") + } + + if !filepath.IsAbs(cfg.GOMODCACHE) { + return fmt.Errorf("GOMODCACHE entry is relative; must be absolute path: %q.\n", cfg.GOMODCACHE) + } + return nil +} diff --git a/src/cmd/go/internal/modfetch/fetch.go b/src/cmd/go/internal/modfetch/fetch.go index c55c3cf253..d5ad277dd0 100644 --- a/src/cmd/go/internal/modfetch/fetch.go +++ b/src/cmd/go/internal/modfetch/fetch.go @@ -37,10 +37,8 @@ var downloadCache par.Cache // local download cache and returns the name of the directory // corresponding to the root of the module's file tree. func Download(ctx context.Context, mod module.Version) (dir string, err error) { - if cfg.GOMODCACHE == "" { - // modload.Init exits if GOPATH[0] is empty, and cfg.GOMODCACHE - // is set to GOPATH[0]/pkg/mod if GOMODCACHE is empty, so this should never happen. - base.Fatalf("go: internal error: cfg.GOMODCACHE not set") + if err := checkCacheDir(); err != nil { + base.Fatalf("go: %v", err) } // The par.Cache here avoids duplicate work. diff --git a/src/cmd/go/testdata/script/env_write.txt b/src/cmd/go/testdata/script/env_write.txt index bda1e57826..4fa39df104 100644 --- a/src/cmd/go/testdata/script/env_write.txt +++ b/src/cmd/go/testdata/script/env_write.txt @@ -173,3 +173,9 @@ go env -w GOOS=linux GOARCH=mips env GOOS=windows ! go env -u GOOS stderr 'unsupported GOOS/GOARCH.*windows/mips$' + +# go env -w should reject relative paths in GOMODCACHE environment. +! go env -w GOMODCACHE=~/test +stderr 'go env -w: GOMODCACHE entry is relative; must be absolute path: "~/test"' +! go env -w GOMODCACHE=./test +stderr 'go env -w: GOMODCACHE entry is relative; must be absolute path: "./test"' diff --git a/src/cmd/go/testdata/script/mod_cache_dir.txt b/src/cmd/go/testdata/script/mod_cache_dir.txt new file mode 100644 index 0000000000..7284ccf8ba --- /dev/null +++ b/src/cmd/go/testdata/script/mod_cache_dir.txt @@ -0,0 +1,11 @@ +env GO111MODULE=on + +# Go should reject relative paths in GOMODCACHE environment. + +env GOMODCACHE="~/test" +! go get example.com/tools/cmd/hello +stderr 'must be absolute path' + +env GOMODCACHE="./test" +! go get example.com/tools/cmd/hello +stderr 'must be absolute path'