diff --git a/src/client/shell/actions.rs b/src/client/shell/actions.rs index 83819b0d1..08a8ae14a 100644 --- a/src/client/shell/actions.rs +++ b/src/client/shell/actions.rs @@ -638,9 +638,8 @@ impl ClientShellState { Ok(crate::api::schema::ResponseResult::PaneSelection { text, .. }) if !text.is_empty() => { - let repaint = self.show_copy_feedback(std::time::Instant::now()); ( - repaint, + false, vec![ClientShellAction::ClipboardWrite(text.into_bytes())], ) } diff --git a/src/client/shell/tests/copy.rs b/src/client/shell/tests/copy.rs index 317950cfc..ee92ac194 100644 --- a/src/client/shell/tests/copy.rs +++ b/src/client/shell/tests/copy.rs @@ -206,17 +206,14 @@ fn client_mouse_selection_highlights_and_copies_through_endpoint_extraction() { text: "LIV".into(), }), ); - assert!(repaint); + assert!(!repaint); assert!(matches!( &actions[..], [ClientShellAction::ClipboardWrite(bytes)] if bytes == b"LIV" )); - assert_eq!( - state - .copy_feedback - .as_ref() - .map(|feedback| feedback.message.as_str()), - Some("copied to clipboard") + assert!( + state.copy_feedback.is_none(), + "feedback must wait for clipboard dispatch" ); } diff --git a/src/client/shell_runtime.rs b/src/client/shell_runtime.rs index 52aba20a6..723cbb950 100644 --- a/src/client/shell_runtime.rs +++ b/src/client/shell_runtime.rs @@ -34,7 +34,11 @@ pub(super) fn dispatch_client_shell_actions( } } shell::ClientShellAction::ClipboardWrite(bytes) => { - crate::selection::write_osc52_bytes(&bytes); + if crate::selection::write_osc52_bytes(&bytes) { + if let Some(shell) = shell.as_deref_mut() { + repaint |= shell.show_copy_feedback(std::time::Instant::now()); + } + } } shell::ClientShellAction::ActivateEndpoint { endpoint_id, diff --git a/src/platform/linux.rs b/src/platform/linux.rs index c4aac7d6b..37bc91f2f 100644 --- a/src/platform/linux.rs +++ b/src/platform/linux.rs @@ -9,10 +9,9 @@ use std::{ pub(super) const REMOTE_BRIDGE_CLOCK: libc::clockid_t = libc::CLOCK_BOOTTIME; -use super::{ - read_limited_reader, ClipboardCommand, ClipboardImage, ForegroundJob, ForegroundProcess, - LimitedRead, Signal, -}; +use super::{ClipboardCommand, ClipboardImage, ForegroundJob, ForegroundProcess, Signal}; + +mod clipboard; pub(crate) use super::unix_common::{ configure_status_command, create_remote_private_dir, create_remote_ssh_config_dir, @@ -23,6 +22,8 @@ pub(crate) use super::unix_common::{ ClientStreamReader, PinnedSshControlSocket, StatusCommandGuard, }; +#[cfg(test)] +mod clipboard_tests; #[cfg(test)] mod config_file_tests; @@ -852,8 +853,9 @@ pub fn process_exists(pid: u32) -> bool { } pub fn write_clipboard(bytes: &[u8]) -> bool { + let deadline = std::time::Instant::now() + clipboard::TIMEOUT; for command in clipboard_commands() { - if run_clipboard_command(&command, bytes) { + if run_clipboard_command_until(&command, bytes, deadline) { return true; } } @@ -861,8 +863,9 @@ pub fn write_clipboard(bytes: &[u8]) -> bool { } pub fn read_clipboard_text() -> Option { + let deadline = std::time::Instant::now() + clipboard::TIMEOUT; for command in read_clipboard_text_commands() { - if let Some(text) = read_clipboard_text_with_command(&command) { + if let Some(text) = read_clipboard_text_with_command_until(&command, deadline) { return Some(text); } } @@ -884,8 +887,10 @@ pub fn open_url(url: &str) -> std::io::Result> { } pub fn read_clipboard_image() -> Option { + let deadline = std::time::Instant::now() + clipboard::TIMEOUT; if running_inside_wsl() { - if let Some(image) = read_wsl_clipboard_image_with_command(|program| Command::new(program)) + if let Some(image) = + read_wsl_clipboard_image_with_command_until(|program| Command::new(program), deadline) { return Some(image); } @@ -900,18 +905,22 @@ pub fn read_clipboard_image() -> Option { ("image/bmp", "bmp"), ] { if std::env::var_os("WAYLAND_DISPLAY").is_some() { - if let Some(image) = - read_validated_clipboard_image("wl-paste", &["--type", mime], extension) - { + if let Some(image) = read_validated_clipboard_image_until( + "wl-paste", + &["--type", mime], + extension, + deadline, + ) { return Some(image); } } if std::env::var_os("DISPLAY").is_some() { - if let Some(image) = read_validated_clipboard_image( + if let Some(image) = read_validated_clipboard_image_until( "xclip", &["-selection", "clipboard", "-t", mime, "-o"], extension, + deadline, ) { return Some(image); } @@ -921,8 +930,9 @@ pub fn read_clipboard_image() -> Option { None } -fn read_wsl_clipboard_image_with_command( +fn read_wsl_clipboard_image_with_command_until( mut command: impl FnMut(&str) -> Command, + deadline: std::time::Instant, ) -> Option { let mut command = command("powershell.exe"); command.args([ @@ -932,19 +942,30 @@ fn read_wsl_clipboard_image_with_command( "-Command", "$ErrorActionPreference='Stop'; Add-Type -AssemblyName System.Windows.Forms; Add-Type -AssemblyName System.Drawing; $image=[System.Windows.Forms.Clipboard]::GetImage(); if ($null -eq $image) { exit 1 }; $stream=[System.IO.MemoryStream]::new(); try { $image.Save($stream, [System.Drawing.Imaging.ImageFormat]::Png); [Console]::OpenStandardOutput().Write($stream.GetBuffer(), 0, [int]$stream.Length) } finally { $stream.Dispose(); $image.Dispose() }", ]); - let bytes = read_clipboard_image_with_spawned_command(command)?; + let bytes = read_clipboard_image_until( + command, + crate::protocol::MAX_CLIPBOARD_IMAGE_PAYLOAD, + deadline, + )?; bytes_match_image_signature("png", &bytes).then_some(ClipboardImage { bytes, extension: "png", }) } -fn read_validated_clipboard_image( +fn read_validated_clipboard_image_until( program: &str, args: &[&str], extension: &'static str, + deadline: std::time::Instant, ) -> Option { - let bytes = read_clipboard_image_with_command(program, args)?; + let mut command = Command::new(program); + command.args(args); + let bytes = read_clipboard_image_until( + command, + crate::protocol::MAX_CLIPBOARD_IMAGE_PAYLOAD, + deadline, + )?; if !bytes_match_image_signature(extension, &bytes) { return None; } @@ -1005,55 +1026,25 @@ fn run_notification_command(mut command: Command) -> std::io::Result { Ok(status.success()) } -fn read_clipboard_image_with_command(program: &str, args: &[&str]) -> Option> { - let mut command = Command::new(program); - command.args(args); - read_clipboard_image_with_spawned_command(command) -} - -fn read_clipboard_image_with_spawned_command(command: Command) -> Option> { - read_clipboard_image_with_spawned_command_max( - command, - crate::protocol::MAX_CLIPBOARD_IMAGE_PAYLOAD, - ) +fn read_clipboard_image_until( + command: Command, + max_bytes: usize, + deadline: std::time::Instant, +) -> Option> { + let bytes = clipboard::run(command, None, max_bytes, deadline).ok()?; + (!bytes.is_empty()).then_some(bytes) } +#[cfg(test)] fn read_clipboard_image_with_spawned_command_max( - mut command: Command, + command: Command, max_bytes: usize, ) -> Option> { - let mut child = command - .stdin(Stdio::null()) - .stdout(Stdio::piped()) - .stderr(Stdio::null()) - .spawn() - .ok()?; - let stdout = child.stdout.take()?; - - let read = match read_limited_reader(stdout, max_bytes) { - Ok(read) => read, - Err(_) => { - let _ = child.kill(); - let _ = child.wait(); - return None; - } - }; - - if read == LimitedRead::Oversized { - let _ = child.kill(); - let _ = child.wait(); - return None; - } - - let status = child.wait().ok()?; - if !status.success() { - return None; - } - - match read { - LimitedRead::Complete(bytes) => Some(bytes), - LimitedRead::Empty | LimitedRead::Oversized => None, - } + read_clipboard_image_until( + command, + max_bytes, + std::time::Instant::now() + clipboard::TIMEOUT, + ) } fn clipboard_commands() -> Vec { @@ -1108,125 +1099,66 @@ fn read_clipboard_text_commands() -> Vec { commands } -fn read_clipboard_text_with_command(command: &ClipboardCommand) -> Option { +fn read_clipboard_text_with_command_until( + command: &ClipboardCommand, + deadline: std::time::Instant, +) -> Option { const MAX_CLIPBOARD_TEXT_BYTES: usize = 1024 * 1024; - - let mut child = Command::new(command.program) - .args(command.args) - .stdin(Stdio::null()) - .stdout(Stdio::piped()) - .stderr(Stdio::null()) - .spawn() - .ok()?; - - let stdout = child.stdout.take()?; - let read = match read_limited_reader(stdout, MAX_CLIPBOARD_TEXT_BYTES) { - Ok(LimitedRead::Oversized) => { - let _ = child.kill(); - let _ = child.wait(); - return None; - } - Ok(read) => read, - Err(_) => { - let _ = child.kill(); - let _ = child.wait(); - return None; - } - }; - - let status = child.wait().ok()?; - if !status.success() { + let mut process = Command::new(command.program); + process.args(command.args); + let bytes = clipboard::run(process, None, MAX_CLIPBOARD_TEXT_BYTES, deadline).ok()?; + if bytes.is_empty() { return None; } - - match read { - LimitedRead::Complete(bytes) => String::from_utf8(bytes).ok(), - LimitedRead::Empty => None, - LimitedRead::Oversized => unreachable!("oversized clipboard text is handled before wait"), - } + String::from_utf8(bytes).ok() } +fn run_clipboard_command_until( + command: &ClipboardCommand, + bytes: &[u8], + deadline: std::time::Instant, +) -> bool { + let mut process = Command::new(command.program); + process.args(command.args); + clipboard::run(process, Some(bytes), 0, deadline).is_ok() +} + +#[cfg(test)] fn run_clipboard_command(command: &ClipboardCommand, bytes: &[u8]) -> bool { - let mut child = match Command::new(command.program) - .args(command.args) - .stdin(Stdio::piped()) - .stdout(Stdio::null()) - .stderr(Stdio::null()) - .spawn() - { - Ok(child) => child, - Err(_) => return false, - }; - - let Some(mut stdin) = child.stdin.take() else { - let _ = child.kill(); - let _ = child.wait(); - return false; - }; - - if stdin.write_all(bytes).is_err() { - let _ = child.kill(); - let _ = child.wait(); - return false; - } - drop(stdin); - - if command.program == "wl-copy" { - return wait_for_wl_copy_startup(child); - } - - child.wait().map(|status| status.success()).unwrap_or(false) + run_clipboard_command_until( + command, + bytes, + std::time::Instant::now() + clipboard::TIMEOUT, + ) } -fn wait_for_wl_copy_startup(mut child: std::process::Child) -> bool { - const STARTUP_WAIT: std::time::Duration = std::time::Duration::from_millis(100); - const POLL_INTERVAL: std::time::Duration = std::time::Duration::from_millis(5); - - let deadline = std::time::Instant::now() + STARTUP_WAIT; - loop { - match child.try_wait() { - Ok(Some(status)) => return status.success(), - Ok(None) if std::time::Instant::now() < deadline => { - std::thread::sleep(POLL_INTERVAL); - } - Ok(None) => return detach_clipboard_owner(child), - Err(_) => { - let _ = child.kill(); - let _ = child.wait(); - return false; - } - } - } +#[cfg(test)] +fn read_clipboard_text_with_command(command: &ClipboardCommand) -> Option { + read_clipboard_text_with_command_until(command, std::time::Instant::now() + clipboard::TIMEOUT) } -fn detach_clipboard_owner(child: std::process::Child) -> bool { - let pid = child.id(); - let child = std::sync::Arc::new(std::sync::Mutex::new(child)); - let reaper_child = std::sync::Arc::clone(&child); - let reaper = std::thread::Builder::new() - .name("herdr-wl-copy-reaper".to_string()) - .spawn(move || { - let wait_result = match reaper_child.lock() { - Ok(mut child) => child.wait(), - Err(poisoned) => poisoned.into_inner().wait(), - }; - if let Err(err) = wait_result { - tracing::warn!(pid, %err, "failed to reap wl-copy clipboard owner"); - } - }); +#[cfg(test)] +fn read_validated_clipboard_image( + program: &str, + args: &[&str], + extension: &'static str, +) -> Option { + read_validated_clipboard_image_until( + program, + args, + extension, + std::time::Instant::now() + clipboard::TIMEOUT, + ) +} - if let Err(err) = reaper { - tracing::warn!(pid, %err, "failed to start wl-copy clipboard owner reaper"); - let mut child = match child.lock() { - Ok(child) => child, - Err(poisoned) => poisoned.into_inner(), - }; - let _ = child.kill(); - let _ = child.wait(); - return false; - } - - true +#[cfg(test)] +fn read_wsl_clipboard_image_with_command( + command: impl FnMut(&str) -> Command, +) -> Option { + read_wsl_clipboard_image_with_command_until( + command, + std::time::Instant::now() + clipboard::TIMEOUT, + ) } fn process_session_id(pid: u32) -> Option { @@ -1762,7 +1694,7 @@ mod tests { } #[test] - fn wl_copy_owner_does_not_block_clipboard_write() { + fn hung_wl_copy_reports_failure_and_is_reaped() { use std::ffi::OsString; use std::os::unix::fs::PermissionsExt; use std::path::PathBuf; @@ -1841,6 +1773,28 @@ mod tests { std::env::set_var("HERDR_TEST_WL_COPY_ARGS", &args); } + for delay in ["", "sleep 0.3"] { + std::fs::write( + &fake_wl_copy, + format!("#!/bin/sh\ncat >/dev/null\n{delay}\nexit 0\n"), + ) + .expect("successful helper should be written"); + let command = ClipboardCommand { + program: "wl-copy", + args: &["--type", "text/plain;charset=utf-8"], + }; + let started = Instant::now(); + assert!(run_clipboard_command(&command, b"clipboard text")); + assert!( + started.elapsed() < Duration::from_secs(2), + "success must not wait for the deadline" + ); + } + std::fs::write( + &fake_wl_copy, + "#!/bin/sh\ncat > \"$HERDR_TEST_WL_COPY_PAYLOAD\"\nprintf '%s\\n' \"$@\" > \"$HERDR_TEST_WL_COPY_ARGS\"\nprintf '%s' \"$$\" > \"$HERDR_TEST_WL_COPY_MARKER\"\nexec sleep 30\n", + ).expect("hung helper should be written"); + let (result_tx, result_rx) = mpsc::channel(); let writer = std::thread::spawn(move || { let command = ClipboardCommand { @@ -1859,12 +1813,13 @@ mod tests { .parse() .expect("owner pid should be numeric"); cleanup.owner_pid = Some(owner_pid); - let returned_while_owner_running = result_rx - .recv_timeout(Duration::from_secs(2)) - .is_ok_and(|result| result); + let reported_success = result_rx + .recv_timeout(Duration::from_secs(7)) + .expect("clipboard write must finish within its deadline"); let actual_payload = std::fs::read(&payload).expect("fake wl-copy should record stdin"); let actual_args = std::fs::read_to_string(&args).expect("fake wl-copy should record args"); + let owner_survived_return = process_exists(owner_pid as u32); unsafe { libc::kill(owner_pid, libc::SIGTERM); } @@ -1878,8 +1833,12 @@ mod tests { drop(cleanup); assert!( - returned_while_owner_running, - "clipboard writes must return while wl-copy remains alive to own the selection" + !reported_success, + "a hung wl-copy must not be mistaken for a clipboard owner" + ); + assert!( + !owner_survived_return, + "timed-out helper must be killed and reaped before return" ); assert_eq!(actual_payload, b"clipboard text"); assert_eq!(actual_args, "--type\ntext/plain;charset=utf-8\n"); diff --git a/src/platform/linux/clipboard.rs b/src/platform/linux/clipboard.rs new file mode 100644 index 000000000..4389f3632 --- /dev/null +++ b/src/platform/linux/clipboard.rs @@ -0,0 +1,134 @@ +use std::io::{self, Read, Write}; +use std::os::fd::{AsRawFd, RawFd}; +use std::os::unix::process::CommandExt; +use std::process::{Child, Command, Stdio}; +use std::time::{Duration, Instant}; + +// Allow slow desktop startup, but never let a clipboard operation wedge the client. +pub(super) const TIMEOUT: Duration = Duration::from_secs(5); + +struct Helper { + child: Child, + completed: bool, +} + +impl Drop for Helper { + fn drop(&mut self) { + if !self.completed { + // Wrappers may leave descendants holding the pipes after their parent exits. + unsafe { + libc::kill(-(self.child.id() as i32), libc::SIGKILL); + } + let _ = self.child.kill(); + let _ = self.child.wait(); + } + } +} + +fn nonblocking(fd: RawFd) -> io::Result<()> { + let flags = unsafe { libc::fcntl(fd, libc::F_GETFL) }; + if flags < 0 || unsafe { libc::fcntl(fd, libc::F_SETFL, flags | libc::O_NONBLOCK) } < 0 { + return Err(io::Error::last_os_error()); + } + Ok(()) +} + +pub(super) fn run( + mut command: Command, + input: Option<&[u8]>, + max_bytes: usize, + deadline: Instant, +) -> io::Result> { + if Instant::now() >= deadline { + return Err(io::ErrorKind::TimedOut.into()); + } + let child = command + .process_group(0) + .stdin(if input.is_some() { + Stdio::piped() + } else { + Stdio::null() + }) + .stdout(if max_bytes > 0 { + Stdio::piped() + } else { + Stdio::null() + }) + .stderr(Stdio::null()) + .spawn()?; + let mut helper = Helper { + child, + completed: false, + }; + let mut stdin = helper.child.stdin.take(); + let mut stdout = helper.child.stdout.take(); + if let Some(pipe) = &stdin { + nonblocking(pipe.as_raw_fd())?; + } + if let Some(pipe) = &stdout { + nonblocking(pipe.as_raw_fd())?; + } + let input = input.unwrap_or_default(); + let mut written = 0; + let mut bytes = Vec::new(); + loop { + if Instant::now() >= deadline { + return Err(io::ErrorKind::TimedOut.into()); + } + let mut progress = false; + if written == input.len() { + stdin = None; + } + if let Some(pipe) = stdin.as_mut() { + match pipe.write(&input[written..]) { + Ok(0) => return Err(io::ErrorKind::WriteZero.into()), + Ok(count) => { + written += count; + progress = true; + } + Err(err) + if matches!( + err.kind(), + io::ErrorKind::WouldBlock | io::ErrorKind::Interrupted + ) => {} + Err(err) => return Err(err), + } + } + if let Some(pipe) = stdout.as_mut() { + let mut buffer = [0; 8192]; + let limit = buffer + .len() + .min(max_bytes.saturating_sub(bytes.len()).saturating_add(1)); + match pipe.read(&mut buffer[..limit]) { + Ok(0) => stdout = None, + Ok(count) => { + if count > max_bytes - bytes.len() { + return Err(io::ErrorKind::FileTooLarge.into()); + } + bytes.extend_from_slice(&buffer[..count]); + progress = true; + } + Err(err) + if matches!( + err.kind(), + io::ErrorKind::WouldBlock | io::ErrorKind::Interrupted + ) => {} + Err(err) => return Err(err), + } + } + if let Some(status) = helper.child.try_wait()? { + if !status.success() { + return Err(io::Error::other("clipboard helper failed")); + } + if written == input.len() && stdout.is_none() { + helper.completed = true; + return Ok(bytes); + } + } + if !progress { + std::thread::sleep( + Duration::from_millis(5).min(deadline.saturating_duration_since(Instant::now())), + ); + } + } +} diff --git a/src/platform/linux/clipboard_tests.rs b/src/platform/linux/clipboard_tests.rs new file mode 100644 index 000000000..1b1702391 --- /dev/null +++ b/src/platform/linux/clipboard_tests.rs @@ -0,0 +1,251 @@ +use super::*; +use std::time::{Duration, Instant}; + +fn hung_helper(case: &str) { + let marker = std::env::temp_dir().join(format!( + "herdr-clipboard-hang-{}-{case}", + std::process::id() + )); + let script = format!("echo $$ > '{}'; {case}", marker.display()); + let (cancel, watchdog) = std::sync::mpsc::channel(); + let watchdog_marker = marker.clone(); + let watchdog = std::thread::spawn(move || { + if watchdog.recv_timeout(Duration::from_secs(8)).is_err() { + if let Ok(pid) = std::fs::read_to_string(&watchdog_marker) { + if let Ok(pid) = pid.trim().parse::() { + unsafe { + libc::kill(pid, libc::SIGKILL); + } + } + } + } + }); + let started = Instant::now(); + let mut command = Command::new("sh"); + command.args(["-c", &script]); + let failed = if case == "exec sleep 30" { + clipboard::run( + command, + Some(&vec![b'x'; 1024 * 1024]), + 0, + Instant::now() + clipboard::TIMEOUT, + ) + .is_err() + } else { + read_clipboard_image_with_spawned_command_max(command, 16).is_none() + }; + let elapsed = started.elapsed(); + let pid: u32 = std::fs::read_to_string(&marker) + .expect("helper pid") + .trim() + .parse() + .expect("numeric pid"); + let survived = process_exists(pid); + let _ = cancel.send(()); + watchdog.join().expect("watchdog joined"); + let _ = std::fs::remove_file(marker); + assert!(failed, "hung helper must fail"); + assert!( + elapsed < Duration::from_secs(7), + "clipboard deadline exceeded: {elapsed:?}" + ); + assert!(!survived, "helper must be killed and reaped before return"); +} + +#[test] +fn clipboard_hung_stdin_is_bounded() { + hung_helper("exec sleep 30"); +} + +#[test] +fn clipboard_hung_stdout_is_bounded() { + hung_helper("exec sleep 30 # stdout"); +} + +#[test] +fn clipboard_hung_exit_after_stdout_closes_is_bounded() { + hung_helper("exec 1>&-; exec sleep 30"); +} + +#[test] +fn clipboard_slow_success_is_not_a_startup_failure() { + let mut command = Command::new("sh"); + command.args(["-c", "sleep 0.3; printf image"]); + assert_eq!( + read_clipboard_image_with_spawned_command_max(command, 16), + Some(b"image".to_vec()) + ); +} + +struct ForkingHelper { + directory: PathBuf, +} + +impl ForkingHelper { + fn new(script: &str) -> Self { + use std::os::unix::fs::PermissionsExt; + let directory = std::env::temp_dir().join(format!( + "herdr-forking-clipboard-{}-{}", + std::process::id(), + std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .expect("timestamp") + .as_nanos(), + )); + std::fs::create_dir(&directory).expect("helper directory"); + let helper = Self { directory }; + std::fs::write( + helper.directory.join("wl-copy"), + format!("#!/bin/sh\n{script}\n"), + ) + .expect("helper script"); + std::fs::set_permissions( + helper.directory.join("wl-copy"), + std::fs::Permissions::from_mode(0o700), + ) + .expect("executable helper"); + helper + } + + fn command(&self) -> Command { + let mut command = Command::new(self.directory.join("wl-copy")); + command.env("OWNER_PID", self.directory.join("owner")); + command + } + + fn owner_pid(&self) -> u32 { + std::fs::read_to_string(self.directory.join("owner")) + .expect("owner pid") + .trim() + .parse() + .expect("numeric owner pid") + } +} + +impl Drop for ForkingHelper { + fn drop(&mut self) { + if let Ok(pid) = std::fs::read_to_string(self.directory.join("owner")) { + if let Ok(pid) = pid.trim().parse::() { + if helper_is_running(pid as u32) { + unsafe { + libc::kill(pid, libc::SIGKILL); + } + } + let deadline = Instant::now() + Duration::from_secs(2); + while helper_is_running(pid as u32) && Instant::now() < deadline { + std::thread::sleep(Duration::from_millis(5)); + } + } + } + let _ = std::fs::remove_dir_all(&self.directory); + } +} + +#[test] +fn clipboard_forking_wl_copy_preserves_successful_owner() { + let helper = ForkingHelper::new( + "cat >/dev/null\nsleep 30 /dev/null 2>&1 &\nprintf '%s' \"$!\" > \"$OWNER_PID\"\nexit 0", + ); + let started = Instant::now(); + let result = clipboard::run( + helper.command(), + Some(b"clipboard text"), + 0, + started + clipboard::TIMEOUT, + ); + let elapsed = started.elapsed(); + let owner = helper.owner_pid(); + assert!(result.is_ok(), "forking helper must succeed: {result:?}"); + assert!( + elapsed < Duration::from_secs(2), + "must not wait on the owner: {elapsed:?}" + ); + let stat = std::fs::read_to_string(format!("/proc/{owner}/stat")).expect("owner must survive"); + assert!( + !stat[stat.rfind(')').expect("process name") + 2..].starts_with('Z'), + "successful owner must not be killed" + ); + let mut status = 0; + assert_eq!( + unsafe { libc::waitpid(owner as i32, &mut status, libc::WNOHANG) }, + -1 + ); + assert_eq!( + std::io::Error::last_os_error().raw_os_error(), + Some(libc::ECHILD) + ); +} + +#[test] +fn clipboard_large_image_round_trip() { + let mut payload: Vec = (0..6 * 1024 * 1024).map(|i| (i % 251) as u8).collect(); + payload[..8].copy_from_slice(b"\x89PNG\r\n\x1a\n"); + let bytes = clipboard::run( + Command::new("cat"), + Some(&payload), + payload.len(), + Instant::now() + clipboard::TIMEOUT, + ) + .expect("large image round trip"); + assert_eq!(bytes, payload); + assert!(bytes_match_image_signature("png", &bytes)); +} + +#[test] +fn clipboard_failed_wrapper_kills_descendants() { + let helper = ForkingHelper::new("sleep 30 &\nprintf '%s' \"$!\" > \"$OWNER_PID\"\nexit 7"); + let started = Instant::now(); + let result = clipboard::run(helper.command(), None, 16, started + clipboard::TIMEOUT); + let owner = helper.owner_pid(); + assert!(result.is_err(), "failed wrapper must fail"); + assert!( + started.elapsed() < Duration::from_secs(2), + "failure must not wait for inherited stdout" + ); + // A container's init may leave the killed descendant as a zombie. + let deadline = Instant::now() + Duration::from_secs(2); + while helper_is_running(owner) && Instant::now() < deadline { + std::thread::sleep(Duration::from_millis(5)); + } + assert!( + !helper_is_running(owner), + "failed wrapper's descendant must not survive" + ); +} + +fn helper_is_running(pid: u32) -> bool { + std::fs::read_to_string(format!("/proc/{pid}/stat")).is_ok_and(|stat| { + stat.rsplit_once(") ") + .is_some_and(|(_, state)| !state.starts_with('Z')) + }) +} + +#[test] +fn clipboard_zombie_helper_is_not_running() { + let mut child = Command::new("sh") + .args(["-c", "exit 0"]) + .spawn() + .expect("child"); + let pid = child.id(); + let deadline = Instant::now() + Duration::from_secs(2); + let mut zombie = false; + while Instant::now() < deadline { + zombie = std::fs::read_to_string(format!("/proc/{pid}/stat")).is_ok_and(|stat| { + stat.rsplit_once(") ") + .is_some_and(|(_, state)| state.starts_with('Z')) + }); + if zombie { + break; + } + std::thread::sleep(Duration::from_millis(5)); + } + let exists = process_exists(pid); + let running = helper_is_running(pid); + let _ = child.kill(); + let _ = child.wait(); + assert!(zombie && exists, "test must observe an unreaped zombie"); + assert!( + !running, + "a zombie cannot keep a clipboard pipe or selection alive" + ); +} diff --git a/src/selection.rs b/src/selection.rs index 9b47b2e06..52bfe9d8c 100644 --- a/src/selection.rs +++ b/src/selection.rs @@ -359,12 +359,23 @@ fn should_prefer_osc52() -> bool { /// Some terminals still only honor BEL-terminated OSC 52 writes, so herdr /// emits BEL here even though ST works in newer emulators. /// -/// Returns false when the clipboard already holds the same text or output fails. +/// Returns true when the text already matches or a clipboard write is dispatched. pub fn write_osc52_bytes(bytes: &[u8]) -> bool { - let prefer_osc52 = should_prefer_osc52(); - if !prefer_osc52 && crate::platform::clipboard_text_matches(bytes) == Some(true) { + write_osc52_bytes_with_match( + bytes, + should_prefer_osc52(), + crate::platform::clipboard_text_matches, + ) +} + +fn write_osc52_bytes_with_match( + bytes: &[u8], + prefer_osc52: bool, + clipboard_text_matches: impl FnOnce(&[u8]) -> Option, +) -> bool { + if !prefer_osc52 && clipboard_text_matches(bytes) == Some(true) { tracing::debug!(bytes = bytes.len(), "suppressed duplicate clipboard write"); - return false; + return true; } if !prefer_osc52 && crate::platform::write_clipboard(bytes) { @@ -380,6 +391,14 @@ pub fn write_osc52_bytes(bytes: &[u8]) -> bool { mod tests { use super::*; + #[test] + fn duplicate_clipboard_copy_reports_success() { + assert!(write_osc52_bytes_with_match(b"same text", false, |bytes| { + assert_eq!(bytes, b"same text"); + Some(true) + })); + } + #[test] fn visible_rects_match_selection_cells_when_scrolled_and_reversed() { let inner = Rect::new(10, 5, 8, 6);