Skip to content
125 changes: 90 additions & 35 deletions src/uu/echo/src/echo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
// file that was distributed with this source code.

use clap::builder::ValueParser;
use clap::{Arg, ArgAction, ArgMatches, Command};
use clap::{Arg, ArgAction, Command};
use std::env;
use std::ffi::{OsStr, OsString};
use std::io::{self, StdoutLock, Write};
Expand All @@ -23,63 +23,118 @@ mod options {
pub const DISABLE_BACKSLASH_ESCAPE: &str = "disable_backslash_escape";
}

fn is_echo_flag(arg: &OsString) -> bool {
matches!(arg.to_str(), Some("-e" | "-E" | "-n"))
/// Holds the parsed flags for echo command:
/// -n (disable newline)
/// -e/-E (escape handling),
/// '-' (stop processing flags).
struct EchoFlags {
/// -n flag: if true, don't output trailing newline
/// Default: false
pub disable_newline: bool,

/// -e enables escape interpretation, -E disables it
/// Default: false (escape interpretation disabled)
pub escape: bool,

/// Single hyphen '-' argument (stops flag processing)
/// Default: false
pub is_single_hyphen: bool,
}

// A workaround because clap interprets the first '--' as a marker that a value
// follows. In order to use '--' as a value, we have to inject an additional '--'
fn handle_double_hyphens(args: impl uucore::Args) -> impl uucore::Args {
let mut result = Vec::new();
let mut is_first_argument = true;
let mut args_iter = args.into_iter();
/// Checks if an argument is a valid echo flag
/// Returns EchoFlags if valid, None otherwise
fn is_echo_flag(arg: &OsString) -> Option<EchoFlags> {
let bytes = arg.as_encoded_bytes();
if bytes.first() == Some(&b'-') {
let mut flags = EchoFlags {
disable_newline: false,
escape: false,
is_single_hyphen: false,
};
// Single hyphen case: "-" (stops flag processing, no other effect)
if arg.len() == 1 {
flags.is_single_hyphen = true;
return Some(flags);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if bytes.first() == Some(&b'-') {
let mut flags = EchoFlags {
disable_newline: false,
escape: false,
is_single_hyphen: false,
};
// Single hyphen case: "-" (stops flag processing, no other effect)
if arg.len() == 1 {
flags.is_single_hyphen = true;
return Some(flags);
}
if bytes.first() == Some(&b'-') && arg != "-" {
let mut flags = EchoFlags {
disable_newline: false,
escape: false,
};

Would this work? I think the is_single_hyphen case is identical to the None case

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also correct. Changed this part too. Thanks!


if let Some(first_val) = args_iter.next() {
// the first argument ('echo') gets pushed before we start with the checks for flags/'--'
result.push(first_val);
// We need to skip any possible Flag arguments until we find the first argument to echo that
// is not a flag. If the first argument is double hyphen we inject an additional '--'
// otherwise we switch is_first_argument boolean to skip the checks for any further arguments
for arg in args_iter {
if is_first_argument && !is_echo_flag(&arg) {
is_first_argument = false;
if arg == "--" {
result.push(OsString::from("--"));
}
// Process characters after the '-'
for c in &bytes[1..] {
match c {
b'e' => flags.escape = true,
b'E' => flags.escape = false,
b'n' => flags.disable_newline = true,
// if there is any char in an argument starting with '-' that doesn't match e/E/n
// present means that this argument is not a flag
_ => return None,
}
result.push(arg);
}

return Some(flags);
}

result.into_iter()
// argument doesn't start with '-' - no flag
None
}

fn collect_args(matches: &ArgMatches) -> Vec<OsString> {
matches
.get_many::<OsString>(options::STRING)
.map_or_else(Vec::new, |values| values.cloned().collect())
/// Processes command line arguments, separating flags from normal arguments
/// Returns:
/// - Vector of non-flag arguments
/// - trailing_newline: whether to print a trailing newline
/// - escape: whether to process escape sequences
fn filter_echo_flags(args: impl uucore::Args) -> (Vec<OsString>, bool, bool) {
let mut result = Vec::new();
let mut trailing_newline = true;
let mut escape = false;
let mut args_iter = args.into_iter();

// Process arguments until first non-flag is found
for arg in &mut args_iter {
if let Some(echo_flags) = is_echo_flag(&arg) {
// Single hyphen stops flag processing
if echo_flags.is_single_hyphen {
result.push(arg);
break;
}
if echo_flags.disable_newline {
trailing_newline = false;
}
escape = echo_flags.escape;
Copy link
Contributor

@blyxxyz blyxxyz Mar 30, 2025

Choose a reason for hiding this comment

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

This is too aggressive since it means -e -n disables escape. Maybe is_echo_flag can mutably borrow a single EchoFlags value that's reused and returned?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great observation. Indeed this was an issue. I fixed this and also added some test as there was no test in the test suite so far to check for this behaviour.

} else {
// First non-flag argument stops flag processing
result.push(arg);
break;
}
}
// Collect remaining arguments
for arg in args_iter {
result.push(arg);
}
(result, trailing_newline, escape)
}

#[uucore::main]
pub fn uumain(args: impl uucore::Args) -> UResult<()> {
// Check POSIX compatibility mode
let is_posixly_correct = env::var("POSIXLY_CORRECT").is_ok();

let args_iter = args.skip(1);
let (args, trailing_newline, escaped) = if is_posixly_correct {
let mut args_iter = args.skip(1).peekable();
let mut args_iter = args_iter.peekable();

if args_iter.peek() == Some(&OsString::from("-n")) {
let matches = uu_app().get_matches_from(handle_double_hyphens(args_iter));
let args = collect_args(&matches);
// if POSIXLY_CORRECT is set and the first argument is the "-n" flag
// we filter flags normally but 'escaped' is activated nonetheless
let (args, _, _) = filter_echo_flags(args_iter);
(args, false, true)
} else {
let args: Vec<_> = args_iter.collect();
// if POSIXLY_CORRECT is set and the first argument is not the "-n" flag
// we just collect all arguments as every argument is considered an argument
let args: Vec<OsString> = args_iter.collect();
(args, true, true)
}
} else {
let matches = uu_app().get_matches_from(handle_double_hyphens(args.into_iter()));
let trailing_newline = !matches.get_flag(options::NO_NEWLINE);
let escaped = matches.get_flag(options::ENABLE_BACKSLASH_ESCAPE);
let args = collect_args(&matches);
// if POSIXLY_CORRECT is not set we filter the flags normally
let (args, trailing_newline, escaped) = filter_echo_flags(args_iter);
(args, trailing_newline, escaped)
};

Expand Down
97 changes: 96 additions & 1 deletion tests/by-util/test_echo.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
//
// For the full copyright and license information, please view the LICENSE
// file that was distributed with this source code.
// spell-checker:ignore (words) araba merci mright
// spell-checker:ignore (words) araba merci efjkow

use uutests::new_ucmd;
use uutests::util::TestScenario;
Expand Down Expand Up @@ -276,6 +276,89 @@ fn test_double_hyphens_at_start() {
.stdout_only("-- a b --\n");
}

#[test]
fn test_double_hyphens_after_single_hyphen() {
new_ucmd!()
.arg("-")
.arg("--")
.succeeds()
.stdout_only("- --\n");

new_ucmd!()
.arg("-")
.arg("-n")
.arg("--")
.succeeds()
.stdout_only("- -n --\n");

new_ucmd!()
.arg("-n")
.arg("-")
.arg("--")
.succeeds()
.stdout_only("- --");
}

#[test]
fn test_flag_like_arguments_which_are_no_flags() {
new_ucmd!()
.arg("-efjkow")
.arg("--")
.succeeds()
.stdout_only("-efjkow --\n");

new_ucmd!()
.arg("--")
.arg("-efjkow")
.succeeds()
.stdout_only("-- -efjkow\n");

new_ucmd!()
.arg("-efjkow")
.arg("-n")
.arg("--")
.succeeds()
.stdout_only("-efjkow -n --\n");

new_ucmd!()
.arg("-n")
.arg("--")
.arg("-efjkow")
.succeeds()
.stdout_only("-- -efjkow");
}

#[test]
fn test_backslash_n_last_char_in_last_argument() {
new_ucmd!()
.arg("-n")
.arg("-e")
.arg("--")
.arg("foo\n")
.succeeds()
.stdout_only("-- foo\n");

new_ucmd!()
.arg("-e")
.arg("--")
.arg("foo\\n")
.succeeds()
.stdout_only("-- foo\n\n");

new_ucmd!()
.arg("-n")
.arg("--")
.arg("foo\n")
.succeeds()
.stdout_only("-- foo\n");

new_ucmd!()
.arg("--")
.arg("foo\n")
.succeeds()
.stdout_only("-- foo\n\n");
}

#[test]
fn test_double_hyphens_after_flags() {
new_ucmd!()
Expand All @@ -292,6 +375,18 @@ fn test_double_hyphens_after_flags() {
.succeeds()
.stdout_only("-- foo\n");

new_ucmd!()
.arg("-ne")
.arg("--")
.succeeds()
.stdout_only("--");

new_ucmd!()
.arg("-neE")
.arg("--")
.succeeds()
.stdout_only("--");

new_ucmd!()
.arg("-e")
.arg("--")
Expand Down
Loading