From 1e6d8a1043d86eab3ebd2f966daef6064accced3 Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Thu, 1 Jul 2021 13:45:07 -0700 Subject: [PATCH] version: don't allocate parsing unsupported versions, empty strings Signed-off-by: Brad Fitzpatrick --- version/cmp.go | 94 +++++++++++++++++++++++++++++++++++---------- version/cmp_test.go | 7 ++++ 2 files changed, 81 insertions(+), 20 deletions(-) diff --git a/version/cmp.go b/version/cmp.go index 73b08585f..3b188897b 100644 --- a/version/cmp.go +++ b/version/cmp.go @@ -5,7 +5,6 @@ package version import ( - "strconv" "strings" ) @@ -68,8 +67,8 @@ type parsed struct { func parse(version string) (parsed, bool) { if strings.HasPrefix(version, "date.") { - stamp, err := strconv.Atoi(version[5:]) - if err != nil { + stamp, ok := atoi(version[5:]) + if !ok { return parsed{}, false } return parsed{Datestamp: stamp}, true @@ -77,8 +76,8 @@ func parse(version string) (parsed, bool) { var ret parsed - major, rest, err := splitNumericPrefix(version) - if err != nil { + major, rest, ok := splitNumericPrefix(version) + if !ok { return parsed{}, false } ret.Major = major @@ -86,8 +85,8 @@ func parse(version string) (parsed, bool) { return ret, true } - ret.Minor, rest, err = splitNumericPrefix(rest[1:]) - if err != nil { + ret.Minor, rest, ok = splitNumericPrefix(rest[1:]) + if !ok { return parsed{}, false } if len(rest) == 0 { @@ -96,8 +95,8 @@ func parse(version string) (parsed, bool) { // Optional patch version, if the next separator is a dot. if rest[0] == '.' { - ret.Patch, rest, err = splitNumericPrefix(rest[1:]) - if err != nil { + ret.Patch, rest, ok = splitNumericPrefix(rest[1:]) + if !ok { return parsed{}, false } if len(rest) == 0 { @@ -112,29 +111,84 @@ func parse(version string) (parsed, bool) { } var trailer string - ret.ExtraCommits, trailer, err = splitNumericPrefix(rest[1:]) - if err != nil || (len(trailer) > 0 && trailer[0] != '-') { + ret.ExtraCommits, trailer, ok = splitNumericPrefix(rest[1:]) + if !ok || (len(trailer) > 0 && trailer[0] != '-') { // rest was probably the string trailer, ignore it. ret.ExtraCommits = 0 } return ret, true } -func splitNumericPrefix(s string) (int, string, error) { +func splitNumericPrefix(s string) (n int, rest string, ok bool) { for i, r := range s { if r >= '0' && r <= '9' { continue } - ret, err := strconv.Atoi(s[:i]) - if err != nil { - return 0, "", err + ret, ok := atoi(s[:i]) + if !ok { + return 0, "", false + } + return ret, s[i:], true + } + + ret, ok := atoi(s) + if !ok { + return 0, "", false + } + return ret, "", true +} + +const ( + maxUint = ^uint(0) + maxInt = int(maxUint >> 1) +) + +// atoi parses an int from a string s. +// The bool result reports whether s is a number +// representable by a value of type int. +// +// From Go's runtime/string.go. +func atoi(s string) (int, bool) { + if s == "" { + return 0, false + } + + neg := false + if s[0] == '-' { + neg = true + s = s[1:] + } + + un := uint(0) + for i := 0; i < len(s); i++ { + c := s[i] + if c < '0' || c > '9' { + return 0, false + } + if un > maxUint/10 { + // overflow + return 0, false } - return ret, s[i:], nil + un *= 10 + un1 := un + uint(c) - '0' + if un1 < un { + // overflow + return 0, false + } + un = un1 } - ret, err := strconv.Atoi(s) - if err != nil { - return 0, "", err + if !neg && un > uint(maxInt) { + return 0, false + } + if neg && un > uint(maxInt)+1 { + return 0, false } - return ret, "", nil + + n := int(un) + if neg { + n = -n + } + + return n, true } diff --git a/version/cmp_test.go b/version/cmp_test.go index 3124a579b..a611fcf77 100644 --- a/version/cmp_test.go +++ b/version/cmp_test.go @@ -28,6 +28,7 @@ func TestParse(t *testing.T) { {"date.20200612", parsed{Datestamp: 20200612}, true}, {"borkbork", parsed{}, false}, {"1a.2.3", parsed{}, false}, + {"", parsed{}, false}, } for _, test := range tests { @@ -38,6 +39,12 @@ func TestParse(t *testing.T) { if diff := cmp.Diff(gotParsed, test.parsed); diff != "" { t.Errorf("parse(%q) diff (-got+want):\n%s", test.version, diff) } + n := int(testing.AllocsPerRun(1000, func() { + gotParsed, got = parse(test.version) + })) + if n != 0 { + t.Errorf("parse(%q) allocs = %d; want 0", test.version, n) + } } }