From 9bd9c66b921a12eb1f63c0d93fc742db5d3e7102 Mon Sep 17 00:00:00 2001 From: pagedmov Date: Sat, 14 Mar 2026 20:04:20 -0400 Subject: [PATCH] implemented '<>' redirects, and the 'seek' builtin 'seek' is a wrapper around the lseek() syscall added noclobber to core shopts and implemented '>|' redirection syntax properly implemented fd close syntax fixed saved fds being leaked into exec'd programs --- src/builtin/mod.rs | 5 +- src/builtin/seek.rs | 253 +++++++++++++++++++++++++++++++++++++++++++ src/libsh/error.rs | 5 +- src/libsh/guards.rs | 6 +- src/libsh/sys.rs | 10 +- src/parse/execute.rs | 29 +---- src/parse/lex.rs | 188 ++++++++++++++++++++------------ src/parse/mod.rs | 157 +++++++++++++++++---------- src/prelude.rs | 2 +- src/procio.rs | 64 +++++++++-- src/shopt.rs | 19 ++++ 11 files changed, 576 insertions(+), 162 deletions(-) create mode 100644 src/builtin/seek.rs diff --git a/src/builtin/mod.rs b/src/builtin/mod.rs index 9bc35b0..9dcc65d 100644 --- a/src/builtin/mod.rs +++ b/src/builtin/mod.rs @@ -24,13 +24,14 @@ pub mod source; pub mod test; // [[ ]] thing pub mod trap; pub mod varcmds; +pub mod seek; -pub const BUILTINS: [&str; 49] = [ +pub const BUILTINS: [&str; 50] = [ "echo", "cd", "read", "export", "local", "pwd", "source", ".", "shift", "jobs", "fg", "bg", "disown", "alias", "unalias", "return", "break", "continue", "exit", "shopt", "builtin", "command", "trap", "pushd", "popd", "dirs", "exec", "eval", "true", "false", ":", "readonly", "unset", "complete", "compgen", "map", "pop", "fpop", "push", "fpush", "rotate", "wait", "type", - "getopts", "keymap", "read_key", "autocmd", "ulimit", "umask", + "getopts", "keymap", "read_key", "autocmd", "ulimit", "umask", "seek" ]; pub fn true_builtin() -> ShResult<()> { diff --git a/src/builtin/seek.rs b/src/builtin/seek.rs new file mode 100644 index 0000000..8e69258 --- /dev/null +++ b/src/builtin/seek.rs @@ -0,0 +1,253 @@ +use nix::{libc::STDOUT_FILENO, unistd::{Whence, lseek, write}}; + +use crate::{getopt::{Opt, OptSpec, get_opts_from_tokens}, libsh::error::{ShErr, ShErrKind, ShResult}, parse::{NdRule, Node, execute::prepare_argv}, procio::borrow_fd, state}; + +pub const LSEEK_OPTS: [OptSpec;2] = [ + OptSpec { + opt: Opt::Short('c'), + takes_arg: false + }, + OptSpec { + opt: Opt::Short('e'), + takes_arg: false + }, +]; + +pub struct LseekOpts { + cursor_rel: bool, + end_rel: bool +} + +pub fn seek(node: Node) -> ShResult<()> { + let NdRule::Command { + assignments: _, + argv, + } = node.class else { unreachable!() }; + + let (argv, opts) = get_opts_from_tokens(argv, &LSEEK_OPTS)?; + let lseek_opts = get_lseek_opts(opts)?; + let mut argv = prepare_argv(argv)?.into_iter(); + argv.next(); // drop 'seek' + + let Some(fd) = argv.next() else { + return Err(ShErr::simple( + ShErrKind::ExecFail, + "lseek: Missing required argument 'fd'", + )); + }; + let Ok(fd) = fd.0.parse::() else { + return Err(ShErr::at( + ShErrKind::ExecFail, + fd.1, + "Invalid file descriptor", + ).with_note("file descriptors are integers")); + }; + + let Some(offset) = argv.next() else { + return Err(ShErr::simple( + ShErrKind::ExecFail, + "lseek: Missing required argument 'offset'", + )); + }; + let Ok(offset) = offset.0.parse::() else { + return Err(ShErr::at( + ShErrKind::ExecFail, + offset.1, + "Invalid offset", + ).with_note("offset can be a positive or negative integer")); + }; + + let whence = if lseek_opts.cursor_rel { + Whence::SeekCur + } else if lseek_opts.end_rel { + Whence::SeekEnd + } else { + Whence::SeekSet + }; + + match lseek(fd as i32, offset, whence) { + Ok(new_offset) => { + let stdout = borrow_fd(STDOUT_FILENO); + let buf = new_offset.to_string() + "\n"; + write(stdout, buf.as_bytes())?; + } + Err(e) => { + state::set_status(1); + return Err(e.into()) + } + } + + state::set_status(0); + Ok(()) +} + +pub fn get_lseek_opts(opts: Vec) -> ShResult { + let mut lseek_opts = LseekOpts { + cursor_rel: false, + end_rel: false, + }; + + for opt in opts { + match opt { + Opt::Short('c') => lseek_opts.cursor_rel = true, + Opt::Short('e') => lseek_opts.end_rel = true, + _ => { + return Err(ShErr::simple( + ShErrKind::ExecFail, + format!("lseek: Unexpected flag '{opt}'"), + )); + } + } + } + + Ok(lseek_opts) +} + +#[cfg(test)] +mod tests { + use crate::testutil::{TestGuard, test_input}; + use pretty_assertions::assert_eq; + + #[test] + fn seek_set_beginning() { + let dir = tempfile::TempDir::new().unwrap(); + let path = dir.path().join("seek.txt"); + std::fs::write(&path, "hello world\n").unwrap(); + let g = TestGuard::new(); + + test_input(format!("exec 9<> {}", path.display())).unwrap(); + test_input("seek 9 0").unwrap(); + + let out = g.read_output(); + assert_eq!(out, "0\n"); + } + + #[test] + fn seek_set_offset() { + let dir = tempfile::TempDir::new().unwrap(); + let path = dir.path().join("seek.txt"); + std::fs::write(&path, "hello world\n").unwrap(); + let g = TestGuard::new(); + + test_input(format!("exec 9<> {}", path.display())).unwrap(); + test_input("seek 9 6").unwrap(); + + let out = g.read_output(); + assert_eq!(out, "6\n"); + } + + #[test] + fn seek_then_read() { + let dir = tempfile::TempDir::new().unwrap(); + let path = dir.path().join("seek.txt"); + std::fs::write(&path, "hello world\n").unwrap(); + let g = TestGuard::new(); + + test_input(format!("exec 9<> {}", path.display())).unwrap(); + test_input("seek 9 6").unwrap(); + // Clear the seek output + g.read_output(); + + test_input("read line <&9").unwrap(); + let val = crate::state::read_vars(|v| v.get_var("line")); + assert_eq!(val, "world"); + } + + #[test] + fn seek_cur_relative() { + let dir = tempfile::TempDir::new().unwrap(); + let path = dir.path().join("seek.txt"); + std::fs::write(&path, "abcdefghij\n").unwrap(); + let g = TestGuard::new(); + + test_input(format!("exec 9<> {}", path.display())).unwrap(); + test_input("seek 9 3").unwrap(); + test_input("seek -c 9 4").unwrap(); + + let out = g.read_output(); + assert_eq!(out, "3\n7\n"); + } + + #[test] + fn seek_end() { + let dir = tempfile::TempDir::new().unwrap(); + let path = dir.path().join("seek.txt"); + std::fs::write(&path, "hello\n").unwrap(); // 6 bytes + let g = TestGuard::new(); + + test_input(format!("exec 9<> {}", path.display())).unwrap(); + test_input("seek -e 9 0").unwrap(); + + let out = g.read_output(); + assert_eq!(out, "6\n"); + } + + #[test] + fn seek_end_negative() { + let dir = tempfile::TempDir::new().unwrap(); + let path = dir.path().join("seek.txt"); + std::fs::write(&path, "hello\n").unwrap(); // 6 bytes + let g = TestGuard::new(); + + test_input(format!("exec 9<> {}", path.display())).unwrap(); + test_input("seek -e 9 -2").unwrap(); + + let out = g.read_output(); + assert_eq!(out, "4\n"); + } + + #[test] + fn seek_write_overwrite() { + let dir = tempfile::TempDir::new().unwrap(); + let path = dir.path().join("seek.txt"); + std::fs::write(&path, "hello world\n").unwrap(); + let _g = TestGuard::new(); + + test_input(format!("exec 9<> {}", path.display())).unwrap(); + test_input("seek 9 6").unwrap(); + test_input("echo -n 'WORLD' >&9").unwrap(); + + let contents = std::fs::read_to_string(&path).unwrap(); + assert_eq!(contents, "hello WORLD\n"); + } + + #[test] + fn seek_rewind_full_read() { + let dir = tempfile::TempDir::new().unwrap(); + let path = dir.path().join("seek.txt"); + std::fs::write(&path, "abc\n").unwrap(); + let g = TestGuard::new(); + + test_input(format!("exec 9<> {}", path.display())).unwrap(); + // Read moves cursor to EOF + test_input("read line <&9").unwrap(); + // Rewind + test_input("seek 9 0").unwrap(); + // Clear output from seek + g.read_output(); + // Read again from beginning + test_input("read line <&9").unwrap(); + + let val = crate::state::read_vars(|v| v.get_var("line")); + assert_eq!(val, "abc"); + } + + #[test] + fn seek_bad_fd() { + let _g = TestGuard::new(); + + let result = test_input("seek 99 0"); + assert!(result.is_err()); + } + + #[test] + fn seek_missing_args() { + let _g = TestGuard::new(); + + let result = test_input("seek"); + assert!(result.is_err()); + + let result = test_input("seek 9"); + assert!(result.is_err()); + } +} diff --git a/src/libsh/error.rs b/src/libsh/error.rs index fa96fd7..910be14 100644 --- a/src/libsh/error.rs +++ b/src/libsh/error.rs @@ -201,6 +201,7 @@ impl ShErr { pub fn is_flow_control(&self) -> bool { self.kind.is_flow_control() } + /// Promotes a shell error from a simple error to an error that blames a span pub fn promote(mut self, span: Span) -> Self { if self.notes.is_empty() { return self; @@ -208,7 +209,9 @@ impl ShErr { let first = self.notes[0].clone(); if self.notes.len() > 1 { self.notes = self.notes[1..].to_vec(); - } + } else { + self.notes = vec![]; + } self.labeled(span, first) } diff --git a/src/libsh/guards.rs b/src/libsh/guards.rs index 35482c9..c669b97 100644 --- a/src/libsh/guards.rs +++ b/src/libsh/guards.rs @@ -147,11 +147,9 @@ impl RawModeGuard { let orig = ORIG_TERMIOS .with(|cell| cell.borrow().clone()) .expect("with_cooked_mode called before raw_mode()"); - tcsetattr(borrow_fd(*TTY_FILENO), termios::SetArg::TCSANOW, &orig) - .expect("Failed to restore cooked mode"); + tcsetattr(borrow_fd(*TTY_FILENO), termios::SetArg::TCSANOW, &orig).ok(); let res = f(); - tcsetattr(borrow_fd(*TTY_FILENO), termios::SetArg::TCSANOW, ¤t) - .expect("Failed to restore raw mode"); + tcsetattr(borrow_fd(*TTY_FILENO), termios::SetArg::TCSANOW, ¤t).ok(); res } } diff --git a/src/libsh/sys.rs b/src/libsh/sys.rs index e84cc6b..73a5b70 100644 --- a/src/libsh/sys.rs +++ b/src/libsh/sys.rs @@ -2,6 +2,14 @@ use std::sync::LazyLock; use crate::prelude::*; +/// Minimum fd number for shell-internal file descriptors. +const MIN_INTERNAL_FD: RawFd = 10; + pub static TTY_FILENO: LazyLock = LazyLock::new(|| { - open("/dev/tty", OFlag::O_RDWR, Mode::empty()).expect("Failed to open /dev/tty") + let fd = open("/dev/tty", OFlag::O_RDWR, Mode::empty()).expect("Failed to open /dev/tty"); + // Move the tty fd above the user-accessible range so that + // `exec 3>&-` and friends don't collide with shell internals. + let high = fcntl(fd, FcntlArg::F_DUPFD_CLOEXEC(MIN_INTERNAL_FD)).expect("Failed to dup /dev/tty high"); + close(fd).ok(); + high }); diff --git a/src/parse/execute.rs b/src/parse/execute.rs index 7c7d8cb..3e47f3f 100644 --- a/src/parse/execute.rs +++ b/src/parse/execute.rs @@ -8,28 +8,7 @@ use ariadne::Fmt; use crate::{ builtin::{ - alias::{alias, unalias}, - arrops::{arr_fpop, arr_fpush, arr_pop, arr_push, arr_rotate}, - autocmd::autocmd, - cd::cd, - complete::{compgen_builtin, complete_builtin}, - dirstack::{dirs, popd, pushd}, - echo::echo, - eval, exec, - flowctl::flowctl, - getopts::getopts, - intro, - jobctl::{self, JobBehavior, continue_job, disown, jobs}, - keymap, map, - pwd::pwd, - read::{self, read_builtin}, - resource::{ulimit, umask_builtin}, - shift::shift, - shopt::shopt, - source::source, - test::double_bracket_test, - trap::{TrapTarget, trap}, - varcmds::{export, local, readonly, unset}, + alias::{alias, unalias}, arrops::{arr_fpop, arr_fpush, arr_pop, arr_push, arr_rotate}, autocmd::autocmd, cd::cd, complete::{compgen_builtin, complete_builtin}, dirstack::{dirs, popd, pushd}, echo::echo, eval, exec, flowctl::flowctl, getopts::getopts, intro, jobctl::{self, JobBehavior, continue_job, disown, jobs}, keymap, seek::seek, map, pwd::pwd, read::{self, read_builtin}, resource::{ulimit, umask_builtin}, shift::shift, shopt::shopt, source::source, test::double_bracket_test, trap::{TrapTarget, trap}, varcmds::{export, local, readonly, unset} }, expand::{expand_aliases, expand_case_pattern, glob_to_regex}, jobs::{ChildProc, JobStack, attach_tty, dispatch_job}, @@ -888,7 +867,10 @@ impl Dispatcher { if fork_builtins { log::trace!("Forking builtin: {}", cmd_raw); - let _guard = self.io_stack.pop_frame().redirect()?; + let guard = self.io_stack.pop_frame().redirect()?; + if cmd_raw.as_str() == "exec" { + guard.persist(); + } self.run_fork(&cmd_raw, |s| { if let Err(e) = s.dispatch_builtin(cmd) { e.print_error(); @@ -1013,6 +995,7 @@ impl Dispatcher { "autocmd" => autocmd(cmd), "ulimit" => ulimit(cmd), "umask" => umask_builtin(cmd), + "seek" => seek(cmd), "true" | ":" => { state::set_status(0); Ok(()) diff --git a/src/parse/lex.rs b/src/parse/lex.rs index b0cdca8..876b12f 100644 --- a/src/parse/lex.rs +++ b/src/parse/lex.rs @@ -411,37 +411,51 @@ impl LexStream { return None; // It's a process sub } pos += 1; + if let Some('|') = chars.peek() { + // noclobber force '>|' + chars.next(); + pos += 1; + tk = self.get_token(self.cursor..pos, TkRule::Redir); + break + } + if let Some('>') = chars.peek() { chars.next(); pos += 1; } - if let Some('&') = chars.peek() { - chars.next(); - pos += 1; - - let mut found_fd = false; - while chars.peek().is_some_and(|ch| ch.is_ascii_digit()) { - chars.next(); - found_fd = true; - pos += 1; - } - - if !found_fd && !self.flags.contains(LexFlags::LEX_UNFINISHED) { - let span_start = self.cursor; - self.cursor = pos; - return Some(Err(ShErr::at( - ShErrKind::ParseErr, - Span::new(span_start..pos, self.source.clone()), - "Invalid redirection", - ))); - } else { - tk = self.get_token(self.cursor..pos, TkRule::Redir); - break; - } - } else { + let Some('&') = chars.peek() else { tk = self.get_token(self.cursor..pos, TkRule::Redir); break; - } + }; + + chars.next(); + pos += 1; + + let mut found_fd = false; + if chars.peek().is_some_and(|ch| *ch == '-') { + chars.next(); + found_fd = true; + pos += 1; + } else { + while chars.peek().is_some_and(|ch| ch.is_ascii_digit()) { + chars.next(); + found_fd = true; + pos += 1; + } + } + + if !found_fd && !self.flags.contains(LexFlags::LEX_UNFINISHED) { + let span_start = self.cursor; + self.cursor = pos; + return Some(Err(ShErr::at( + ShErrKind::ParseErr, + Span::new(span_start..pos, self.source.clone()), + "Invalid redirection", + ))); + } else { + tk = self.get_token(self.cursor..pos, TkRule::Redir); + break; + } } '<' => { if chars.peek() == Some(&'(') { @@ -449,53 +463,92 @@ impl LexStream { } pos += 1; - if let Some('<') = chars.peek() { - chars.next(); - pos += 1; + match chars.peek() { + Some('<') => { + chars.next(); + pos += 1; - match chars.peek() { - Some('<') => { - chars.next(); - pos += 1; - } - - Some(ch) => { - let mut ch = *ch; - while is_field_sep(ch) { - let Some(next_ch) = chars.next() else { - // Incomplete input — fall through to emit << as Redir - break; - }; - pos += next_ch.len_utf8(); - ch = next_ch; + match chars.peek() { + Some('<') => { + chars.next(); + pos += 1; } - if is_field_sep(ch) { - // Ran out of input while skipping whitespace — fall through - } else { - let saved_cursor = self.cursor; - match self.read_heredoc(pos) { - Ok(Some(heredoc_tk)) => { - // cursor is set to after the delimiter word; - // heredoc_skip is set to after the body - pos = self.cursor; - self.cursor = saved_cursor; - tk = heredoc_tk; + Some(ch) => { + let mut ch = *ch; + while is_field_sep(ch) { + let Some(next_ch) = chars.next() else { + // Incomplete input — fall through to emit << as Redir break; + }; + pos += next_ch.len_utf8(); + ch = next_ch; + } + + if is_field_sep(ch) { + // Ran out of input while skipping whitespace — fall through + } else { + let saved_cursor = self.cursor; + match self.read_heredoc(pos) { + Ok(Some(heredoc_tk)) => { + // cursor is set to after the delimiter word; + // heredoc_skip is set to after the body + pos = self.cursor; + self.cursor = saved_cursor; + tk = heredoc_tk; + break; + } + Ok(None) => { + // Incomplete heredoc — restore cursor and fall through + self.cursor = saved_cursor; + } + Err(e) => return Some(Err(e)), } - Ok(None) => { - // Incomplete heredoc — restore cursor and fall through - self.cursor = saved_cursor; - } - Err(e) => return Some(Err(e)), } } - } - _ => { - // No delimiter yet — input is incomplete - // Fall through to emit the << as a Redir token + _ => { + // No delimiter yet — input is incomplete + // Fall through to emit the << as a Redir token + } } } + Some('>') => { + chars.next(); + pos += 1; + tk = self.get_token(self.cursor..pos, TkRule::Redir); + break; + } + Some('&') => { + chars.next(); + pos += 1; + + let mut found_fd = false; + if chars.peek().is_some_and(|ch| *ch == '-') { + chars.next(); + found_fd = true; + pos += 1; + } else { + while chars.peek().is_some_and(|ch| ch.is_ascii_digit()) { + chars.next(); + found_fd = true; + pos += 1; + } + } + + if !found_fd && !self.flags.contains(LexFlags::LEX_UNFINISHED) { + let span_start = self.cursor; + self.cursor = pos; + return Some(Err(ShErr::at( + ShErrKind::ParseErr, + Span::new(span_start..pos, self.source.clone()), + "Invalid redirection", + ))); + } else { + tk = self.get_token(self.cursor..pos, TkRule::Redir); + break; + } + } + _ => {} } tk = self.get_token(self.cursor..pos, TkRule::Redir); @@ -1049,11 +1102,10 @@ impl Iterator for LexStream { // If a heredoc was parsed on this line, skip past the body // Only on newline — ';' is a command separator within the same line - if ch == '\n' || ch == '\r' { - if let Some(skip) = self.heredoc_skip.take() { - self.cursor = skip; - } - } + if (ch == '\n' || ch == '\r') + && let Some(skip) = self.heredoc_skip.take() { + self.cursor = skip; + } while let Some(ch) = get_char(&self.source, self.cursor) { match ch { diff --git a/src/parse/mod.rs b/src/parse/mod.rs index 5049327..02bc519 100644 --- a/src/parse/mod.rs +++ b/src/parse/mod.rs @@ -12,7 +12,7 @@ use crate::{ }, parse::lex::clean_input, prelude::*, - procio::IoMode, + procio::IoMode, state::read_shopts, }; pub mod execute; @@ -280,12 +280,17 @@ bitflags! { pub struct Redir { pub io_mode: IoMode, pub class: RedirType, + pub span: Option } impl Redir { pub fn new(io_mode: IoMode, class: RedirType) -> Self { - Self { io_mode, class } + Self { io_mode, class, span: None } } + pub fn with_span(mut self, span: Span) -> Self { + self.span = Some(span); + self + } } #[derive(Default, Debug)] @@ -293,6 +298,7 @@ pub struct RedirBldr { pub io_mode: Option, pub class: Option, pub tgt_fd: Option, + pub span: Option, } impl RedirBldr { @@ -300,43 +306,36 @@ impl RedirBldr { Default::default() } pub fn with_io_mode(self, io_mode: IoMode) -> Self { - let Self { - io_mode: _, - class, - tgt_fd, - } = self; - Self { - io_mode: Some(io_mode), - class, - tgt_fd, - } + Self { + io_mode: Some(io_mode), + ..self + } } pub fn with_class(self, class: RedirType) -> Self { - let Self { - io_mode, - class: _, - tgt_fd, - } = self; - Self { - io_mode, - class: Some(class), - tgt_fd, - } + Self { + class: Some(class), + ..self + } } pub fn with_tgt(self, tgt_fd: RawFd) -> Self { - let Self { - io_mode, - class, - tgt_fd: _, - } = self; - Self { - io_mode, - class, - tgt_fd: Some(tgt_fd), - } + Self { + tgt_fd: Some(tgt_fd), + ..self + } } + pub fn with_span(self, span: Span) -> Self { + Self { + span: Some(span), + ..self + } + } pub fn build(self) -> Redir { - Redir::new(self.io_mode.unwrap(), self.class.unwrap()) + let new = Redir::new(self.io_mode.unwrap(), self.class.unwrap()); + if let Some(span) = self.span { + new.with_span(span) + } else { + new + } } } @@ -355,16 +354,24 @@ impl FromStr for RedirBldr { if let Some('>') = chars.peek() { chars.next(); redir = redir.with_class(RedirType::Append); - } + } else if let Some('|') = chars.peek() { + chars.next(); + redir = redir.with_class(RedirType::OutputForce); + } } '<' => { redir = redir.with_class(RedirType::Input); let mut count = 0; - while count < 2 && matches!(chars.peek(), Some('<')) { - chars.next(); - count += 1; - } + if chars.peek() == Some(&'>') { + chars.next(); // consume the '>' + redir = redir.with_class(RedirType::ReadWrite); + } else { + while count < 2 && matches!(chars.peek(), Some('<')) { + chars.next(); + count += 1; + } + } redir = match count { 1 => redir.with_class(RedirType::HereDoc), @@ -373,13 +380,18 @@ impl FromStr for RedirBldr { }; } '&' => { - while let Some(next_ch) = chars.next() { - if next_ch.is_ascii_digit() { - src_fd.push(next_ch) - } else { - break; - } - } + if chars.peek() == Some(&'-') { + chars.next(); + src_fd.push('-'); + } else { + while let Some(next_ch) = chars.next() { + if next_ch.is_ascii_digit() { + src_fd.push(next_ch) + } else { + break; + } + } + } if src_fd.is_empty() { return Err(ShErr::simple( ShErrKind::ParseErr, @@ -405,15 +417,20 @@ impl FromStr for RedirBldr { } } - // FIXME: I am 99.999999999% sure that tgt_fd and src_fd are backwards here let tgt_fd = tgt_fd .parse::() .unwrap_or_else(|_| match redir.class.unwrap() { - RedirType::Input | RedirType::HereDoc | RedirType::HereString => 0, + RedirType::Input | + RedirType::ReadWrite | + RedirType::HereDoc | + RedirType::HereString => 0, _ => 1, }); redir = redir.with_tgt(tgt_fd); - if let Ok(src_fd) = src_fd.parse::() { + if src_fd.as_str() == "-" { + let io_mode = IoMode::Close { tgt_fd }; + redir = redir.with_io_mode(io_mode); + } else if let Ok(src_fd) = src_fd.parse::() { let io_mode = IoMode::fd(tgt_fd, src_fd); redir = redir.with_io_mode(io_mode); } @@ -424,6 +441,7 @@ impl FromStr for RedirBldr { impl TryFrom for RedirBldr { type Error = ShErr; fn try_from(tk: Tk) -> Result { + let span = tk.span.clone(); if tk.flags.contains(TkFlags::IS_HEREDOC) { let flags = tk.flags; let mut heredoc_body = if flags.contains(TkFlags::LIT_HEREDOC) { @@ -466,10 +484,14 @@ impl TryFrom for RedirBldr { Ok(RedirBldr { io_mode: Some(IoMode::loaded_pipe(0, heredoc_body.as_bytes())?), class: Some(RedirType::HereDoc), - tgt_fd: Some(0) + tgt_fd: Some(0), + span: Some(span) }) } else { - Self::from_str(tk.as_str()) + match Self::from_str(tk.as_str()) { + Ok(bldr) => Ok(bldr.with_span(span)), + Err(e) => Err(e.promote(span)), + } } } } @@ -481,10 +503,12 @@ pub enum RedirType { PipeAnd, // |&, redirs stderr and stdout Input, // < Output, // > + OutputForce,// >| Append, // >> HereDoc, // << IndentHereDoc, // <<-, strips leading tabs HereString, // <<< + ReadWrite, // <>, fd is opened for reading and writing } #[derive(Clone, Debug)] @@ -1881,11 +1905,34 @@ pub fn get_redir_file>(class: RedirType, path: P) -> ShResult OpenOptions::new().read(true).open(Path::new(&path)), - RedirType::Output => OpenOptions::new() - .write(true) - .create(true) - .truncate(true) - .open(path), + RedirType::Output => { + if read_shopts(|o| o.core.noclobber) && path.is_file() { + return Err(ShErr::simple( + ShErrKind::ExecFail, + format!("shopt core.noclobber is set, refusing to overwrite existing file `{}`", path.display()), + )); + } + OpenOptions::new() + .write(true) + .create(true) + .truncate(true) + .open(path) + }, + RedirType::ReadWrite => { + OpenOptions::new() + .write(true) + .read(true) + .create(true) + .truncate(false) + .open(path) + } + RedirType::OutputForce => { + OpenOptions::new() + .write(true) + .create(true) + .truncate(true) + .open(path) + } RedirType::Append => OpenOptions::new().create(true).append(true).open(path), _ => unimplemented!("Unimplemented redir type: {:?}", class), }; diff --git a/src/prelude.rs b/src/prelude.rs index 01df639..b74ff28 100644 --- a/src/prelude.rs +++ b/src/prelude.rs @@ -19,7 +19,7 @@ pub use std::os::unix::io::{AsRawFd, BorrowedFd, FromRawFd, IntoRawFd, OwnedFd, pub use bitflags::bitflags; pub use nix::{ errno::Errno, - fcntl::{OFlag, open}, + fcntl::{FcntlArg, OFlag, fcntl, open}, libc::{self, STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO}, sys::{ signal::{self, SigHandler, SigSet, SigmaskHow, Signal, kill, killpg, pthread_sigmask, signal}, diff --git a/src/procio.rs b/src/procio.rs index fa2ed65..99a68b0 100644 --- a/src/procio.rs +++ b/src/procio.rs @@ -8,6 +8,7 @@ use crate::{ expand::Expander, libsh::{ error::{ShErr, ShErrKind, ShResult}, + sys::TTY_FILENO, utils::RedirVecUtils, }, parse::{Redir, RedirType, get_redir_file, lex::TkFlags}, @@ -17,6 +18,16 @@ use crate::{ // Credit to fish-shell for many of the implementation ideas present in this // module https://fishshell.com/ +/// Minimum fd number for shell-internal file descriptors. +/// User-visible fds (0-9) are kept clear so `exec 3>&-` etc. work as expected. +const MIN_INTERNAL_FD: RawFd = 10; + +/// Like `dup()`, but places the new fd at `MIN_INTERNAL_FD` or above so it +/// doesn't collide with user-managed fds. +fn dup_high(fd: RawFd) -> nix::Result { + fcntl(fd, FcntlArg::F_DUPFD_CLOEXEC(MIN_INTERNAL_FD)) +} + #[derive(Clone, Debug)] pub enum IoMode { Fd { @@ -84,9 +95,16 @@ impl IoMode { let expanded_pathbuf = PathBuf::from(expanded_path); let file = get_redir_file(mode, expanded_pathbuf)?; + // Move the opened fd above the user-accessible range so it never + // collides with the target fd (e.g. `3>/tmp/foo` where open() returns 3, + // causing dup2(3,3) to be a no-op and then OwnedFd drop closes it). + let raw = file.as_raw_fd(); + let high = fcntl(raw, FcntlArg::F_DUPFD_CLOEXEC(MIN_INTERNAL_FD)) + .map_err(ShErr::from)?; + drop(file); // closes the original low fd self = IoMode::OpenedFile { tgt_fd, - file: Arc::new(OwnedFd::from(file)), + file: Arc::new(unsafe { OwnedFd::from_raw_fd(high) }), } } Ok(self) @@ -210,23 +228,53 @@ impl<'e> IoFrame { ) } pub fn save(&'e mut self) { - let saved_in = dup(STDIN_FILENO).unwrap(); - let saved_out = dup(STDOUT_FILENO).unwrap(); - let saved_err = dup(STDERR_FILENO).unwrap(); + let saved_in = dup_high(STDIN_FILENO).unwrap(); + let saved_out = dup_high(STDOUT_FILENO).unwrap(); + let saved_err = dup_high(STDERR_FILENO).unwrap(); self.saved_io = Some(IoGroup(saved_in, saved_out, saved_err)); } pub fn redirect(mut self) -> ShResult { self.save(); + if let Err(e) = self.apply_redirs() { + // Restore saved fds before propagating the error so they don't leak. + self.restore().ok(); + return Err(e); + } + Ok(RedirGuard::new(self)) + } + fn apply_redirs(&mut self) -> ShResult<()> { for redir in &mut self.redirs { let io_mode = &mut redir.io_mode; + if let IoMode::Close { tgt_fd } = io_mode { + if *tgt_fd == *TTY_FILENO { + // Don't let user close the shell's tty fd. + continue; + } + close(*tgt_fd).ok(); + continue; + } if let IoMode::File { .. } = io_mode { - *io_mode = io_mode.clone().open_file()?; + match io_mode.clone().open_file() { + Ok(file) => *io_mode = file, + Err(e) => { + if let Some(span) = redir.span.as_ref() { + return Err(e.promote(span.clone())); + } + return Err(e) + } + } }; let tgt_fd = io_mode.tgt_fd(); let src_fd = io_mode.src_fd(); - dup2(src_fd, tgt_fd)?; + if let Err(e) = dup2(src_fd, tgt_fd) { + if let Some(span) = redir.span.as_ref() { + return Err(ShErr::from(e).promote(span.clone())); + } else { + return Err(e.into()); + } + } } - Ok(RedirGuard::new(self)) + Ok(()) } pub fn restore(&mut self) -> ShResult<()> { if let Some(saved) = self.saved_io.take() { @@ -338,6 +386,8 @@ pub fn borrow_fd<'f>(fd: i32) -> BorrowedFd<'f> { } type PipeFrames = Map, Option)) -> IoFrame>; + +/// An iterator that lazily creates a specific number of pipes. pub struct PipeGenerator { num_cmds: usize, cursor: usize, diff --git a/src/shopt.rs b/src/shopt.rs index 963f68a..e174586 100644 --- a/src/shopt.rs +++ b/src/shopt.rs @@ -146,6 +146,7 @@ pub struct ShOptCore { pub bell_enabled: bool, pub max_recurse_depth: usize, pub xpg_echo: bool, + pub noclobber: bool, } impl ShOptCore { @@ -238,6 +239,15 @@ impl ShOptCore { }; self.xpg_echo = val; } + "noclobber" => { + let Ok(val) = val.parse::() else { + return Err(ShErr::simple( + ShErrKind::SyntaxErr, + "shopt: expected 'true' or 'false' for noclobber value", + )); + }; + self.noclobber = val; + } _ => { return Err(ShErr::simple( ShErrKind::SyntaxErr, @@ -304,6 +314,11 @@ impl ShOptCore { output.push_str(&format!("{}", self.xpg_echo)); Ok(Some(output)) } + "noclobber" => { + let mut output = String::from("Prevent > from overwriting existing files (use >| to override)\n"); + output.push_str(&format!("{}", self.noclobber)); + Ok(Some(output)) + } _ => Err(ShErr::simple( ShErrKind::SyntaxErr, format!("shopt: Unexpected 'core' option '{query}'"), @@ -327,6 +342,7 @@ impl Display for ShOptCore { output.push(format!("bell_enabled = {}", self.bell_enabled)); output.push(format!("max_recurse_depth = {}", self.max_recurse_depth)); output.push(format!("xpg_echo = {}", self.xpg_echo)); + output.push(format!("noclobber = {}", self.noclobber)); let final_output = output.join("\n"); @@ -346,6 +362,7 @@ impl Default for ShOptCore { bell_enabled: true, max_recurse_depth: 1000, xpg_echo: false, + noclobber: false, } } } @@ -589,6 +606,7 @@ mod tests { bell_enabled, max_recurse_depth, xpg_echo, + noclobber, } = ShOptCore::default(); // If a field is added to the struct, this destructure fails to compile. let _ = ( @@ -601,6 +619,7 @@ mod tests { bell_enabled, max_recurse_depth, xpg_echo, + noclobber, ); }