ls: preserve bytes in non-UTF-8 ignore patterns

src/uu/ls/src/config.rs

@@ -13,9 +13,12 @@
     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 @@
     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 @@
     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<Self, glob::PatternError> {
+        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(crate) recursive: bool,
     pub(crate) reverse: bool,
     pub(crate) dereference: Dereference,
-    pub(crate) ignore_patterns: Vec<Pattern>,
+    pub(crate) ignore_patterns: Vec<IgnorePattern>,
     pub(crate) size_format: SizeFormat,
     pub(crate) directory: bool,
     pub(crate) time: MetadataTimeField,
@@ -795,19 +858,19 @@
         let mut quoting_style = extract_quoting_style(options, show_control, mode);
         let indicator_style = extract_indicator_style(options);
 
-        let mut ignore_patterns: Vec<Pattern> = Vec::new();
+        let mut ignore_patterns: Vec<IgnorePattern> = 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::<String>(options::IGNORE)
+            .get_many::<OsString>(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 @@
 
         if files == Files::Normal {
             for pattern in options
-                .get_many::<String>(options::HIDE)
+                .get_many::<OsString>(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!(

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(

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(

tests/by-util/test_ls.rs

@@ -4687,22 +4687,16 @@
         .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 @@
         .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);
+    }
+}