Skip to content
2 changes: 2 additions & 0 deletions src/uu/ls/locales/en-US.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,8 @@ ls-invalid-quoting-style = {$program}: Ignoring invalid value of environment var
ls-invalid-columns-width = ignoring invalid width in environment variable COLUMNS: {$width}
ls-invalid-ignore-pattern = Invalid pattern for ignore: {$pattern}
ls-invalid-hide-pattern = Invalid pattern for hide: {$pattern}
ls-warning-unrecognized-ls-colors-prefix = unrecognized prefix: {$prefix}
ls-warning-unparsable-ls-colors = unparsable value for LS_COLORS environment variable
ls-total = total {$size}

# Security context warnings
Expand Down
2 changes: 2 additions & 0 deletions src/uu/ls/locales/fr-FR.ftl
Original file line number Diff line number Diff line change
Expand Up @@ -123,4 +123,6 @@ ls-invalid-quoting-style = {$program} : Ignorer la valeur invalide de la variabl
ls-invalid-columns-width = ignorer la largeur invalide dans la variable d'environnement COLUMNS : {$width}
ls-invalid-ignore-pattern = Motif invalide pour ignore : {$pattern}
ls-invalid-hide-pattern = Motif invalide pour hide : {$pattern}
ls-warning-unrecognized-ls-colors-prefix = préfixe non reconnu : {$prefix}
ls-warning-unparsable-ls-colors = valeur illisible pour la variable d'environnement LS_COLORS
ls-total = total {$size}
291 changes: 249 additions & 42 deletions src/uu/ls/src/colors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,11 @@ const ANSI_RESET: &str = "\x1b[0m";
const ANSI_CLEAR_EOL: &str = "\x1b[K";
const EMPTY_STYLE: &str = "\x1b[m";

enum RawIndicatorStyle {
Empty,
Code(String),
}

