Visitar URL original
fix(engine): reject duration values that wrap time.Duration by weirong-ui · Pull Request #32841 · helm/helm · GitHub
Skip to content

fix(engine): reject duration values that wrap time.Duration - #32841

Open
weirong-ui wants to merge 1 commit into
helm:mainfrom
weirong-ui:fix-duration-float-boundary
Open

weirong-ui wants to merge 1 commit into
helm:mainfrom
weirong-ui:fix-duration-float-boundary

Conversation

@weirong-ui

@weirong-ui weirong-ui commented Nov 2, 2026 •

Copy link
Copy Markdown

What this PR does / why we need it:

asDuration wraps to MinInt64 instead of returning an error at the upper
float boundary:

{{ durationSeconds 9223372036.854776 }}  -> -9.223372036854776e+09
{{ mustToDuration 9223372036.854776 }}   -> -2562047h47m16.854775808s

float64(math.MaxInt64) rounds up to 2^63, so for
seconds == maxDurationSecondsFloat the computed nanos equals 2^63, passes
the > check, and time.Duration(nanos) wraps to MinInt64. Both helpers go
through asDuration.

Use >= for the upper bound. The lower bound stays < because -2^63 is
representable and is a valid duration. Added boundary cases to
TestDurationHelpers.

Special notes for your reviewer:

Found this while reading the overflow guard added in #31695. The new cases only
pin down the boundary; values below it are unchanged.

If applicable:

  • this PR contains user facing changes (the docs needed label should be applied if so)
  • this PR contains unit tests
  • this PR has been tested for backwards compatibility

Signed-off-by: weirong-ui <2771986015@qq.com>
@pull-request-size pull-request-size Bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Nov 2, 2026
@github-actions github-actions Bot added the v4.x Issues and Pull Requests related to the major version v4 label Nov 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/S Denotes a PR that changes 10-29 lines, ignoring generated files. v4.x Issues and Pull Requests related to the major version v4

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant