mirror of
https://github.com/uutils/coreutils.git
synced 2026-06-10 15:48:22 -07:00
ls: fix link-count column inflating per ACL file
The `+1` accounting for the ACL/context indicator was applied once per item inside the padding loop, so N files with ACLs inflated the link-count column width by N. Apply it a single time after the loop. Fixes #10980
This commit is contained in:
@@ -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 {
|
||||
|
||||
@@ -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 <date> 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
|
||||
|
||||
Reference in New Issue
Block a user