/// We need this struct to be able to store the previous style.
/// This because we need to check the previous value in case we don't need
/// the reset
Expand Down Expand Up @@ -66,46 +71,24 @@ impl<'a> StyleManager<'a> {
// Fast-path: apply LS_COLORS raw SGR codes verbatim,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe move this block into a function to decrease complexity ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Extracted raw LS_COLORS application into helpers to reduce complexity.

// bypassing LsColors fallbacks so the entry from LS_COLORS
// is honored exactly as specified.
if let Some(indicator) = self.indicator_for_raw_code(path) {
let should_skip = indicator == Indicator::SymbolicLink
&& self.ln_color_from_target
&& path.path().exists();

if !should_skip {
if let Some(raw) = self.indicator_codes.get(&indicator).cloned() {
if raw.is_empty() {
return self.apply_empty_style(name, wrap);
}
style_code.push_str(self.reset(!self.initial_reset_is_done));
style_code.push_str(ANSI_CSI);
style_code.push_str(&raw);
style_code.push_str(ANSI_SGR_END);
applied_raw_code = true;
self.current_style = None;
force_suffix_reset = true;
}
match self.raw_indicator_style_for_path(path) {
Some(RawIndicatorStyle::Empty) => {
// An explicit empty entry (e.g. "or=") disables coloring and
// bypasses fallbacks, matching GNU ls behavior.
return self.apply_empty_style(name, wrap);
}
Some(RawIndicatorStyle::Code(raw)) => {
style_code.push_str(&self.build_raw_style_code(&raw));
applied_raw_code = true;
self.current_style = None;
force_suffix_reset = true;
}
None => {}
}
}

if !applied_raw_code {
if let Some(new_style) = new_style {
// we only need to apply a new style if it's not the same as the current
// style for example if normal is the current style and a file with
// normal style is to be printed we could skip printing new color
// codes
if !self.is_current_style(new_style) {
style_code.push_str(self.reset(!self.initial_reset_is_done));
style_code.push_str(&self.get_style_code(new_style));
}
}
// if new style is None and current style is Normal we should reset it
else if matches!(self.get_normal_style().copied(), Some(norm_style) if self.is_current_style(&norm_style))
{
style_code.push_str(self.reset(false));
// even though this is an unnecessary reset for gnu compatibility we allow it here
force_suffix_reset = true;
}
self.append_style_code_for_style(new_style, &mut style_code, &mut force_suffix_reset);
}

// we need this clear to eol code in some terminals, for instance if the
Expand All @@ -122,6 +105,58 @@ impl<'a> StyleManager<'a> {
ret
}

fn raw_indicator_style_for_path(&self, path: &PathData) -> Option<RawIndicatorStyle> {
let indicator = self.indicator_for_raw_code(path)?;
let should_skip = indicator == Indicator::SymbolicLink
&& self.ln_color_from_target
&& path.path().exists();

if should_skip {
return None;
}

let raw = self.indicator_codes.get(&indicator)?;
if raw.is_empty() {
Some(RawIndicatorStyle::Empty)
} else {
Some(RawIndicatorStyle::Code(raw.clone()))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please remove the clone()

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the clone by switching to an Indicator-based path and re-fetching the raw code.

}
}

fn build_raw_style_code(&mut self, raw: &str) -> String {
let mut style_code = String::new();
style_code.push_str(self.reset(!self.initial_reset_is_done));
style_code.push_str(ANSI_CSI);
style_code.push_str(raw);
style_code.push_str(ANSI_SGR_END);
style_code
}

fn append_style_code_for_style(
&mut self,
new_style: Option<&Style>,
style_code: &mut String,
force_suffix_reset: &mut bool,
) {
if let Some(new_style) = new_style {
// we only need to apply a new style if it's not the same as the current
// style for example if normal is the current style and a file with
// normal style is to be printed we could skip printing new color
// codes
if !self.is_current_style(new_style) {
style_code.push_str(self.reset(!self.initial_reset_is_done));
style_code.push_str(&self.get_style_code(new_style));
}
}
// if new style is None and current style is Normal we should reset it
else if matches!(self.get_normal_style().copied(), Some(norm_style) if self.is_current_style(&norm_style))
{
style_code.push_str(self.reset(false));
// even though this is an unnecessary reset for gnu compatibility we allow it here
*force_suffix_reset = true;
}
}

/// Resets the current style and returns the default ANSI reset code to
/// reset all text formatting attributes. If `force` is true, the reset is
/// done even if the reset has been applied before.
Expand Down Expand Up @@ -201,13 +236,7 @@ impl<'a> StyleManager<'a> {
return self.apply_empty_style(name, wrap);
}

let mut style_code = String::new();
style_code.push_str(self.reset(!self.initial_reset_is_done));
style_code.push_str(ANSI_CSI);
style_code.push_str(&raw);
style_code.push_str(ANSI_SGR_END);

let mut ret: OsString = style_code.into();
let mut ret: OsString = self.build_raw_style_code(&raw).into();
ret.push(name);
ret.push(self.reset(true));
if wrap {
Expand Down Expand Up @@ -474,10 +503,188 @@ pub(crate) fn color_name(
style_manager.apply_style_based_on_metadata(path, md_option.as_ref(), name, wrap)
}

#[derive(Debug)]
pub(crate) enum LsColorsParseError {
UnrecognizedPrefix(String),
InvalidSyntax,
}

pub(crate) fn validate_ls_colors_env() -> Result<(), LsColorsParseError> {
let Ok(ls_colors) = env::var("LS_COLORS") else {
return Ok(());
};

if ls_colors.is_empty() {
return Ok(());
}

validate_ls_colors(&ls_colors)
}

fn validate_ls_colors(ls_colors: &str) -> Result<(), LsColorsParseError> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think this function could benefit from some comments

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a short comment describing the GNU-like LS_COLORS validation intent.

let bytes = ls_colors.as_bytes();
let mut idx = 0;

while idx < bytes.len() {
match bytes[idx] {
b':' => {
idx += 1;
}
b'*' => {
idx += 1;
idx = parse_funky_string(bytes, idx, true)?;
if idx >= bytes.len() || bytes[idx] != b'=' {
return Err(LsColorsParseError::InvalidSyntax);
}
idx += 1;
idx = parse_funky_string(bytes, idx, false)?;
if idx < bytes.len() && bytes[idx] == b':' {
idx += 1;
}
}
_ => {
if idx + 1 >= bytes.len() {
return Err(LsColorsParseError::InvalidSyntax);
}
let label = [bytes[idx], bytes[idx + 1]];
idx += 2;
if idx >= bytes.len() || bytes[idx] != b'=' {
return Err(LsColorsParseError::InvalidSyntax);
}
if !is_valid_ls_colors_prefix(label) {
let prefix = String::from_utf8_lossy(&label).into_owned();
return Err(LsColorsParseError::UnrecognizedPrefix(prefix));
}
idx += 1;
idx = parse_funky_string(bytes, idx, false)?;
if idx < bytes.len() && bytes[idx] == b':' {
idx += 1;
}
}
}
}

Ok(())
}

fn parse_funky_string(
bytes: &[u8],
mut idx: usize,
equals_end: bool,
) -> Result<usize, LsColorsParseError> {
enum State {
Ground,
Backslash,
Octal(u8),
Hex(u8),
Caret,
}

let mut state = State::Ground;
loop {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a short comment explaining that the parser handles GNU-compatible escapes.

let byte = if idx < bytes.len() { bytes[idx] } else { 0 };
match state {
State::Ground => match byte {
b':' | 0 => return Ok(idx),
b'=' if equals_end => return Ok(idx),
b'\\' => {
state = State::Backslash;
idx += 1;
}
b'^' => {
state = State::Caret;
idx += 1;
}
_ => idx += 1,
},
State::Backslash => match byte {
0 => return Err(LsColorsParseError::InvalidSyntax),
b'0'..=b'7' => {
state = State::Octal(byte - b'0');
idx += 1;
}
b'x' | b'X' => {
state = State::Hex(0);
idx += 1;
}
b'a' | b'b' | b'e' | b'f' | b'n' | b'r' | b't' | b'v' | b'?' | b'_' => {
state = State::Ground;
idx += 1;
}
_ => {
state = State::Ground;
idx += 1;
}
},
State::Octal(num) => match byte {
b'0'..=b'7' => {
state = State::Octal(num.wrapping_mul(8).wrapping_add(byte - b'0'));
idx += 1;
}
_ => state = State::Ground,
},
State::Hex(num) => match byte {
b'0'..=b'9' => {
state = State::Hex(num.wrapping_mul(16).wrapping_add(byte - b'0'));
idx += 1;
}
b'a'..=b'f' => {
state = State::Hex(num.wrapping_mul(16).wrapping_add(byte - b'a' + 10));
idx += 1;
}
b'A'..=b'F' => {
state = State::Hex(num.wrapping_mul(16).wrapping_add(byte - b'A' + 10));
idx += 1;
}
_ => state = State::Ground,
},
State::Caret => match byte {
b'@'..=b'~' | b'?' => {
state = State::Ground;
idx += 1;
}
_ => return Err(LsColorsParseError::InvalidSyntax),
},
}
}
}

fn is_valid_ls_colors_prefix(label: [u8; 2]) -> bool {
matches!(
label,
[b'l', b'c']
| [b'r', b'c']
| [b'e', b'c']
| [b'r', b's']
| [b'n', b'o']
| [b'f', b'i']
| [b'd', b'i']
| [b'l', b'n']
| [b'p', b'i']
| [b's', b'o']
| [b'b', b'd']
| [b'c', b'd']
| [b'm', b'i']
| [b'o', b'r']
| [b'e', b'x']
| [b'd', b'o']
| [b's', b'u']
| [b's', b'g']
| [b's', b't']
| [b'o', b'w']
| [b't', b'w']
| [b'c', b'a']
| [b'm', b'h']
| [b'c', b'l']
)
}

fn parse_indicator_codes() -> (HashMap<Indicator, String>, bool) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe it could benefit from better error handling for malformed LS_COLOR

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added GNU-like LS_COLORS validation with warnings and color disabled on parse errors.

let mut indicator_codes = HashMap::new();
let mut ln_color_from_target = false;

// LS_COLORS validity is checked before enabling color output, so parse
// entries directly here for raw indicator overrides.
if let Ok(ls_colors) = env::var("LS_COLORS") {
for entry in ls_colors.split(':') {
if entry.is_empty() {
Expand Down
Loading
Loading