numfmt: fix char-boundary panic on a multibyte locale decimal separator

[leeewee]

Fixes #13937

numfmt aborts with a str char-boundary panic on an input like 1٫€K under a locale whose decimal separator is multibyte (e.g. LC_ALL=ar_SA.UTF-8, where the separator is the Arabic ٫ U+066B), while GNU rejects it gracefully:

$ LC_ALL=ar_SA.UTF-8 numfmt --from=si '1٫€K'
thread 'main' panicked at src/uu/numfmt/src/format.rs:70:20:
byte index 4 is not a char boundary; it is inside '€' (bytes 3..6) of `1٫€K`
# aborts, exit 134

$ LC_ALL=ar_SA.UTF-8 /usr/bin/numfmt --from=si '1٫€K'
numfmt: invalid suffix in input: '1٫€K'
# exit 2

Cause

find_valid_number_with_suffix located the suffix with s.chars().skip(numeric_part.len()), using numeric_part.len() — a byte length — as a char skip count. When the numeric part holds a multibyte char (here the locale decimal separator ٫, so 1٫ is the numeric part with len() == 3 bytes but only 2 chars), the byte count over-skips past the following multibyte € onto a later valid suffix (K), taking a slicing arm; &s[..=numeric_part.len()] then slices to a byte offset inside € and panics.

Fix

Index the suffix from the byte offset numeric_part.len() (s[numeric_part.len()..].chars()) rather than skipping by chars. numeric_part is always a prefix of s, so numeric_part.len() is a char boundary and the slice is safe; the real next char (€) is now examined, is not a valid suffix, and the input takes the same graceful "invalid suffix" rejection GNU gives. The existing byte-slices are unaffected — a valid suffix is ASCII, so they remain char-aligned.

A regression test covers the --from=si and --from=auto variants under LC_ALL=ar_SA.UTF-8.

[Copilot]

Pull request overview

Fixes a crash in numfmt when parsing inputs under locales with a multibyte decimal separator by avoiding mixing byte offsets and char iteration, and adds a regression test for the reported panic case.

Changes:

• Fix find_valid_number_with_suffix to derive the suffix iterator from a byte-slice (s[numeric_part.len()..].chars()) instead of chars().skip(byte_len).
• Add a regression test covering --from=si and --from=auto with LC_ALL=ar_SA.UTF-8 and an invalid multibyte suffix sequence (€…).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
src/uu/numfmt/src/format.rs	Prevents char-boundary panics by aligning suffix scanning with the numeric prefix byte boundary.
tests/by-util/test_numfmt.rs	Adds regression coverage for malformed inputs under a multibyte-decimal locale.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[codspeed-hq]

Merging this PR will improve performance by 4.02%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

⚡ 3 improved benchmarks
✅ 350 untouched benchmarks
⏩ 50 skipped benchmarks¹

Performance Changes

	Mode	Benchmark	BASE	HEAD	Efficiency
⚡	Simulation	numfmt_large_numbers_si[10000]	98.1 ms	93.2 ms	+5.22%
⚡	Simulation	complex_relative_date	330.6 µs	318.7 µs	+3.72%
⚡	Simulation	ls_recursive_balanced_tree[(6, 4, 15)]	120.7 ms	117.1 ms	+3.12%

Tip

Curious why this is faster? Comment @codspeedbot explain why this is faster on this PR, or directly use the CodSpeed MCP with your agent.

---

_{Comparing leeewee:numfmt-fix-locale-char-boundary (96e37c8) with main (ebaf6e9)}

Footnotes

1. 50 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. ↩

[github-actions]

Binary size comparison:

Individual binary size comparison VS main (threshold: >=5% AND >=4 KB).

Total size of compared binaries: 148.96 MB (-140 KB, -0.09%)

Significant per-binary changes:
  [           1.10 MB ->    1.18 MB  (+76 KB, +6.74%)
  test        1.10 MB ->    1.18 MB  (+76 KB, +6.74%)
  install     1.25 MB ->    1.32 MB  (+72 KB, +5.62%)
  mkdir       1.10 MB ->    1.17 MB  (+72 KB, +6.38%)
  mkfifo      1.09 MB ->    1.16 MB  (+72 KB, +6.45%)
  mknod       1.10 MB ->    1.17 MB  (+72 KB, +6.41%)
  chmod       1.15 MB ->    1.22 MB  (+68 KB, +5.76%)

[github-actions]

GNU testsuite comparison:

Skipping an intermittent issue tests/tail/symlink (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/tail/tail-n0f (passes in this run but fails in the 'main' branch)
Note: The gnu test tests/seq/seq-epipe is now being skipped but was previously passing.

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Suppressed comments (1)

tests/by-util/test_numfmt.rs:463

• The new regression test sets LC_ALL to fr_FR. UTF-8 (note the stray space) while using the Arabic decimal separator ٫ in the input. Under fr_FR.UTF-8 the decimal separator is ,, so this test won't exercise the reported multibyte-decimal-separator path and is likely to fail (or even fail to set the locale). Also, since this test relies on locale env vars, it should be ignored under wasi_runner like the existing locale tests in this file, and it’s safer to early-return when ar_SA.UTF-8 isn’t available on Unix CI.

#[test]
fn test_from_multibyte_decimal_separator_invalid_suffix() {
    new_ucmd!()
        .env("LC_ALL", "fr_FR. UTF-8")
        .args(&["--from=si", "1٫€K"])

[sylvestre]

I broke it, sorry