Skip to content

numfmt: find the suffix by byte, not by character - #14296

Open
arbelonson-source wants to merge 1 commit into
uutils:mainfrom
arbelonson-source:fix/numfmt-multibyte-separator-panic
Open

numfmt: find the suffix by byte, not by character#14296
arbelonson-source wants to merge 1 commit into
uutils:mainfrom
arbelonson-source:fix/numfmt-multibyte-separator-panic

Conversation

@arbelonson-source

Copy link
Copy Markdown
Contributor

Fixes #13937.

find_valid_number_with_suffix ends the number at a byte offset and then looks for the suffix that many characters further in. The two agree only while every character is one byte — which the decimal separator is not in a locale like ar-SA, where it is U+066B ٫, two bytes:

$ 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`
$ echo $?                                  # 134 with panic=abort, 101 otherwise

The number here is , three bytes but two characters. Skipping three characters walks past the and lands on the K, so K is accepted as the suffix; slicing to three bytes plus one then cuts the in half and the str slice aborts the process. GNU rejects the input and exits 2.

The fix is to take the suffix from s[numeric_part.len()..], so there is only one accounting. The two arms that returned the same slice are folded into one, since duplicating the index arithmetic three times is what let it drift.

@leeewee's write-up in the issue has the full table of the three arms and the input shape that reaches each — this covers all three.

Behaviour

Every affected input now exits 2 as GNU does, instead of aborting:

input (LC_ALL=ar_SA.UTF-8) before after
numfmt --from=si '1٫€K' abort numfmt: invalid suffix in input '1٫€K': '€K'
numfmt --from=si '1٫€Kx' abort numfmt: invalid suffix in input '1٫€Kx': '€Kx'
numfmt --from=auto '1٫€Ki' abort numfmt: invalid suffix in input '1٫€Ki': '€Ki'
numfmt --from=si '1٫Ki' ... '1٫Ki': 'Ki' ... '1٫Ki': 'i'

The last row is the same desync seen from the other side: K is the suffix and i is what is wrong with the input, but the character-skip had landed early and blamed both.

That numfmt accepts 1٫5K at all where GNU rejects it is a separate matter, tracked in #14232; this PR does not change it.

Testing

  • Old-vs-new comparison over 876 invocations--from in all five modes and --to in four, crossed with 40 inputs (plain, suffixed, i-suffixed, malformed, and the multibyte shapes) under LC_ALL of C, ar_SA.UTF-8, de_DE.UTF-8 and fr_FR.UTF-8, plus --padding, --suffix and --field: 860 identical, 16 changed, and all 16 are under ar_SA.UTF-8 — the 12 aborts above, plus the 4 1٫Ki cases. Nothing outside the multibyte-separator locale moves.
  • Checked against GNU 9.11 under a locally built ar_SA.UTF-8: every changed case now agrees with GNU on the exit code.
  • New test_locale_multibyte_separator_before_a_multibyte_char, verified to fail on main.
  • cargo test --features numfmt --test tests -- test_numfmt: 186 passed, 0 failed (185 pre-existing, 1 new); cargo test -p uu_numfmt: 53 passed.
  • cargo fmt --check and cargo clippy -p uu_numfmt --all-targets -- -D warnings: clean.

Disclosure

Prepared with AI assistance (Claude Opus 5, via Claude Code), per the AI policy in CONTRIBUTING.md. GNU's behaviour was established by running the installed GNU binary as a black box; I did not read GNU coreutils source. All testing was run locally.

`find_valid_number_with_suffix` ends the number at a byte offset and then
looks for the suffix that many *characters* further in. The two agree
only while every character is one byte, which the decimal separator is
not in a locale like ar-SA, where it is the two-byte U+066B:

    $ 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)

The number is `1٫`, three bytes. Skipping three characters walks past
the `€` and finds the `K`, so `K` is taken as the suffix; slicing to
three bytes plus one then cuts the `€` in half and the slice aborts the
process. GNU rejects the input and exits 2.

Take the suffix from the bytes after the number instead, so the two
accountings cannot disagree. The two arms that returned the same slice
are now one, since duplicating the arithmetic is what let it drift.

Fixes uutils#13937.
@github-actions

Copy link
Copy Markdown

GNU testsuite comparison:

Skip an intermittent issue tests/date/resolution (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/tail/symlink (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/cut/bounded-memory (passes in this run but fails in the 'main' branch)
Skipping an intermittent issue tests/date/date-locale-hour (passes in this run but fails in the 'main' branch)
Skip an intermittent issue tests/tail/tail-n0f (was skipped on 'main', now failing)

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.

numfmt panics (str char-boundary) on a number using a multibyte locale decimal separator before a multibyte char and a suffix

1 participant