From 8be72f4054dfcdd82a149e807ea45c72f91e5dad Mon Sep 17 00:00:00 2001 From: Matt Date: Thu, 13 Aug 2026 11:29:53 -0400 Subject: [PATCH] Adding increment functions that error This provides a way to increment without recovering from panics. Signed-off-by: Matt --- version.go | 66 ++++++++++++++++-- version_test.go | 177 +++++++++++++++++++++++++++++++++++++++++------- 2 files changed, 214 insertions(+), 29 deletions(-) diff --git a/version.go b/version.go index da42876..84544f4 100644 --- a/version.go +++ b/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,12 +430,12 @@ 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. @@ -421,17 +443,33 @@ func (v Version) IncPatch() Version { // 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. diff --git a/version_test.go b/version_test.go index 305d277..5ebe9e0 100644 --- a/version_test.go +++ b/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) {