From 46a66cb6414ca20429ce8e633edc01b1efb341b5 Mon Sep 17 00:00:00 2001 From: arbelonson-source <269032023+arbelonson-source@users.noreply.github.com> Date: Mon, 31 Aug 2026 10:03:40 +0300 Subject: [PATCH 1/2] numfmt: stop a multi-byte locale decimal separator from panicking MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes #13937. Under a locale whose decimal separator is multi-byte (e.g. Arabic ٫, 2 bytes), find_valid_number_with_suffix skipped past the numeric part by treating numeric_part.len() -- a byte length -- as a character count for Iterator::skip. Whenever the numeric part contained the multi-byte separator, that landed one or more characters too far, examining the wrong character as the "suffix" -- one that could itself be multi-byte (e.g. €) and, if it happened to look like a recognized suffix from that wrong position, get byte-sliced using an end index computed for a hardcoded 1-byte suffix width. On an actual multi-byte character there, that index falls inside it rather than on a char boundary, which panics: 'byte index 4 is not a char boundary; it is inside '€' (bytes 3..6)'. panic=abort turns that into a SIGABRT (exit 134) instead of GNU's clean 'invalid suffix in input' (exit 2). Slice by numeric_part.len() -- safe, since numeric_part is itself a byte-aligned prefix of the input -- and read characters directly from that point instead of skipping character-by-character from the start, and size each computed slice by the suffix's own real UTF-8 byte width rather than assuming 1. Also merged two match arms clippy correctly flagged as identical once both used the same real-width computation, rather than the previous hardcoded-width arithmetic that happened to differ only by coincidence. AI-assisted-by: Claude Opus 5, via Claude Code --- src/uu/numfmt/src/format.rs | 17 ++++++++++------- tests/by-util/test_numfmt.rs | 22 ++++++++++++++++++++++ 2 files changed, 32 insertions(+), 7 deletions(-) diff --git a/src/uu/numfmt/src/format.rs b/src/uu/numfmt/src/format.rs index 9c03801ef7d..f20c789a9b4 100644 --- a/src/uu/numfmt/src/format.rs +++ b/src/uu/numfmt/src/format.rs @@ -57,7 +57,13 @@ fn find_valid_number_with_suffix(s: &str, unit: Unit) -> Option<&str> { let accepts_suffix = unit != Unit::None; let accepts_i = [Unit::Auto, Unit::Iec(true)].contains(&unit); - let mut characters = s.chars().skip(numeric_part.len()); + // `numeric_part.len()` is a byte length; skipping that many *characters* + // would land past the real next character whenever the numeric part + // contains a multi-byte decimal separator (e.g. Arabic ٫). Slice by that + // same byte length instead, which -- since numeric_part is itself a + // prefix of s -- always falls on a char boundary, then read characters + // from there directly. + let mut characters = s[numeric_part.len()..].chars(); let potential_suffix = characters.next(); let potential_i = characters.next(); @@ -66,14 +72,11 @@ fn find_valid_number_with_suffix(s: &str, unit: Unit) -> Option<&str> { } match (potential_suffix, potential_i) { - (Some(suffix), None) if RawSuffix::try_from(&suffix).is_ok() => { - Some(&s[..=numeric_part.len()]) - } (Some(suffix), Some('i')) if accepts_i && RawSuffix::try_from(&suffix).is_ok() => { - Some(&s[..numeric_part.len() + 2]) + Some(&s[..numeric_part.len() + suffix.len_utf8() + 'i'.len_utf8()]) } - (Some(suffix), Some(_)) if RawSuffix::try_from(&suffix).is_ok() => { - Some(&s[..=numeric_part.len()]) + (Some(suffix), _) if RawSuffix::try_from(&suffix).is_ok() => { + Some(&s[..numeric_part.len() + suffix.len_utf8()]) } _ => Some(numeric_part), } diff --git a/tests/by-util/test_numfmt.rs b/tests/by-util/test_numfmt.rs index 4db86944771..b64cf206074 100644 --- a/tests/by-util/test_numfmt.rs +++ b/tests/by-util/test_numfmt.rs @@ -1614,6 +1614,28 @@ fn test_locale_fr_rejects_period() { .stderr_contains("invalid"); } +#[test] +#[cfg_attr(wasi_runner, ignore = "WASI: locale env vars not propagated")] +fn test_locale_multibyte_separator_does_not_panic_on_invalid_suffix() { + // Regression test for #13937: under a locale whose decimal separator is + // multi-byte (Arabic ٫, 2 bytes), an invalid multi-byte suffix character + // right after the numeric part used to abort the process instead of + // being cleanly rejected, since the suffix search skipped by the + // separator's *byte* length as though it were a *character* count. + for arg in ["1٫€K", "1٫€Kx"] { + new_ucmd!() + .env("LC_ALL", "ar_SA.UTF-8") + .args(&["--from=si", arg]) + .fails() + .stderr_contains("invalid suffix"); + } + new_ucmd!() + .env("LC_ALL", "ar_SA.UTF-8") + .args(&["--from=auto", "1٫€Ki"]) + .fails() + .stderr_contains("invalid suffix"); +} + #[test] fn test_locale_c_uses_period() { // C locale should still use '.' as usual From 7b231b00693a095557c2712c813e00bb29ad5e35 Mon Sep 17 00:00:00 2001 From: arbelonson-source <269032023+arbelonson-source@users.noreply.github.com> Date: Mon, 31 Aug 2026 14:23:24 +0300 Subject: [PATCH 2/2] numfmt: shorten the byte-length slice comment --- src/uu/numfmt/src/format.rs | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/uu/numfmt/src/format.rs b/src/uu/numfmt/src/format.rs index f20c789a9b4..ab41906e20e 100644 --- a/src/uu/numfmt/src/format.rs +++ b/src/uu/numfmt/src/format.rs @@ -57,12 +57,8 @@ fn find_valid_number_with_suffix(s: &str, unit: Unit) -> Option<&str> { let accepts_suffix = unit != Unit::None; let accepts_i = [Unit::Auto, Unit::Iec(true)].contains(&unit); - // `numeric_part.len()` is a byte length; skipping that many *characters* - // would land past the real next character whenever the numeric part - // contains a multi-byte decimal separator (e.g. Arabic ٫). Slice by that - // same byte length instead, which -- since numeric_part is itself a - // prefix of s -- always falls on a char boundary, then read characters - // from there directly. + // Slice by byte length, not `.chars().skip()`: the numeric part may + // contain a multi-byte decimal separator (e.g. Arabic ٫). let mut characters = s[numeric_part.len()..].chars(); let potential_suffix = characters.next(); let potential_i = characters.next();