cmd/go: validating version format in mod edit

Version strings set by -retract and -exclude are not canonicalized
by go mod commands. This change adds validation to go mod edit to
prevent invalid version strings from being added to the go.mod file.

For golang/go#43280

Change-Id: I3708b7a09111a56effac1fe1165122772e3f2d75
Reviewed-on: https://go-review.googlesource.com/c/mod/+/279394
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
Reviewed-by: Bryan C. Mills <bcmills@google.com>
Trust: Michael Matloob <matloob@golang.org>

modfile/rule.go

diff --git a/modfile/rule.go b/modfile/rule.go
index 83398dd..c6a189d 100644
--- a/modfile/rule.go
+++ b/modfile/rule.go
@@ -832,7 +832,16 @@
 	return nil
 }
 
+// AddExclude adds a exclude statement to the mod file. Errors if the provided
+// version is not a canonical version string
 func (f *File) AddExclude(path, vers string) error {
+	if !isCanonicalVersion(vers) {
+		return &module.InvalidVersionError{
+			Version: vers,
+			Err:     errors.New("must be of the form v1.2.3"),
+		}
+	}
+
 	var hint *Line
 	for _, x := range f.Exclude {
 		if x.Mod.Path == path && x.Mod.Version == vers {
@@ -904,7 +913,22 @@
 	return nil
 }
 
+// AddRetract adds a retract statement to the mod file. Errors if the provided
+// version interval does not consist of canonical version strings
 func (f *File) AddRetract(vi VersionInterval, rationale string) error {
+	if !isCanonicalVersion(vi.High) {
+		return &module.InvalidVersionError{
+			Version: vi.High,
+			Err:     errors.New("must be of the form v1.2.3"),
+		}
+	}
+	if !isCanonicalVersion(vi.Low) {
+		return &module.InvalidVersionError{
+			Version: vi.Low,
+			Err:     errors.New("must be of the form v1.2.3"),
+		}
+	}
+
 	r := &Retract{
 		VersionInterval: vi,
 	}
@@ -1061,3 +1085,9 @@
 	}
 	return semver.Compare(vii.High, vij.High) > 0
 }
+
+// isCanonicalVersion tests if the provided version string represents a valid
+// canonical version.
+func isCanonicalVersion(vers string) bool {
+	return vers != "" && semver.Canonical(vers) == vers
+}

modfile/rule_test.go

diff --git a/modfile/rule_test.go b/modfile/rule_test.go
index fbf144d..03123ed 100644
--- a/modfile/rule_test.go
+++ b/modfile/rule_test.go
@@ -568,6 +568,48 @@
 	},
 }
 
+var addRetractValidateVersionTests = []struct {
+	dsc, low, high string
+}{
+	{
+		"blank_version",
+		"",
+		"",
+	},
+	{
+		"missing_prefix",
+		"1.0.0",
+		"1.0.0",
+	},
+	{
+		"non_canonical",
+		"v1.2",
+		"v1.2",
+	},
+	{
+		"invalid_range",
+		"v1.2.3",
+		"v1.3",
+	},
+}
+
+var addExcludeValidateVersionTests = []struct {
+	dsc, ver string
+}{
+	{
+		"blank_version",
+		"",
+	},
+	{
+		"missing_prefix",
+		"1.0.0",
+	},
+	{
+		"non_canonical",
+		"v1.2",
+	},
+}
+
 func TestAddRequire(t *testing.T) {
 	for _, tt := range addRequireTests {
 		t.Run(tt.desc, func(t *testing.T) {
@@ -699,3 +741,31 @@
 
 	return f
 }
+
+func TestAddRetractValidateVersion(t *testing.T) {
+	for _, tt := range addRetractValidateVersionTests {
+		t.Run(tt.dsc, func(t *testing.T) {
+			f, err := Parse("in", []byte("module m"), nil)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if err = f.AddRetract(VersionInterval{Low: tt.low, High: tt.high}, ""); err == nil {
+				t.Fatal("expected AddRetract to complain about version format")
+			}
+		})
+	}
+}
+
+func TestAddExcludeValidateVersion(t *testing.T) {
+	for _, tt := range addExcludeValidateVersionTests {
+		t.Run(tt.dsc, func(t *testing.T) {
+			f, err := Parse("in", []byte("module m"), nil)
+			if err != nil {
+				t.Fatal(err)
+			}
+			if err = f.AddExclude("aa", tt.ver); err == nil {
+				t.Fatal("expected AddExclude to complain about version format")
+			}
+		})
+	}
+}