Merge branch 'main' into fix/stty-glibc242-panic

This commit is contained in:
Sylvestre Ledru 2025-11-26 10:12:11 +01:00 committed by GitHub
commit bf3656547f
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 266 additions and 17 deletions

View file

@ -12,7 +12,6 @@ RUN apt-get update \
gcc \
gdb \
gperf \
jq \
libacl1-dev \
libattr1-dev \
libcap-dev \

View file

@ -10,7 +10,7 @@
// spell-checker:ignore isig icanon iexten echoe crterase echok echonl noflsh xcase tostop echoprt prterase echoctl ctlecho echoke crtkill flusho extproc
// spell-checker:ignore lnext rprnt susp swtch vdiscard veof veol verase vintr vkill vlnext vquit vreprint vstart vstop vsusp vswtc vwerase werase
// spell-checker:ignore sigquit sigtstp
// spell-checker:ignore cbreak decctlq evenp litout oddp tcsadrain exta extb
// spell-checker:ignore cbreak decctlq evenp litout oddp tcsadrain exta extb NCCS
mod flags;
@ -31,7 +31,7 @@ use std::os::fd::{AsFd, BorrowedFd};
use std::os::unix::fs::OpenOptionsExt;
use std::os::unix::io::{AsRawFd, RawFd};
use std::panic;
use uucore::error::{UError, UResult, USimpleError};
use uucore::error::{UError, UResult, USimpleError, UUsageError};
use uucore::format_usage;
use uucore::translate;
@ -151,6 +151,7 @@ enum ArgOptions<'a> {
Mapping((S, u8)),
Special(SpecialSetting),
Print(PrintSetting),
SavedState(Vec<u32>),
}
impl<'a> From<AllFlags<'a>> for ArgOptions<'a> {
@ -353,8 +354,12 @@ fn stty(opts: &Options) -> UResult<()> {
valid_args.push(ArgOptions::Print(PrintSetting::Size));
}
_ => {
// Try to parse saved format (hex string like "6d02:5:4bf:8a3b:...")
if let Some(state) = parse_saved_state(arg) {
valid_args.push(ArgOptions::SavedState(state));
}
// control char
if let Some(char_index) = cc_to_index(arg) {
else if let Some(char_index) = cc_to_index(arg) {
if let Some(mapping) = args_iter.next() {
let cc_mapping = string_to_control_char(mapping).map_err(|e| {
let message = match e {
@ -371,7 +376,7 @@ fn stty(opts: &Options) -> UResult<()> {
)
}
};
USimpleError::new(1, message)
UUsageError::new(1, message)
})?;
valid_args.push(ArgOptions::Mapping((char_index, cc_mapping)));
} else {
@ -419,6 +424,9 @@ fn stty(opts: &Options) -> UResult<()> {
ArgOptions::Print(setting) => {
print_special_setting(setting, opts.file.as_raw_fd())?;
}
ArgOptions::SavedState(state) => {
apply_saved_state(&mut termios, state)?;
}
}
}
tcsetattr(opts.file.as_fd(), set_arg, &termios)?;
@ -430,8 +438,9 @@ fn stty(opts: &Options) -> UResult<()> {
Ok(())
}
// The GNU implementation adds the --help message when the args are incorrectly formatted
fn missing_arg<T>(arg: &str) -> Result<T, Box<dyn UError>> {
Err::<T, Box<dyn UError>>(USimpleError::new(
Err(UUsageError::new(
1,
translate!(
"stty-error-missing-argument",
@ -441,7 +450,7 @@ fn missing_arg<T>(arg: &str) -> Result<T, Box<dyn UError>> {
}
fn invalid_arg<T>(arg: &str) -> Result<T, Box<dyn UError>> {
Err::<T, Box<dyn UError>>(USimpleError::new(
Err(UUsageError::new(
1,
translate!(
"stty-error-invalid-argument",
@ -451,7 +460,7 @@ fn invalid_arg<T>(arg: &str) -> Result<T, Box<dyn UError>> {
}
fn invalid_integer_arg<T>(arg: &str) -> Result<T, Box<dyn UError>> {
Err::<T, Box<dyn UError>>(USimpleError::new(
Err(UUsageError::new(
1,
translate!(
"stty-error-invalid-integer-argument",
@ -479,6 +488,43 @@ fn parse_rows_cols(arg: &str) -> Option<u16> {
None
}
/// Parse a saved terminal state string in stty format.
///
/// The format is colon-separated hexadecimal values:
/// `input_flags:output_flags:control_flags:local_flags:cc0:cc1:cc2:...`
///
/// - Must have exactly 4 + NCCS parts (4 flags + platform-specific control characters)
/// - All parts must be non-empty valid hex values
/// - Control characters must fit in u8 (0-255)
/// - Returns `None` if format is invalid
fn parse_saved_state(arg: &str) -> Option<Vec<u32>> {
let parts: Vec<&str> = arg.split(':').collect();
let expected_parts = 4 + nix::libc::NCCS;
// GNU requires exactly the right number of parts for this platform
if parts.len() != expected_parts {
return None;
}
// Validate all parts are non-empty valid hex
let mut values = Vec::with_capacity(expected_parts);
for (i, part) in parts.iter().enumerate() {
if part.is_empty() {
return None; // GNU rejects empty hex values
}
let val = u32::from_str_radix(part, 16).ok()?;
// Control characters (indices 4+) must fit in u8
if i >= 4 && val > 255 {
return None;
}
values.push(val);
}
Some(values)
}
fn check_flag_group<T>(flag: &Flag<T>, remove: bool) -> bool {
remove && flag.group.is_some()
}
@ -881,6 +927,39 @@ fn apply_char_mapping(termios: &mut Termios, mapping: &(S, u8)) {
termios.control_chars[mapping.0 as usize] = mapping.1;
}
/// Apply a saved terminal state to the current termios.
///
/// The state array contains:
/// - `state[0]`: input flags
/// - `state[1]`: output flags
/// - `state[2]`: control flags
/// - `state[3]`: local flags
/// - `state[4..]`: control characters (optional)
///
/// If state has fewer than 4 elements, no changes are applied. This is a defensive
/// check that should never trigger since `parse_saved_state` rejects such states.
fn apply_saved_state(termios: &mut Termios, state: &[u32]) -> nix::Result<()> {
// Require at least 4 elements for the flags (defensive check)
if state.len() < 4 {
return Ok(()); // No-op for invalid state (already validated by parser)
}
// Apply the four flag groups, done (as _) for MacOS size compatibility
termios.input_flags = InputFlags::from_bits_truncate(state[0] as _);
termios.output_flags = OutputFlags::from_bits_truncate(state[1] as _);
termios.control_flags = ControlFlags::from_bits_truncate(state[2] as _);
termios.local_flags = LocalFlags::from_bits_truncate(state[3] as _);
// Apply control characters if present (stored as u32 but used as u8)
for (i, &cc_val) in state.iter().skip(4).enumerate() {
if i < termios.control_chars.len() {
termios.control_chars[i] = cc_val as u8;
}
}
Ok(())
}
fn apply_special_setting(
_termios: &mut Termios,
setting: &SpecialSetting,

View file

@ -2,10 +2,18 @@
//
// For the full copyright and license information, please view the LICENSE
// file that was distributed with this source code.
// spell-checker:ignore parenb parmrk ixany iuclc onlcr icanon noflsh econl igpar ispeed ospeed
// spell-checker:ignore parenb parmrk ixany iuclc onlcr icanon noflsh econl igpar ispeed ospeed NCCS nonhex gstty
use uutests::new_ucmd;
use uutests::util::pty_path;
use uutests::util::{expected_result, pty_path};
use uutests::{at_and_ts, new_ucmd, unwrap_or_return};
/// Normalize stderr by replacing the full binary path with just the utility name
/// This allows comparison between GNU (which shows "stty" or "gstty") and ours (which shows full path)
fn normalize_stderr(stderr: &str) -> String {
// Replace patterns like "Try 'gstty --help'" or "Try '/path/to/stty --help'" with "Try 'stty --help'"
let re = regex::Regex::new(r"Try '[^']*(?:g)?stty --help'").unwrap();
re.replace_all(stderr, "Try 'stty --help'").to_string()
}
#[test]
fn test_invalid_arg() {
@ -349,3 +357,172 @@ fn non_negatable_combo() {
.fails()
.stderr_contains("invalid argument '-ek'");
}
// Tests for saved state parsing and restoration
#[test]
#[cfg(unix)]
fn test_save_and_restore() {
let (path, _controller, _replica) = pty_path();
let saved = new_ucmd!()
.args(&["--save", "--file", &path])
.succeeds()
.stdout_move_str();
let saved = saved.trim();
assert!(saved.contains(':'));
new_ucmd!().args(&["--file", &path, saved]).succeeds();
}
#[test]
#[cfg(unix)]
fn test_save_with_g_flag() {
let (path, _controller, _replica) = pty_path();
let saved = new_ucmd!()
.args(&["-g", "--file", &path])
.succeeds()
.stdout_move_str();
let saved = saved.trim();
assert!(saved.contains(':'));
new_ucmd!().args(&["--file", &path, saved]).succeeds();
}
#[test]
#[cfg(unix)]
fn test_save_restore_after_change() {
let (path, _controller, _replica) = pty_path();
let saved = new_ucmd!()
.args(&["--save", "--file", &path])
.succeeds()
.stdout_move_str();
let saved = saved.trim();
new_ucmd!()
.args(&["--file", &path, "intr", "^A"])
.succeeds();
new_ucmd!().args(&["--file", &path, saved]).succeeds();
new_ucmd!()
.args(&["--file", &path])
.succeeds()
.stdout_str_check(|s| !s.contains("intr = ^A"));
}
// These tests both validate what we expect each input to return and their error codes
// and also use the GNU coreutils results to validate our results match expectations
#[test]
#[cfg(unix)]
fn test_saved_state_valid_formats() {
let (path, _controller, _replica) = pty_path();
let (_at, ts) = at_and_ts!();
// Generate valid saved state from the actual terminal
let saved = unwrap_or_return!(expected_result(&ts, &["-g", "--file", &path])).stdout_move_str();
let saved = saved.trim();
let result = ts.ucmd().args(&["--file", &path, saved]).run();
result.success().no_stderr();
let exp_result = unwrap_or_return!(expected_result(&ts, &["--file", &path, saved]));
let normalized_stderr = normalize_stderr(result.stderr_str());
result
.stdout_is(exp_result.stdout_str())
.code_is(exp_result.code());
assert_eq!(normalized_stderr, exp_result.stderr_str());
}
#[test]
#[cfg(unix)]
fn test_saved_state_invalid_formats() {
let (path, _controller, _replica) = pty_path();
let (_at, ts) = at_and_ts!();
let num_cc = nix::libc::NCCS;
// Build test strings with platform-specific counts
let cc_zeros = vec!["0"; num_cc].join(":");
let cc_with_invalid = if num_cc > 0 {
let mut parts = vec!["1c"; num_cc];
parts[0] = "100"; // First control char > 255
parts.join(":")
} else {
String::new()
};
let cc_with_space = if num_cc > 0 {
let mut parts = vec!["1c"; num_cc];
parts[0] = "1c "; // Space in hex
parts.join(":")
} else {
String::new()
};
let cc_with_nonhex = if num_cc > 0 {
let mut parts = vec!["1c"; num_cc];
parts[0] = "xyz"; // Non-hex
parts.join(":")
} else {
String::new()
};
let cc_with_empty = if num_cc > 0 {
let mut parts = vec!["1c"; num_cc];
parts[0] = ""; // Empty
parts.join(":")
} else {
String::new()
};
// Cannot test single value since it would be interpreted as baud rate
let invalid_states = vec![
"500:5:4bf".to_string(), // fewer than expected parts
"500:5:4bf:8a3b".to_string(), // only 4 parts
format!("500:5:{}:8a3b:{}", cc_zeros, "extra"), // too many parts
format!("500::4bf:8a3b:{}", cc_zeros), // empty hex value in flags
format!("500:5:4bf:8a3b:{}", cc_with_empty), // empty hex value in cc
format!("500:5:4bf:8a3b:{}", cc_with_nonhex), // non-hex characters
format!("500:5:4bf:8a3b:{}", cc_with_space), // space in hex value
format!("500:5:4bf:8a3b:{}", cc_with_invalid), // control char > 255
];
for state in &invalid_states {
let result = ts.ucmd().args(&["--file", &path, state]).run();
result.failure().stderr_contains("invalid argument");
let exp_result = unwrap_or_return!(expected_result(&ts, &["--file", &path, state]));
let normalized_stderr = normalize_stderr(result.stderr_str());
let exp_normalized_stderr = normalize_stderr(exp_result.stderr_str());
result
.stdout_is(exp_result.stdout_str())
.code_is(exp_result.code());
assert_eq!(normalized_stderr, exp_normalized_stderr);
}
}
#[test]
#[cfg(unix)]
#[ignore = "Fails because the implementation of print state is not correctly printing flags on certain platforms"]
fn test_saved_state_with_control_chars() {
let (path, _controller, _replica) = pty_path();
let (_at, ts) = at_and_ts!();
// Build a valid saved state with platform-specific number of control characters
let num_cc = nix::libc::NCCS;
let cc_values: Vec<String> = (1..=num_cc).map(|_| format!("{:x}", 0)).collect();
let saved_state = format!("500:5:4bf:8a3b:{}", cc_values.join(":"));
ts.ucmd().args(&["--file", &path, &saved_state]).succeeds();
let result = ts.ucmd().args(&["-g", "--file", &path]).run();
result.success().stdout_contains(":");
let exp_result = unwrap_or_return!(expected_result(&ts, &["-g", "--file", &path]));
result
.stdout_is(exp_result.stdout_str())
.stderr_is(exp_result.stderr_str())
.code_is(exp_result.code());
}

View file

@ -135,7 +135,6 @@ else
"$([ "${SELINUX_ENABLED}" = 1 ] && echo --with-selinux || echo --without-selinux)"
#Add timeout to to protect against hangs
"${SED}" -i 's|^"\$@|'"${SYSTEM_TIMEOUT}"' 600 "\$@|' build-aux/test-driver
"${SED}" -i 's| tr | /usr/bin/tr |' tests/init.sh
# Use a better diff
"${SED}" -i 's|diff -c|diff -u|g' tests/Coreutils.pm
"${MAKE}" -j "$("${NPROC}")"
@ -342,11 +341,6 @@ test \$n_stat1 -ge \$n_stat2 \\' tests/ls/stat-free-color.sh
# Slightly different error message
"${SED}" -i 's/not supported/unexpected argument/' tests/mv/mv-exchange.sh
# Most tests check that `/usr/bin/tr` is working correctly before running.
# However in NixOS/Nix-based distros, the tr util is located somewhere in
# /nix/store/xxxxxxxxxxxx...xxxx/bin/tr
# We just replace the references to `/usr/bin/tr`
"${SED}" -i 's/\/usr\/bin\/tr/$(command -v tr)/' tests/init.sh
# upstream doesn't having the program name in the error message
# but we do. We should keep it that way.