Skip to content

ls: fix panic on a color style with no attributes - #14373

Merged
sylvestre merged 1 commit into
uutils:mainfrom
harshasiddartha:fix/ls-color-style-code-underflow
Sep 4, 2026
Merged

ls: fix panic on a color style with no attributes#14373
sylvestre merged 1 commit into
uutils:mainfrom
harshasiddartha:fix/ls-color-style-code-underflow

Conversation

@harshasiddartha

@harshasiddartha harshasiddartha commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Fixes #14360

ls --color=always panics with "attempt to subtract with overflow" in debug builds when an LS_COLORS entry yields a style that has no attributes, for example:

$ mkdir d && touch d/f
$ LS_COLORS='no=39' ./target/debug/ls --color=always d >/dev/null
thread 'main' panicked at src/uu/ls/src/colors.rs:210:22:
attempt to subtract with overflow

get_style_code painted an empty string with nu-ansi-term and then stripped the trailing \x1b[0m with ret.truncate(ret.len() - 4). A style with no attributes paints to an empty string, so there is no trailing reset to strip and the subtraction underflows on usize.

Rather than painting and then stripping, this asks nu-ansi-term for the prefix directly with nu_a_style.prefix().to_string(). That returns the escape sequence on its own, and an empty string for a style with no attributes, so there is nothing to strip and the underflow cannot happen. Style::write_prefix and write_suffix are gated on the same is_plain() check, so the result is byte-identical to the old code for every style that previously worked: \x1b[07m\x1b[0m still becomes \x1b[07m.

Tested with the new test_ls_color_empty_style in tests/by-util/test_ls.rs, which pins the exact output. Without the fix it fails with the panic and exit code 101; with the fix it passes. The other --color tests still pass (cargo test --test tests --features ls test_ls_color, 7 passed), cargo clippy -p uu_ls --all-targets is clean, and rustfmt --check passes on both touched files.

One thing I did not change and want to flag: with no=39 the style is normalized away entirely, so we emit no \x1b[39m for it. That is a separate compatibility question from the panic, so I left it out of this PR. The test pins current behavior, which makes that gap easy to spot if someone fixes it later. I built and tested only the ls crate and the tests target locally, not the full workspace.

get_style_code stripped the trailing reset from the rendered style with
`ret.truncate(ret.len() - 4)`. A style that carries no attribute, such as
the one built from `LS_COLORS='no=39'`, renders to an empty string, so the
subtraction underflowed and `ls --color` panicked in debug builds.

Strip the reset with strip_suffix instead, and leave the string untouched
when there is none.
@harshasiddartha
harshasiddartha force-pushed the fix/ls-color-style-code-underflow branch from 543e3ee to 8e9e484 Compare September 3, 2026 18:23
@codspeed-hq

codspeed-hq Bot commented Sep 4, 2026

Copy link
Copy Markdown

Merging this PR will improve performance by 6.11%

⚡ 1 improved benchmark
✅ 15 untouched benchmarks
⏩ 402 skipped benchmarks1

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation ls_recursive_deep_tree[(200, 2)] 3.4 ms 3.2 ms +6.11%

Tip

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


Comparing harshasiddartha:fix/ls-color-style-code-underflow (8e9e484) with main (52eec64)2

Open in CodSpeed

Footnotes

  1. 402 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.

  2. No successful run was found on main (5426f41) during the generation of this report, so 52eec64 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report.

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown

GNU testsuite comparison:

Skip an intermittent issue tests/cut/bounded-memory (fails in this run but passes in the 'main' branch)
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)
Skip an intermittent issue tests/tail/retry (fails in this run but passes in the 'main' branch)
Skipping an intermittent issue tests/date/resolution (passes in this run but fails in the 'main' branch)
Note: The gnu test tests/tail/pipe-f is now being skipped but was previously passing.
Note: The gnu test tests/tail/tail-n0f is now being skipped but was previously passing.

@sylvestre
sylvestre merged commit f0c1305 into uutils:main Sep 4, 2026
103 checks passed
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.

ls: arithmetic overflow (overflow-checks) on an empty color style — get_style_code len - 4 underflow

2 participants