Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ls: Fix quoting for dirnames with a colon : in recursive mode, and patch the quote-align GNU test #6559

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 24 additions & 10 deletions src/uu/ls/src/ls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ use uucore::libc::{dev_t, major, minor};
#[cfg(unix)]
use uucore::libc::{S_IXGRP, S_IXOTH, S_IXUSR};
use uucore::line_ending::LineEnding;
use uucore::quoting_style::{escape_name, QuotingStyle};
use uucore::quoting_style::{escape_dir_name, escape_name, QuotingStyle};
use uucore::{
display::Quotable,
error::{set_exit_code, UError, UResult},
Expand Down Expand Up @@ -2036,14 +2036,27 @@ impl PathData {
}
}

/// Show the directory name in the case where several arguments are given to ls
/// or the recursive flag is passed.
///
/// ```no-exec
/// $ ls -R
/// .: <- This is printed by this function
/// dir1 file1 file2
///
/// dir1: <- This as well
/// file11
/// ```
fn show_dir_name(path_data: &PathData, out: &mut BufWriter<Stdout>, config: &Config) {
if config.hyperlink && !config.dired {
let name = escape_name(path_data.p_buf.as_os_str(), &config.quoting_style);
let hyperlink = create_hyperlink(&name, path_data);
write!(out, "{hyperlink}:").unwrap();
let escaped_name = escape_dir_name(path_data.p_buf.as_os_str(), &config.quoting_style);

let name = if config.hyperlink && !config.dired {
create_hyperlink(&escaped_name, path_data)
} else {
write!(out, "{}:", path_data.p_buf.display()).unwrap();
}
escaped_name
};

write!(out, "{name}:").unwrap();
}

#[allow(clippy::cognitive_complexity)]
Expand Down Expand Up @@ -2327,9 +2340,10 @@ fn enter_directory(
for e in entries
.iter()
.skip(if config.files == Files::All { 2 } else { 0 })
.filter(|p| p.ft.get().is_some())
.filter(|p| p.ft.get().unwrap().is_some())
.filter(|p| p.ft.get().unwrap().unwrap().is_dir())
.filter(|p| {
p.ft.get()
.is_some_and(|o_ft| o_ft.is_some_and(|ft| ft.is_dir()))
})
{
match fs::read_dir(&e.p_buf) {
Err(err) => {
Expand Down
56 changes: 46 additions & 10 deletions src/uucore/src/lib/features/quoting_style.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ use std::fmt;
// These are characters with special meaning in the shell (e.g. bash).
// The first const contains characters that only have a special meaning when they appear at the beginning of a name.
const SPECIAL_SHELL_CHARS_START: &[char] = &['~', '#'];
const SPECIAL_SHELL_CHARS: &str = "`$&*()|[]{};\\'\"<>?! ";
// PR#6559 : Remove `]{}` from special shell chars.
const SPECIAL_SHELL_CHARS: &str = "`$&*()|[;\\'\"<>?! ";

/// The quoting style to use when escaping a name.
#[derive(Clone, Copy, Debug, Eq, PartialEq)]
Expand Down Expand Up @@ -123,7 +124,7 @@ impl EscapedChar {
}
}

fn new_c(c: char, quotes: Quotes) -> Self {
fn new_c(c: char, quotes: Quotes, dirname: bool) -> Self {
use EscapeState::*;
let init_state = match c {
'\x07' => Backslash('a'),
Expand All @@ -142,10 +143,11 @@ impl EscapedChar {
Quotes::Double => Backslash('"'),
_ => Char('"'),
},
' ' => match quotes {
' ' if !dirname => match quotes {
Quotes::None => Backslash(' '),
_ => Char(' '),
},
':' if dirname => Backslash(':'),
_ if c.is_ascii_control() => Octal(EscapeOctal::from(c)),
_ => Char(c),
};
Expand Down Expand Up @@ -284,8 +286,29 @@ fn shell_with_escape(name: &str, quotes: Quotes) -> (String, bool) {
(escaped_str, must_quote)
}

/// Return a set of characters that implies quoting of the word in
/// shell-quoting mode.
fn shell_escaped_char_set(is_dirname: bool) -> &'static [char] {
const ESCAPED_CHARS: &[char] = &[
// the ':' colon character only induce quoting in the
// context of ls displaying a directory name before listing its content.
// (e.g. with the recursive flag -R)
':',
// Under this line are the control characters that should be
// quoted in shell mode in all cases.
'"', '`', '$', '\\', '^', '\n', '\t', '\r', '=',
];

let start_index = if is_dirname { 0 } else { 1 };

&ESCAPED_CHARS[start_index..]
}

/// Escape a name according to the given quoting style.
pub fn escape_name(name: &OsStr, style: &QuotingStyle) -> String {
///
/// This inner function provides an additional flag `dirname` which
/// is meant for ls' directory name display.
fn escape_name_inner(name: &OsStr, style: &QuotingStyle, dirname: bool) -> String {
match style {
QuotingStyle::Literal { show_control } => {
if *show_control {
Expand All @@ -301,7 +324,7 @@ pub fn escape_name(name: &OsStr, style: &QuotingStyle) -> String {
let escaped_str: String = name
.to_string_lossy()
.chars()
.flat_map(|c| EscapedChar::new_c(c, *quotes))
.flat_map(|c| EscapedChar::new_c(c, *quotes, dirname))
.collect();

match quotes {
Expand All @@ -316,7 +339,8 @@ pub fn escape_name(name: &OsStr, style: &QuotingStyle) -> String {
show_control,
} => {
let name = name.to_string_lossy();
let (quotes, must_quote) = if name.contains(&['"', '`', '$', '\\'][..]) {

let (quotes, must_quote) = if name.contains(shell_escaped_char_set(dirname)) {
(Quotes::Single, true)
} else if name.contains('\'') {
(Quotes::Double, true)
Expand All @@ -341,6 +365,18 @@ pub fn escape_name(name: &OsStr, style: &QuotingStyle) -> String {
}
}

/// Escape a filename with respect to the given style.
pub fn escape_name(name: &OsStr, style: &QuotingStyle) -> String {
RenjiSann marked this conversation as resolved.
Show resolved Hide resolved
escape_name_inner(name, style, false)
}

/// Escape a directory name with respect to the given style.
/// This is mainly meant to be used for ls' directory name printing and is not
/// likely to be used elsewhere.
pub fn escape_dir_name(dir_name: &OsStr, style: &QuotingStyle) -> String {
escape_name_inner(dir_name, style, true)
}

impl fmt::Display for QuotingStyle {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
match *self {
Expand Down Expand Up @@ -575,8 +611,8 @@ mod tests {
("one\ntwo", "literal-show"),
("one\\ntwo", "escape"),
("\"one\\ntwo\"", "c"),
("one?two", "shell"),
("one\ntwo", "shell-show"),
("'one?two'", "shell"),
("'one\ntwo'", "shell-show"),
("'one?two'", "shell-always"),
("'one\ntwo'", "shell-always-show"),
("'one'$'\\n''two'", "shell-escape"),
Expand Down Expand Up @@ -619,9 +655,9 @@ mod tests {
"\"\\000\\001\\002\\003\\004\\005\\006\\a\\b\\t\\n\\v\\f\\r\\016\\017\"",
"c",
),
("????????????????", "shell"),
("'????????????????'", "shell"),
(
"\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0A\x0B\x0C\x0D\x0E\x0F",
"'\x00\x01\x02\x03\x04\x05\x06\x07\x08\x09\x0A\x0B\x0C\x0D\x0E\x0F'",
"shell-show",
),
("'????????????????'", "shell-always"),
Expand Down
Loading
Loading