diff --git a/src/uu/ls/src/display.rs b/src/uu/ls/src/display.rs index d1c35e45c..74180ea1e 100644 --- a/src/uu/ls/src/display.rs +++ b/src/uu/ls/src/display.rs @@ -86,6 +86,10 @@ pub(crate) struct PaddingCollection { #[cfg(unix)] pub(crate) minor: usize, pub(crate) block_size: usize, + /// True if any listed item has an ACL or non-trivial security context, + /// which requires reserving one extra column for the `+`/`.` indicator + /// so link-count columns align across items with and without it. + pub(crate) has_alt_access: bool, } pub(crate) struct DisplayItemName { @@ -905,9 +909,10 @@ fn display_item_long( state.display_buf.push(b' '); } - state - .display_buf - .extend_pad_left(&display_symlink_count(md), padding.link_count); + state.display_buf.extend_pad_left( + &display_symlink_count(md), + padding.link_count + usize::from(padding.has_alt_access), + ); if config.long.owner { state.display_buf.push(b' '); @@ -1058,7 +1063,10 @@ fn display_item_long( state.display_buf.push(b'.'); } state.display_buf.push(b' '); - state.display_buf.extend_pad_left("?", padding.link_count); + state.display_buf.extend_pad_left( + "?", + padding.link_count + usize::from(padding.has_alt_access), + ); if config.long.owner { state.display_buf.push(b' '); @@ -1241,6 +1249,7 @@ fn calculate_padding_collection( major: 1, minor: 1, block_size: 1, + has_alt_access: false, }; for item in items { @@ -1272,7 +1281,8 @@ fn calculate_padding_collection( padding_collections.context = context_len.max(padding_collections.context); } - // correctly align columns when some files have capabilities/ACLs and others do not + // Track whether any item has an ACL or non-trivial security context so + // rendering can reserve a single extra column for the `+`/`.` indicator. { #[cfg(any(not(unix), target_os = "android", target_os = "macos"))] // TODO: See how Mac should work here @@ -1280,7 +1290,7 @@ fn calculate_padding_collection( #[cfg(all(unix, not(any(target_os = "android", target_os = "macos"))))] let is_acl_set = has_acl(item.display_name()); if context_len > 1 || is_acl_set { - padding_collections.link_count += 1; + padding_collections.has_alt_access = true; } } @@ -1321,6 +1331,7 @@ fn calculate_padding_collection( context: 1, size: 1, block_size: 1, + has_alt_access: false, }; for item in items { diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index fb9a00fa2..5058de3e0 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -6214,6 +6214,56 @@ fn test_acl_display() { .stdout_matches(&re_without_acl); } +// Regression test for https://github.com/uutils/coreutils/issues/10980 +// Each file with an ACL must not inflate the link-count column width. +#[cfg(all(unix, not(target_os = "macos")))] +#[test] +fn test_acl_padding_not_inflated() { + use std::process::Command; + + let scene = TestScenario::new(util_name!()); + let at = &scene.fixtures; + + let uid = unsafe { libc::getuid() }; + let names = ["file1", "file2", "file3", "file4", "file5"]; + for name in &names { + at.touch(name); + let status = Command::new("setfacl") + .args(["-m", &format!("u:{uid}:rw"), &at.plus_as_string(name)]) + .status() + .map(|s| s.code()); + if !matches!(status, Ok(Some(0))) { + println!("test skipped: setfacl failed"); + return; + } + } + + let out = scene + .ucmd() + .current_dir(at.as_string()) + .arg("-l") + .succeeds() + .stdout_move_str(); + + // Each line with an ACL file should look like: + // -rw-------+ 1 user group 0 fileN + // i.e. single space between the `+` and the link count `1`. + // The bug inflates that gap to one extra space per ACL file. + let mut checked = 0; + for line in out.lines() { + if line.starts_with('-') && line.contains('+') { + let after_plus = line.split('+').nth(1).unwrap(); + let leading_spaces = after_plus.len() - after_plus.trim_start().len(); + assert_eq!( + leading_spaces, 1, + "expected exactly 1 space between '+' and link count, got {leading_spaces} in line: {line:?}" + ); + checked += 1; + } + } + assert!(checked > 0, "no ACL lines were validated"); +} + // Make sure that "ls --color" correctly applies color "normal" to text and // files. Specifically, it should use the NORMAL setting to format non-file name // output and file names that don't have a designated color (unless the FILE