From e922c7705ee0df995063c02e6d50ad404afd0a42 Mon Sep 17 00:00:00 2001 From: Adam Bernot Date: Fri, 26 Jun 2026 11:38:53 -0700 Subject: [PATCH] fix: update strfmt duration parsing Update pkg/validation/strfmt/duration.go to backport the duration parsing improvements from upstream github.com/go-openapi/strfmt@v0.26.4. --- pkg/validation/strfmt/duration.go | 333 ++++++++++++++++++++----- pkg/validation/strfmt/duration_test.go | 263 +++++++++++++++++-- pkg/validation/strfmt/errors.go | 24 ++ 3 files changed, 543 insertions(+), 77 deletions(-) create mode 100644 pkg/validation/strfmt/errors.go diff --git a/pkg/validation/strfmt/duration.go b/pkg/validation/strfmt/duration.go index 04545296b..6e805e40d 100644 --- a/pkg/validation/strfmt/duration.go +++ b/pkg/validation/strfmt/duration.go @@ -17,45 +17,77 @@ package strfmt import ( "encoding/json" "fmt" - "regexp" - "strconv" "strings" "time" + "unicode" ) -func init() { +func init() { //nolint:gochecknoinits // registers duration format in the default registry d := Duration(0) - // register this format in the default registry Default.Add("duration", &d, IsDuration) } -var ( - timeUnits = [][]string{ - {"ns", "nano"}, - {"us", "µs", "micro"}, - {"ms", "milli"}, - {"s", "sec"}, - {"m", "min"}, - {"h", "hr", "hour"}, - {"d", "day"}, - {"w", "wk", "week"}, - } - - timeMultiplier = map[string]time.Duration{ - "ns": time.Nanosecond, - "us": time.Microsecond, - "ms": time.Millisecond, - "s": time.Second, - "m": time.Minute, - "h": time.Hour, - "d": 24 * time.Hour, - "w": 7 * 24 * time.Hour, - } - - durationMatcher = regexp.MustCompile(`((\d+)\s*([A-Za-zµ]+))`) +const ( + hoursInDay = 24 + daysInWeek = 7 + nanos = uint64(time.Nanosecond) + micros = uint64(time.Microsecond) + millis = uint64(time.Millisecond) + seconds = uint64(time.Second) + minutes = uint64(time.Minute) + hours = uint64(time.Hour) + days = uint64(hoursInDay * time.Hour) + weeks = uint64(hoursInDay * daysInWeek * time.Hour) + maxUint64 = uint64(1 << 63) ) -// IsDuration returns true if the provided string is a valid duration +// timeMultiplier holds all supported aliases for duration units, including their plural form. +// +//nolint:gochecknoglobals // package-level lookup tables for duration parsing +var timeMultiplier = map[string]uint64{ + "ns": nanos, + "nano": nanos, + "nanosecond": nanos, + "nanoseconds": nanos, + "nanos": nanos, + "us": micros, + "µs": micros, // U+00B5 = micro symbol + "μs": micros, // U+03BC = Greek letter mu + "micro": micros, + "micros": micros, + "microsecond": micros, + "microseconds": micros, + "ms": millis, + "milli": millis, + "millis": millis, + "millisecond": millis, + "milliseconds": millis, + "s": seconds, + "sec": seconds, + "secs": seconds, + "second": seconds, + "seconds": seconds, + "m": minutes, + "min": minutes, + "mins": minutes, + "minute": minutes, + "minutes": minutes, + "h": hours, + "hr": hours, + "hrs": hours, + "hour": hours, + "hours": hours, + "d": days, + "day": days, + "days": days, + "w": weeks, + "wk": weeks, + "wks": weeks, + "week": weeks, + "weeks": weeks, +} + +// IsDuration returns true if the provided string is a valid duration. func IsDuration(str string) bool { _, err := ParseDuration(str) return err == nil @@ -64,17 +96,17 @@ func IsDuration(str string) bool { // Duration represents a duration // // Duration stores a period of time as a nanosecond count, with the largest -// repesentable duration being approximately 290 years. +// representable duration being approximately 290 years. // -// swagger:strfmt duration +// swagger:strfmt duration. type Duration time.Duration -// MarshalText turns this instance into text +// MarshalText turns this instance into text. func (d Duration) MarshalText() ([]byte, error) { return []byte(time.Duration(d).String()), nil } -// UnmarshalText hydrates this instance from text +// UnmarshalText hydrates this instance from text. func (d *Duration) UnmarshalText(data []byte) error { // validation is performed later on dd, err := ParseDuration(string(data)) if err != nil { @@ -84,52 +116,170 @@ func (d *Duration) UnmarshalText(data []byte) error { // validation is performed return nil } -// ParseDuration parses a duration from a string, compatible with scala duration syntax -func ParseDuration(cand string) (time.Duration, error) { - if dur, err := time.ParseDuration(cand); err == nil { - return dur, nil +// ParseDuration parses a duration from a string +// +// It is similar to [time.ParseDuration] but support additional units like days and weeks, +// additional abreviations for units and is more tolerant on the presence of blank spaces. +// +// A duration may be negative or fractional. +// +// # Differences with [time.ParseDuration] +// +// - more supported units and aliases (see below) +// - sign followed by blank space is tolerated +// - tolerates blanks between duration and unit (e.g. "300 ms") +// +// # Supported units +// +// Units may be specified using aliases or a plural form. +// +// - "ns", "nano", "nanosecond", "nanoseconds", "nanos" +// - "us", "µs" (U+00B5 = micro symbol), "μs" (U+03BC = Greek letter mu), "micro", "micros", "microsecond", "microseconds" +// - "ms", "milli", "millis", "millisecond", "milliseconds" +// - "s", "sec", "secs", "second", "seconds" +// - "m", "min", "mins", "minute", "minutes" +// - "h", "hr", "hrs", "hour", "hours" +// - "d", "day", "days" +// - "w", "wk", "wks", "week", "weeks" +// +// NOTE: inspired by scala duration syntax. +// +// # Examples +// +// "300ms", "-1.5h", "2h45m", +// ".5 week", +// "2 minutes 45 seconds". +// +//nolint:gocognit,gocyclo,cyclop // complexity is only slightly above the usual level, may be tolerated as it mimicks the stdlib. +func ParseDuration(s string) (time.Duration, error) { + // NOTE: this code is largely inspired by the standard library. + orig := s + var d uint64 + neg := false + + // Consume [-+]? + if s != "" { + c := s[0] + if c == '-' || c == '+' { + neg = c == '-' + s = s[1:] + } + } + + // Consume space + s = strings.TrimLeftFunc(s, unicode.IsSpace) + + // Special case: if all that is left is "0", this is zero. + if s == "0" { + return 0, nil } - var dur time.Duration - ok := false - for _, match := range durationMatcher.FindAllStringSubmatch(cand, -1) { + if s == "" { + return 0, parseDurationError(orig, "empty duration") + } + + for s != "" { + var ( + v, f uint64 // integers before, after decimal point + scale float64 = 1 // value = v + f/scale + ) + s = strings.TrimLeftFunc(s, unicode.IsSpace) + + // The next character must be 0-9.] + if s[0] != '.' && ('0' > s[0] || s[0] > '9') { + return 0, parseDurationError(orig, fmt.Sprintf("expected a numerical value, but got %q", s[0])) + } + + // Consume integer part [0-9]* + pl := len(s) + var ok bool + v, s, ok = leadingInt(s) + if !ok { + return 0, parseDurationError(orig, "expected a leading integer part") + } + pre := pl != len(s) // whether we consumed anything before a period + + // Consume fractional part (\.[0-9]*)? + post := false + if s != "" && s[0] == '.' { + s = s[1:] + pl := len(s) + f, scale, s = leadingFraction(s) + post = pl != len(s) + } + + if !pre && !post { + // no digits (e.g. ".s" or "-.s") + return 0, parseDurationError(orig, "expected digits") + } + + // Consume space. + s = strings.TrimLeftFunc(s, unicode.IsSpace) + + // Consume unit. + i := 0 + for ; i < len(s); i++ { + c := s[i] + if c == '.' || '0' <= c && c <= '9' || unicode.IsSpace(rune(c)) { + break + } + } - factor, err := strconv.Atoi(match[2]) // converts string to int - if err != nil { - return 0, err + if i == 0 { + return 0, parseDurationError(orig, "missing unit in duration") } - unit := strings.ToLower(strings.TrimSpace(match[3])) - for _, variants := range timeUnits { - last := len(variants) - 1 - multiplier := timeMultiplier[variants[0]] + u := s[:i] + s = s[i:] + unit, ok := timeMultiplier[u] + if !ok { + return 0, parseDurationError(orig, fmt.Sprintf("unknown unit %q in duration", u)) + } + + if v > maxUint64/unit { + // overflow + return 0, parseDurationError(orig, "numerical overflow") + } - for i, variant := range variants { - if (last == i && strings.HasPrefix(unit, variant)) || strings.EqualFold(variant, unit) { - ok = true - dur += time.Duration(factor) * multiplier - } + v *= unit + if f > 0 { + // float64 is needed to be nanosecond accurate for fractions of hours. + // v >= 0 && (f*unit/scale) <= 3.6e+12 (ns/h, h is the largest unit) + v += uint64(float64(f) * (float64(unit) / scale)) + if v > maxUint64 { + // overflow + return 0, parseDurationError(orig, "numerical overflow") } } + + d += v + if d > maxUint64 { + return 0, parseDurationError(orig, "numerical overflow") + } + } + + if neg { + return -time.Duration(d), nil } - if ok { - return dur, nil + if d > maxUint64-1 { + return 0, parseDurationError(orig, "numerical overflow") } - return 0, fmt.Errorf("unable to parse %s as duration", cand) + + return time.Duration(d), nil } -// String converts this duration to a string +// String converts this duration to a string. func (d Duration) String() string { return time.Duration(d).String() } -// MarshalJSON returns the Duration as JSON +// MarshalJSON returns the Duration as JSON. func (d Duration) MarshalJSON() ([]byte, error) { return json.Marshal(time.Duration(d).String()) } -// UnmarshalJSON sets the Duration from JSON +// UnmarshalJSON sets the Duration from JSON. func (d *Duration) UnmarshalJSON(data []byte) error { if string(data) == jsonNull { return nil @@ -161,3 +311,70 @@ func (d *Duration) DeepCopy() *Duration { d.DeepCopyInto(out) return out } + +func parseDurationError(s, msg string) error { + if msg == "" { + return fmt.Errorf("invalid duration: %s: %w", s, ErrFormat) + } + + return fmt.Errorf("invalid duration: %s: %s: %w", s, msg, ErrFormat) +} + +// leadingInt consumes the leading [0-9]* from s. +func leadingInt[bytes []byte | string](s bytes) (x uint64, rem bytes, ok bool) { //nolint:ireturn // false positive + i := 0 + for ; i < len(s); i++ { + c := s[i] + if c < '0' || c > '9' { + break + } + + if x > maxUint64/10 { // overflow + return 0, rem, false + } + + x = x*10 + uint64(c) - '0' + if x > maxUint64 { // overflow + return 0, rem, false + } + } + + return x, s[i:], true +} + +// leadingFraction consumes the leading [0-9]* from s. +// // +// It is used only for fractions, so it does not return an error on overflow, +// it just stops accumulating precision. +func leadingFraction(s string) (x uint64, scale float64, rem string) { + i := 0 + scale = 1 + overflow := false + for ; i < len(s); i++ { + c := s[i] + if c < '0' || c > '9' { + break + } + + if overflow { + continue + } + + if x > (maxUint64-1)/10 { + // It's possible for overflow to give a positive number, so take care. + overflow = true + continue + } + + y := x*10 + uint64(c) - '0' + if y > maxUint64 { + overflow = true + continue + } + + x = y + scale *= 10 + } + + return x, scale, s[i:] +} diff --git a/pkg/validation/strfmt/duration_test.go b/pkg/validation/strfmt/duration_test.go index eeb4ff35d..6b7697871 100644 --- a/pkg/validation/strfmt/duration_test.go +++ b/pkg/validation/strfmt/duration_test.go @@ -15,64 +15,68 @@ package strfmt import ( + "fmt" "testing" "time" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestDuration(t *testing.T) { pp := Duration(0) err := pp.UnmarshalText([]byte("0ms")) - assert.NoError(t, err) + require.NoError(t, err) err = pp.UnmarshalText([]byte("yada")) - assert.Error(t, err) + require.Error(t, err) orig := "2ms" b := []byte(orig) bj := []byte("\"" + orig + "\"") err = pp.UnmarshalText(b) - assert.NoError(t, err) + require.NoError(t, err) err = pp.UnmarshalText([]byte("three week")) - assert.Error(t, err) + require.Error(t, err) err = pp.UnmarshalText([]byte("9999999999999999999999999999999999999999999999999999999 weeks")) - assert.Error(t, err) + require.Error(t, err) txt, err := pp.MarshalText() - assert.NoError(t, err) + require.NoError(t, err) assert.Equal(t, orig, string(txt)) err = pp.UnmarshalJSON(bj) - assert.NoError(t, err) - assert.EqualValues(t, orig, pp.String()) + require.NoError(t, err) + assert.Equal(t, orig, pp.String()) err = pp.UnmarshalJSON([]byte("yada")) - assert.Error(t, err) + require.Error(t, err) err = pp.UnmarshalJSON([]byte(`"12 parsecs"`)) - assert.Error(t, err) + require.Error(t, err) err = pp.UnmarshalJSON([]byte(`"12 y"`)) - assert.Error(t, err) + require.Error(t, err) b, err = pp.MarshalJSON() - assert.NoError(t, err) + require.NoError(t, err) assert.Equal(t, bj, b) } func testDurationParser(t *testing.T, toParse string, expected time.Duration) { + t.Helper() + r, e := ParseDuration(toParse) - assert.NoError(t, e) + require.NoError(t, e) assert.Equal(t, expected, r) } func TestDurationParser_Failed(t *testing.T) { _, e := ParseDuration("45 wekk") - assert.Error(t, e) + require.Error(t, e) } func TestIsDuration_Failed(t *testing.T) { @@ -82,7 +86,6 @@ func TestIsDuration_Failed(t *testing.T) { func TestDurationParser(t *testing.T) { testcases := map[string]time.Duration{ - // parse the short forms without spaces "1ns": 1 * time.Nanosecond, "1us": 1 * time.Microsecond, @@ -138,20 +141,179 @@ func TestDurationParser(t *testing.T) { "1 hour": 1 * time.Hour, "1 day": 24 * time.Hour, "1 week": 7 * 24 * time.Hour, + + // parse composite forms + "1m45s": time.Minute + 45*time.Second, + "1 m45 s": time.Minute + 45*time.Second, + "1m 45s": time.Minute + 45*time.Second, + "1 minute 45 seconds": time.Minute + 45*time.Second, } for str, dur := range testcases { - testDurationParser(t, str, dur) + t.Run(str, func(t *testing.T) { + testDurationParser(t, str, dur) + + // negative duration + testDurationParser(t, "-"+str, -dur) + testDurationParser(t, "- "+str, -dur) + }) } } + +// TestDurationParser_EdgeCases covers ParseDuration branches that the happy-path +// tests don't exercise: the "0" shortcut, empty input left after a sign or +// spaces, and inputs that have a decimal point but no digit on either side. +func TestDurationParser_EdgeCases(t *testing.T) { + t.Run("zero shortcut returns 0 with no error", func(t *testing.T) { + for _, in := range []string{"0", "-0", "+0", "- 0", "+ 0"} { + input := in + t.Run(fmt.Sprintf("%q", input), func(t *testing.T) { + t.Parallel() + + d, err := ParseDuration(input) + require.NoError(t, err) + assert.Equal(t, time.Duration(0), d) + }) + } + }) + + t.Run("empty payload after sign or spaces is rejected", func(t *testing.T) { + for _, in := range []string{"", "-", "+", " ", "- ", "+ "} { + input := in + t.Run(fmt.Sprintf("%q", input), func(t *testing.T) { + t.Parallel() + + _, err := ParseDuration(input) + require.Error(t, err) + }) + } + }) + + t.Run("decimal point without digits is rejected", func(t *testing.T) { + // A leading '.' passes the first numeric check but produces neither an + // integer nor a fractional part, exercising the "I dare you" branch. + for _, in := range []string{".s", ".h", ".d", "-.s", "+.h", "1m .s"} { + input := in + t.Run(input, func(t *testing.T) { + t.Parallel() + + _, err := ParseDuration(input) + require.Error(t, err) + }) + } + }) +} + +// TestDurationParser_Overflow covers every numerical-overflow branch in +// ParseDuration, leadingInt, and leadingFraction. +// +// The boundary values are derived from maxUint64 = 1<<63 (the magnitude of +// math.MinInt64). The fractional cases hinge on (1<<63 - 1)/10 = 922337203685477580. +func TestDurationParser_Overflow(t *testing.T) { + overflows := []struct { + name string + input string + }{ + { + name: "leadingInt overflow after multiply-add", + // 19 digits where the last one pushes x past 1<<63 even though + // x <= maxUint64/10 still held before the final multiply-add. + input: "9223372036854775809ns", + }, + { + name: "v*unit fits but adding fraction overflows", + // 2562047*hours = 9223369200000000000 (under 1<<63), then + // +0.9*hours = +3240000000000 pushes the sum past 1<<63. + input: "2562047.9h", + }, + { + name: "running total d exceeds maxUint64 across tokens", + // Each token alone fits (9e18 < 1<<63 ~= 9.223e18), but the sum + // (1.8e19) overflows in the in-loop d > maxUint64 check. + input: "9000000000000ms 9000000000000ms", + }, + { + name: "single positive value equals maxUint64", + // 1<<63 ns parses through leadingInt successfully, then fails the + // final d > maxUint64-1 check (only the negative form fits, as + // time.Duration(math.MinInt64)). + input: "9223372036854775808ns", + }, + } + + for _, tt := range overflows { + tc := tt + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + d, err := ParseDuration(tc.input) + require.Errorf(t, err, "parsed %q as %v, expected overflow error", tc.input, d) + }) + } +} + +// TestDurationParser_FractionPrecisionLoss covers the three overflow-handling +// branches in leadingFraction. Unlike integer overflow, fractional overflow is +// not an error: per the helper's contract it stops accumulating precision and +// returns the partial value, so the parser still produces a valid duration — +// just truncated near the 18th fractional digit. +func TestDurationParser_FractionPrecisionLoss(t *testing.T) { + // All three inputs share the same first 18 fractional digits; precision + // caps there regardless of which overflow branch fires, so the parsed + // value is identical. + const truncated = time.Duration(922337203) // 0.922337203 * 1s, rounded down + + cases := []struct { + name string + input string + }{ + { + name: "y overflow on 19th digit", + // 19th digit '9' makes y = x*10+9 > 1<<63. + input: "0.9223372036854775809s", + }, + { + name: "x exceeds threshold on 20th digit", + // 19th digit '8' lands y exactly at 1<<63 (still accepted), + // 20th digit then trips the x > (1<<63-1)/10 guard. + input: "0.92233720368547758089s", + }, + { + name: "trailing digits skipped once overflow is set", + // 21st digit hits the early `if overflow` continue branch. + input: "0.922337203685477580891s", + }, + } + + for _, tt := range cases { + tc := tt + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + + d, err := ParseDuration(tc.input) + require.NoError(t, err) + assert.Equal(t, truncated, d) + }) + } +} + +// TestDurationParser_NegativeMinDuration verifies that the smallest +// representable duration parses successfully even though the equivalent +// positive value overflows. +func TestDurationParser_NegativeMinDuration(t *testing.T) { + d, err := ParseDuration("-9223372036854775808ns") + require.NoError(t, err) + assert.Equal(t, time.Duration(-1<<63), d) +} + func TestIsDuration_Caveats(t *testing.T) { // This works too e := IsDuration("45 weeks") assert.True(t, e) - // This works too + // This no longer works e = IsDuration("45 weekz") - assert.True(t, e) + assert.False(t, e) // This works too e = IsDuration("12 hours") @@ -164,7 +326,6 @@ func TestIsDuration_Caveats(t *testing.T) { // This does not work e = IsDuration("12 phours") assert.False(t, e) - } func TestDeepCopyDuration(t *testing.T) { @@ -182,3 +343,67 @@ func TestDeepCopyDuration(t *testing.T) { out3 := inNil.DeepCopy() assert.Nil(t, out3) } + +func TestIssue169FractionalDuration(t *testing.T) { + for _, tt := range []struct { + Input string + Expected string + ExpectError bool + }{ + { + Input: "1.5 h", + Expected: "1h30m0s", + }, + { + Input: "1.5 d", + Expected: "36h0m0s", + }, + { + Input: "3.14159 d", + Expected: "75h23m53.376s", + }, + { + Input: "- 3.14159 d", + Expected: "-75h23m53.376s", + }, + { + Input: "3.141.59 d", + ExpectError: true, + }, + { + Input: ".314159 d", + Expected: "7h32m23.3376s", + }, + { + Input: "314159. d", + ExpectError: true, + }, + } { + fractionalDuration := tt + + if fractionalDuration.ExpectError { + t.Run(fmt.Sprintf("invalid fractional duration %s should NOT parse", fractionalDuration.Input), func(t *testing.T) { + t.Parallel() + + require.False(t, IsDuration(fractionalDuration.Input)) + }) + + continue + } + + t.Run(fmt.Sprintf("fractional duration %s should parse", fractionalDuration.Input), func(t *testing.T) { + t.Parallel() + + require.True(t, IsDuration(fractionalDuration.Input)) + + var d Duration + require.NoError(t, d.UnmarshalText([]byte(fractionalDuration.Input))) + + require.Equal(t, fractionalDuration.Expected, d.String()) + + dd, err := ParseDuration(fractionalDuration.Input) + require.NoError(t, err) + require.Equal(t, fractionalDuration.Expected, dd.String()) + }) + } +} diff --git a/pkg/validation/strfmt/errors.go b/pkg/validation/strfmt/errors.go new file mode 100644 index 000000000..1d960ad84 --- /dev/null +++ b/pkg/validation/strfmt/errors.go @@ -0,0 +1,24 @@ +// Copyright 2015 go-swagger maintainers +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package strfmt + +type strfmtError string + +// ErrFormat is an error raised by the [strfmt] package. +const ErrFormat strfmtError = "format error" + +func (e strfmtError) Error() string { + return string(e) +}