From 61478bcf930d71188f9750b6c5c0b91aac9ee254 Mon Sep 17 00:00:00 2001 From: Christopher Dryden Date: Sun, 28 Dec 2025 02:55:55 +0000 Subject: [PATCH 1/2] stty: use stdin for TTY operations instead of /dev/tty --- src/uu/stty/src/stty.rs | 71 ++++++++++++++++++-------------------- tests/by-util/test_stty.rs | 10 ++++++ 2 files changed, 44 insertions(+), 37 deletions(-) diff --git a/src/uu/stty/src/stty.rs b/src/uu/stty/src/stty.rs index d60d4d985ba..9153c1528fe 100644 --- a/src/uu/stty/src/stty.rs +++ b/src/uu/stty/src/stty.rs @@ -26,12 +26,12 @@ use nix::sys::termios::{ use nix::{ioctl_read_bad, ioctl_write_ptr_bad}; use std::cmp::Ordering; use std::fs::File; -use std::io::{self, Stdout, stdout}; +use std::io::{self, Stdin, stdin, stdout}; use std::num::IntErrorKind; use std::os::fd::{AsFd, BorrowedFd}; use std::os::unix::fs::OpenOptionsExt; use std::os::unix::io::{AsRawFd, RawFd}; -use uucore::error::{UError, UResult, USimpleError, UUsageError}; +use uucore::error::{FromIo, UError, UResult, USimpleError, UUsageError}; use uucore::format_usage; use uucore::parser::num_parser::ExtendedParser; use uucore::translate; @@ -124,12 +124,13 @@ struct Options<'a> { all: bool, save: bool, file: Device, + device_name: String, settings: Option>, } enum Device { File(File), - Stdout(Stdout), + Stdin(Stdin), } #[derive(Debug)] @@ -166,7 +167,7 @@ impl AsFd for Device { fn as_fd(&self) -> BorrowedFd<'_> { match self { Self::File(f) => f.as_fd(), - Self::Stdout(stdout) => stdout.as_fd(), + Self::Stdin(stdin) => stdin.as_fd(), } } } @@ -175,45 +176,42 @@ impl AsRawFd for Device { fn as_raw_fd(&self) -> RawFd { match self { Self::File(f) => f.as_raw_fd(), - Self::Stdout(stdout) => stdout.as_raw_fd(), + Self::Stdin(stdin) => stdin.as_raw_fd(), } } } impl<'a> Options<'a> { fn from(matches: &'a ArgMatches) -> io::Result { - Ok(Self { - all: matches.get_flag(options::ALL), - save: matches.get_flag(options::SAVE), - file: match matches.get_one::(options::FILE) { - // Two notes here: - // 1. O_NONBLOCK is needed because according to GNU docs, a - // POSIX tty can block waiting for carrier-detect if the - // "clocal" flag is not set. If your TTY is not connected - // to a modem, it is probably not relevant though. - // 2. We never close the FD that we open here, but the OS - // will clean up the FD for us on exit, so it doesn't - // matter. The alternative would be to have an enum of - // BorrowedFd/OwnedFd to handle both cases. - Some(f) => Device::File( + let (file, device_name) = match matches.get_one::(options::FILE) { + // Two notes here: + // 1. O_NONBLOCK is needed because according to GNU docs, a + // POSIX tty can block waiting for carrier-detect if the + // "clocal" flag is not set. If your TTY is not connected + // to a modem, it is probably not relevant though. + // 2. We never close the FD that we open here, but the OS + // will clean up the FD for us on exit, so it doesn't + // matter. The alternative would be to have an enum of + // BorrowedFd/OwnedFd to handle both cases. + Some(f) => ( + Device::File( std::fs::OpenOptions::new() .read(true) .custom_flags(O_NONBLOCK) .open(f)?, ), - // default to /dev/tty, if that does not exist then default to stdout - None => { - if let Ok(f) = std::fs::OpenOptions::new() - .read(true) - .custom_flags(O_NONBLOCK) - .open("/dev/tty") - { - Device::File(f) - } else { - Device::Stdout(stdout()) - } - } - }, + f.clone(), + ), + // Per POSIX, stdin is used for TTY operations when no device is specified. + // This matches GNU coreutils behavior: if stdin is not a TTY, + // tcgetattr will fail with "Inappropriate ioctl for device". + None => (Device::Stdin(stdin()), "standard input".to_string()), + }; + Ok(Self { + all: matches.get_flag(options::ALL), + save: matches.get_flag(options::SAVE), + file, + device_name, settings: matches .get_many::(options::SETTINGS) .map(|v| v.map(|s| s.as_ref()).collect()), @@ -412,8 +410,8 @@ fn stty(opts: &Options) -> UResult<()> { } } - // TODO: Figure out the right error message for when tcgetattr fails - let mut termios = tcgetattr(opts.file.as_fd())?; + let mut termios = + tcgetattr(opts.file.as_fd()).map_err_context(|| opts.device_name.clone())?; // iterate over valid_args, match on the arg type, do the matching apply function for arg in &valid_args { @@ -433,8 +431,7 @@ fn stty(opts: &Options) -> UResult<()> { } tcsetattr(opts.file.as_fd(), set_arg, &termios)?; } else { - // TODO: Figure out the right error message for when tcgetattr fails - let termios = tcgetattr(opts.file.as_fd())?; + let termios = tcgetattr(opts.file.as_fd()).map_err_context(|| opts.device_name.clone())?; print_settings(&termios, opts)?; } Ok(()) @@ -997,7 +994,7 @@ fn apply_char_mapping(termios: &mut Termios, mapping: &(S, u8)) { /// /// The state array contains: /// - `state[0]`: input flags -/// - `state[1]`: output flags +/// - `state[1]`: output flags /// - `state[2]`: control flags /// - `state[3]`: local flags /// - `state[4..]`: control characters (optional) diff --git a/tests/by-util/test_stty.rs b/tests/by-util/test_stty.rs index 136ea2768af..1abe0913d61 100644 --- a/tests/by-util/test_stty.rs +++ b/tests/by-util/test_stty.rs @@ -1557,6 +1557,16 @@ fn test_saved_state_with_control_chars() { .code_is(exp_result.code()); } +// Per POSIX, stty uses stdin for TTY operations. When stdin is a pipe, it should fail. +#[test] +#[cfg(unix)] +fn test_stdin_not_tty_fails() { + new_ucmd!() + .pipe_in("") + .fails() + .stderr_contains("standard input: Inappropriate ioctl for device"); +} + #[test] #[cfg(unix)] fn test_columns_env_wrapping() { From cef23f01c8bd7f38bdaaa82df57812e37d68f706 Mon Sep 17 00:00:00 2001 From: Christopher Dryden Date: Tue, 30 Dec 2025 19:44:27 +0000 Subject: [PATCH 2/2] Add tests for stty stdin behavior and fix platform-specific error messages --- .../workspace.wordlist.txt | 1 + tests/by-util/test_stty.rs | 62 ++++++++++++++++++- 2 files changed, 62 insertions(+), 1 deletion(-) diff --git a/.vscode/cspell.dictionaries/workspace.wordlist.txt b/.vscode/cspell.dictionaries/workspace.wordlist.txt index 8a8a1474a92..f9c8d686bff 100644 --- a/.vscode/cspell.dictionaries/workspace.wordlist.txt +++ b/.vscode/cspell.dictionaries/workspace.wordlist.txt @@ -379,6 +379,7 @@ istrip litout opost parodd +ENOTTY # translation tests CLICOLOR diff --git a/tests/by-util/test_stty.rs b/tests/by-util/test_stty.rs index 1abe0913d61..ae64eb6aeb5 100644 --- a/tests/by-util/test_stty.rs +++ b/tests/by-util/test_stty.rs @@ -1561,10 +1561,70 @@ fn test_saved_state_with_control_chars() { #[test] #[cfg(unix)] fn test_stdin_not_tty_fails() { + // ENOTTY error message varies by platform/libc: + // - glibc: "Inappropriate ioctl for device" + // - musl: "Not a tty" + // - Android: "Not a typewriter" + #[cfg(target_os = "android")] + let expected_error = "standard input: Not a typewriter"; + #[cfg(all(not(target_os = "android"), target_env = "musl"))] + let expected_error = "standard input: Not a tty"; + #[cfg(all(not(target_os = "android"), not(target_env = "musl")))] + let expected_error = "standard input: Inappropriate ioctl for device"; + new_ucmd!() .pipe_in("") .fails() - .stderr_contains("standard input: Inappropriate ioctl for device"); + .stderr_contains(expected_error); +} + +// Test that stty uses stdin for TTY operations per POSIX. +// Verifies: output redirection (#8012), save/restore pattern (#8608), stdin redirection (#8848) +#[test] +#[cfg(unix)] +fn test_stty_uses_stdin() { + use std::fs::File; + use std::process::Stdio; + + let (path, _controller, _replica) = pty_path(); + + // Output redirection: stty > file (stdin is still TTY) + let stdin = File::open(&path).unwrap(); + new_ucmd!() + .set_stdin(stdin) + .set_stdout(Stdio::piped()) + .succeeds() + .stdout_contains("speed"); + + // Save/restore: stty $(stty -g) pattern + let stdin = File::open(&path).unwrap(); + let saved = new_ucmd!() + .arg("-g") + .set_stdin(stdin) + .set_stdout(Stdio::piped()) + .succeeds() + .stdout_str() + .trim() + .to_string(); + assert!(saved.contains(':'), "Expected colon-separated saved state"); + + let stdin = File::open(&path).unwrap(); + new_ucmd!().arg(&saved).set_stdin(stdin).succeeds(); + + // Stdin redirection: stty rows 30 cols 100 < /dev/pts/N + let stdin = File::open(&path).unwrap(); + new_ucmd!() + .args(&["rows", "30", "cols", "100"]) + .set_stdin(stdin) + .succeeds(); + + let stdin = File::open(&path).unwrap(); + new_ucmd!() + .arg("--all") + .set_stdin(stdin) + .succeeds() + .stdout_contains("rows 30") + .stdout_contains("columns 100"); } #[test]