Skip to content

date: safely handle oversized format modifier widths#11085

Closed
jorgitin02 wants to merge 4 commits intouutils:mainfrom
jorgitin02:date-11044-clean
Closed

date: safely handle oversized format modifier widths#11085
jorgitin02 wants to merge 4 commits intouutils:mainfrom
jorgitin02:date-11044-clean

Conversation

@jorgitin02
Copy link

Fixes #11044 by making Mon Feb 23 22:18:00 EST 2026 reject huge format-width modifiers safely instead of
panicking.\n\nChanges:\n- make modifier width handling fallible and return a normal format
error for oversized widths\n- add localized error message\n- add regression coverage for
extreme width format specifiers

@cakebaker
Copy link
Contributor

Can you please run cargo fmt? Thanks.

lorsqu'une option est utilisée pour spécifier une ou plusieurs dates, tout argument autre
qu'une option doit être une chaîne de format commençant par un signe '+'.
date-error-format-modifier-width-too-large = la largeur du modificateur de format '{$width}' est trop grande pour le spécificateur '%{$specifier}'
date-error-format-modifier-width-too-large = la largeur du modificateur de format '{$width}' est trop grande pour le spécificateur '%{$specifier}'
Copy link
Contributor

Choose a reason for hiding this comment

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

The entry is a duplicate.

Suggested change
date-error-format-modifier-width-too-large = la largeur du modificateur de format '{$width}' est trop grande pour le spécificateur '%{$specifier}'

@sylvestre
Copy link
Contributor

how is it different from #11053 ?

@jorgitin02
Copy link
Author

Correction to previous comment (shell ate inline formatting):\n\nAddressed in cf48f33:\n- ran rustfmt on the touched Rust files\n- removed the duplicate date-error-format-modifier-width-too-large entry in src/uu/date/locales/fr-FR.ftl\n\nRegarding #11053: that earlier PR uses a hard cap (1024) on width. This PR takes a different approach by making width/padding handling fully fallible and returning a normal format error for oversized widths (no arbitrary cap), while preserving normal behavior for valid widths.

@sylvestre
Copy link
Contributor

@jorgitin02 your previous comment isn't correctly formatted

also, why:

  • you didn't updated the previous PR instead?
  • closed it?
    thanks

@github-actions
Copy link

GNU testsuite comparison:

GNU test failed: tests/tail/retry. tests/tail/retry is passing on 'main'. Maybe you have to rebase?
Skip an intermittent issue tests/date/date-locale-hour (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/pr/bounded-memory (fails in this run but passes in the 'main' branch)

@jorgitin02
Copy link
Author

CI follow-up: the previous run reported a GNU regression in tests/tail/retry (aggregate gate) while that test passes on main. I rebased date-11044-clean onto current upstream/main and force-pushed. New head is f0e2af6; rerun is in progress.

@codspeed-hq
Copy link

codspeed-hq bot commented Feb 24, 2026

Merging this PR will not alter performance

✅ 294 untouched benchmarks
⏩ 42 skipped benchmarks1


Comparing jorgitin02:date-11044-clean (b8948ff) with main (72ec8cf)

Open in CodSpeed

Footnotes

  1. 42 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@sylvestre
Copy link
Contributor

retry is unrelated

@jorgitin02
Copy link
Author

CI status clarification:\n- The failure emails currently arriving are from older runs on previous head cf48f33 (including GNU aggregate).\n- Current PR head is f0e2af6 (rebased onto latest main).\n- New workflow runs for this head were created in action_required state (0 jobs), so they need a maintainer to click Approve and run before checks can execute.\n\nOnce approved, the new runs should give the real status for the rebased head.

@github-actions
Copy link

GNU testsuite comparison:

GNU test failed: tests/date/resolution. tests/date/resolution is passing on 'main'. Maybe you have to rebase?
Skip an intermittent issue tests/tail/inotify-dir-recreate (fails in this run but passes in the 'main' branch)

@github-actions
Copy link

GNU testsuite comparison:

Skip an intermittent issue tests/date/date-locale-hour (fails in this run but passes in the 'main' branch)
Note: The gnu test tests/expand/bounded-memory is now being skipped but was previously passing.

@sylvestre
Copy link
Contributor

CI status clarification:\n- The failure emails currently arriving are from older runs on previous head cf48f33 (including GNU aggregate).\n- Current PR head is f0e2af6 (rebased onto latest main).\n- New workflow runs for this head were created in action_required state (0 jobs), so they need a maintainer to click Approve and run before checks can execute.\n\nOnce approved, the new runs should give the real status for the rebased head.

please ask your LLM to format your message correctly...
and you still haven't closed #11053

write!(
f,
"{}",
get_message_with_args("date-error-format-modifier-width-too-large", args)
Copy link
Contributor

Choose a reason for hiding this comment

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

please use the translate! macro

} else {
// Default: pad on the left (e.g., " -22" or " 1999")
result = format!("{}{result}", pad_char.to_string().repeat(padding));
let target_len = result.len().checked_add(padding).ok_or_else(|| {
Copy link
Contributor

Choose a reason for hiding this comment

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

please ask your LLM to deduplicate this code

Copy link
Author

Choose a reason for hiding this comment

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

Thanks for the callout. You’re right about the formatting; I reformatted the status message.
To avoid duplicate review, I’m closing #11053 and keeping the scoped fix in this PR only.

@jorgitin02 jorgitin02 closed this Feb 26, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

date: date +%9223372036854775807c panic

3 participants