From 2612968378290b7daade408205abb4292e5cbd70 Mon Sep 17 00:00:00 2001 From: limadog9 Date: Sun, 18 Oct 2026 14:28:54 -0400 Subject: [PATCH] ls: preserve bytes in non-UTF-8 ignore patterns Use native glob matching for non-UTF-8 Unix filenames and ignore/hide patterns. Add regressions distinguishing invalid bytes from literal replacement characters. Closes #7161. --- src/uu/ls/src/config.rs | 81 ++++++++++++++++++++++++---- src/uu/ls/src/display.rs | 25 +-------- src/uu/ls/src/ls.rs | 2 + tests/by-util/test_ls.rs | 111 +++++++++++++++++++++++++++++++++------ 4 files changed, 170 insertions(+), 49 deletions(-) diff --git a/src/uu/ls/src/config.rs b/src/uu/ls/src/config.rs index 29f4d9415af..22a0887b0ac 100644 --- a/src/uu/ls/src/config.rs +++ b/src/uu/ls/src/config.rs @@ -13,9 +13,12 @@ use std::{ num::IntErrorKind, }; +#[cfg(not(all(unix, not(target_os = "redox"))))] use glob::Pattern; use lscolors::LsColors; use term_grid::SPACES_IN_TAB; +#[cfg(not(all(unix, not(target_os = "redox"))))] +use uucore::parser::parse_glob; use uucore::{ diagnostics::OptionValue, @@ -24,7 +27,7 @@ use uucore::{ format::human::SizeFormat, fsext::MetadataTimeField, line_ending::LineEnding, - parser::{parse_block_size, parse_glob, parse_size::parse_size_non_zero_u64}, + parser::{parse_block_size, parse_size::parse_size_non_zero_u64}, quoting_style::{QuotingStyle, quoting_style_from_env}, show_error, show_warning, time::format, @@ -38,6 +41,66 @@ use crate::{ options::QUOTING_STYLE, }; +#[derive(Debug, Clone)] +pub(crate) struct IgnorePattern { + #[cfg(not(all(unix, not(target_os = "redox"))))] + pattern: Pattern, + #[cfg(all(unix, not(target_os = "redox")))] + bytes: std::ffi::CString, +} + +impl IgnorePattern { + #[allow( + clippy::unnecessary_wraps, + reason = "glob validation is fallible on other platforms" + )] + fn new(pattern: &OsStr) -> Result { + Ok(Self { + #[cfg(not(all(unix, not(target_os = "redox"))))] + pattern: parse_glob::from_str(&pattern.to_string_lossy())?, + #[cfg(all(unix, not(target_os = "redox")))] + // Patterns come from argv or fixed strings, so they cannot contain NUL. + bytes: std::ffi::CString::new(pattern.as_encoded_bytes()) + .expect("command-line patterns cannot contain NUL"), + }) + } + + pub(crate) fn matches_any(patterns: &[Self], name: &OsStr) -> bool { + if patterns.is_empty() { + return false; + } + #[cfg(all(unix, not(target_os = "redox")))] + { + let Ok(name) = std::ffi::CString::new(name.as_encoded_bytes()) else { + return false; + }; + patterns.iter().any(|pattern| { + // SAFETY: both arguments are NUL-terminated. FNM_PERIOD + // prevents wildcards from matching a leading dot. + unsafe { + uucore::libc::fnmatch( + pattern.bytes.as_ptr(), + name.as_ptr(), + uucore::libc::FNM_PERIOD, + ) == 0 + } + }) + } + #[cfg(not(all(unix, not(target_os = "redox"))))] + { + let options = glob::MatchOptions { + require_literal_leading_dot: true, + require_literal_separator: false, + case_sensitive: true, + }; + let name = name.to_string_lossy(); + patterns + .iter() + .any(|p| p.pattern.matches_with(&name, options)) + } + } +} + pub mod options { pub mod format { pub static ONE_LINE: &str = "1"; @@ -217,7 +280,7 @@ pub struct Config { pub(crate) recursive: bool, pub(crate) reverse: bool, pub(crate) dereference: Dereference, - pub(crate) ignore_patterns: Vec, + pub(crate) ignore_patterns: Vec, pub(crate) size_format: SizeFormat, pub(crate) directory: bool, pub(crate) time: MetadataTimeField, @@ -795,19 +858,19 @@ impl Config { let mut quoting_style = extract_quoting_style(options, show_control, mode); let indicator_style = extract_indicator_style(options); - let mut ignore_patterns: Vec = Vec::new(); + let mut ignore_patterns: Vec = Vec::new(); if options.get_flag(options::IGNORE_BACKUPS) { - ignore_patterns.push(Pattern::new("*~").unwrap()); - ignore_patterns.push(Pattern::new(".*~").unwrap()); + ignore_patterns.push(IgnorePattern::new(OsStr::new("*~")).unwrap()); + ignore_patterns.push(IgnorePattern::new(OsStr::new(".*~")).unwrap()); } for pattern in options - .get_many::(options::IGNORE) + .get_many::(options::IGNORE) .into_iter() .flatten() { - if let Ok(p) = parse_glob::from_str(pattern) { + if let Ok(p) = IgnorePattern::new(pattern) { ignore_patterns.push(p); } else { show_warning!( @@ -819,11 +882,11 @@ impl Config { if files == Files::Normal { for pattern in options - .get_many::(options::HIDE) + .get_many::(options::HIDE) .into_iter() .flatten() { - if let Ok(p) = parse_glob::from_str(pattern) { + if let Ok(p) = IgnorePattern::new(pattern) { ignore_patterns.push(p); } else { show_warning!( diff --git a/src/uu/ls/src/display.rs b/src/uu/ls/src/display.rs index 7dbd4db3a3f..5029b3c1e67 100644 --- a/src/uu/ls/src/display.rs +++ b/src/uu/ls/src/display.rs @@ -26,7 +26,6 @@ use std::{ }; use ansi_width::ansi_width; -use glob::MatchOptions; #[cfg(unix)] use rustc_hash::FxHashMap; use term_grid::{DEFAULT_SEPARATOR_SIZE, Direction, Filling, Grid, GridOptions}; @@ -223,30 +222,8 @@ pub fn should_display(entry: &DirEntry, config: &Config) -> bool { return false; } - // check if it is among ignore_patterns - let options = MatchOptions { - // setting require_literal_leading_dot to match behavior in GNU ls - require_literal_leading_dot: true, - require_literal_separator: false, - case_sensitive: true, - }; - let file_name = entry.file_name(); - // If the decoding fails, still match best we can - // FIXME: use OsStrings or Paths once we have a glob crate that supports it: - // https://github.com/rust-lang/glob/issues/23 - // https://github.com/rust-lang/glob/issues/78 - // https://github.com/BurntSushi/ripgrep/issues/1250 - - let file_name = match file_name.to_str() { - Some(s) => Cow::Borrowed(s), - None => file_name.to_string_lossy(), - }; - - !config - .ignore_patterns - .iter() - .any(|p| p.matches_with(&file_name, options)) + !crate::config::IgnorePattern::matches_any(&config.ignore_patterns, &file_name) } fn display_dir_entry_size( diff --git a/src/uu/ls/src/ls.rs b/src/uu/ls/src/ls.rs index 31a6c974999..82ce1ddb4b0 100644 --- a/src/uu/ls/src/ls.rs +++ b/src/uu/ls/src/ls.rs @@ -410,6 +410,7 @@ pub fn uu_app() -> Command { .long(options::HIDE) .action(ArgAction::Append) .value_name("PATTERN") + .value_parser(ValueParser::os_string()) .help(translate!("ls-help-hide-pattern")), ) .arg( @@ -418,6 +419,7 @@ pub fn uu_app() -> Command { .long(options::IGNORE) .action(ArgAction::Append) .value_name("PATTERN") + .value_parser(ValueParser::os_string()) .help(translate!("ls-help-ignore-pattern")), ) .arg( diff --git a/tests/by-util/test_ls.rs b/tests/by-util/test_ls.rs index 23833bfc1d4..dacc097f41d 100644 --- a/tests/by-util/test_ls.rs +++ b/tests/by-util/test_ls.rs @@ -4687,22 +4687,16 @@ fn test_ls_ignore_hide() { .succeeds() .stdout_only("some_other_file\n"); - // Invalid patterns - scene - .ucmd() - .arg("--ignore=READ[ME") - .arg("-1") - .succeeds() - .stderr_contains("Invalid pattern") - .stdout_is("CONTRIBUTING.md\nREADME.md\nREADMECAREFULLY.md\nsome_other_file\n"); - - scene - .ucmd() - .arg("--hide=READ[ME") - .arg("-1") - .succeeds() - .stderr_contains("Invalid pattern") - .stdout_is("CONTRIBUTING.md\nREADME.md\nREADMECAREFULLY.md\nsome_other_file\n"); + // fnmatch treats the unmatched bracket literally; the glob fallback rejects it. + for option in ["--ignore=READ[ME", "--hide=READ[ME"] { + let result = scene.ucmd().arg(option).arg("-1").succeeds(); + if cfg!(all(unix, not(target_os = "redox"))) { + result.no_stderr(); + } else { + result.stderr_contains("Invalid pattern"); + } + result.stdout_is("CONTRIBUTING.md\nREADME.md\nREADMECAREFULLY.md\nsome_other_file\n"); + } } #[test] @@ -8507,3 +8501,88 @@ fn test_ls_dereference_sibling_alias_not_already_listed() { .stdout_contains("link:\nfile") .no_stderr(); } + +#[cfg(all(unix, not(target_vendor = "apple")))] +#[test] +fn test_ignore_patterns_preserve_non_utf8_names() { + use std::ffi::OsStr; + use std::os::unix::ffi::OsStrExt; + let ts = TestScenario::new(util_name!()); + for name in [b"bad\xff.txt".as_slice(), "bad�.txt".as_bytes(), b"keep"] { + ts.fixtures.touch(OsStr::from_bytes(name)); + } + for option in ["--ignore", "--hide"] { + let result = ts + .ucmd() + .args(&["--zero", option]) + .arg("bad�.txt") + .succeeds(); + result.stdout_only_bytes(b"bad\xff.txt\0keep\0"); + let result = ts + .ucmd() + .args(&["--zero", option]) + .arg(OsStr::from_bytes(b"bad\xff.*")) + .succeeds(); + result.stdout_only_bytes(b"bad\xef\xbf\xbd.txt\0keep\0"); + ts.ucmd() + .args(&["--zero", option, "bad*.txt"]) + .succeeds() + .stdout_only("keep\0"); + } + ts.ucmd() + .args(&["-a", "--zero", "--ignore"]) + .arg(OsStr::from_bytes(b"bad\xff.*")) + .succeeds() + .stdout_only_bytes(b".\0..\0bad\xef\xbf\xbd.txt\0keep\0"); + // --hide is ignored with -a. + ts.ucmd() + .args(&["-a", "--zero", "--hide"]) + .arg(OsStr::from_bytes(b"bad\xff.*")) + .succeeds() + .stdout_contains_bytes(b"bad\xff.txt\0"); +} + +#[cfg(all(unix, not(target_os = "redox"), not(target_vendor = "apple")))] +#[test] +fn test_ignore_patterns_use_fnmatch_for_all_filename_encodings() { + use std::ffi::OsStr; + use std::os::unix::ffi::OsStrExt; + let ts = TestScenario::new(util_name!()); + for name in [b"a*\xff".as_slice(), b"a*x", b"a1\xff", b"a1x", b"keep"] { + ts.fixtures.touch(OsStr::from_bytes(name)); + } + for option in ["--ignore", "--hide"] { + ts.ucmd() + .env("LC_ALL", "C") + .args(&["--zero", option, r"a\*?", option, "a[[:digit:]]?"]) + .succeeds() + .stdout_only("keep\0"); + } +} + +#[cfg(all(unix, not(target_os = "redox")))] +#[test] +fn test_ignore_patterns_accept_unix_fnmatch_syntax() { + let ts = TestScenario::new(util_name!()); + for name in ["ab", "axxb", "[", "keep"] { + ts.fixtures.touch(name); + } + for option in ["--ignore", "--hide"] { + ts.ucmd() + .env("LC_ALL", "C") + .args(&["--zero", option, "a**b"]) + .succeeds() + .stdout_only("[\0keep\0"); + // Darwin's fnmatch does not treat an unclosed bracket as a literal. + let expected = if cfg!(target_vendor = "apple") { + "[\0ab\0axxb\0keep\0" + } else { + "ab\0axxb\0keep\0" + }; + ts.ucmd() + .env("LC_ALL", "C") + .args(&["--zero", option, "["]) + .succeeds() + .stdout_only(expected); + } +}