Adding increment functions that error

version.go

@@ -53,6 +53,12 @@ var (
 	// ErrVersionTooLong is returned when a version string exceeds the
 	// maximum allowed length.
 	ErrVersionTooLong = fmt.Errorf("version string is too long (max %d bytes)", MaxVersionLen)
+
+	// ErrIncrementOverflow is returned when incrementing a version segment
+	// would exceed the maximum value of a uint64. Errors returned by
+	// IncMajorE, IncMinorE, and IncPatchE wrap this value and name the segment
+	// that overflowed, so they can be detected with errors.Is.
+	ErrIncrementOverflow = errors.New("version increment would overflow uint64")
 )
 
 // MaxVersionLen is the maximum allowed length of a version string. This guards
@@ -395,7 +401,23 @@ func (v Version) originalVPrefix() string {
 // it unsets metadata and prerelease values, increments patch number.
 // If the current version has any of prerelease or metadata information,
 // it unsets both values and keeps current patch value
+//
+// Note, this panics if the patch segment is math.MaxUint64 and would overflow.
+// A version parsed from untrusted input can reach that value, so callers
+// handling input they do not control should use IncPatchE instead.
 func (v Version) IncPatch() Version {
+	vNext, err := v.IncPatchE()
+	if err != nil {
+		panic(err)
+	}
+	return vNext
+}
+
+// IncPatchE produces the next patch version. It behaves the same as IncPatch
+// but returns an error wrapping ErrIncrementOverflow instead of panicking when
+// the patch segment is math.MaxUint64. On error the version is returned
+// unchanged.
+func (v Version) IncPatchE() (Version, error) {
 	vNext := v
 	// according to http://semver.org/#spec-item-9
 	// Pre-release versions have a lower precedence than the associated normal version.
@@ -408,30 +430,46 @@ func (v Version) IncPatch() Version {
 		vNext.metadata = ""
 		vNext.pre = ""
 		if v.patch == math.MaxUint64 {
-			panic("patch version increment would overflow uint64")
+			return v, fmt.Errorf("patch %w", ErrIncrementOverflow)
 		}
 		vNext.patch = v.patch + 1
 	}
 	vNext.original = v.originalVPrefix() + "" + vNext.String()
-	return vNext
+	return vNext, nil
 }
 
 // IncMinor produces the next minor version.
 // Sets patch to 0.
 // Increments minor number.
 // Unsets metadata.
 // Unsets prerelease status.
+//
+// Note, this panics if the minor segment is math.MaxUint64 and would overflow.
+// A version parsed from untrusted input can reach that value, so callers
+// handling input they do not control should use IncMinorE instead.
 func (v Version) IncMinor() Version {
+	vNext, err := v.IncMinorE()
+	if err != nil {
+		panic(err)
+	}
+	return vNext
+}
+
+// IncMinorE produces the next minor version. It behaves the same as IncMinor
+// but returns an error wrapping ErrIncrementOverflow instead of panicking when
+// the minor segment is math.MaxUint64. On error the version is returned
+// unchanged.
+func (v Version) IncMinorE() (Version, error) {
 	vNext := v
 	vNext.metadata = ""
 	vNext.pre = ""
 	vNext.patch = 0
 	if v.minor == math.MaxUint64 {
-		panic("minor version increment would overflow uint64")
+		return v, fmt.Errorf("minor %w", ErrIncrementOverflow)
 	}
 	vNext.minor = v.minor + 1
 	vNext.original = v.originalVPrefix() + "" + vNext.String()
-	return vNext
+	return vNext, nil
 }
 
 // IncMajor produces the next major version.
@@ -440,18 +478,34 @@ func (v Version) IncMinor() Version {
 // Increments major number.
 // Unsets metadata.
 // Unsets prerelease status.
+//
+// Note, this panics if the major segment is math.MaxUint64 and would overflow.
+// A version parsed from untrusted input can reach that value, so callers
+// handling input they do not control should use IncMajorE instead.
 func (v Version) IncMajor() Version {
+	vNext, err := v.IncMajorE()
+	if err != nil {
+		panic(err)
+	}
+	return vNext
+}
+
+// IncMajorE produces the next major version. It behaves the same as IncMajor
+// but returns an error wrapping ErrIncrementOverflow instead of panicking when
+// the major segment is math.MaxUint64. On error the version is returned
+// unchanged.
+func (v Version) IncMajorE() (Version, error) {
 	vNext := v
 	vNext.metadata = ""
 	vNext.pre = ""
 	vNext.patch = 0
 	vNext.minor = 0
 	if v.major == math.MaxUint64 {
-		panic("major version increment would overflow uint64")
+		return v, fmt.Errorf("major %w", ErrIncrementOverflow)
 	}
 	vNext.major = v.major + 1
 	vNext.original = v.originalVPrefix() + "" + vNext.String()
-	return vNext
+	return vNext, nil
 }
 
 // SetPrerelease defines the prerelease value.

version_test.go

@@ -3,6 +3,7 @@ package semver
 import (
 	"database/sql"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"math"
 	"strings"
@@ -748,34 +749,164 @@ func TestInc(t *testing.T) {
 	}
 }
 
-func TestIncPatchOverflow(t *testing.T) {
-	v := Version{major: 1, minor: 2, patch: math.MaxUint64}
-	defer func() {
-		if r := recover(); r == nil {
-			t.Error("Expected panic on patch overflow, but did not get one")
+func TestIncE(t *testing.T) {
+	tests := []struct {
+		v1               string
+		expected         string
+		how              string
+		expectedOriginal string
+	}{
+		{"1.2.3", "1.2.4", "patch", "1.2.4"},
+		{"v1.2.4", "1.2.5", "patch", "v1.2.5"},
+		{"1.2.3", "1.3.0", "minor", "1.3.0"},
+		{"v1.2.4", "1.3.0", "minor", "v1.3.0"},
+		{"1.2.3", "2.0.0", "major", "2.0.0"},
+		{"v1.2.4", "2.0.0", "major", "v2.0.0"},
+		{"1.2.3+meta", "1.2.4", "patch", "1.2.4"},
+		{"1.2.3-beta+meta", "1.2.3", "patch", "1.2.3"},
+		{"v1.2.4-beta+meta", "1.2.4", "patch", "v1.2.4"},
+		{"1.2.3-beta+meta", "1.3.0", "minor", "1.3.0"},
+		{"v1.2.4-beta+meta", "1.3.0", "minor", "v1.3.0"},
+		{"1.2.3-beta+meta", "2.0.0", "major", "2.0.0"},
+		{"v1.2.4-beta+meta", "2.0.0", "major", "v2.0.0"},
+	}
+
+	// The E variants must agree with the panicking ones for every input that
+	// does not overflow.
+	for _, tc := range tests {
+		v1, err := NewVersion(tc.v1)
+		if err != nil {
+			t.Errorf("Error parsing version: %s", err)
+			continue
 		}
-	}()
-	v.IncPatch()
-}
 
-func TestIncMinorOverflow(t *testing.T) {
-	v := Version{major: 1, minor: math.MaxUint64, patch: 0}
-	defer func() {
-		if r := recover(); r == nil {
-			t.Error("Expected panic on minor overflow, but did not get one")
+		var v2 Version
+		var incErr error
+		switch tc.how {
+		case "patch":
+			v2, incErr = v1.IncPatchE()
+		case "minor":
+			v2, incErr = v1.IncMinorE()
+		case "major":
+			v2, incErr = v1.IncMajorE()
 		}
-	}()
-	v.IncMinor()
-}
 
-func TestIncMajorOverflow(t *testing.T) {
-	v := Version{major: math.MaxUint64, minor: 0, patch: 0}
-	defer func() {
-		if r := recover(); r == nil {
-			t.Error("Expected panic on major overflow, but did not get one")
+		if incErr != nil {
+			t.Errorf("Inc %q of %q returned unexpected error: %s", tc.how, tc.v1, incErr)
+			continue
 		}
-	}()
-	v.IncMajor()
+
+		if a, e := v2.String(), tc.expected; a != e {
+			t.Errorf("IncE %q failed. Expected %q got %q", tc.how, e, a)
+		}
+
+		if a, e := v2.Original(), tc.expectedOriginal; a != e {
+			t.Errorf("IncE %q failed. Expected original %q got %q", tc.how, e, a)
+		}
+	}
+}
+
+func TestIncOverflow(t *testing.T) {
+	tests := []struct {
+		name    string
+		v       Version
+		inc     func(Version) Version
+		incE    func(Version) (Version, error)
+		wantMsg string
+	}{
+		{
+			name:    "patch",
+			v:       Version{major: 1, minor: 2, patch: math.MaxUint64},
+			inc:     Version.IncPatch,
+			incE:    Version.IncPatchE,
+			wantMsg: "patch version increment would overflow uint64",
+		},
+		{
+			name:    "minor",
+			v:       Version{major: 1, minor: math.MaxUint64, patch: 0},
+			inc:     Version.IncMinor,
+			incE:    Version.IncMinorE,
+			wantMsg: "minor version increment would overflow uint64",
+		},
+		{
+			name:    "major",
+			v:       Version{major: math.MaxUint64, minor: 0, patch: 0},
+			inc:     Version.IncMajor,
+			incE:    Version.IncMajorE,
+			wantMsg: "major version increment would overflow uint64",
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			// The E variant reports the overflow instead of panicking and
+			// leaves the version untouched.
+			got, err := tc.incE(tc.v)
+			if err == nil {
+				t.Fatalf("Expected an error on %s overflow, but did not get one", tc.name)
+			}
+			if !errors.Is(err, ErrIncrementOverflow) {
+				t.Errorf("Expected error to wrap ErrIncrementOverflow, got: %v", err)
+			}
+			if err.Error() != tc.wantMsg {
+				t.Errorf("Expected error message %q, got %q", tc.wantMsg, err.Error())
+			}
+			if got != tc.v {
+				t.Errorf("Expected the version to be unchanged on overflow. Expected %q got %q", tc.v, got)
+			}
+
+			// The original method still panics, and the panic value is the
+			// same error so it can be inspected after a recover.
+			defer func() {
+				r := recover()
+				if r == nil {
+					t.Fatalf("Expected panic on %s overflow, but did not get one", tc.name)
+				}
+				rerr, ok := r.(error)
+				if !ok {
+					t.Fatalf("Expected the panic value to be an error, got %T", r)
+				}
+				if !errors.Is(rerr, ErrIncrementOverflow) {
+					t.Errorf("Expected panic value to wrap ErrIncrementOverflow, got: %v", rerr)
+				}
+			}()
+			tc.inc(tc.v)
+		})
+	}
+}
+
+// A version with a maxed out segment is reachable from untrusted input, so the
+// non-panicking path has to cover values that arrive through the parser.
+func TestIncOverflowFromParsedInput(t *testing.T) {
+	v, err := NewVersion("1.2.18446744073709551615")
+	if err != nil {
+		t.Fatalf("Error parsing version: %s", err)
+	}
+
+	if _, err = v.IncPatchE(); !errors.Is(err, ErrIncrementOverflow) {
+		t.Errorf("Expected ErrIncrementOverflow from parsed input, got: %v", err)
+	}
+
+	// The other segments are nowhere near the limit and must still increment.
+	if _, err = v.IncMinorE(); err != nil {
+		t.Errorf("Unexpected error incrementing minor: %s", err)
+	}
+	if _, err = v.IncMajorE(); err != nil {
+		t.Errorf("Unexpected error incrementing major: %s", err)
+	}
+}
+
+// A prerelease does not increment the patch segment, so it cannot overflow
+// even when the patch is already at the maximum.
+func TestIncPatchOverflowWithPrerelease(t *testing.T) {
+	v := Version{major: 1, minor: 2, patch: math.MaxUint64, pre: "beta"}
+	got, err := v.IncPatchE()
+	if err != nil {
+		t.Fatalf("Unexpected error: %s", err)
+	}
+	if e := "1.2.18446744073709551615"; got.String() != e {
+		t.Errorf("Expected %q got %q", e, got.String())
+	}
 }
 
 func TestSetPrerelease(t *testing.T) {