Skip to content

data: return an error for too large numbers in ParseDuration - #22163

Merged
MichaelEischer merged 2 commits into
restic:masterfrom
b84330808:fix-duration-overflow-panic
Nov 5, 2026
Merged

MichaelEischer merged 2 commits into
restic:masterfrom
b84330808:fix-duration-overflow-panic

Conversation

@b84330808

Copy link
Copy Markdown
Contributor

What does this PR change? What problem does it solve?

restic forget --keep-within 99999999999999999999999999y crashes with a stack trace, because nextNumber in internal/data/duration.go panics when strconv.Atoi fails on a number that doesn't fit into an int. It now returns the error, so the flag reports an invalid argument like other bad durations.

Was the change previously discussed in an issue or on the forum?

No. #5486 fixed a different panic in the same function.

Checklist

  • I have added tests for all code changes, see writing tests
  • I have added documentation for relevant changes (in the manual).
  • There's a new file in changelog/unreleased/ that describes the changes for our users (see template).
  • I'm done! This pull request is ready for review.

nextNumber panicked when strconv.Atoi failed, which happens when a
number in a duration does not fit into an int. As ParseDuration backs
the --keep-within* flags, `restic forget --keep-within
99999999999999999999999999y` crashed with a stack trace instead of
reporting an invalid argument. Return the error instead.
@github-actions github-actions Bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Nov 4, 2026
wplapper added a commit to wplapper/restic that referenced this pull request Nov 5, 2026
incorporate PR restic#22163 and add comments about 7 or 8 digit day offsets.
These offsets are unlikely to be seen in the wild, but could be constructed as
corner tests cases.

internal/data/duration.go:
adopt fix in PR restic#22163

doc/045_working_with_repos.rst:
reference to the ambiguity of 7 or 8 digit decimal offset in days and how to
differentiate from snapshots which end in the character "d".

@MichaelEischer MichaelEischer left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@MichaelEischer
MichaelEischer enabled auto-merge (squash) November 5, 2026 19:29
@MichaelEischer
MichaelEischer merged commit 9a0b9e0 into restic:master Nov 5, 2026
13 checks passed
mchobbit pushed a commit to mchobbit/restic that referenced this pull request Nov 6, 2026
mchobbit added a commit to mchobbit/restic that referenced this pull request Nov 6, 2026
Five fixes from upstream master since v0.19.1, cherry-picked with -x: the
three in changelog/unreleased marked Bugfix (restic#21977 truncated key/config
files, restic#22007 Swift password in debug logs, restic#22163 large --keep-within*
durations), forget's failedSnIDs lock (restic#22100) and prune's fatal pack
listing error. BACKOTTER.md lists them under a new section on taking
upstream fixes between releases. Two of the picked tests call names
upstream added after 0.19.1 (openKey, NoopTerminalCounterFactory,
NewNoopPrinter); they call 0.19.1's instead.

Co-Authored-By: Claude Opus 5.5 <[email protected]>
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants