cmd/go/internal/modload: reject some bad module paths

This change rejects module paths that don't conform to
the new checkModulePathLax function, when loading a go.mod
file. The change uses the checkModulePathLax function instead of
CheckPath because there are still many users who are using
unpublished modules with unpublishable paths, and we don't
want to break them all.

Next, before this change, when go mod init is run in GOPATH,
it would try to use the location of the directory within GOPATH
to infer the module path. After this change, it will only use
that inferred module path if it conforms to module.CheckPath.

Change-Id: Idb36d1655cc76aae82671e87ba634609503ad1a2
Reviewed-on: https://go-review.googlesource.com/c/go/+/211597
Run-TryBot: Michael Matloob <matloob@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Bryan C. Mills <bcmills@google.com>

src/cmd/go/internal/modload/init.go

@@ -379,6 +379,10 @@ func InitMod(ctx context.Context) {
 		legacyModInit()
 	}
 
+	if err := checkModulePathLax(f.Module.Mod.Path); err != nil {
+		base.Fatalf("go: %v", err)
+	}
+
 	setDefaultBuildMod()
 	modFileToBuildList()
 	if cfg.BuildMod == "vendor" {
@@ -387,6 +391,49 @@ func InitMod(ctx context.Context) {
 	}
 }
 
+// checkModulePathLax checks that the path meets some minimum requirements
+// to avoid confusing users or the module cache. The requirements are weaker
+// than those of module.CheckPath to allow room for weakening module path
+// requirements in the future, but strong enough to help users avoid significant
+// problems.
+func checkModulePathLax(p string) error {
+	// TODO(matloob): Replace calls of this function in this CL with calls
+	// to module.CheckImportPath once it's been laxened, if it becomes laxened.
+	// See golang.org/issue/29101 for a discussion about whether to make CheckImportPath
+	// more lax or more strict.
+
+	errorf := func(format string, args ...interface{}) error {
+		return fmt.Errorf("invalid module path %q: %s", p, fmt.Sprintf(format, args...))
+	}
+
+	// Disallow shell characters " ' * < > ? ` | to avoid triggering bugs
+	// with file systems and subcommands. Disallow file path separators : and \
+	// because path separators other than / will confuse the module cache.
+	// See fileNameOK in golang.org/x/mod/module/module.go.
+	shellChars := "`" + `\"'*<>?|`
+	fsChars := `\:`
+	if i := strings.IndexAny(p, shellChars); i >= 0 {
+		return errorf("contains disallowed shell character %q", p[i])
+	}
+	if i := strings.IndexAny(p, fsChars); i >= 0 {
+		return errorf("contains disallowed path separator character %q", p[i])
+	}
+
+	// Ensure path.IsAbs and build.IsLocalImport are false, and that the path is
+	// invariant under path.Clean, also to avoid confusing the module cache.
+	if path.IsAbs(p) {
+		return errorf("is an absolute path")
+	}
+	if build.IsLocalImport(p) {
+		return errorf("is a local import path")
+	}
+	if path.Clean(p) != p {
+		return errorf("is not clean")
+	}
+
+	return nil
+}
+
 // fixVersion returns a modfile.VersionFixer implemented using the Query function.
 //
 // It resolves commit hashes and branch names to versions,
@@ -678,16 +725,35 @@ func findModulePath(dir string) (string, error) {
 	}
 
 	// Look for path in GOPATH.
+	var badPathErr error
 	for _, gpdir := range filepath.SplitList(cfg.BuildContext.GOPATH) {
 		if gpdir == "" {
 			continue
 		}
 		if rel := search.InDir(dir, filepath.Join(gpdir, "src")); rel != "" && rel != "." {
-			return filepath.ToSlash(rel), nil
+			path := filepath.ToSlash(rel)
+			// TODO(matloob): replace this with module.CheckImportPath
+			// once it's been laxened.
+			// Only checkModulePathLax here. There are some unpublishable
+			// module names that are compatible with checkModulePathLax
+			// but they already work in GOPATH so don't break users
+			// trying to do a build with modules. gorelease will alert users
+			// publishing their modules to fix their paths.
+			if err := checkModulePathLax(path); err != nil {
+				badPathErr = err
+				break
+			}
+			return path, nil
 		}
 	}
 
-	msg := `cannot determine module path for source directory %s (outside GOPATH, module path must be specified)
+	reason := "outside GOPATH, module path must be specified"
+	if badPathErr != nil {
+		// return a different error message if the module was in GOPATH, but
+		// the module path determined above would be an invalid path.
+		reason = fmt.Sprintf("bad module path inferred from directory in GOPATH: %v", badPathErr)
+	}
+	msg := `cannot determine module path for source directory %s (%s)
 
 Example usage:
 	'go mod init example.com/m' to initialize a v0 or v1 module
@@ -695,7 +761,7 @@ Example usage:
 
 Run 'go help mod init' for more information.
 `
-	return "", fmt.Errorf(msg, dir)
+	return "", fmt.Errorf(msg, dir, reason)
 }
 
 var (

src/cmd/go/testdata/script/mod_invalid_path.txt

@@ -1,12 +1,40 @@
-# Test that mod files with missing paths produce an error.
+# Test that mod files with invalid or missing paths produce an error.
 
 # Test that go list fails on a go.mod with no module declaration.
 cd $WORK/gopath/src/mod
 ! go list .
 stderr '^go: no module declaration in go.mod.\n\tRun ''go mod edit -module=example.com/mod'' to specify the module path.$'
 
+# Test that go mod init in GOPATH doesn't add a module declaration
+# with a path that can't possibly be a module path, because
+# it isn't even a valid import path.
+# The single quote and backtick are the only characters  we don't allow
+# in checkModulePathLax, but is allowed in a Windows file name.
+# TODO(matloob): choose a different character once
+# module.CheckImportPath is laxened and replaces
+# checkModulePathLax.
+cd $WORK/'gopath/src/m''d'
+! go mod init
+stderr 'cannot determine module path'
+
+# Test that a go.mod file is rejected when its module declaration has a path that can't
+# possibly be a module path, because it isn't even a valid import path
+cd $WORK/gopath/src/badname
+! go list .
+stderr 'invalid module path'
+
 -- mod/go.mod --
 
 -- mod/foo.go --
 package foo
 
+-- m'd/foo.go --
+package mad
+
+-- badname/go.mod --
+
+module .\.
+
+-- badname/foo.go --
+package badname
+