diff --git a/CHANGELOG.md b/CHANGELOG.md index 59b5043b..f080b349 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,9 @@ Versions follow [Semantic Versioning](https://semver.org/). ### Features - **Settings shows the installed version.** On Windows and Linux the desktop app showed its version nowhere. Settings now ends with it, and it can be copied into a bug report. - **Group and arrange hosts on the dashboard.** With many hosts the dashboard was one long grid in config order. Both apps now group it with `g`: each host goes under its first tag, untagged hosts last. In the desktop app, View options also sorts the cards by name or status, sections fold from their header, and a card's handle drags it, or moves it with the arrow keys, into an order of your own that is kept across restarts. +- **Drag files and folders between the SFTP panes, or onto the remote pane from your file manager.** In the desktop app, dragging a row to the other pane uploads or downloads it into the folder row it is dropped on, or else into the folder on show, and dragging a ticked row takes every ticked row along. Files and folders dropped from Explorer, Finder or a Linux file manager onto the remote pane upload to it. Escape cancels a drag. +- **Both apps transfer whole folders.** Upload and Download in the desktop app, and copy and paste in the terminal app, take a folder with everything in it, merge it into a folder of the same name and replace files that clash, and the desktop app asks first when the folder on show already has those names. Files and folders keep their permission bits, so scripts stay executable, and a copy stays writable for you so the next transfer can replace it; symbolic links and special files are skipped, and the transfer ends by saying how many items were skipped or failed and why the first one was. Cancel in the desktop app's progress strip, closing its tab, or Esc in the terminal app stops a transfer and removes the file it was part way through. +- **The file panes show when each file was last changed.** The desktop app's panes gain a Modified column, with the full date and time on hover, once the window is wide enough. The terminal app's file manager shows the time at the end of each row when its panel is wide enough. ### Bug Fixes - **Security: a server that offers a host key of another type than the one saved for it is refused.** OmnySSH compared the offered key only with saved keys of the same type. A host saved with its Ed25519 key that suddenly offered only an RSA or ECDSA key was therefore taken for a new host: the key was saved and the login went ahead, password included, and the dashboard did this in the background without anyone connecting. Someone intercepting the connection could get past the saved key this way. Like `ssh`, OmnySSH now refuses any key that matches none of those saved for the host, whatever its type, and says which type is saved. A server that really dropped the type of key it was saved with needs its old key removed once, as `ssh` also asks. @@ -27,6 +30,9 @@ Versions follow [Semantic Versioning](https://semver.org/). - **A snippet with the same tag twice no longer empties the Snippets screen.** Tags typed as `db, db` in a snippet form, or written so into `snippets.toml`, left the desktop app's Snippets screen blank, so the snippet could not even be opened to fix it. The Run dialog broke the same way when two hosts share a name, as two `Host` blocks with one name in `~/.ssh/config` give. It now lists that name once, for the host a run goes to. The host picker and the command palette listed such a name twice, and either row opened the first host; they now list it once too. - **Sizes in the terminal app's file manager stay in their column next to wide file names.** A name with Chinese, Japanese or Korean characters or emoji, which take two cells each, pushed its size past the panel's edge, so a 14-byte file read `1`. Names are now padded and cut by the cells they take. - **Closing the host picker with Esc gives the keyboard back to the terminal.** In the desktop app, clicking SFTP or Terminal in the sidebar over an open terminal and then pressing Esc left the keyboard on the sidebar, so typing reached nothing until you clicked in the terminal. The command palette's sidebar button had the same problem, and so did clicking the terminal's own tab. A terminal that finishes connecting while the palette is open, or comes into view just as a passphrase prompt opens, no longer takes the keyboard from it. +- **Pasting a folder in the terminal app copies it instead of leaving an empty file.** The file manager handed a folder to the transfer as if it were a file, so the paste failed after creating an empty file of that name on the other side. +- **A paste or delete of more than 64 items in the terminal app handles them all.** They went to the SFTP queue at once, and whatever did not fit in its 64 places was dropped without a word while the app kept waiting for it. They now go one at a time. +- **The terminal app no longer crashes when a file grows while it is copied.** A log still being written to could end up larger than when its transfer started, which took the progress bar past 100% and brought the app down. --- diff --git a/README.md b/README.md index 1ce1a9ce..ff4b51bc 100644 --- a/README.md +++ b/README.md @@ -55,7 +55,7 @@ Cards for every host with CPU, RAM and disk bars, uptime, OS version, top proces Full PTY sessions in tabs. Open as many servers as you need, switch between them from the sidebar, and keep them running while you work in the dashboard. ### Two panel SFTP -Local on the left, remote on the right. Tick the files you want and move them across, watch the progress bar, select many at once. Nobody remembers `scp -r` syntax anyway. +Local on the left, remote on the right. Tick the files and folders you want and move them across, or just drag them from one side to the other, or straight from your file manager onto the server. Watch the progress bar, select many at once, see when each file was last changed. Nobody remembers `scp -r` syntax anyway. ### Snippets Save the commands you paste every week. Pick a snippet, tick the hosts to send it to, and it runs on all of them at once. Snippets take parameters, so `sudo systemctl restart {{service}}` asks you for the name. diff --git a/crates/omnyssh-core/src/ssh/sftp.rs b/crates/omnyssh-core/src/ssh/sftp.rs index f92f5468..b8ddedf1 100644 --- a/crates/omnyssh-core/src/ssh/sftp.rs +++ b/crates/omnyssh-core/src/ssh/sftp.rs @@ -6,11 +6,16 @@ //! All operations are non-blocking from the UI perspective. //! Progress is reported via [`CoreEvent::FileTransferProgress`]. +use std::future::Future; +use std::path::{Path, PathBuf}; +use std::sync::atomic::{AtomicU64, Ordering}; +use std::sync::Arc; use std::time::Duration; use anyhow::Context; -use tokio::io::{AsyncReadExt, AsyncWriteExt}; -use tokio::sync::mpsc; +use russh_sftp::protocol::{FileAttributes, FileType, OpenFlags}; +use tokio::io::{AsyncRead, AsyncReadExt, AsyncWriteExt}; +use tokio::sync::{mpsc, Notify}; use tokio::time; use crate::event::{CoreEvent, TransferId}; @@ -36,6 +41,30 @@ pub struct FileEntry { pub size: u64, /// `true` when this entry is a directory. pub is_dir: bool, + /// Last modification time, in seconds since the Unix epoch, when known. + pub modified: Option, +} + +impl FileEntry { + /// The synthetic `..` entry pointing at `parent`. + fn parent(parent: &str) -> Self { + Self { + name: "..".to_string(), + path: parent.to_string(), + size: 0, + is_dir: true, + modified: None, + } + } +} + +/// Seconds since the Unix epoch for a file time, or `None` if it is unavailable. +fn unix_secs(time: std::io::Result) -> Option { + let time = time.ok()?; + match time.duration_since(std::time::UNIX_EPOCH) { + Ok(d) => i64::try_from(d.as_secs()).ok(), + Err(e) => i64::try_from(e.duration().as_secs()).ok().map(|s| -s), + } } // --------------------------------------------------------------------------- @@ -46,13 +75,18 @@ pub struct FileEntry { pub enum SftpCommand { /// List the entries in a remote directory. ListDir(String), - /// Download a remote file to a local path. + /// Download a remote file, or a remote folder with everything in it, to a local + /// path. An existing folder is merged into and an existing file replaced; new + /// files and folders get the source's permission bits. Inside a folder, symlinks, + /// special files and names this system cannot create are skipped; the transfer + /// goes on past them and past failed files, and its [`CoreEvent::SftpOpDone`] + /// error counts them. [`SftpManager::cancel`] stops it. Download { remote: String, local: String, transfer_id: TransferId, }, - /// Upload a local file to a remote path. + /// Upload a local file or folder to a remote path, by the rules of `Download`. Upload { local: String, remote: String, @@ -70,6 +104,9 @@ pub enum SftpCommand { Disconnect, } +/// A command and the cancel count it was sent under (see [`SftpManager::cancel`]). +type Queued = (u64, SftpCommand); + // --------------------------------------------------------------------------- // SftpManager — handle held by App to communicate with the background task // --------------------------------------------------------------------------- @@ -80,7 +117,10 @@ pub enum SftpCommand { /// commands, and [`SftpManager::disconnect`] for a clean shutdown. #[derive(Debug)] pub struct SftpManager { - cmd_tx: mpsc::Sender, + cmd_tx: mpsc::Sender, + /// Shared with the task rather than queued: the transfer to stop holds the queue + /// until it ends. + cancels: Arc, } impl SftpManager { @@ -114,34 +154,51 @@ impl SftpManager { .await .map_err(|_| anyhow::anyhow!("SFTP did not start within {}s", OPEN_TIMEOUT.as_secs()))??; - let (cmd_tx, cmd_rx) = mpsc::channel::(64); + let (cmd_tx, cmd_rx) = mpsc::channel::(64); + let cancels = Arc::new(Cancels::default()); let host_name = host.name.clone(); // `session` and `sftp` are owned by this async block. If the task // panics, Rust's unwind machinery calls their Drop impls before the // panic propagates to tokio — the TCP connection is therefore always // released even in the panic path. No explicit catch_unwind needed. + let task_cancels = Arc::clone(&cancels); tokio::spawn(async move { let _ = event_tx .send(CoreEvent::SftpConnected { host_name: host_name.clone(), }) .await; - sftp_task_loop(session, sftp, cmd_rx, event_tx.clone()).await; + sftp_task_loop(session, sftp, cmd_rx, event_tx.clone(), task_cancels).await; tracing::info!("SFTP task for '{}' exited", host_name); }); - Ok(Self { cmd_tx }) + Ok(Self { cmd_tx, cancels }) } /// Enqueues a command (fire-and-forget). Silently drops if the task exited. pub fn send(&self, cmd: SftpCommand) { - let _ = self.cmd_tx.try_send(cmd); + let _ = self.cmd_tx.try_send((self.cancels.count(), cmd)); + } + + /// Cancels every transfer sent so far. The running one stops at its next step and + /// removes the file it was part way through; files it finished stay. Queued ones + /// stop before they start. Each still ends with its [`CoreEvent::SftpOpDone`], + /// whose error says it was cancelled. + pub fn cancel(&self) { + self.cancels.bump(); } - /// Sends [`SftpCommand::Disconnect`] and drops the sender. + /// Cancels the transfers, sends [`SftpCommand::Disconnect`] and drops the sender. pub fn disconnect(self) { - let _ = self.cmd_tx.try_send(SftpCommand::Disconnect); + self.send(SftpCommand::Disconnect); + } +} + +impl Drop for SftpManager { + // Nothing can follow a transfer's progress once its manager is gone. + fn drop(&mut self) { + self.cancel(); } } @@ -152,10 +209,15 @@ impl SftpManager { async fn sftp_task_loop( _ssh: SshSession, // kept alive to hold the SSH connection open sftp: russh_sftp::client::SftpSession, - mut cmd_rx: mpsc::Receiver, + mut cmd_rx: mpsc::Receiver, event_tx: mpsc::Sender, + cancels: Arc, ) { - while let Some(cmd) = cmd_rx.recv().await { + while let Some((sent, cmd)) = cmd_rx.recv().await { + let cancel = Cancel { + cancels: &cancels, + sent, + }; match cmd { SftpCommand::ListDir(path) => match do_list_dir(&sftp, &path).await { Ok(entries) => { @@ -166,7 +228,7 @@ async fn sftp_task_loop( Err(e) => { let _ = event_tx .send(CoreEvent::SftpDisconnected { - reason: format!("ListDir failed: {e}"), + reason: format!("ListDir failed: {e:#}"), }) .await; } @@ -177,9 +239,9 @@ async fn sftp_task_loop( local, transfer_id, } => { - let result = do_download(&sftp, &remote, &local, transfer_id, &event_tx) + let result = do_download(&sftp, &remote, &local, transfer_id, &event_tx, &cancel) .await - .map_err(|e| e.to_string()); + .map_err(|e| format!("{e:#}")); let _ = event_tx.send(CoreEvent::SftpOpDone { result }).await; } @@ -188,9 +250,9 @@ async fn sftp_task_loop( remote, transfer_id, } => { - let result = do_upload(&local, &sftp, &remote, transfer_id, &event_tx) + let result = do_upload(&local, &sftp, &remote, transfer_id, &event_tx, &cancel) .await - .map_err(|e| e.to_string()); + .map_err(|e| format!("{e:#}")); let _ = event_tx.send(CoreEvent::SftpOpDone { result }).await; } @@ -249,12 +311,7 @@ async fn do_list_dir( } else { &parent_str }; - entries.push(FileEntry { - name: "..".to_string(), - path: parent_str.to_string(), - size: 0, - is_dir: true, - }); + entries.push(FileEntry::parent(parent_str)); } for entry in read_dir { @@ -262,17 +319,14 @@ async fn do_list_dir( let ft = entry.file_type(); let meta = entry.metadata(); - let full_path = if path.ends_with('/') { - format!("{path}{name}") - } else { - format!("{path}/{name}") - }; + let full_path = join_remote(path, &name); entries.push(FileEntry { name, path: full_path, size: meta.size.unwrap_or(0), is_dir: ft.is_dir(), + modified: meta.mtime.map(i64::from), }); } @@ -294,110 +348,663 @@ async fn do_list_dir( Ok(entries) } +/// `name` joined onto the remote directory `dir`. +fn join_remote(dir: &str, name: &str) -> String { + if dir.ends_with('/') { + format!("{dir}{name}") + } else { + format!("{dir}/{name}") + } +} + +/// Whether a name the server gave can be created locally as itself: one plain path +/// component, so it neither climbs out of the destination nor, as Windows' `C:x` does, +/// replaces it. +fn is_safe_name(name: &str) -> bool { + let mut parts = Path::new(name).components(); + let single = matches!( + (parts.next(), parts.next()), + (Some(std::path::Component::Normal(part)), None) if part == name + ); + single && !name.contains('\0') && (!cfg!(windows) || is_windows_name(name)) +} + +/// Windows' own rules: no `:` (drives, alternate streams) or other reserved character, +/// no trailing dot or space (Win32 drops them, so `a.` would overwrite `a`), and no +/// device name, with or without an extension (`aux.c` opens the AUX device). +fn is_windows_name(name: &str) -> bool { + if name.chars().any(|c| c < ' ' || "<>:\"/\\|?*".contains(c)) || name.ends_with(['.', ' ']) { + return false; + } + let stem = name.split('.').next().unwrap_or(name).trim_end_matches(' '); + let upper = stem.to_ascii_uppercase(); + let device = match upper.get(..3) { + Some("CON" | "PRN" | "AUX" | "NUL") => upper.len() == 3, + Some("COM" | "LPT") => { + let port = &upper[3..]; + port.chars().count() == 1 && "0123456789\u{b9}\u{b2}\u{b3}".contains(port) + } + _ => false, + }; + !device +} + +/// `name` from the server joined onto the local folder `dir`, unless it would be +/// anything but a direct child of `dir`. +fn join_local(dir: &Path, name: &str) -> Option { + let path = dir.join(name); + (is_safe_name(name) && path.parent() == Some(dir)).then_some(path) +} + +/// Rejects a download whose own name, as the server lists it or as it ends `local`, +/// could not be created here as itself: the frontends build `local` from that name, and +/// on Windows `a\b` would turn into a folder and a file. +fn check_download_name(remote: &str, local: &str) -> anyhow::Result<()> { + let local_name = Path::new(local).file_name().and_then(|n| n.to_str()); + for name in [remote.rsplit('/').next(), local_name] { + let name = name.unwrap_or_default(); + if !is_safe_name(name) { + anyhow::bail!("'{name}' is not a name that can be created here"); + } + } + Ok(()) +} + +/// Rejects a transfer whose local path climbs out with `..` or either path holds a +/// null byte. +fn check_paths(local: &str, remote: &str) -> anyhow::Result<()> { + if Path::new(local) + .components() + .any(|c| c == std::path::Component::ParentDir) + { + anyhow::bail!("Local path contains '..': {local}"); + } + if local.contains('\0') || remote.contains('\0') { + anyhow::bail!("Path contains null bytes"); + } + Ok(()) +} + +// --------------------------------------------------------------------------- +// Transfers +// --------------------------------------------------------------------------- + +/// How deep a folder transfer goes: past any real tree, short of one a server makes +/// up to never end. +const MAX_DEPTH: usize = 64; + +/// How many times a manager's transfers were cancelled, and a wake-up for a step +/// waiting on that. +#[derive(Debug, Default)] +struct Cancels { + count: AtomicU64, + changed: Notify, +} + +impl Cancels { + fn count(&self) -> u64 { + self.count.load(Ordering::Relaxed) + } + + fn bump(&self) { + self.count.fetch_add(1, Ordering::Relaxed); + self.changed.notify_waiters(); + } +} + +/// Whether the transfer at hand was cancelled: the manager's cancel count has moved +/// on from the one its command was sent under. +struct Cancel<'a> { + cancels: &'a Cancels, + sent: u64, +} + +impl Cancel<'_> { + fn check(&self) -> anyhow::Result<()> { + if self.cancels.count() == self.sent { + Ok(()) + } else { + Err(Cancelled.into()) + } + } + + /// Resolves once the transfer is cancelled, for a step a server can keep going. + async fn cancelled(&self) { + loop { + // Made before the check, so a cancel in between still wakes it. + let changed = self.cancels.changed.notified(); + if self.check().is_err() { + return; + } + changed.await; + } + } +} + +/// The error a cancelled transfer ends with. +#[derive(Debug, thiserror::Error)] +#[error("Transfer cancelled")] +struct Cancelled; + +/// What a folder transfer left out. It goes on past every skip and failure and +/// reports them in its one result. +#[derive(Debug, Default)] +struct Skipped { + count: usize, + first: Option, +} + +impl Skipped { + fn add(&mut self, path: &str, reason: impl std::fmt::Display) { + self.count += 1; + self.first + .get_or_insert_with(|| format!("'{path}': {reason}")); + } + + fn into_result(self) -> anyhow::Result<()> { + let Some(first) = self.first else { + return Ok(()); + }; + let items = if self.count == 1 { "item" } else { "items" }; + anyhow::bail!("{} {items} skipped or failed, first {first}", self.count) + } +} + +/// One folder or file of a folder transfer. +#[derive(Debug, PartialEq)] +struct Planned { + src: String, + dst: String, + size: u64, + /// The source's permission bits, when known. + mode: Option, +} + +/// Everything a folder transfer creates, worked out before the first byte moves so +/// the progress bar covers the whole folder. `dirs` lists parents before children. +#[derive(Debug, Default)] +struct TreePlan { + dirs: Vec, + files: Vec, +} + +impl TreePlan { + /// Sizes come from the server, which may claim anything. + fn total(&self) -> u64 { + self.files + .iter() + .fold(0, |total, f| total.saturating_add(f.size)) + } +} + +/// Byte progress across every file of one transfer. +struct Progress<'a> { + transfer_id: TransferId, + done: u64, + total: u64, + event_tx: &'a mpsc::Sender, +} + +impl<'a> Progress<'a> { + /// Starts the count. Its first tick tells the frontends that planning is over. + async fn start( + transfer_id: TransferId, + total: u64, + event_tx: &'a mpsc::Sender, + ) -> Self { + let mut progress = Self { + transfer_id, + done: 0, + total, + event_tx, + }; + progress.advance(0).await; + progress + } + + async fn advance(&mut self, n: usize) { + self.done += n as u64; + let _ = self + .event_tx + .send(CoreEvent::FileTransferProgress( + self.transfer_id, + self.done, + self.total, + )) + .await; + } +} + +/// A local file's permission bits; Windows has none to give. +fn local_mode(meta: &std::fs::Metadata) -> Option { + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + Some(meta.permissions().mode() & 0o777) + } + #[cfg(not(unix))] + { + let _ = meta; + None + } +} + +/// A remote entry's permission bits, when the server sent them. +fn remote_mode(attrs: &FileAttributes) -> Option { + attrs.permissions.map(|p| p & 0o777) +} + +/// Walks the local folder `root`, whose permission bits are `mode`, mapping it onto +/// the remote folder `dst_root`. Nothing is followed: a symlink is skipped, as is +/// anything else that is not a plain file or folder. +async fn plan_local_tree( + root: &str, + mode: Option, + dst_root: &str, + cancel: &Cancel<'_>, + skipped: &mut Skipped, +) -> anyhow::Result { + let mut plan = TreePlan::default(); + let mut stack = vec![(PathBuf::from(root), dst_root.to_string(), mode, 0)]; + while let Some((src, dst, mode, depth)) = stack.pop() { + cancel.check()?; + // Every name on the way is UTF-8, so the path converts losslessly. + let src_str = src.to_string_lossy().into_owned(); + let mut read_dir = match tokio::fs::read_dir(&src).await { + Ok(read_dir) => read_dir, + Err(e) => { + skipped.add(&src_str, format_args!("read local dir: {e}")); + continue; + } + }; + plan.dirs.push(Planned { + src: src_str.clone(), + dst: dst.clone(), + size: 0, + mode, + }); + loop { + let entry = match read_dir.next_entry().await { + Ok(Some(entry)) => entry, + Ok(None) => break, + Err(e) => { + skipped.add(&src_str, format_args!("read local dir: {e}")); + break; + } + }; + let path = entry.path(); + let shown = path.to_string_lossy(); + let Ok(name) = entry.file_name().into_string() else { + skipped.add(&shown, "name is not valid UTF-8"); + continue; + }; + // `DirEntry::metadata` does not follow symlinks. + let meta = match entry.metadata().await { + Ok(meta) => meta, + Err(e) => { + skipped.add(&shown, e); + continue; + } + }; + let remote = join_remote(&dst, &name); + let file_type = meta.file_type(); + if file_type.is_dir() && depth < MAX_DEPTH { + stack.push((path.clone(), remote, local_mode(&meta), depth + 1)); + } else if file_type.is_dir() { + skipped.add(&shown, "nested too deep"); + } else if file_type.is_file() { + plan.files.push(Planned { + src: shown.into_owned(), + dst: remote, + size: meta.len(), + mode: local_mode(&meta), + }); + } else if file_type.is_symlink() { + skipped.add(&shown, "symbolic link, not followed"); + } else { + skipped.add(&shown, "not a regular file or folder"); + } + } + } + Ok(plan) +} + +/// Walks the remote folder `root`, whose permission bits are `mode`, mapping it onto +/// the local folder `dst_root`; `list` reads one remote folder. A symlink is skipped +/// (servers report links, not their targets), as is anything that is not a plain +/// file or folder and any name that cannot be created here. +async fn plan_remote_tree( + mut list: F, + root: &str, + mode: Option, + dst_root: &Path, + cancel: &Cancel<'_>, + skipped: &mut Skipped, +) -> anyhow::Result +where + F: FnMut(String) -> Fut, + Fut: Future>>, +{ + let mut plan = TreePlan::default(); + let mut stack = vec![(root.to_string(), dst_root.to_path_buf(), mode, 0)]; + while let Some((src, dst, mode, depth)) = stack.pop() { + cancel.check()?; + // A server can answer one listing for ever. + let listing = tokio::select! { + listing = list(src.clone()) => listing, + () = cancel.cancelled() => return Err(Cancelled.into()), + }; + let entries = match listing { + Ok(entries) => entries, + Err(e) => { + skipped.add(&src, format_args!("read remote dir: {e:#}")); + continue; + } + }; + plan.dirs.push(Planned { + src: src.clone(), + dst: dst.to_string_lossy().into_owned(), + size: 0, + mode, + }); + for (name, attrs) in entries { + let path = join_remote(&src, &name); + let Some(local) = join_local(&dst, &name) else { + skipped.add(&path, "name not allowed here"); + continue; + }; + match attrs.file_type() { + FileType::Dir if depth < MAX_DEPTH => { + stack.push((path, local, remote_mode(&attrs), depth + 1)); + } + FileType::Dir => skipped.add(&path, "nested too deep"), + FileType::File => plan.files.push(Planned { + src: path, + dst: local.to_string_lossy().into_owned(), + size: attrs.size.unwrap_or(0), + mode: remote_mode(&attrs), + }), + FileType::Symlink => skipped.add(&path, "symbolic link, not followed"), + FileType::Other => skipped.add(&path, "not a regular file or folder"), + } + } + } + Ok(plan) +} + +/// Downloads a remote file or, recursively, a remote folder to `local`. async fn do_download( sftp: &russh_sftp::client::SftpSession, remote: &str, local: &str, transfer_id: TransferId, event_tx: &mpsc::Sender, + cancel: &Cancel<'_>, ) -> anyhow::Result<()> { - // Guard against path traversal in the local destination. - if std::path::Path::new(local) - .components() - .any(|c| c == std::path::Component::ParentDir) + check_paths(local, remote)?; + check_download_name(remote, local)?; + cancel.check()?; + + // Size, mode and type best-effort: a failed stat downloads as a file, and the open + // below reports why. Without permissions the type is unknown too. + let meta = sftp.metadata(remote).await.ok(); + let mode = meta.as_ref().and_then(remote_mode); + match meta + .as_ref() + .map(|m| (m.permissions.is_some(), m.file_type())) { - anyhow::bail!("Download destination path contains '..': {local}"); - } - if local.contains('\0') || remote.contains('\0') { - anyhow::bail!("Path contains null bytes"); + Some((_, FileType::Dir)) => {} + Some((true, FileType::Symlink | FileType::Other)) => { + anyhow::bail!("'{remote}' is not a regular file or folder") + } + _ => { + let total = meta.and_then(|m| m.size).unwrap_or(0); + let mut progress = Progress::start(transfer_id, total, event_tx).await; + return download_file(sftp, remote, local, mode, &mut progress, cancel).await; + } } - // Fetch size for progress (best-effort). - let total = sftp - .metadata(remote) - .await - .map(|m| m.size.unwrap_or(0)) - .unwrap_or(0); + let mut skipped = Skipped::default(); + let list = |dir: String| async move { + let entries = sftp.read_dir(dir).await?; + anyhow::Ok(entries.map(|e| (e.file_name(), e.metadata())).collect()) + }; + let plan = plan_remote_tree(list, remote, mode, Path::new(local), cancel, &mut skipped).await?; + let mut progress = Progress::start(transfer_id, plan.total(), event_tx).await; + for dir in &plan.dirs { + cancel.check()?; + if let Err(e) = create_local_dir(&dir.dst, dir.mode).await { + skipped.add(&dir.src, format_args!("create local dir: {e}")); + } + } + for file in &plan.files { + match download_file(sftp, &file.src, &file.dst, file.mode, &mut progress, cancel).await { + Err(e) if e.is::() => return Err(e), + Err(e) => skipped.add(&file.src, format_args!("{e:#}")), + Ok(()) => {} + } + } + skipped.into_result() +} - let mut remote_file = sftp +async fn download_file( + sftp: &russh_sftp::client::SftpSession, + remote: &str, + local: &str, + mode: Option, + progress: &mut Progress<'_>, + cancel: &Cancel<'_>, +) -> anyhow::Result<()> { + cancel.check()?; + let remote_file = sftp .open(remote) .await .context("open remote file for download")?; - let mut local_file = tokio::fs::File::create(local) + write_local(remote_file, local, mode, progress, cancel).await +} + +/// Writes everything `source` yields to the local file `local`. A cancel or a failure +/// removes the part written so far, which would otherwise pass for the whole file. +async fn write_local( + mut source: impl AsyncRead + Unpin, + local: &str, + mode: Option, + progress: &mut Progress<'_>, + cancel: &Cancel<'_>, +) -> anyhow::Result<()> { + // A cancel during the remote open leaves a file already there as it was. + cancel.check()?; + let mut local_file = create_local_file(local, mode) .await .context("create local file")?; let mut buf = vec![0u8; 65_536]; - let mut done: u64 = 0; - - loop { - let n = remote_file - .read(&mut buf) - .await - .context("read remote file")?; - if n == 0 { - break; + let copied = async { + loop { + cancel.check()?; + // A stalled server would otherwise hold a cancel until its request times out. + let n = tokio::select! { + n = source.read(&mut buf) => n.context("read remote file")?, + () = cancel.cancelled() => return Err(Cancelled.into()), + }; + if n == 0 { + return Ok(()); + } + local_file + .write_all(&buf[..n]) + .await + .context("write local file")?; + progress.advance(n).await; } - local_file - .write_all(&buf[..n]) - .await - .context("write local file")?; - done += n as u64; - let _ = event_tx - .send(CoreEvent::FileTransferProgress(transfer_id, done, total)) - .await; } + .await; - Ok(()) + // The last write may still be in flight; this is where it fails, if it does. + let flushed = local_file.flush().await.context("write local file"); + let result = copied.and(flushed); + if result.is_err() { + drop(local_file); + let _ = tokio::fs::remove_file(local).await; + } + result +} + +/// Creates or truncates the local file `path`. A new file gets the permission bits +/// `mode` less the umask, as sftp gives it, owner write included so the next transfer +/// can replace it; an existing one keeps its own. +async fn create_local_file(path: &str, mode: Option) -> std::io::Result { + let mut options = tokio::fs::OpenOptions::new(); + options.write(true).create(true).truncate(true); + #[cfg(unix)] + if let Some(mode) = mode { + options.mode(mode | 0o200); + } + #[cfg(not(unix))] + let _ = mode; + options.open(path).await } +/// Creates the local folder `path`, or merges into the one there. A new folder gets +/// the permission bits `mode` less the umask, owner access added so it can be filled. +async fn create_local_dir(path: &str, mode: Option) -> std::io::Result<()> { + let mut builder = tokio::fs::DirBuilder::new(); + builder.recursive(true); + #[cfg(unix)] + if let Some(mode) = mode { + builder.mode(mode | 0o700); + } + #[cfg(not(unix))] + let _ = mode; + builder.create(path).await +} + +/// Uploads a local file or, recursively, a local folder to `remote`. async fn do_upload( local: &str, sftp: &russh_sftp::client::SftpSession, remote: &str, transfer_id: TransferId, event_tx: &mpsc::Sender, + cancel: &Cancel<'_>, ) -> anyhow::Result<()> { - // Guard against path traversal in the local source. - if std::path::Path::new(local) - .components() - .any(|c| c == std::path::Component::ParentDir) - { - anyhow::bail!("Upload source path contains '..': {local}"); + check_paths(local, remote)?; + cancel.check()?; + + // Follows a symlink: a linked folder the user picked is uploaded as a folder. + let meta = tokio::fs::metadata(local) + .await + .context("open local file for upload")?; + let mode = local_mode(&meta); + if meta.is_file() { + let mut progress = Progress::start(transfer_id, meta.len(), event_tx).await; + return upload_file(local, sftp, remote, mode, &mut progress, cancel).await; } - if local.contains('\0') || remote.contains('\0') { - anyhow::bail!("Path contains null bytes"); + if !meta.is_dir() { + anyhow::bail!("'{local}' is not a regular file or folder"); + } + + let mut skipped = Skipped::default(); + let plan = plan_local_tree(local, mode, remote, cancel, &mut skipped).await?; + let mut progress = Progress::start(transfer_id, plan.total(), event_tx).await; + for dir in &plan.dirs { + cancel.check()?; + if let Err(e) = create_remote_dir(sftp, &dir.dst, dir.mode).await { + skipped.add(&dir.src, format_args!("{e:#}")); + } + } + for file in &plan.files { + match upload_file(&file.src, sftp, &file.dst, file.mode, &mut progress, cancel).await { + Err(e) if e.is::() => return Err(e), + Err(e) => skipped.add(&file.src, format_args!("{e:#}")), + Ok(()) => {} + } + } + skipped.into_result() +} + +/// Creates the remote folder `path`, or merges into the one there. A new folder gets +/// the permission bits `mode`, owner access added so it can be filled. +async fn create_remote_dir( + sftp: &russh_sftp::client::SftpSession, + path: &str, + mode: Option, +) -> anyhow::Result<()> { + if let Err(e) = sftp.create_dir(path).await { + if sftp + .metadata(path) + .await + .is_ok_and(|m| m.file_type().is_dir()) + { + return Ok(()); + } + return Err(e).context("create remote dir"); + } + // SFTP's mkdir takes a mode, but this client sends none. A server may refuse to set + // one; the folder is there all the same. + if let Some(mode) = mode { + let attrs = FileAttributes { + permissions: Some(mode | 0o700), + ..FileAttributes::empty() + }; + let _ = sftp.set_metadata(path, attrs).await; } + Ok(()) +} +async fn upload_file( + local: &str, + sftp: &russh_sftp::client::SftpSession, + remote: &str, + mode: Option, + progress: &mut Progress<'_>, + cancel: &Cancel<'_>, +) -> anyhow::Result<()> { + cancel.check()?; let mut local_file = tokio::fs::File::open(local) .await .context("open local file for upload")?; - let total = local_file.metadata().await.map(|m| m.len()).unwrap_or(0); - + // A new file gets the source's mode less the server's umask, owner write included + // so the next upload can replace it; an existing one keeps its own. + let attrs = FileAttributes { + permissions: mode.map(|mode| mode | 0o200), + ..FileAttributes::empty() + }; + let flags = OpenFlags::CREATE | OpenFlags::TRUNCATE | OpenFlags::WRITE; let mut remote_file = sftp - .create(remote) + .open_with_flags_and_attributes(remote, flags, attrs) .await .context("create remote file for upload")?; let mut buf = vec![0u8; 65_536]; - let mut done: u64 = 0; - - loop { - let n = local_file.read(&mut buf).await.context("read local file")?; - if n == 0 { - break; + let copied = async { + loop { + cancel.check()?; + let n = local_file.read(&mut buf).await.context("read local file")?; + if n == 0 { + return Ok(()); + } + // A stalled server would otherwise hold a cancel until its request times out. + tokio::select! { + written = remote_file.write_all(&buf[..n]) => { + written.context("write remote file")?; + } + () = cancel.cancelled() => return Err(Cancelled.into()), + } + progress.advance(n).await; } - remote_file - .write_all(&buf[..n]) - .await - .context("write remote file")?; - done += n as u64; - let _ = event_tx - .send(CoreEvent::FileTransferProgress(transfer_id, done, total)) - .await; } + .await; - Ok(()) + if copied.is_err() { + // Bounded: a server that stalled the copy may not answer these either. + let _ = time::timeout(Duration::from_secs(5), async { + // Closed first, or the handle would close only after the removal. + let _ = remote_file.shutdown().await; + let _ = sftp.remove_file(remote).await; + }) + .await; + } + copied } async fn do_read_preview( @@ -436,12 +1043,7 @@ pub async fn list_local_dir(path: &str) -> anyhow::Result> { } else { &parent_str }; - entries.push(FileEntry { - name: "..".to_string(), - path: parent_str.to_string(), - size: 0, - is_dir: true, - }); + entries.push(FileEntry::parent(parent_str)); } while let Some(entry) = read_dir @@ -462,6 +1064,7 @@ pub async fn list_local_dir(path: &str) -> anyhow::Result> { path: path_str, size, is_dir, + modified: meta.and_then(|m| unix_secs(m.modified())), }); } @@ -538,6 +1141,881 @@ mod tests { assert!(drive_roots(0).is_empty()); } + fn no_cancel(cancels: &Cancels) -> Cancel<'_> { + Cancel { cancels, sent: 0 } + } + + fn attrs(permissions: u32, size: u64) -> FileAttributes { + FileAttributes { + permissions: Some(permissions), + size: Some(size), + ..FileAttributes::empty() + } + } + + #[tokio::test] + async fn local_listing_carries_modification_time() { + let dir = tempfile::tempdir().expect("tempdir"); + std::fs::write(dir.path().join("a.txt"), b"hello").expect("write scratch file"); + + let entries = list_local_dir(&dir.path().to_string_lossy()) + .await + .expect("list scratch dir"); + let file = entries + .iter() + .find(|e| e.name == "a.txt") + .expect("file listed"); + assert_eq!(file.size, 5); + assert!(!file.is_dir); + assert!(file.modified.is_some_and(|t| t > 0)); + let parent = entries.first().expect("parent entry"); + assert_eq!(parent.name, ".."); + assert!(parent.modified.is_none()); + } + + #[tokio::test] + async fn the_local_plan_maps_files_and_folders_and_reports_the_rest() { + let tmp = tempfile::tempdir().expect("tempdir"); + let dir = tmp.path(); + std::fs::create_dir_all(dir.join("sub").join("deep")).expect("create scratch tree"); + std::fs::write(dir.join("a.txt"), b"abc").expect("write scratch file"); + std::fs::write(dir.join("sub").join("deep").join("b.bin"), b"hello") + .expect("write scratch file"); + #[cfg(unix)] + { + std::os::unix::fs::symlink(dir, dir.join("sub").join("loop")).expect("symlink"); + std::os::unix::net::UnixListener::bind(dir.join("sock")).expect("socket"); + } + + let cancels = Cancels::default(); + let mut skipped = Skipped::default(); + let root = dir.to_str().expect("utf-8 temp dir"); + let plan = plan_local_tree( + root, + Some(0o750), + "/srv/up", + &no_cancel(&cancels), + &mut skipped, + ) + .await + .expect("plan tree"); + assert_eq!(plan.dirs[0].dst, "/srv/up"); + assert_eq!(plan.dirs[0].mode, Some(0o750)); + let mut dirs: Vec<_> = plan.dirs.iter().map(|d| d.dst.as_str()).collect(); + dirs.sort(); + assert_eq!(dirs, ["/srv/up", "/srv/up/sub", "/srv/up/sub/deep"]); + // Parents come before their children, so creating in order never fails. + let sub = plan.dirs.iter().position(|d| d.dst == "/srv/up/sub"); + let deep = plan.dirs.iter().position(|d| d.dst == "/srv/up/sub/deep"); + assert!(sub < deep); + let mut files: Vec<_> = plan + .files + .iter() + .map(|f| (f.dst.as_str(), f.size, f.mode.is_some())) + .collect(); + files.sort(); + let moded = cfg!(unix); + assert_eq!( + files, + [ + ("/srv/up/a.txt", 3, moded), + ("/srv/up/sub/deep/b.bin", 5, moded) + ] + ); + assert_eq!(plan.total(), 8); + // The link back up and the socket are left out, and said so. + assert_eq!(skipped.count, if cfg!(unix) { 2 } else { 0 }); + } + + #[cfg(target_os = "linux")] + #[tokio::test] + async fn a_local_name_that_is_not_utf8_is_reported_not_fatal() { + use std::os::unix::ffi::OsStrExt; + let tmp = tempfile::tempdir().expect("tempdir"); + std::fs::write(tmp.path().join("ok.txt"), b"ok").expect("write"); + let bad = std::ffi::OsStr::from_bytes(b"bad\xff.txt"); + std::fs::write(tmp.path().join(bad), b"bad").expect("write"); + + let cancels = Cancels::default(); + let mut skipped = Skipped::default(); + let root = tmp.path().to_str().expect("utf-8 temp dir"); + let plan = plan_local_tree(root, None, "/up", &no_cancel(&cancels), &mut skipped) + .await + .expect("plan tree"); + assert_eq!(plan.files.len(), 1); + assert_eq!(plan.files[0].dst, "/up/ok.txt"); + let err = skipped.into_result().expect_err("the bad name is reported"); + assert!( + err.to_string().ends_with("': name is not valid UTF-8"), + "{err}" + ); + } + + /// A remote file system: folder -> listing. A folder not in it cannot be read. + fn fake_server( + tree: Vec<(&'static str, Vec<(&'static str, FileAttributes)>)>, + ) -> impl FnMut(String) -> std::future::Ready>> + { + move |dir| { + let listing = tree + .iter() + .find(|(path, _)| *path == dir) + .map(|(_, entries)| { + entries + .iter() + .map(|(name, attrs)| (name.to_string(), attrs.clone())) + .collect() + }); + std::future::ready(listing.ok_or_else(|| anyhow::anyhow!("Permission denied"))) + } + } + + #[tokio::test] + async fn the_remote_plan_maps_a_tree_and_reports_what_it_leaves_out() { + let server = fake_server(vec![ + ( + "/srv/app", + vec![ + ("run.sh", attrs(0o100_755, 3)), + ("conf", attrs(0o40_700, 0)), + ("current", attrs(0o120_777, 0)), + ("fifo", attrs(0o10_644, 0)), + ("a/b", attrs(0o100_644, 1)), + ], + ), + ( + "/srv/app/conf", + vec![ + ("app.toml", attrs(0o100_600, 5)), + ("locked", attrs(0o40_000, 0)), + ], + ), + ]); + let dst = Path::new("dl").join("app"); + let cancels = Cancels::default(); + let mut skipped = Skipped::default(); + let plan = plan_remote_tree( + server, + "/srv/app", + Some(0o755), + &dst, + &no_cancel(&cancels), + &mut skipped, + ) + .await + .expect("plan tree"); + + let local = |rel: &[&str]| { + rel.iter() + .fold(dst.clone(), |p, part| p.join(part)) + .to_string_lossy() + .into_owned() + }; + let dirs: Vec<_> = plan.dirs.iter().map(|d| (d.dst.clone(), d.mode)).collect(); + assert_eq!( + dirs, + [(local(&[]), Some(0o755)), (local(&["conf"]), Some(0o700))] + ); + assert_eq!( + plan.files, + [ + Planned { + src: "/srv/app/run.sh".into(), + dst: local(&["run.sh"]), + size: 3, + mode: Some(0o755), + }, + Planned { + src: "/srv/app/conf/app.toml".into(), + dst: local(&["conf", "app.toml"]), + size: 5, + mode: Some(0o600), + }, + ] + ); + // The symlink, the FIFO, the name with a separator and the unreadable folder: + // left out, counted, and the first one named. + assert_eq!(skipped.count, 4); + let err = skipped.into_result().expect_err("skips are reported"); + assert_eq!( + err.to_string(), + "4 items skipped or failed, first '/srv/app/current': symbolic link, not followed" + ); + } + + #[test] + fn sizes_a_server_makes_up_do_not_overflow_the_total() { + let file = |size| Planned { + src: String::new(), + dst: String::new(), + size, + mode: None, + }; + let plan = TreePlan { + dirs: Vec::new(), + files: vec![file(u64::MAX), file(2)], + }; + assert_eq!(plan.total(), u64::MAX); + } + + #[tokio::test] + async fn the_remote_plan_stops_at_the_depth_cap() { + // A server that makes up one more folder at every level. + let server = |_dir: String| { + std::future::ready(anyhow::Ok(vec![("d".to_string(), attrs(0o40_755, 0))])) + }; + let cancels = Cancels::default(); + let mut skipped = Skipped::default(); + let plan = plan_remote_tree( + server, + "/x", + None, + Path::new("dl"), + &no_cancel(&cancels), + &mut skipped, + ) + .await + .expect("plan tree"); + assert_eq!(plan.dirs.len(), MAX_DEPTH + 1); + let err = skipped.into_result().expect_err("the cut is reported"); + assert!(err + .to_string() + .starts_with("1 item skipped or failed, first '/x/d/d/")); + assert!(err.to_string().ends_with("/d': nested too deep"), "{err}"); + } + + #[tokio::test] + async fn planning_stops_at_the_next_folder_once_cancelled() { + let cancels = Cancels::default(); + let server = |_dir: String| { + cancels.bump(); + std::future::ready(anyhow::Ok(vec![("d".to_string(), attrs(0o40_755, 0))])) + }; + let mut skipped = Skipped::default(); + let err = plan_remote_tree( + server, + "/x", + None, + Path::new("dl"), + &no_cancel(&cancels), + &mut skipped, + ) + .await + .expect_err("cancelled"); + assert!(err.is::()); + assert_eq!(cancels.count(), 1, "one folder read, no more"); + } + + #[tokio::test] + async fn a_cancel_stops_a_listing_that_never_ends() { + let cancels = Cancels::default(); + // A server that keeps the first listing going. + let server = + |_dir: String| std::future::pending::>>(); + let cancel = no_cancel(&cancels); + let mut skipped = Skipped::default(); + let plan = plan_remote_tree(server, "/x", None, Path::new("dl"), &cancel, &mut skipped); + let press_cancel = async { + cancels.bump(); + std::future::pending::<()>().await + }; + let stopped = time::timeout(Duration::from_secs(5), async { + tokio::select! { + biased; + result = plan => result, + () = press_cancel => unreachable!("never ends"), + } + }) + .await + .expect("planning stopped"); + assert!(stopped.expect_err("cancelled").is::()); + } + + #[test] + fn skips_are_counted_and_the_first_reason_kept_whole() { + assert!(Skipped::default().into_result().is_ok()); + + let mut skipped = Skipped::default(); + let cause = std::io::Error::from(std::io::ErrorKind::PermissionDenied); + let err = anyhow::Error::from(cause).context("open remote file for download"); + skipped.add("/srv/a", format_args!("{err:#}")); + let one = format!( + "1 item skipped or failed, first '/srv/a': open remote file for download: {}", + std::io::Error::from(std::io::ErrorKind::PermissionDenied) + ); + let mut again = Skipped::default(); + again.add("/srv/a", format_args!("{err:#}")); + assert_eq!(again.into_result().expect_err("one").to_string(), one); + + skipped.add("/srv/b", "symbolic link, not followed"); + assert_eq!( + skipped.into_result().expect_err("two").to_string(), + one.replacen("1 item", "2 items", 1) + ); + } + + #[test] + fn a_cancel_stops_what_was_sent_before_it_and_a_closed_manager_stops_all() { + let (cmd_tx, mut cmd_rx) = mpsc::channel(8); + let manager = SftpManager { + cmd_tx, + cancels: Arc::default(), + }; + let cancels = Arc::clone(&manager.cancels); + let cancelled = |sent| { + Cancel { + cancels: &cancels, + sent, + } + .check() + .is_err_and(|e| e.is::()) + }; + + manager.send(SftpCommand::ListDir("/before".into())); + manager.cancel(); + manager.send(SftpCommand::ListDir("/after".into())); + let (before, _) = cmd_rx.try_recv().expect("queued"); + let (after, _) = cmd_rx.try_recv().expect("queued"); + assert!(cancelled(before)); + assert!(!cancelled(after)); + + manager.disconnect(); + assert!(cancelled(after)); + assert!(matches!( + cmd_rx.try_recv(), + Ok((_, SftpCommand::Disconnect)) + )); + } + + /// Hands out `data` in one read and cancels the transfer as it does. + struct CancellingReader<'a> { + data: &'a [u8], + cancels: &'a Cancels, + } + + impl AsyncRead for CancellingReader<'_> { + fn poll_read( + mut self: std::pin::Pin<&mut Self>, + _: &mut std::task::Context<'_>, + buf: &mut tokio::io::ReadBuf<'_>, + ) -> std::task::Poll> { + let n = self.data.len().min(buf.remaining()); + buf.put_slice(&self.data[..n]); + self.data = &self.data[n..]; + self.cancels.bump(); + std::task::Poll::Ready(Ok(())) + } + } + + #[tokio::test] + async fn a_cancelled_download_removes_the_part_it_wrote() { + let tmp = tempfile::tempdir().expect("tempdir"); + let local = tmp.path().join("part.bin"); + let cancels = Cancels::default(); + let (tx, _rx) = mpsc::channel(8); + let mut progress = Progress::start(1, 100, &tx).await; + let source = CancellingReader { + data: b"the first chunk of many", + cancels: &cancels, + }; + + let local_str = local.to_str().expect("utf-8 temp dir"); + let err = write_local(source, local_str, None, &mut progress, &no_cancel(&cancels)) + .await + .expect_err("cancelled"); + assert!(err.is::()); + assert_eq!(progress.done, 23, "the chunk was written before the cancel"); + assert!(!local.exists(), "the partial file is gone"); + } + + #[tokio::test] + async fn a_cancel_before_the_first_byte_leaves_the_local_file_alone() { + let tmp = tempfile::tempdir().expect("tempdir"); + let local = tmp.path().join("keep.txt"); + std::fs::write(&local, "the user's own").expect("write"); + let cancels = Cancels::default(); + let cancel = no_cancel(&cancels); + cancels.bump(); + let (tx, _rx) = mpsc::channel(8); + let mut progress = Progress::start(1, 100, &tx).await; + + let local_str = local.to_str().expect("utf-8 temp dir"); + let err = write_local(&b"new"[..], local_str, None, &mut progress, &cancel) + .await + .expect_err("cancelled"); + assert!(err.is::()); + assert_eq!( + std::fs::read(&local).expect("still there"), + b"the user's own" + ); + } + + /// Hands out `data` once, then stalls like a server that stopped answering, or + /// fails like a dropped connection. + struct StoppingReader<'a> { + data: &'a [u8], + fail: bool, + } + + impl AsyncRead for StoppingReader<'_> { + fn poll_read( + mut self: std::pin::Pin<&mut Self>, + _: &mut std::task::Context<'_>, + buf: &mut tokio::io::ReadBuf<'_>, + ) -> std::task::Poll> { + if self.data.is_empty() { + return if self.fail { + std::task::Poll::Ready(Err(std::io::ErrorKind::ConnectionReset.into())) + } else { + std::task::Poll::Pending + }; + } + let n = self.data.len().min(buf.remaining()); + buf.put_slice(&self.data[..n]); + self.data = &self.data[n..]; + std::task::Poll::Ready(Ok(())) + } + } + + #[tokio::test] + async fn a_cancel_stops_a_download_the_server_stalled() { + let tmp = tempfile::tempdir().expect("tempdir"); + let local = tmp.path().join("stalled.bin"); + let cancels = Cancels::default(); + let cancel = no_cancel(&cancels); + let (tx, _rx) = mpsc::channel(8); + let mut progress = Progress::start(1, 100, &tx).await; + let source = StoppingReader { + data: b"a first chunk", + fail: false, + }; + + let local_str = local.to_str().expect("utf-8 temp dir"); + let copy = write_local(source, local_str, None, &mut progress, &cancel); + let press_cancel = async { + time::sleep(Duration::from_millis(50)).await; + cancels.bump(); + std::future::pending::<()>().await + }; + let err = time::timeout(Duration::from_secs(5), async { + tokio::select! { + biased; + result = copy => result, + () = press_cancel => unreachable!("never ends"), + } + }) + .await + .expect("the cancel did not wait for the server") + .expect_err("cancelled"); + assert!(err.is::()); + assert!(!local.exists(), "the partial file is gone"); + } + + #[tokio::test] + async fn a_download_that_breaks_off_leaves_no_partial_file() { + let tmp = tempfile::tempdir().expect("tempdir"); + let local = tmp.path().join("cut.bin"); + let cancels = Cancels::default(); + let (tx, _rx) = mpsc::channel(8); + let mut progress = Progress::start(1, 100, &tx).await; + let source = StoppingReader { + data: b"a first chunk", + fail: true, + }; + + let local_str = local.to_str().expect("utf-8 temp dir"); + let err = write_local(source, local_str, None, &mut progress, &no_cancel(&cancels)) + .await + .expect_err("broke off"); + assert!(!err.is::()); + assert!(!local.exists(), "no cut-off file passes for the whole one"); + } + + #[cfg(unix)] + fn mode_of(path: &Path) -> u32 { + use std::os::unix::fs::PermissionsExt; + std::fs::metadata(path).expect("stat").permissions().mode() & 0o777 + } + + #[cfg(unix)] + #[tokio::test] + async fn a_download_keeps_the_source_mode() { + let tmp = tempfile::tempdir().expect("tempdir"); + let cancels = Cancels::default(); + let (tx, _rx) = mpsc::channel(8); + let mut progress = Progress::start(1, 10, &tx).await; + + let script = tmp.path().join("run.sh"); + let script_str = script.to_str().expect("utf-8 temp dir"); + write_local( + &b"#!/bin/sh\n"[..], + script_str, + Some(0o755), + &mut progress, + &no_cancel(&cancels), + ) + .await + .expect("written"); + assert_eq!(std::fs::read(&script).expect("read"), b"#!/bin/sh\n"); + // The umask may take group and other bits, never the owner's. + assert_eq!(mode_of(&script) & 0o700, 0o700); + + // A read-only folder still gets the owner's access, to be filled. + let dir = tmp.path().join("ro"); + create_local_dir(dir.to_str().expect("utf-8"), Some(0o500)) + .await + .expect("created"); + assert_eq!(mode_of(&dir) & 0o700, 0o700); + // An existing folder is merged into. + create_local_dir(dir.to_str().expect("utf-8"), Some(0o755)) + .await + .expect("merged"); + } + + /// An SFTP session on this machine's own files through OpenSSH's sftp-server, run + /// with `args`, where one is installed. + #[cfg(unix)] + async fn local_sftp(args: &[&str]) -> Option { + let server = [ + "/usr/lib/openssh/sftp-server", + "/usr/libexec/openssh/sftp-server", + "/usr/libexec/sftp-server", + ] + .into_iter() + .find(|path| Path::new(path).exists())?; + let mut child = tokio::process::Command::new(server) + .args(args) + .stdin(std::process::Stdio::piped()) + .stdout(std::process::Stdio::piped()) + .spawn() + .ok()?; + // The server exits once the session drops its end of the pipes. + let pipes = tokio::io::join(child.stdout.take()?, child.stdin.take()?); + russh_sftp::client::SftpSession::new(pipes).await.ok() + } + + #[cfg(unix)] + #[tokio::test] + async fn a_folder_round_trips_through_sftp_with_its_modes_and_skips_reported() { + use std::os::unix::fs::PermissionsExt; + let Some(sftp) = local_sftp(&[]).await else { + eprintln!("no sftp-server on this machine; skipped"); + return; + }; + let tmp = tempfile::tempdir().expect("tempdir"); + let path = |rel: &str| tmp.path().join(rel); + let str_of = |p: &Path| p.to_str().expect("utf-8 temp dir").to_string(); + let set_mode = |p: PathBuf, mode| { + std::fs::set_permissions(p, std::fs::Permissions::from_mode(mode)).expect("chmod") + }; + std::fs::create_dir_all(path("src/private")).expect("mkdir"); + std::fs::write(path("src/run.sh"), "#!/bin/sh\n").expect("write"); + set_mode(path("src/run.sh"), 0o755); + std::fs::write(path("src/private/data.bin"), vec![7u8; 200_000]).expect("write"); + set_mode(path("src/private"), 0o700); + std::os::unix::fs::symlink(path("src/run.sh"), path("src/link")).expect("symlink"); + + let cancels = Cancels::default(); + let (tx, mut rx) = mpsc::channel(64); + let err = do_upload( + &str_of(&path("src")), + &sftp, + &str_of(&path("up")), + 1, + &tx, + &no_cancel(&cancels), + ) + .await + .expect_err("the link is reported"); + let link = str_of(&path("src/link")); + assert_eq!( + err.to_string(), + format!("1 item skipped or failed, first '{link}': symbolic link, not followed") + ); + assert_eq!( + std::fs::read(path("up/private/data.bin")) + .expect("read") + .len(), + 200_000 + ); + assert!(!path("up/link").exists()); + assert_eq!( + mode_of(&path("up/run.sh")) & 0o700, + 0o700, + "still executable" + ); + assert_eq!(mode_of(&path("up/private")), 0o700, "still private"); + // The first tick (nothing done yet) marks the end of planning. + assert!(matches!( + rx.try_recv(), + Ok(CoreEvent::FileTransferProgress(1, 0, 200_010)) + )); + + // Back down, into a folder that already holds one of the files. + std::fs::create_dir_all(path("down")).expect("mkdir"); + std::fs::write(path("down/run.sh"), "old").expect("write"); + do_download( + &sftp, + &str_of(&path("up")), + &str_of(&path("down")), + 2, + &tx, + &no_cancel(&cancels), + ) + .await + .expect("downloaded"); + assert_eq!( + std::fs::read(path("down/run.sh")).expect("read"), + b"#!/bin/sh\n" + ); + assert_eq!( + std::fs::read(path("down/private/data.bin")) + .expect("read") + .len(), + 200_000 + ); + assert_eq!(mode_of(&path("down/private")), 0o700); + + // An unreadable folder is reported with the server's reason; the rest still + // arrives. Root reads it anyway, so only a plain user can check this. + set_mode(path("up/private"), 0o000); + let readable = std::fs::read_dir(path("up/private")).is_ok(); + let result = do_download( + &sftp, + &str_of(&path("up")), + &str_of(&path("again")), + 3, + &tx, + &no_cancel(&cancels), + ) + .await; + set_mode(path("up/private"), 0o700); + if !readable { + let err = result.expect_err("the folder is reported"); + let private = str_of(&path("up/private")); + assert!( + err.to_string().starts_with(&format!( + "1 item skipped or failed, first '{private}': read remote dir: Permission denied" + )), + "{err}" + ); + assert!(path("again/run.sh").exists()); + } + } + + #[cfg(unix)] + #[tokio::test] + async fn a_cancelled_upload_removes_the_partial_remote_file() { + let Some(sftp) = local_sftp(&[]).await else { + eprintln!("no sftp-server on this machine; skipped"); + return; + }; + let tmp = tempfile::tempdir().expect("tempdir"); + let src = tmp.path().join("big.bin"); + std::fs::write(&src, vec![1u8; 1_000_000]).expect("write"); + let dst = tmp.path().join("copy.bin"); + + // One slot: the upload cannot run more than a chunk ahead of this watcher, + // which cancels once the first chunk is out. + let (tx, mut rx) = mpsc::channel(1); + let cancels = Cancels::default(); + let cancel = no_cancel(&cancels); + let upload = do_upload( + src.to_str().expect("utf-8"), + &sftp, + dst.to_str().expect("utf-8"), + 1, + &tx, + &cancel, + ); + let watch = async { + while let Some(CoreEvent::FileTransferProgress(_, done, _)) = rx.recv().await { + if done > 0 { + cancels.bump(); + } + } + }; + let result = tokio::select! { + result = upload => result, + () = watch => unreachable!("the channel outlives the upload"), + }; + assert!(result.expect_err("cancelled").is::()); + assert!(!dst.exists(), "the partial file is gone"); + } + + #[cfg(unix)] + #[tokio::test] + async fn a_folder_uploads_to_a_server_that_will_not_set_modes() { + let Some(sftp) = local_sftp(&["-P", "setstat"]).await else { + eprintln!("no sftp-server on this machine; skipped"); + return; + }; + let tmp = tempfile::tempdir().expect("tempdir"); + let path = |rel: &str| tmp.path().join(rel); + let str_of = |rel: &str| path(rel).to_str().expect("utf-8 temp dir").to_string(); + std::fs::create_dir_all(path("src/a/b")).expect("mkdir"); + std::fs::write(path("src/a/b/f.txt"), "hello").expect("write"); + + let cancels = Cancels::default(); + let cancel = no_cancel(&cancels); + let (tx, _rx) = mpsc::channel(64); + do_upload(&str_of("src"), &sftp, &str_of("up"), 1, &tx, &cancel) + .await + .expect("uploaded"); + assert_eq!(std::fs::read(path("up/a/b/f.txt")).expect("read"), b"hello"); + } + + #[cfg(unix)] + #[tokio::test] + async fn a_read_only_file_is_replaced_by_the_next_transfer() { + use std::os::unix::fs::PermissionsExt; + let Some(sftp) = local_sftp(&[]).await else { + eprintln!("no sftp-server on this machine; skipped"); + return; + }; + let tmp = tempfile::tempdir().expect("tempdir"); + let path = |rel: &str| tmp.path().join(rel); + let str_of = |rel: &str| path(rel).to_str().expect("utf-8 temp dir").to_string(); + std::fs::create_dir(path("src")).expect("mkdir"); + for (file, mode) in [("src/obj", 0o444), ("key", 0o400)] { + std::fs::write(path(file), file).expect("write"); + std::fs::set_permissions(path(file), std::fs::Permissions::from_mode(mode)) + .expect("chmod"); + } + + let cancels = Cancels::default(); + let cancel = no_cancel(&cancels); + let (tx, _rx) = mpsc::channel(64); + for round in 0..2 { + do_download(&sftp, &str_of("src"), &str_of("tree"), round, &tx, &cancel) + .await + .expect("folder downloaded"); + do_download(&sftp, &str_of("key"), &str_of("dl"), round, &tx, &cancel) + .await + .expect("file downloaded"); + do_upload(&str_of("key"), &sftp, &str_of("up"), round, &tx, &cancel) + .await + .expect("file uploaded"); + } + // As sftp's get leaves it: the owner may write, nobody else gains anything. + assert_eq!(mode_of(&path("dl")), 0o600); + assert_eq!(std::fs::read(path("tree/obj")).expect("read"), b"src/obj"); + } + + #[test] + fn a_server_name_must_be_one_plain_component() { + for name in ["report.txt", "..hidden", "with space", "caf\u{e9}.txt"] { + assert!(is_safe_name(name), "{name:?} should be safe"); + } + for name in ["", ".", "..", "a/b", "/abs", "dir/", "a\0b"] { + assert!(!is_safe_name(name), "{name:?} should not be safe"); + } + assert_eq!(join_remote("/", "x"), "/x"); + assert_eq!(join_remote("/srv", "x"), "/srv/x"); + } + + #[test] + fn windows_names_refuse_drives_devices_and_trimmed_endings() { + for name in [ + "C:x", + "D:", + "a:b", + "..\\x", + "a\\b", + "CON", + "aux.c", + "NUL.tar.gz", + "Com1", + "lpt9.txt", + "COM\u{b9}", + "CON .txt", + "a.", + "a ", + "a, session_id: u64) -> Result<(), CommandError> { + state.cancel_sftp(session_id); + Ok(()) +} + +/// Close an SFTP session and its connection, stopping its transfer (tech-gui.md §4.2). #[tauri::command] #[specta::specta] pub fn sftp_close(state: State<'_, GuiState>, session_id: u64) -> Result<(), CommandError> { diff --git a/crates/omnyssh-gui/src/dto.rs b/crates/omnyssh-gui/src/dto.rs index cf86c283..944d9ced 100644 --- a/crates/omnyssh-gui/src/dto.rs +++ b/crates/omnyssh-gui/src/dto.rs @@ -243,6 +243,7 @@ pub struct SnippetDto { /// A file or directory in an SFTP panel listing (tech-gui.md §4.1). Maps from the /// core `FileEntry`; `path` is the absolute path the frontend marks entries by. +/// `modified` is Unix seconds, `null` when unknown. #[derive(Debug, Clone, Serialize, Deserialize, specta::Type)] #[serde(rename_all = "camelCase")] pub struct FileEntryDto { @@ -250,6 +251,7 @@ pub struct FileEntryDto { pub path: String, pub size: u64, pub is_dir: bool, + pub modified: Option, } /// Live progress for one SFTP upload/download (tech-gui.md §4.1). The GUI allocates @@ -561,6 +563,7 @@ impl From<&FileEntry> for FileEntryDto { path: entry.path.clone(), size: entry.size, is_dir: entry.is_dir, + modified: entry.modified, } } } @@ -945,22 +948,26 @@ mod tests { path: "/etc/omnyssh/config.toml".to_string(), size: 4096, is_dir: false, + modified: Some(1_700_000_000), }; let dto = FileEntryDto::from(&file); assert_eq!(dto.name, "config.toml"); assert_eq!(dto.path, "/etc/omnyssh/config.toml"); assert_eq!(dto.size, 4096); assert!(!dto.is_dir); + assert_eq!(dto.modified, Some(1_700_000_000)); let dir = FileEntry { name: "..".to_string(), path: "/etc".to_string(), size: 0, is_dir: true, + modified: None, }; let dto = FileEntryDto::from(&dir); assert!(dto.is_dir); assert_eq!(dto.size, 0); + assert!(dto.modified.is_none()); } #[test] @@ -972,11 +979,12 @@ mod tests { path: "/srv".to_string(), size: 0, is_dir: true, + modified: Some(1_700_000_000), })) .expect("serialise FileEntryDto"); assert_eq!( json, - r#"{"name":"srv","path":"/srv","size":0,"isDir":true}"# + r#"{"name":"srv","path":"/srv","size":0,"isDir":true,"modified":1700000000}"# ); } diff --git a/crates/omnyssh-gui/src/main.rs b/crates/omnyssh-gui/src/main.rs index 0148f6c9..e9ad0c92 100644 --- a/crates/omnyssh-gui/src/main.rs +++ b/crates/omnyssh-gui/src/main.rs @@ -18,8 +18,8 @@ use commands::auth::{answer_password, unlock_identity}; use commands::hosts::{delete_host, list_hosts, refresh_metrics, reload_hosts, save_host}; use commands::keysetup::start_key_setup; use commands::sftp::{ - list_local_dir, list_local_roots, preview_local_file, sftp_close, sftp_delete, sftp_download, - sftp_list, sftp_mkdir, sftp_open, sftp_preview, sftp_rename, sftp_upload, + list_local_dir, list_local_roots, preview_local_file, sftp_cancel, sftp_close, sftp_delete, + sftp_download, sftp_list, sftp_mkdir, sftp_open, sftp_preview, sftp_rename, sftp_upload, }; use commands::snippets::{delete_snippet, execute_snippet, list_snippets, save_snippet}; use commands::terminal::{ @@ -112,6 +112,7 @@ fn specta_builder() -> Builder { sftp_rename, sftp_delete, sftp_preview, + sftp_cancel, sftp_close, list_local_dir, list_local_roots, diff --git a/crates/omnyssh-gui/src/state.rs b/crates/omnyssh-gui/src/state.rs index 5aad0516..10911170 100644 --- a/crates/omnyssh-gui/src/state.rs +++ b/crates/omnyssh-gui/src/state.rs @@ -458,6 +458,18 @@ impl GuiState { } } + /// Cancel the transfer a live SFTP session is running; unknown ids are a no-op. + pub fn cancel_sftp(&self, session_id: SessionId) { + if let Some(manager) = self + .sftp + .lock() + .expect("sftp lock poisoned") + .get(&session_id) + { + manager.cancel(); + } + } + /// Allocate a transfer id owned by `session_id`, so its `FileTransferProgress` /// events route back to the right tab via `transfer_owner` (§3.4). pub fn next_transfer(&self, session_id: SessionId) -> TransferId { @@ -478,8 +490,8 @@ impl GuiState { .copied() } - /// User-initiated SFTP close: drop the manager (a graceful `Disconnect` to its - /// task) and prune this session's transfer-owner entries (§3.4). + /// User-initiated SFTP close: drop the manager (its transfer cancelled, a graceful + /// `Disconnect` to its task) and prune this session's transfer-owner entries (§3.4). pub fn close_sftp(&self, session_id: SessionId) { if let Some(manager) = self .sftp diff --git a/crates/omnyssh-gui/ui/e2e/sftp.spec.ts b/crates/omnyssh-gui/ui/e2e/sftp.spec.ts index 3d0457d2..6eadfd3c 100644 --- a/crates/omnyssh-gui/ui/e2e/sftp.spec.ts +++ b/crates/omnyssh-gui/ui/e2e/sftp.spec.ts @@ -1,4 +1,4 @@ -import { expect, test, type Page } from '@playwright/test'; +import { expect, test, type Locator, type Page } from '@playwright/test'; // SFTP dual-pane vertical (tech-gui.md §3.2). e2e runs against the static SPA with // Tauri absent, so we stub `__TAURI_INTERNALS__` at the boundary (§6.4). The stub owns @@ -13,9 +13,19 @@ const HOSTS = [ ]; // `windows` swaps in a Windows-shaped local side: a home on C: and a second drive, D:. -async function boot(page: Page, windows = false): Promise { +// `holdProgress` keeps a transfer's first tick back until `__tick()`, as while the core +// walks a folder. `windowScale` is the window's scale factor, the page's own ratio +// unless given. +async function boot( + page: Page, + { + windows = false, + holdProgress = false, + windowScale + }: { windows?: boolean; holdProgress?: boolean; windowScale?: number } = {} +): Promise { await page.addInitScript( - ({ hosts, windows }) => { + ({ hosts, windows, holdProgress, windowScale }) => { let cbid = 0; const win = window as unknown as Record; const listeners: Record = {}; @@ -23,9 +33,29 @@ async function boot(page: Page, windows = false): Promise { let nextTransfer = 0; // A pending transfer holds until the test fires its op-done, so the progress bar // is observable mid-flight (the core is sequential — one transfer at a time). - const completions: Array<() => void> = []; + const completions: Array<{ sessionId: number; tid: number; finish: () => void }> = []; + // Remote listings asked for meanwhile wait behind it, in the core's one queue. + const waiting: Array<{ sessionId: number; list: () => void }> = []; + function busy(sessionId: number): boolean { + return completions.some((c) => c.sessionId === sessionId); + } + function release(sessionId: number): void { + if (busy(sessionId)) return; + for (const w of waiting.filter((w) => w.sessionId === sessionId)) { + waiting.splice(waiting.indexOf(w), 1); + w.list(); + } + } - type Entry = { name: string; path: string; size: number; isDir: boolean }; + type Entry = { + name: string; + path: string; + size: number; + isDir: boolean; + modified?: number | null; + }; + // 2024-06-15 12:00 UTC: mid-month and midday, so the year shows in any time zone. + const JUNE_2024 = 1718452800; const home = windows ? 'C:\\Users\\me' : '/home/user'; const roots = windows ? ['C:\\', 'D:\\', 'E:\\', 'Z:\\'] : ['/']; const local: Record = windows @@ -35,17 +65,38 @@ async function boot(page: Page, windows = false): Promise { } : { [home]: [ - { name: 'notes.txt', path: '/home/user/notes.txt', size: 24, isDir: false }, + { + name: 'notes.txt', + path: '/home/user/notes.txt', + size: 24, + isDir: false, + modified: JUNE_2024 + }, { name: 'work', path: '/home/user/work', size: 0, isDir: true } ] }; - // Where each download was sent, for the test to read back. + // Where each transfer was sent, and which sessions cancelled or closed, for the + // test to read back. const downloads: string[] = []; + const uploads: string[] = []; + const cancels: number[] = []; + const closes: number[] = []; (win as { __downloads?: string[] }).__downloads = downloads; + (win as { __uploads?: string[] }).__uploads = uploads; + (win as { __cancels?: number[] }).__cancels = cancels; + (win as { __closes?: number[] }).__closes = closes; + (win as { __fireEvent?: unknown }).__fireEvent = (event: string, payload: unknown) => + fireEvent(event, payload); const remote: Record = { '/': [ - { name: 'config.yml', path: '/config.yml', size: 64, isDir: false }, + { name: 'config.yml', path: '/config.yml', size: 64, isDir: false, modified: JUNE_2024 }, { name: 'var', path: '/var', size: 0, isDir: true } + ], + // A hostile server's names, each joined onto the folder as it was listed. + '/var': [ + { name: 'a/b', path: '/var/a/b', size: 8, isDir: false }, + { name: './x', path: '/var/./x', size: 8, isDir: false }, + { name: 'ok.log', path: '/var/ok.log', size: 8, isDir: false } ] }; @@ -75,9 +126,25 @@ async function boot(page: Page, windows = false): Promise { } // Fire the oldest still-pending transfer's op-done (deterministic completion). - (win as { __completeTransfer?: () => void }).__completeTransfer = () => completions.shift()?.(); + (win as { __completeTransfer?: () => void }).__completeTransfer = () => { + const c = completions.shift(); + c?.finish(); + if (c) release(c.sessionId); + }; + // A progress tick for the oldest pending transfer. + function tick(done: number, total: number): void { + const c = completions[0]; + if (!c) return; + fireEvent('transfer-progress', { sessionId: c.sessionId, transferId: c.tid, done, total }); + } + (win as { __tick?: typeof tick }).__tick = tick; (win as { __TAURI_INTERNALS__: unknown }).__TAURI_INTERNALS__ = { + // `getCurrentWebview()` reads these to scope OS drag-drop listeners. + metadata: { + currentWindow: { label: 'main' }, + currentWebview: { label: 'main', windowLabel: 'main' } + }, invoke: (cmd: string, args: Record) => { if (cmd.startsWith('plugin:path')) return Promise.resolve(home); switch (cmd) { @@ -104,19 +171,27 @@ async function boot(page: Page, windows = false): Promise { } case 'sftp_list': { const { sessionId, path } = args as { sessionId: number; path: string }; - setTimeout( - () => fireEvent('sftp-dir-listed', { sessionId, path, entries: withParent(path, remote[path] ?? []) }), - 0 - ); + const list = () => + setTimeout( + () => fireEvent('sftp-dir-listed', { sessionId, path, entries: withParent(path, remote[path] ?? []) }), + 0 + ); + if (busy(sessionId)) waiting.push({ sessionId, list }); + else list(); return Promise.resolve(null); } case 'sftp_upload': { const { sessionId, remote: dest } = args as { sessionId: number; remote: string }; + uploads.push(dest); const tid = ++nextTransfer; - setTimeout(() => fireEvent('transfer-progress', { sessionId, transferId: tid, done: 4, total: 8 }), 0); - completions.push(() => { - addFile(remote, parentOf(dest), baseName(dest)); - fireEvent('sftp-op-done', { sessionId, ok: true }); + if (!holdProgress) setTimeout(() => tick(4, 8), 0); + completions.push({ + sessionId, + tid, + finish: () => { + addFile(remote, parentOf(dest), baseName(dest)); + fireEvent('sftp-op-done', { sessionId, ok: true }); + } }); return Promise.resolve(null); } @@ -124,15 +199,36 @@ async function boot(page: Page, windows = false): Promise { const { sessionId, local: dest } = args as { sessionId: number; local: string }; downloads.push(dest); const tid = ++nextTransfer; - setTimeout(() => fireEvent('transfer-progress', { sessionId, transferId: tid, done: 2, total: 8 }), 0); - completions.push(() => { - addFile(local, parentOf(dest), baseName(dest)); - fireEvent('sftp-op-done', { sessionId, ok: true }); + if (!holdProgress) setTimeout(() => tick(2, 8), 0); + completions.push({ + sessionId, + tid, + finish: () => { + addFile(local, parentOf(dest), baseName(dest)); + fireEvent('sftp-op-done', { sessionId, ok: true }); + } }); return Promise.resolve(null); } + case 'sftp_cancel': { + // As the core does: the session's running transfer stops and says so. + const { sessionId } = args as { sessionId: number }; + cancels.push(sessionId); + const i = completions.findIndex((c) => c.sessionId === sessionId); + if (i >= 0) { + completions.splice(i, 1); + setTimeout(() => { + fireEvent('sftp-op-done', { sessionId, ok: false, error: 'Transfer cancelled' }); + release(sessionId); + }, 0); + } + return Promise.resolve(null); + } case 'sftp_close': + closes.push((args as { sessionId: number }).sessionId); return Promise.resolve(null); + case 'plugin:window|scale_factor': + return Promise.resolve(windowScale ?? window.devicePixelRatio); case 'plugin:event|listen': { const { event, handler } = args as { event: string; handler: number }; (listeners[event] ||= []).push(handler); @@ -152,13 +248,18 @@ async function boot(page: Page, windows = false): Promise { } }; }, - { hosts: HOSTS, windows } + { hosts: HOSTS, windows, holdProgress, windowScale } ); await page.goto('/'); await expect(page.getByText('2 hosts')).toBeVisible(); } +// Fire the oldest pending transfer's op-done. +async function complete(page: Page): Promise { + await page.evaluate(() => (window as unknown as { __completeTransfer: () => void }).__completeTransfer()); +} + test('host-first: a card’s files opens SFTP and browses both sides', async ({ page }) => { await boot(page); @@ -203,6 +304,525 @@ test('round-trip: upload a local file to the remote, then download a remote file await expect(localPane.getByText('config.yml')).toBeVisible(); }); +test('Cancel stops the running transfer and drops the queued ones', async ({ page }) => { + await boot(page); + await page.getByTitle('files on web-1').click(); + const localPane = page.getByRole('region', { name: 'Local' }); + const remotePane = page.getByRole('region', { name: 'web-1' }); + await expect(localPane.getByText('notes.txt')).toBeVisible(); + await expect(remotePane.getByText('config.yml')).toBeVisible(); + + // A file, then a folder queued behind it. + await localPane.getByRole('checkbox', { name: 'Mark notes.txt' }).click(); + await localPane.getByRole('checkbox', { name: 'Mark work' }).click(); + await page.getByRole('button', { name: 'Upload' }).click(); + const strip = page.getByLabel('transfer progress'); + await expect(strip).toBeVisible(); + + await strip.getByRole('button', { name: 'Cancel transfer' }).click(); + await expect(strip).toHaveCount(0); + await expect(page.getByText('Transfer cancelled')).toBeVisible(); + const sent = await page.evaluate(() => { + const w = window as unknown as { __uploads: string[]; __cancels: number[] }; + return { uploads: w.__uploads, cancels: w.__cancels }; + }); + expect(sent).toEqual({ uploads: ['/notes.txt'], cancels: [1] }); +}); + +test('closing the tab mid-transfer closes its session, which stops the transfer', async ({ + page +}) => { + await boot(page); + await page.getByTitle('files on web-1').click(); + const localPane = page.getByRole('region', { name: 'Local' }); + await expect(localPane.getByText('notes.txt')).toBeVisible(); + await localPane.getByRole('checkbox', { name: 'Mark notes.txt' }).click(); + await page.getByRole('button', { name: 'Upload' }).click(); + await expect(page.getByLabel('transfer progress')).toBeVisible(); + + await page.getByRole('button', { name: /^Close web-1/ }).click(); + await expect(page.getByLabel('transfer progress')).toHaveCount(0); + await expect + .poll(() => page.evaluate(() => (window as unknown as { __closes: number[] }).__closes)) + .toEqual([1]); +}); + +test('a transfer shows as preparing until its first tick, a folder of empty files too', async ({ + page +}) => { + await boot(page, { holdProgress: true }); + await page.getByTitle('files on web-1').click(); + const localPane = page.getByRole('region', { name: 'Local' }); + const remotePane = page.getByRole('region', { name: 'web-1' }); + await expect(localPane.getByText('work')).toBeVisible(); + await expect(remotePane.getByText('config.yml')).toBeVisible(); + + await localPane.getByRole('checkbox', { name: 'Mark work' }).click(); + await page.getByRole('button', { name: 'Upload' }).click(); + const strip = page.getByLabel('transfer progress'); + await expect(strip).toContainText('Preparing work…'); + await expect(strip.getByRole('button', { name: 'Cancel transfer' })).toBeVisible(); + + // Planned, and nothing in it has a byte: the core's first tick is 0 of 0. + await page.evaluate(() => { + (window as unknown as { __tick: (done: number, total: number) => void }).__tick(0, 0); + }); + await expect(strip).toContainText('Uploading work'); + await expect(strip).toContainText('0 B'); + + await complete(page); + await expect(strip).toHaveCount(0); + await expect(remotePane.getByText('work')).toBeVisible(); +}); + +test('a remote folder opened during a transfer shows loading, then opens', async ({ page }) => { + await boot(page); + await page.getByTitle('files on web-1').click(); + const localPane = page.getByRole('region', { name: 'Local' }); + const remotePane = page.getByRole('region', { name: 'web-1' }); + await expect(localPane.getByText('notes.txt')).toBeVisible(); + await expect(remotePane.getByText('config.yml')).toBeVisible(); + + await localPane.getByRole('checkbox', { name: 'Mark notes.txt' }).click(); + await page.getByRole('button', { name: 'Upload' }).click(); + await expect(page.getByLabel('transfer progress')).toBeVisible(); + + // The listing waits behind the transfer. + await remotePane.getByTitle('var', { exact: true }).click(); + await expect(remotePane).toHaveAttribute('aria-busy', 'true'); + await expect(remotePane.getByText('Loading…')).toBeVisible(); + + // Once it is done the pane opens the folder asked for, not the one it was on. + await complete(page); + await expect(remotePane).toHaveAttribute('aria-busy', 'false'); + await expect(remotePane.getByTitle('/var', { exact: true })).toBeVisible(); + await expect(remotePane.getByText('config.yml')).toHaveCount(0); + await page.waitForTimeout(100); + await expect(remotePane.getByTitle('/var', { exact: true })).toBeVisible(); +}); + +test('a transfer that would replace items asks first, once for the batch', async ({ page }) => { + await boot(page); + await page.getByTitle('files on web-1').click(); + const localPane = page.getByRole('region', { name: 'Local' }); + const remotePane = page.getByRole('region', { name: 'web-1' }); + await expect(localPane.getByText('notes.txt')).toBeVisible(); + await expect(remotePane.getByText('config.yml')).toBeVisible(); + + await localPane.getByRole('checkbox', { name: 'Mark notes.txt' }).click(); + await page.getByRole('button', { name: 'Upload' }).click(); + await complete(page); + await expect(remotePane.getByText('notes.txt')).toBeVisible(); + + // notes.txt is on the server now; the folder ticked beside it is not. + await localPane.getByRole('checkbox', { name: 'Mark work' }).click(); + await page.getByRole('button', { name: 'Upload' }).click(); + const ask = page.getByRole('dialog', { name: 'Replace existing items' }); + await expect(ask).toContainText( + '1 item(s) already exist in /. Folders are merged and files with the same name are replaced.' + ); + await ask.getByRole('button', { name: 'Cancel' }).click(); + await expect(ask).toHaveCount(0); + expect(await uploads(page)).toEqual(['/notes.txt']); + + await page.getByRole('button', { name: 'Upload' }).click(); + await ask.getByRole('button', { name: 'Replace' }).click(); + await expect(ask).toHaveCount(0); + await expect(page.getByLabel('transfer progress')).toBeVisible(); + await complete(page); + await expect.poll(() => uploads(page)).toEqual(['/notes.txt', '/notes.txt', '/work']); +}); + +test('a Replace prompt left for another tab is dropped, not brought back', async ({ page }) => { + await boot(page); + await page.getByTitle('files on web-1').click(); + await expect(page.getByRole('region', { name: 'web-1' }).getByText('config.yml')).toBeVisible(); + await page.getByRole('button', { name: 'Dashboard', exact: true }).click(); + await page.getByTitle('files on db-1').click(); + await expect(page.getByRole('region', { name: 'db-1' }).getByText('config.yml')).toBeVisible(); + await page.getByRole('button', { name: 'web-1 · sftp', exact: true }).click(); + + const localPane = page.getByRole('region', { name: 'Local' }); + await page.getByRole('region', { name: 'web-1' }).getByRole('checkbox', { name: 'Mark config.yml' }).click(); + await page.getByRole('button', { name: 'Download' }).click(); + await complete(page); + await expect(localPane.getByText('config.yml')).toBeVisible(); + + await page.getByRole('button', { name: 'Download' }).click(); + const ask = page.getByRole('dialog', { name: 'Replace existing items' }); + await expect(ask).toBeVisible(); + await page.keyboard.press('Control+k'); + await page.getByRole('dialog', { name: 'Command palette' }).getByRole('textbox').fill('db-1'); + await page.keyboard.press('Enter'); + await expect(page.getByRole('region', { name: 'db-1' })).toBeVisible(); + await expect(ask).toHaveCount(0); + + await page.getByRole('button', { name: 'web-1 · sftp', exact: true }).click(); + await expect(page.getByRole('region', { name: 'web-1' })).toBeVisible(); + await expect(ask).toHaveCount(0); + const downloads = await page.evaluate(() => (window as unknown as { __downloads: string[] }).__downloads); + expect(downloads).toEqual(['/home/user/config.yml']); +}); + +test('a listed name that is not one plain name is refused, the rest downloads', async ({ + page +}) => { + await boot(page); + await page.getByTitle('files on web-1').click(); + const remotePane = page.getByRole('region', { name: 'web-1' }); + await remotePane.getByTitle('var', { exact: true }).click(); + await expect(remotePane.getByText('ok.log')).toBeVisible(); + + for (const name of ['a/b', './x', 'ok.log']) { + await remotePane.getByRole('checkbox', { name: `Mark ${name}` }).click(); + } + await page.getByRole('button', { name: 'Download' }).click(); + await expect(page.getByText("'a/b' is not a name that can be created here.")).toBeVisible(); + await expect(page.getByLabel('transfer progress')).toBeVisible(); + const downloads = await page.evaluate(() => (window as unknown as { __downloads: string[] }).__downloads); + expect(downloads).toEqual(['/home/user/ok.log']); +}); + +test('a drop on a folder row does not ask: that folder is not on show', async ({ page }) => { + await boot(page); + await page.getByTitle('files on web-1').click(); + const localPane = page.getByRole('region', { name: 'Local' }); + const remotePane = page.getByRole('region', { name: 'web-1' }); + await expect(localPane.getByText('notes.txt')).toBeVisible(); + await localPane.getByRole('checkbox', { name: 'Mark notes.txt' }).click(); + await page.getByRole('button', { name: 'Upload' }).click(); + await complete(page); + await expect(remotePane.getByText('notes.txt')).toBeVisible(); + + // The local pane shows a notes.txt, but the drop goes into work/. + await dragOnto( + page, + remotePane.getByTitle('notes.txt', { exact: true }), + localPane.getByTitle('work', { exact: true }) + ); + await expect(page.getByLabel('transfer progress')).toBeVisible(); + await expect(page.getByRole('dialog', { name: 'Replace existing items' })).toHaveCount(0); + const downloads = await page.evaluate(() => (window as unknown as { __downloads: string[] }).__downloads); + expect(downloads).toEqual(['/home/user/work/notes.txt']); +}); + +test('the panes show modification times', async ({ page }) => { + await boot(page); + await page.getByTitle('files on web-1').click(); + const localPane = page.getByRole('region', { name: 'Local' }); + const remotePane = page.getByRole('region', { name: 'web-1' }); + await expect(localPane.getByText('notes.txt')).toBeVisible(); + await expect(remotePane.getByText('config.yml')).toBeVisible(); + + await expect(localPane.getByTitle(/^Modified .*2024/)).toBeVisible(); + await expect(remotePane.getByTitle(/^Modified .*2024/)).toBeVisible(); + await expect(page.getByText('Created')).toHaveCount(0); +}); + +test('a date stays on one line, and the column gives way in a narrow window', async ({ page }) => { + await boot(page); + await page.getByTitle('files on web-1').click(); + const remotePane = page.getByRole('region', { name: 'web-1' }); + const date = remotePane.getByTitle(/^Modified /); + await expect(date).toBeVisible(); + + // As under GDK_SCALE=2 with GDK_DPI_SCALE=0.5: the rem the column is sized in shrinks, + // the text it holds does not. + await page.addStyleTag({ + content: 'html { font-size: 8px } [title^="Modified "] { font-size: 12px; line-height: 16px }' + }); + const box = await date.boundingBox(); + expect(box?.height).toBeLessThan(24); + + await page.setViewportSize({ width: 900, height: 720 }); + await expect(date).toBeHidden(); + await expect(remotePane.getByText('Modified', { exact: true })).toBeHidden(); +}); + +async function centre(locator: Locator): Promise<{ x: number; y: number }> { + const box = await locator.boundingBox(); + if (!box) throw new Error('not laid out'); + return { x: box.x + box.width / 2, y: box.y + box.height / 2 }; +} + +// Press on `from` and move past the drag threshold, holding the button. +async function startDrag(page: Page, from: Locator): Promise { + const a = await centre(from); + await page.mouse.move(a.x, a.y); + await page.mouse.down(); + await page.mouse.move(a.x + 20, a.y, { steps: 4 }); +} + +async function dragOnto( + page: Page, + from: Locator, + to: Locator | { x: number; y: number } +): Promise { + await startDrag(page, from); + // Several steps, so the drag tracks the target. + const b = 'x' in to ? to : await centre(to); + await page.mouse.move(b.x, b.y, { steps: 8 }); + await page.mouse.up(); +} + +test('drag and drop: a local file dropped on the remote pane uploads there', async ({ page }) => { + await boot(page); + await page.getByTitle('files on web-1').click(); + const localPane = page.getByRole('region', { name: 'Local' }); + const remotePane = page.getByRole('region', { name: 'web-1' }); + await expect(remotePane.getByText('config.yml')).toBeVisible(); + + await dragOnto(page, localPane.getByTitle('notes.txt', { exact: true }), remotePane.getByText('config.yml')); + await expect(page.getByLabel('transfer progress')).toBeVisible(); + const uploads = await page.evaluate(() => (window as unknown as { __uploads: string[] }).__uploads); + expect(uploads).toEqual(['/notes.txt']); + // The drag ended away from the row it started on, so nothing was previewed. + await expect(page.getByRole('dialog', { name: 'File preview' })).toHaveCount(0); + + await page.evaluate(() => (window as unknown as { __completeTransfer: () => void }).__completeTransfer()); + await expect(remotePane.getByText('notes.txt')).toBeVisible(); +}); + +test('drag and drop: a remote file dropped on a local folder downloads into it', async ({ page }) => { + await boot(page); + await page.getByTitle('files on web-1').click(); + const localPane = page.getByRole('region', { name: 'Local' }); + const remotePane = page.getByRole('region', { name: 'web-1' }); + await expect(localPane.getByText('work')).toBeVisible(); + + await dragOnto(page, remotePane.getByTitle('config.yml', { exact: true }), localPane.getByTitle('work', { exact: true })); + await expect(page.getByLabel('transfer progress')).toBeVisible(); + const downloads = await page.evaluate(() => (window as unknown as { __downloads: string[] }).__downloads); + expect(downloads).toEqual(['/home/user/work/config.yml']); +}); + +test('drag and drop: dropping back on the same pane transfers nothing', async ({ page }) => { + await boot(page); + await page.getByTitle('files on web-1').click(); + const localPane = page.getByRole('region', { name: 'Local' }); + await expect(localPane.getByText('work')).toBeVisible(); + + await dragOnto(page, localPane.getByTitle('notes.txt', { exact: true }), localPane.getByTitle('work', { exact: true })); + await expect(page.getByLabel('transfer progress')).toHaveCount(0); + const uploads = await page.evaluate(() => (window as unknown as { __uploads: string[] }).__uploads); + expect(uploads).toEqual([]); +}); + +test('drag and drop: Escape drops the drag, and the release after it clicks nothing', async ({ + page +}) => { + await boot(page); + await page.getByTitle('files on web-1').click(); + const remotePane = page.getByRole('region', { name: 'web-1' }); + const folder = remotePane.getByTitle('var', { exact: true }); + await expect(folder).toBeVisible(); + + await startDrag(page, folder); + const named = page.getByText('var', { exact: true }).filter({ visible: true }); + await expect(named).toHaveCount(2); // the row and the ghost + await page.keyboard.press('Escape'); + await expect(named).toHaveCount(1); + + // Back on the row it started from: no click, so the pane stays where it was. + const back = await centre(folder); + await page.mouse.move(back.x, back.y, { steps: 4 }); + await page.mouse.up(); + await page.waitForTimeout(100); + await expect(remotePane.getByText('config.yml')).toBeVisible(); + const downloads = await page.evaluate(() => (window as unknown as { __downloads: string[] }).__downloads); + expect(downloads).toEqual([]); +}); + +test('drag and drop: `..` dragged away and back does not go up, like any row', async ({ page }) => { + await boot(page); + await page.getByTitle('files on web-1').click(); + const localPane = page.getByRole('region', { name: 'Local' }); + await localPane.getByTitle('work', { exact: true }).click(); + await expect(localPane.getByTitle('/home/user/work', { exact: true })).toBeVisible(); + + const parent = localPane.getByTitle('..', { exact: true }); + const at = await centre(parent); + await page.mouse.move(at.x, at.y); + await page.mouse.down(); + await page.mouse.move(at.x, at.y + 200, { steps: 6 }); + await page.mouse.move(at.x, at.y, { steps: 6 }); + await page.mouse.up(); + await page.waitForTimeout(100); + await expect(localPane.getByTitle('/home/user/work', { exact: true })).toBeVisible(); +}); + +test('drag and drop: a drop on the transfer strip goes to the pane above it', async ({ page }) => { + await boot(page); + await page.getByTitle('files on web-1').click(); + const localPane = page.getByRole('region', { name: 'Local' }); + const remotePane = page.getByRole('region', { name: 'web-1' }); + await expect(localPane.getByText('notes.txt')).toBeVisible(); + await expect(remotePane.getByText('config.yml')).toBeVisible(); + + await localPane.getByRole('checkbox', { name: 'Mark notes.txt' }).click(); + await page.getByRole('button', { name: 'Upload' }).click(); + const strip = page.getByLabel('transfer progress'); + await expect(strip).toBeVisible(); + + // Released on the strip's local half: a download into the local pane's directory, + // queued behind the upload. + const x = (await centre(localPane)).x; + const y = (await centre(strip)).y; + await dragOnto(page, remotePane.getByTitle('config.yml', { exact: true }), { x, y }); + await complete(page); + await expect + .poll(() => page.evaluate(() => (window as unknown as { __downloads: string[] }).__downloads)) + .toEqual(['/home/user/config.yml']); +}); + +test('drag and drop: a remote folder dropped on the local pane downloads it whole', async ({ page }) => { + await boot(page); + await page.getByTitle('files on web-1').click(); + const localPane = page.getByRole('region', { name: 'Local' }); + const remotePane = page.getByRole('region', { name: 'web-1' }); + await expect(localPane.getByText('notes.txt')).toBeVisible(); + await expect(remotePane.getByText('var')).toBeVisible(); + + await dragOnto(page, remotePane.getByTitle('var', { exact: true }), localPane.getByTitle('notes.txt', { exact: true })); + await expect(page.getByLabel('transfer progress')).toBeVisible(); + const downloads = await page.evaluate(() => (window as unknown as { __downloads: string[] }).__downloads); + expect(downloads).toEqual(['/home/user/var']); + // The press on the folder row did not navigate into it. + await expect(remotePane.getByText('config.yml')).toBeVisible(); +}); + +// A drop from the OS file manager, as Tauri delivers it: the drag comes in, then drops. +async function osDrop( + page: Page, + paths: string[], + position: { x: number; y: number } +): Promise { + for (const event of ['tauri://drag-enter', 'tauri://drag-drop']) { + await page.evaluate( + ({ event, paths, position }) => { + const fire = (window as unknown as { __fireEvent: (e: string, p: unknown) => void }).__fireEvent; + fire(event, { paths, position }); + }, + { event, paths, position } + ); + } +} + +function uploads(page: Page): Promise { + return page.evaluate(() => (window as unknown as { __uploads: string[] }).__uploads); +} + +// Each webview reports the drop position in its own pixels and Tauri passes it on as is: +// physical on Windows (WebView2), logical on macOS (WKWebView) and Linux (WebKitGTK). +const PLATFORMS = [ + { + name: 'Windows', + userAgent: + 'Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/149.0 Safari/537.36', + physical: true + }, + { + name: 'macOS', + userAgent: + 'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/605.1.15 (KHTML, like Gecko)', + physical: false + }, + { + name: 'Linux', + userAgent: 'Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/605.1.15 (KHTML, like Gecko)', + physical: false + } +]; + +for (const platform of PLATFORMS) { + test.describe(`on ${platform.name} at 2x`, () => { + test.use({ userAgent: platform.userAgent, deviceScaleFactor: 2 }); + + test('files and folders dropped from the OS onto the remote pane upload', async ({ page }) => { + await boot(page); + await page.getByTitle('files on web-1').click(); + const remotePane = page.getByRole('region', { name: 'web-1' }); + await expect(remotePane.getByText('config.yml')).toBeVisible(); + + const at = await centre(remotePane.getByText('config.yml')); + const scale = platform.physical ? 2 : 1; + await osDrop(page, ['/tmp/photo.png', '/tmp/album/'], { x: at.x * scale, y: at.y * scale }); + + // One transfer at a time: the folder goes once the file is done. + await expect(page.getByLabel('transfer progress')).toBeVisible(); + expect(await uploads(page)).toEqual(['/photo.png']); + await complete(page); + await expect.poll(() => uploads(page)).toEqual(['/photo.png', '/album']); + }); + }); +} + +test.describe('on Linux with the page at 1x in a 2x window', () => { + // As WebKitGTK does under GDK_SCALE=2 with GDK_DPI_SCALE=0.5: it still reports the + // window's logical pixels, now half a CSS pixel each. + test.use({ userAgent: PLATFORMS[2].userAgent, deviceScaleFactor: 1 }); + + test('a file dropped from the OS onto the remote pane uploads', async ({ page }) => { + await boot(page, { windowScale: 2 }); + await page.getByTitle('files on web-1').click(); + const remotePane = page.getByRole('region', { name: 'web-1' }); + await expect(remotePane.getByText('config.yml')).toBeVisible(); + + const at = await centre(remotePane.getByText('config.yml')); + await osDrop(page, ['/tmp/photo.png'], { x: at.x / 2, y: at.y / 2 }); + await expect(page.getByLabel('transfer progress')).toBeVisible(); + expect(await uploads(page)).toEqual(['/photo.png']); + }); +}); + +test('drag and drop: a drive or volume dropped from the OS is refused, the rest uploads', async ({ + page +}) => { + await boot(page); + await page.getByTitle('files on web-1').click(); + const remotePane = page.getByRole('region', { name: 'web-1' }); + await expect(remotePane.getByText('config.yml')).toBeVisible(); + + const at = await centre(remotePane.getByText('config.yml')); + await osDrop(page, ['/', 'D:\\', '/tmp/photo.png'], at); + await expect(page.getByText('A drive or volume cannot be uploaded.')).toBeVisible(); + await expect(page.getByLabel('transfer progress')).toBeVisible(); + expect(await uploads(page)).toEqual(['/photo.png']); +}); + +test('two dropped items of one name are refused, not uploaded over each other', async ({ + page +}) => { + await boot(page); + await page.getByTitle('files on web-1').click(); + const remotePane = page.getByRole('region', { name: 'web-1' }); + await expect(remotePane.getByText('config.yml')).toBeVisible(); + + const at = await centre(remotePane.getByText('config.yml')); + await osDrop(page, ['/tmp/a/readme.txt', '/tmp/b/readme.txt'], at); + await expect(page.getByText("Two of the items would both be 'readme.txt' in /.")).toBeVisible(); + await expect(page.getByLabel('transfer progress')).toHaveCount(0); + expect(await uploads(page)).toEqual([]); +}); + +test('an OS drop reaches only the tab on show (§2 exactly-one-active)', async ({ page }) => { + await boot(page); + await page.getByTitle('files on web-1').click(); + await expect(page.getByRole('region', { name: 'web-1' }).getByText('config.yml')).toBeVisible(); + await page.getByRole('button', { name: 'Dashboard', exact: true }).click(); + await page.getByTitle('files on db-1').click(); + const dbRemote = page.getByRole('region', { name: 'db-1' }); + await expect(dbRemote.getByText('config.yml')).toBeVisible(); + + // The hidden web-1 tab hears the drop too, and must leave it to db-1. + await osDrop(page, ['/tmp/photo.png'], await centre(dbRemote.getByText('config.yml'))); + await expect(page.getByLabel('transfer progress')).toBeVisible(); + expect(await uploads(page)).toEqual(['/photo.png']); + await page.getByRole('button', { name: 'web-1 · sftp', exact: true }).click(); + await expect(page.getByRole('region', { name: 'web-1' })).toBeVisible(); + await expect(page.getByLabel('transfer progress').filter({ visible: true })).toHaveCount(0); +}); + test('an inactive tab’s modal never overlays another entity (§2 exactly-one-active)', async ({ page }) => { @@ -228,6 +848,37 @@ test('an inactive tab’s modal never overlays another entity (§2 exactly-one-a await expect(page.getByRole('dialog', { name: 'New folder' })).toHaveCount(0); }); +test('a drag lets go when its tab is hidden (§2 exactly-one-active)', async ({ page }) => { + await boot(page); + await page.getByTitle('files on web-1').click(); + await expect(page.getByRole('region', { name: 'web-1' }).getByText('config.yml')).toBeVisible(); + await page.getByRole('button', { name: 'Dashboard', exact: true }).click(); + await page.getByTitle('files on db-1').click(); + const dbRemote = page.getByRole('region', { name: 'db-1' }); + await expect(dbRemote.getByText('config.yml')).toBeVisible(); + await page.getByRole('button', { name: 'web-1 · sftp', exact: true }).click(); + + // A drag in web-1, and db-1 brought up with the keyboard while the button is held. + const localPane = page.getByRole('region', { name: 'Local' }); + await startDrag(page, localPane.getByTitle('notes.txt', { exact: true })); + const named = page.getByText('notes.txt', { exact: true }).filter({ visible: true }); + await expect(named).toHaveCount(2); // the row and the ghost + await page.keyboard.press('Control+k'); + await page.getByRole('dialog', { name: 'Command palette' }).getByRole('textbox').fill('db-1'); + await page.keyboard.press('Enter'); + await expect(dbRemote).toBeVisible(); + + // No ghost over db-1, and a release on its remote pane sends nothing. + await expect(named).toHaveCount(1); + const over = await centre(dbRemote.getByText('config.yml')); + await page.mouse.move(over.x, over.y, { steps: 4 }); + await page.mouse.up(); + await page.waitForTimeout(100); + const uploads = await page.evaluate(() => (window as unknown as { __uploads: string[] }).__uploads); + expect(uploads).toEqual([]); + await expect(page.getByLabel('transfer progress')).toHaveCount(0); +}); + test('action-first: the SFTP spawner opens the host picker, then a live session', async ({ page }) => { @@ -243,7 +894,7 @@ test('action-first: the SFTP spawner opens the host picker, then a live session' test('Windows: the local pane switches drives, and downloads land on the chosen one', async ({ page }) => { - await boot(page, true); + await boot(page, { windows: true }); await page.getByTitle('files on web-1').click(); const localPane = page.getByRole('region', { name: 'Local' }); @@ -266,7 +917,7 @@ test('Windows: the local pane switches drives, and downloads land on the chosen test('Windows: a drive that fails to list says why, and the selector stays on the pane’s drive', async ({ page }) => { - await boot(page, true); + await boot(page, { windows: true }); await page.getByTitle('files on web-1').click(); const localPane = page.getByRole('region', { name: 'Local' }); const drive = localPane.getByRole('combobox', { name: 'Local drive' }); @@ -289,8 +940,27 @@ test('a single-root system shows no drive switch', async ({ page }) => { await expect(localPane.getByRole('combobox', { name: 'Local drive' })).toHaveCount(0); }); +test('Windows: a slow drive opened during a download stays open once it is done', async ({ + page +}) => { + await boot(page, { windows: true }); + await page.getByTitle('files on web-1').click(); + const localPane = page.getByRole('region', { name: 'Local' }); + const remotePane = page.getByRole('region', { name: 'web-1' }); + await expect(localPane.getByText('notes.txt')).toBeVisible(); + await remotePane.getByRole('checkbox', { name: 'Mark config.yml' }).click(); + await page.getByRole('button', { name: 'Download' }).click(); + await expect(page.getByLabel('transfer progress')).toBeVisible(); + + // Z: is still listing when the download ends. + await localPane.getByRole('combobox', { name: 'Local drive' }).selectOption('Z:\\'); + await complete(page); + await expect(localPane.getByText('Share')).toBeVisible(); + await expect(localPane.getByText('notes.txt')).toHaveCount(0); +}); + test('Windows: a slow drive left for another does not take the pane back', async ({ page }) => { - await boot(page, true); + await boot(page, { windows: true }); await page.getByTitle('files on web-1').click(); const localPane = page.getByRole('region', { name: 'Local' }); const drive = localPane.getByRole('combobox', { name: 'Local drive' }); diff --git a/crates/omnyssh-gui/ui/src/lib/bindings.ts b/crates/omnyssh-gui/ui/src/lib/bindings.ts index e497bf6c..3fab408b 100644 --- a/crates/omnyssh-gui/ui/src/lib/bindings.ts +++ b/crates/omnyssh-gui/ui/src/lib/bindings.ts @@ -196,8 +196,9 @@ async sftpList(sessionId: number, path: string) : Promise> { try { @@ -208,8 +209,10 @@ async sftpUpload(sessionId: number, local: string, remote: string) : Promise> { try { @@ -264,7 +267,19 @@ async sftpPreview(sessionId: number, path: string) : Promise> { + try { + return { status: "ok", data: await TAURI_INVOKE("sftp_cancel", { sessionId }) }; +} catch (e) { + if(e instanceof Error) throw e; + else return { status: "error", error: e as any }; +} +}, +/** + * Close an SFTP session and its connection, stopping its transfer (tech-gui.md §4.2). */ async sftpClose(sessionId: number) : Promise> { try { @@ -515,8 +530,9 @@ export type Error = { message: string } /** * A file or directory in an SFTP panel listing (tech-gui.md §4.1). Maps from the * core `FileEntry`; `path` is the absolute path the frontend marks entries by. + * `modified` is Unix seconds, `null` when unknown. */ -export type FileEntryDto = { name: string; path: string; size: number; isDir: boolean } +export type FileEntryDto = { name: string; path: string; size: number; isDir: boolean; modified: number | null } /** * Preview bytes for a remote file (tech-gui.md §4.3). Stamped with `sessionId`; the * core `FilePreviewReady` carries only the path + content (§3.4). diff --git a/crates/omnyssh-gui/ui/src/lib/ipc/commands.ts b/crates/omnyssh-gui/ui/src/lib/ipc/commands.ts index cf40818d..89a73e81 100644 --- a/crates/omnyssh-gui/ui/src/lib/ipc/commands.ts +++ b/crates/omnyssh-gui/ui/src/lib/ipc/commands.ts @@ -112,13 +112,14 @@ export async function sftpList(sessionId: number, path: string): Promise { if (res.status === 'error') throw new Error(res.error.message); } -/** Upload a local file to a remote path; progress arrives as `transfer-progress`. */ +/** Upload a local file or folder to a remote path; progress arrives as `transfer-progress`. */ export async function sftpUpload(sessionId: number, local: string, remote: string): Promise { const res = await commands.sftpUpload(sessionId, local, remote); if (res.status === 'error') throw new Error(res.error.message); } -/** Download a remote file to a local path; progress arrives as `transfer-progress`. */ +/** Download a remote file or folder to a local path; progress arrives as + * `transfer-progress`. */ export async function sftpDownload( sessionId: number, local: string, @@ -152,6 +153,12 @@ export async function sftpPreview(sessionId: number, path: string): Promise { + const res = await commands.sftpCancel(sessionId); + if (res.status === 'error') throw new Error(res.error.message); +} + /** Close an SFTP session and its connection. Idempotent for an already-closed id. */ export async function sftpClose(sessionId: number): Promise { const res = await commands.sftpClose(sessionId); diff --git a/crates/omnyssh-gui/ui/src/lib/ipc/subscribe.test.ts b/crates/omnyssh-gui/ui/src/lib/ipc/subscribe.test.ts index 8775343d..dcbd11db 100644 --- a/crates/omnyssh-gui/ui/src/lib/ipc/subscribe.test.ts +++ b/crates/omnyssh-gui/ui/src/lib/ipc/subscribe.test.ts @@ -139,7 +139,9 @@ describe('startEventBridge', () => { payload: { sessionId: 11, path: '/srv', - entries: [{ name: 'app.log', path: '/srv/app.log', size: 12, isDir: false }] + entries: [ + { name: 'app.log', path: '/srv/app.log', size: 12, isDir: false, modified: null } + ] } }); diff --git a/crates/omnyssh-gui/ui/src/lib/platform.test.ts b/crates/omnyssh-gui/ui/src/lib/platform.test.ts new file mode 100644 index 00000000..3cca80f0 --- /dev/null +++ b/crates/omnyssh-gui/ui/src/lib/platform.test.ts @@ -0,0 +1,26 @@ +import { describe, expect, it } from 'vitest'; +import { dropPoint } from './platform'; + +// Where an OS drop lands, from the position each webview reports (tauri-runtime-wry +// wraps it unscaled): it decides which pane and folder row take the drop. +describe('dropPoint', () => { + const reported = { x: 1800, y: 600 }; + + it('scales WebView2 physical pixels down on Windows', () => { + expect(dropPoint(reported, 1.5, true, 1.5)).toEqual({ x: 1200, y: 400 }); + expect(dropPoint(reported, 1, true, 1)).toEqual(reported); + }); + + it('keeps WKWebView points on a Retina Mac and WebKitGTK ones under GDK_SCALE=2', () => { + expect(dropPoint(reported, 2, false, 2)).toEqual(reported); + }); + + it('scales WebKitGTK logical pixels up when the page renders at 1 in a 2x window', () => { + expect(dropPoint(reported, 2, false, 1)).toEqual({ x: 3600, y: 1200 }); + }); + + it('keeps the position while the window scale is unknown', () => { + expect(dropPoint(reported, undefined, false, 2)).toEqual(reported); + expect(dropPoint(reported, null, false, 2)).toEqual(reported); + }); +}); diff --git a/crates/omnyssh-gui/ui/src/lib/platform.ts b/crates/omnyssh-gui/ui/src/lib/platform.ts index 45c438d4..dbb07aef 100644 --- a/crates/omnyssh-gui/ui/src/lib/platform.ts +++ b/crates/omnyssh-gui/ui/src/lib/platform.ts @@ -3,3 +3,18 @@ // title-bar inset, and the e2e suite emulates a platform by its user agent alone. export const isMac = /Mac/.test(navigator.userAgent); export const isWindows = /Windows/.test(navigator.userAgent); + +/** An OS drag-and-drop position in CSS pixels. Tauri passes on what the webview + * reports: WebView2 counts physical pixels, WKWebView and WebKitGTK the window's + * logical ones, each `scale` (the window's scale factor) physical pixels wide. Those + * are CSS pixels while the page renders at the window's scale; WebKitGTK under + * GDK_SCALE=2 with GDK_DPI_SCALE=0.5 renders it at 1. */ +export function dropPoint( + position: { x: number; y: number }, + scale?: number | null, + windows = isWindows, + ratio = globalThis.devicePixelRatio || 1 +): { x: number; y: number } { + const k = windows ? 1 / ratio : (scale ?? ratio) / ratio; + return { x: position.x * k, y: position.y * k }; +} diff --git a/crates/omnyssh-gui/ui/src/lib/screens/SftpPane.svelte b/crates/omnyssh-gui/ui/src/lib/screens/SftpPane.svelte index 0a74a3cd..9a20fd8a 100644 --- a/crates/omnyssh-gui/ui/src/lib/screens/SftpPane.svelte +++ b/crates/omnyssh-gui/ui/src/lib/screens/SftpPane.svelte @@ -2,34 +2,57 @@ // One side of the dual-pane SFTP browser (tech-gui.md §3.2): a current-path header // with a parent-supplied toolbar, then the entry list. Clicking a directory (or the // `..` row) navigates; clicking a file previews; the leading checkbox marks it for a - // batch transfer/delete. Semantic tokens only — no colour literals (§5.1). + // batch transfer/delete. A press on a row may start a drag (the parent owns the drag + // and marks the pane `dropActive` while it is the target); `data-pane` and + // `data-dir-path` are how the parent finds the pane and folder under the pointer. + // A listing on its way dims the current one: a remote one can wait behind a transfer. + // Semantic tokens only — no colour literals (§5.1). import type { Snippet } from 'svelte'; import { Icon } from '$lib/theme'; import type { FileEntryDto } from '$lib/bindings'; - import { formatBytes, type Pane } from '$lib/stores/sftp'; + import { formatBytes, formatDate, type Pane, type PaneSide } from '$lib/stores/sftp'; let { title, + side, pane, + dropActive = false, + dropDir, onNavigate, onToggleMark, onPreview, + onDragStart, toolbar }: { title: string; + side: PaneSide; pane: Pane; + /** The pane is the target of a drag in progress. */ + dropActive?: boolean; + /** The folder row under the pointer while this pane is the drop target. */ + dropDir?: string; onNavigate: (entry: FileEntryDto) => void; onToggleMark: (path: string) => void; onPreview: (entry: FileEntryDto) => void; + onDragStart?: (entry: FileEntryDto, event: PointerEvent) => void; toolbar?: Snippet; } = $props(); const rowBase = 'flex w-full min-w-0 items-center gap-2 rounded px-2 py-1.5 text-left text-sm transition ' + 'focus-visible:outline-none focus-visible:ring-2 focus-visible:ring-focus'; + // The date column appears once the pane is wide enough to keep names readable. It + // never wraps: under GDK_DPI_SCALE the rem it is sized in can shrink below its text. + const modifiedCol = 'hidden min-w-36 shrink-0 whitespace-nowrap text-right lg:inline'; -
+

-
- {pane.path || '—'} +
+ {pane.path || '—'} + {#if pane.loading && pane.entries.length > 0} + Loading… + {/if}
@@ -55,11 +81,24 @@ {:else if pane.entries.length === 0}

Empty directory

{:else} -
    + +
      {#each pane.entries as entry, i (i)} {@const isParent = entry.name === '..'} {@const marked = pane.marked.has(entry.path)} -
    • + {@const dropHere = dropActive && entry.isDir && dropDir === entry.path} +
    • {#if isParent} {:else} @@ -81,6 +120,7 @@ class="{rowBase} text-muted hover:bg-surface-inset hover:text-fg" title={entry.name} onclick={() => (entry.isDir ? onNavigate(entry) : onPreview(entry))} + onpointerdown={(e) => onDragStart?.(entry, e)} > @@ -89,6 +129,14 @@ {#if !entry.isDir} {formatBytes(entry.size)} {/if} + {#if !isParent} + + {formatDate(entry.modified)} + + {/if}
      • {/each} diff --git a/crates/omnyssh-gui/ui/src/lib/screens/SftpView.svelte b/crates/omnyssh-gui/ui/src/lib/screens/SftpView.svelte index 6954dbed..3dd256c4 100644 --- a/crates/omnyssh-gui/ui/src/lib/screens/SftpView.svelte +++ b/crates/omnyssh-gui/ui/src/lib/screens/SftpView.svelte @@ -7,14 +7,29 @@ // remote uses sftp_list (arrives as an event). Semantic tokens only (§5.1). import { onMount, onDestroy } from 'svelte'; import { homeDir } from '@tauri-apps/api/path'; - import { Icon } from '$lib/theme'; + import { getCurrentWebview } from '@tauri-apps/api/webview'; + import { Button, Icon } from '$lib/theme'; import Modal from '$lib/components/Modal.svelte'; import Select from '$lib/components/Select.svelte'; import SftpPane from './SftpPane.svelte'; import type { FileEntryDto } from '$lib/bindings'; import { sessions, type Session } from '$lib/stores/sessions'; - import { sftp, markedEntries, formatBytes, rootOf, type PaneSide } from '$lib/stores/sftp'; + import { + sftp, + markedEntries, + dragPayload, + clashCount, + repeatedName, + formatBytes, + rootOf, + baseName, + isPlainName, + transferState, + CANCELLED, + type PaneSide + } from '$lib/stores/sftp'; import { lastError } from '$lib/stores/notifications'; + import { dropPoint, isMac, isWindows } from '$lib/platform'; import { sftpOpen, sftpList, @@ -25,6 +40,7 @@ sftpRename, sftpDelete, sftpPreview, + sftpCancel, listLocalDir, listLocalRoots, previewLocalFile @@ -40,8 +56,9 @@ // Queued mutations, dispatched one at a time (see the pump effect). The core's SFTP // command channel is bounded and drops on overflow, so a large batch fired at once // would silently lose commands and wedge the op-done FIFO; gating on the previous - // op's completion keeps at most one command outstanding. - let outbox = $state void>>([]); + // op's completion keeps at most one command outstanding. Transfers are flagged so a + // cancel drops the queued ones too. + let outbox = $state void }>>([]); // A pending mkdir/rename input. Rename carries the entry being renamed. let prompt = $state<{ kind: 'mkdir' | 'rename'; value: string; target?: FileEntryDto } | null>( @@ -65,11 +82,11 @@ await refreshLocal(root); selectedDrive = drive; } - const transfer = $derived(view?.transfer); + const transfer = $derived(view ? transferState(view) : undefined); - const localMarkedFiles = $derived(view ? markedEntries(view.local).filter((e) => !e.isDir) : []); + // Folders transfer whole, so the Upload/Download buttons take them like files. + const localMarked = $derived(view ? markedEntries(view.local) : []); const remoteMarked = $derived(view ? markedEntries(view.remote) : []); - const remoteMarkedFiles = $derived(remoteMarked.filter((e) => !e.isDir)); const singleRemoteMark = $derived(remoteMarked.length === 1 ? remoteMarked[0] : undefined); function errMsg(err: unknown): string { @@ -169,7 +186,7 @@ if (!view || view.pending.length > 0 || outbox.length === 0) return; const [next, ...rest] = outbox; outbox = rest; - next(); + next.run(); }); // Re-list the affected pane once every queued mutation has drained — the FS changed @@ -179,8 +196,15 @@ if (id == null || !view || view.pending.length > 0 || outbox.length > 0 || !view.refresh) return; const target = view.refresh; sftp.clearRefresh(id); - if (target === 'local' || target === 'both') void refreshLocal(view.local.path); - if (target === 'remote' || target === 'both') refreshRemote(view.remote.path); + // A listing asked for meanwhile is where the user went; the old path again would + // take the pane back. A remote one also waited in the core's queue behind the + // transfer, so it shows the change. + if ((target === 'local' || target === 'both') && !view.local.loading) { + void refreshLocal(view.local.path); + } + if ((target === 'remote' || target === 'both') && !view.remote.loading) { + refreshRemote(view.remote.path); + } }); function navigate(side: PaneSide, entry: FileEntryDto): void { @@ -216,45 +240,266 @@ }; } - function enqueue(...actions: Array<() => void>): void { + function enqueue(actions: Array<() => void>, transfer = false): void { if (!actions.length) return; // Clear the prior batch's lingering error only when starting from idle. Piling onto a // batch that is still draining must not wipe a failure it already recorded (that error // stays visible until the next fresh action — see applyOpDone). const draining = outbox.length > 0 || (view?.pending.length ?? 0) > 0; if (backendId != null && !draining) sftp.clearError(backendId); - outbox = [...outbox, ...actions]; + outbox = [...outbox, ...actions.map((run) => ({ transfer, run }))]; } - function upload(): void { + // Stops the running transfer and drops the queued ones; the core cancels what was + // sent before this, so a transfer started afterwards runs. + function cancelTransfers(): void { const id = backendId; - if (id == null || !view) return; - const dir = view.remote.path; - enqueue( - ...localMarkedFiles.map((file) => () => { - sftp.pushOp(id, { kind: 'upload', name: file.name, refresh: 'remote' }); - void sftpUpload(id, file.path, joinRemote(dir, file.name)).catch(onDispatchError(id)); - }) + if (id == null) return; + outbox = outbox.filter((op) => !op.transfer); + void sftpCancel(id).catch((err) => lastError.set(errMsg(err))); + } + + // A transfer held back until the user agrees to replace what is there. + let overwrite = $state<{ count: number; dir: string; run: () => void } | null>(null); + + // Runs a transfer of `names` into `dir`, asking first when the listing on show there + // already has some of them. A folder row's listing is unknown, so a drop on one goes. + // Two items bound for one name do not go: the second would replace the first unasked. + function unlessClashing(side: PaneSide, names: string[], dir: string, run: () => void): void { + const pane = view?.[side]; + const caseless = side === 'local' && (isWindows || isMac); + const twice = repeatedName(names, caseless); + if (twice) { + lastError.set(`Two of the items would both be '${twice}' in ${dir}.`); + return; + } + const count = pane?.path === dir ? clashCount(names, pane.entries, caseless) : 0; + if (count > 0) overwrite = { count, dir, run }; + else run(); + } + + function replace(): void { + const run = overwrite?.run; + overwrite = null; + run?.(); + } + + // `dir` defaults to the other pane's current directory; a drop onto a folder row + // passes that folder instead. + function upload(files: Array>, dir = view?.remote.path): void { + const id = backendId; + if (id == null || !view || dir == null) return; + unlessClashing('remote', files.map((file) => file.name), dir, () => + enqueue( + files.map((file) => () => { + sftp.pushOp(id, { kind: 'upload', name: file.name, refresh: 'remote' }); + void sftpUpload(id, file.path, joinRemote(dir, file.name)).catch(onDispatchError(id)); + }), + true + ) ); } - function download(): void { + // The local path is the listed name joined onto `dir`, so only a plain name goes. + function download(files: FileEntryDto[], dir = view?.local.path): void { const id = backendId; - if (id == null || !view) return; - const dir = view.local.path; - enqueue( - ...remoteMarkedFiles.map((file) => () => { - sftp.pushOp(id, { kind: 'download', name: file.name, refresh: 'local' }); - void sftpDownload(id, joinLocal(dir, file.name), file.path).catch(onDispatchError(id)); - }) + if (id == null || !view || dir == null) return; + const odd = files.find((file) => !isPlainName(file.name, isWindows)); + if (odd) lastError.set(`'${odd.name}' is not a name that can be created here.`); + const plain = files.filter((file) => isPlainName(file.name, isWindows)); + unlessClashing('local', plain.map((file) => file.name), dir, () => + enqueue( + plain.map((file) => () => { + sftp.pushOp(id, { kind: 'download', name: file.name, refresh: 'local' }); + void sftpDownload(id, joinLocal(dir, file.name), file.path).catch(onDispatchError(id)); + }), + true + ) ); } + // --- Drag and drop ----------------------------------------------------------------- + // Pane to pane uses pointer events, not HTML5 drag and drop: Tauri's native file-drop + // handler (on by default, and needed for drops from the OS below) swallows HTML5 drag + // events in WebView2. A press becomes a drag only past DRAG_THRESHOLD px, so a click + // still navigates or previews. + const DRAG_THRESHOLD = 5; + + interface DropTarget { + side: PaneSide; + /** The directory the drop lands in: a folder row under the pointer, else the + * pane's current directory. */ + dir: string; + /** Set when `dir` is a folder row, for its highlight. */ + row?: string; + } + + let root = $state(); + let press: { side: PaneSide; entry: FileEntryDto; x: number; y: number } | null = null; + let drag = $state<{ + from: PaneSide; + files: FileEntryDto[]; + x: number; + y: number; + target: DropTarget | null; + } | null>(null); + // The press went past the threshold, carrying something or not (`..`): the release + // must not click the row it ends on, even after Escape. + let moved = false; + let swallowClick = false; + // A drag from the OS hovering this view, for the remote pane's highlight. + let osDrop = $state(null); + + function endGesture(): void { + press = null; + drag = null; + moved = false; + } + + // A hidden view lets go of what it held (§2): no ghost over the entity now shown, no + // transfer from a release there, and no Replace prompt to come back with a stale count. + $effect(() => { + if (!active) { + endGesture(); + osDrop = null; + overwrite = null; + } + }); + + /** This view's pane (and folder row, if any) under a viewport point. Below the panes, + * on the transfer strip, a point goes to the pane above it. */ + function dropTargetAt(x: number, y: number): DropTarget | null { + if (!view || !root) return null; + const el = document.elementFromPoint(x, y); + if (!el || !root.contains(el)) return null; + const paneEl = + el.closest('[data-pane]') ?? + [...root.querySelectorAll('[data-pane]')].find((pane) => { + const r = pane.getBoundingClientRect(); + return x >= r.left && x < r.right; + }); + const side = paneEl?.dataset.pane; + if (side !== 'local' && side !== 'remote') return null; + const row = el.closest('[data-dir-path]')?.dataset.dirPath; + return { side, dir: row ?? view[side].path, row }; + } + + function startPress(side: PaneSide, entry: FileEntryDto, e: PointerEvent): void { + if (e.button !== 0 || !view || !active) return; + press = { side, entry, x: e.clientX, y: e.clientY }; + moved = false; + } + + function onPointerMove(e: PointerEvent): void { + if (!press && !drag) return; + // The button came up where no pointerup reached this window. + if ((e.buttons & 1) === 0) { + endGesture(); + return; + } + if (drag) { + const target = dropTargetAt(e.clientX, e.clientY); + drag = { + ...drag, + x: e.clientX, + y: e.clientY, + target: target && target.side !== drag.from ? target : null + }; + return; + } + if (!press || !view) return; + if (Math.hypot(e.clientX - press.x, e.clientY - press.y) < DRAG_THRESHOLD) return; + moved = true; + const files = dragPayload(view[press.side], press.entry); + const from = press.side; + press = null; + if (files.length === 0) return; + drag = { from, files, x: e.clientX, y: e.clientY, target: null }; + } + + function onPointerUp(): void { + const done = drag; + if (moved) { + swallowClick = true; + setTimeout(() => (swallowClick = false), 0); + } + endGesture(); + if (!active || !done?.target) return; + if (done.from === 'local') upload(done.files, done.target.dir); + else download(done.files, done.target.dir); + } + + function onKeyDown(e: KeyboardEvent): void { + if (e.key === 'Escape' && drag) drag = null; + } + + // Drops from the OS file manager, with absolute paths and a position in the webview's + // own pixels. Only the remote pane accepts them; every SFTP view stays mounted, so + // only the visible one reacts. + onMount(() => { + let unlisten: (() => void) | undefined; + let webview: ReturnType; + try { + webview = getCurrentWebview(); + } catch { + // Off the Tauri runtime there is no webview to drop onto; pane to pane still works. + return; + } + // Read again as each drag comes in: the window may have moved to a screen of + // another scale. + let scale: number | null = null; + const readScale = () => + void webview.window + .scaleFactor() + .then((s) => (scale = s)) + .catch(() => {}); + readScale(); + void webview + .onDragDropEvent((event) => { + if (!active || !view) return; + const p = event.payload; + if (p.type === 'leave') { + osDrop = null; + return; + } + if (p.type === 'enter') readScale(); + const at = dropPoint(p.position, scale); + const target = dropTargetAt(at.x, at.y); + const remote = target?.side === 'remote' ? target : null; + if (p.type === 'drop') { + osDrop = null; + if (remote) uploadDropped(p.paths, remote.dir); + } else { + osDrop = remote; + } + }) + .then((fn) => { + if (destroyed) fn(); + else unlisten = fn; + }) + .catch(() => {}); + return () => unlisten?.(); + }); + + // Dropped paths upload as they are: the core tells files from folders and walks the + // folders. + function uploadDropped(paths: string[], dir: string): void { + const items = paths.map((path) => ({ path, name: baseName(path) })); + if (items.some((item) => !item.name)) lastError.set('A drive or volume cannot be uploaded.'); + upload(items.filter((item) => item.name), dir); + } + + function guarded(fn: (arg: T) => void): (arg: T) => void { + return (arg) => { + if (!swallowClick) fn(arg); + }; + } + function remove(): void { const id = backendId; if (id == null) return; enqueue( - ...remoteMarked.map((entry) => () => { + remoteMarked.map((entry) => () => { sftp.pushOp(id, { kind: 'delete', name: entry.name, refresh: 'remote' }); void sftpDelete(id, entry.path).catch(onDispatchError(id)); }) @@ -276,16 +521,20 @@ if (!value) return; const dir = view.remote.path; if (prompt.kind === 'mkdir') { - enqueue(() => { - sftp.pushOp(id, { kind: 'mkdir', refresh: 'remote' }); - void sftpMkdir(id, joinRemote(dir, value)).catch(onDispatchError(id)); - }); + enqueue([ + () => { + sftp.pushOp(id, { kind: 'mkdir', refresh: 'remote' }); + void sftpMkdir(id, joinRemote(dir, value)).catch(onDispatchError(id)); + } + ]); } else if (prompt.target) { const from = prompt.target.path; - enqueue(() => { - sftp.pushOp(id, { kind: 'rename', refresh: 'remote' }); - void sftpRename(id, from, joinRemote(dir, value)).catch(onDispatchError(id)); - }); + enqueue([ + () => { + sftp.pushOp(id, { kind: 'rename', refresh: 'remote' }); + void sftpRename(id, from, joinRemote(dir, value)).catch(onDispatchError(id)); + } + ]); } prompt = null; } @@ -313,9 +562,34 @@ 'focus-visible:ring-2 focus-visible:ring-focus placeholder:text-faint'; + + +{#if active && drag} + + +{/if} + -
      + panes below them. The select-none while dragging keeps the drag from selecting text. --> +
      {#if openError}

      Could not open SFTP on {session.hostName}

      @@ -329,10 +603,14 @@
      navigate('local', e)} + dropActive={drag?.target?.side === 'local'} + dropDir={drag?.target?.side === 'local' ? drag.target.row : undefined} + onNavigate={guarded((e) => navigate('local', e))} onToggleMark={(p) => toggleMark('local', p)} - onPreview={(e) => preview('local', e)} + onPreview={guarded((e) => preview('local', e))} + onDragStart={(entry, e) => startPress('local', entry, e)} > {#snippet toolbar()} {#if roots.length > 1} @@ -354,9 +632,9 @@
      -
      + {#if transfer.preparing} + +
      + {:else} +
      + {/if}
      {:else if view.error} -
      + +
      {view.error}
      {/if} @@ -502,6 +811,22 @@ {/if} +{#if active && overwrite} + (overwrite = null)}> +
      +

      Replace existing items

      +

      + {overwrite.count} item(s) already exist in {overwrite.dir}. + Folders are merged and files with the same name are replaced. +

      +
      + + +
      +
      +       +{/if} + {#if active && view?.preview}
      diff --git a/crates/omnyssh-gui/ui/src/lib/stores/sftp.test.ts b/crates/omnyssh-gui/ui/src/lib/stores/sftp.test.ts index 5e8f3111..d3b5216f 100644 --- a/crates/omnyssh-gui/ui/src/lib/stores/sftp.test.ts +++ b/crates/omnyssh-gui/ui/src/lib/stores/sftp.test.ts @@ -10,8 +10,16 @@ import { mergeRefresh, applyProgress, applyOpDone, + transferState, formatBytes, + formatDate, + dragPayload, + clashCount, + repeatedName, rootOf, + baseName, + isPlainName, + CANCELLED, type Pane, type SftpSession } from './sftp'; @@ -20,13 +28,106 @@ import { // it is unit-testable without a Tauri runtime (tech-gui.md §3.2, §6.4). function entry(name: string, isDir = false, size = 0): FileEntryDto { - return { name, path: `/srv/${name}`, size, isDir }; + return { name, path: `/srv/${name}`, size, isDir, modified: null }; } function paneWith(entries: FileEntryDto[], marked: string[] = []): Pane { return { path: '/srv', entries, loading: false, marked: new Set(marked) }; } +describe('formatDate', () => { + it('renders unknown times as an em dash', () => { + expect(formatDate(null)).toBe('—'); + expect(formatDate(undefined)).toBe('—'); + expect(formatDate(Number.NaN)).toBe('—'); + }); + + it('renders a known time with its year, short and long', () => { + const secs = Date.UTC(2024, 5, 15, 12, 0) / 1000; + expect(formatDate(secs)).toContain('2024'); + expect(formatDate(secs, true)).toContain('2024'); + expect(formatDate(secs, true).length).toBeGreaterThan(formatDate(secs).length); + }); +}); + +describe('dragPayload', () => { + const a = entry('a.txt'); + const b = entry('b.txt'); + const dir = entry('logs', true); + const parent = { ...entry('..', true), path: '/' }; + + it('carries only the dragged file when it is not marked', () => { + expect(dragPayload(paneWith([a, b], [b.path]), a)).toEqual([a]); + }); + + it('carries every marked entry, folders included, in listing order', () => { + const pane = paneWith([a, dir, b], [b.path, dir.path, a.path]); + expect(dragPayload(pane, b)).toEqual([a, dir, b]); + expect(dragPayload(pane, dir)).toEqual([a, dir, b]); + }); + + it('carries an unmarked folder on its own', () => { + expect(dragPayload(paneWith([a, dir], [a.path]), dir)).toEqual([dir]); + }); + + it('carries nothing for the parent row', () => { + expect(dragPayload(paneWith([parent]), parent)).toEqual([]); + }); +}); + +describe('clashCount', () => { + const listing = [{ ...entry('..', true), path: '/' }, entry('App.log'), entry('www', true)]; + + it('counts the names the listing already has, files and folders alike', () => { + expect(clashCount(['www', 'App.log', 'new.txt'], listing, false)).toBe(2); + expect(clashCount(['new.txt'], listing, false)).toBe(0); + }); + + it('never counts the parent row', () => { + expect(clashCount(['..'], listing, false)).toBe(0); + }); + + it('matches across case only where the file system ignores it', () => { + expect(clashCount(['app.log', 'WWW'], listing, false)).toBe(0); + expect(clashCount(['app.log', 'WWW'], listing, true)).toBe(2); + }); + + it('matches an accented letter composed or not where case is ignored, as macOS does', () => { + const decomposed = [entry('cafe\u0301.txt')]; + expect(clashCount(['caf\u00e9.txt'], decomposed, true)).toBe(1); + expect(clashCount(['caf\u00e9.txt'], decomposed, false)).toBe(0); + }); +}); + +describe('repeatedName', () => { + it('finds two items that would land on one name', () => { + expect(repeatedName(['readme.txt', 'a.png', 'readme.txt'], false)).toBe('readme.txt'); + expect(repeatedName(['readme.txt', 'a.png'], false)).toBeUndefined(); + }); + + it('matches across case only where the file system ignores it', () => { + expect(repeatedName(['A.txt', 'a.txt'], false)).toBeUndefined(); + expect(repeatedName(['A.txt', 'a.txt'], true)).toBe('a.txt'); + }); +}); + +describe('isPlainName', () => { + it('takes one plain component and nothing that would land elsewhere', () => { + for (const name of ['notes.txt', '..hidden', 'a b', 'a\\b']) { + expect(isPlainName(name, false)).toBe(true); + } + for (const name of ['', '.', '..', 'a/b', './x', 'x/', '.config/autostart']) { + expect(isPlainName(name, false)).toBe(false); + } + }); + + it('splits on a backslash only on Windows', () => { + expect(isPlainName('a\\b', true)).toBe(false); + expect(isPlainName('..\\x', true)).toBe(false); + expect(isPlainName('notes.txt', true)).toBe(true); + }); +}); + describe('sftp reducers', () => { it('starts a session connecting with both panes empty and loading', () => { const s = newSession('web-1'); @@ -125,6 +226,18 @@ describe('sftp reducers', () => { expect(s.pending).toEqual([]); }); + it('applyOpDone keeps a prior op error when the rest of the batch is cancelled', () => { + const upload = (name: string) => ({ kind: 'upload' as const, name, refresh: 'remote' as const }); + let s: SftpSession = { ...newSession('web-1'), pending: [upload('a.txt'), upload('b.txt')] }; + s = applyOpDone(s, false, "open 'a.txt': Permission denied"); + s = applyOpDone(s, false, CANCELLED); + expect(s.error).toBe("open 'a.txt': Permission denied"); + + // A cancel on its own still says so. + s = applyOpDone({ ...newSession('web-1'), pending: [upload('c.txt')] }, false, CANCELLED); + expect(s.error).toBe(CANCELLED); + }); + it('correlates a two-file batch by FIFO order across progress + op-done', () => { // The core is sequential, so the front pending op is always the one running: A's // progress shows A; A's op-done pops it; then B's progress shows B (§3.2/§4.3). @@ -146,6 +259,35 @@ describe('sftp reducers', () => { expect(s.refresh).toBe('remote'); }); + it('transferState shows a dispatched transfer as preparing until its first tick', () => { + const idle = newSession('web-1'); + expect(transferState(idle)).toBeUndefined(); + const mkdir: SftpSession = { ...idle, pending: [{ kind: 'mkdir', refresh: 'remote' }] }; + expect(transferState(mkdir)).toBeUndefined(); + + let s: SftpSession = { + ...idle, + pending: [{ kind: 'download', name: 'logs', refresh: 'local' }] + }; + expect(transferState(s)).toEqual({ + kind: 'download', + name: 'logs', + done: 0, + total: 0, + preparing: true + }); + // A folder of empty files still ticks once planned: 0 of 0, no longer preparing. + s = applyProgress(s, { sessionId: 1, transferId: 3, done: 0, total: 0 }); + expect(transferState(s)).toEqual({ + kind: 'download', + name: 'logs', + done: 0, + total: 0, + preparing: false + }); + expect(transferState(applyOpDone(s, true))).toBeUndefined(); + }); + it('formatBytes is human readable', () => { expect(formatBytes(512)).toBe('512 B'); expect(formatBytes(2048)).toBe('2.0 KB'); @@ -172,6 +314,31 @@ describe('rootOf', () => { }); }); +describe('baseName', () => { + it('takes the last component of a dropped path', () => { + expect(baseName('/tmp/album/')).toBe('album'); + expect(baseName('/home/me/back\\slash.txt')).toBe('back\\slash.txt'); + expect(baseName('C:\\Users\\me\\photo.png')).toBe('photo.png'); + }); + + it('has no name for a root', () => { + expect(baseName('/')).toBe(''); + expect(baseName('C:\\')).toBe(''); + expect(baseName('d:')).toBe(''); + expect(baseName('\\\\server\\share')).toBe(''); + expect(baseName('\\\\server\\share\\')).toBe(''); + expect(baseName('\\\\?\\C:\\')).toBe(''); + expect(baseName('\\\\?\\Volume{0b1c}\\')).toBe(''); + expect(baseName('\\\\?\\UNC\\server\\share\\')).toBe(''); + }); + + it('names what lies on a share or behind a long path', () => { + expect(baseName('\\\\server\\share\\album')).toBe('album'); + expect(baseName('\\\\?\\C:\\Users\\me\\photo.png')).toBe('photo.png'); + expect(baseName('\\\\?\\UNC\\server\\share\\album\\')).toBe('album'); + }); +}); + describe('sftp store', () => { it('keeps concurrent sessions isolated and prunes on close', () => { sftp.open(1, 'web-1'); diff --git a/crates/omnyssh-gui/ui/src/lib/stores/sftp.ts b/crates/omnyssh-gui/ui/src/lib/stores/sftp.ts index a03fee6d..433dcf80 100644 --- a/crates/omnyssh-gui/ui/src/lib/stores/sftp.ts +++ b/crates/omnyssh-gui/ui/src/lib/stores/sftp.ts @@ -34,6 +34,9 @@ interface Preview { content: string; } +/** The `sftp-op-done` error of a cancelled transfer, in the core's words. */ +export const CANCELLED = 'Transfer cancelled'; + // A mutating op awaiting its `sftp-op-done`. The core processes commands sequentially, // so op-done events arrive in issue order — this FIFO correlates each op-done to the op // that produced it (the contract carries no op id, §4.3). `refresh` is the pane whose @@ -92,6 +95,51 @@ export function markedEntries(pane: Pane): FileEntryDto[] { return pane.entries.filter((e) => pane.marked.has(e.path)); } +/** A name as a file system compares it: `caseless` for one that ignores case, as + * Windows and macOS do by default. macOS also takes an accented letter composed or not + * as the same; Windows does not, and gets a needless question at most. */ +function nameKey(name: string, caseless: boolean): string { + return caseless ? name.normalize('NFC').toLowerCase() : name; +} + +/** How many of `names` the listing `entries` already holds: what a transfer into its + * directory would replace, or merge into for a folder. */ +export function clashCount(names: string[], entries: FileEntryDto[], caseless: boolean): number { + const there = new Set( + entries.filter((e) => e.name !== '..').map((e) => nameKey(e.name, caseless)) + ); + return names.filter((name) => there.has(nameKey(name, caseless))).length; +} + +/** A name in `names` that an earlier one would land on too: two files dropped from + * different folders, or `A.txt` and `a.txt` going where case is ignored. */ +export function repeatedName(names: string[], caseless: boolean): string | undefined { + const seen = new Set(); + return names.find((name) => { + const key = nameKey(name, caseless); + if (seen.has(key)) return true; + seen.add(key); + return false; + }); +} + +/** Whether a name the server listed can be created locally as itself: one plain path + * component. Joined onto a local folder, anything else would land below it, unseen by + * the Replace check. On Windows `\\` separates too. */ +export function isPlainName(name: string, windows: boolean): boolean { + if (['', '.', '..'].includes(name) || name.includes('/')) return false; + return !(windows && name.includes('\\')); +} + +/** What dragging `entry` carries: every marked entry when `entry` is one of them, else + * `entry` alone. Folders travel with their whole contents; `..` is not transferable, + * so dragging it carries nothing. */ +export function dragPayload(pane: Pane, entry: FileEntryDto): FileEntryDto[] { + if (entry.name === '..') return []; + if (pane.marked.has(entry.path)) return markedEntries(pane); + return [entry]; +} + /** The root in `roots` that `path` lies under, or '' for none (a network share). * Drive letters compare case-insensitively, as Windows does. */ export function rootOf(path: string, roots: string[]): string { @@ -99,6 +147,19 @@ export function rootOf(path: string, roots: string[]): string { return roots.find((root) => lower.startsWith(root.toLowerCase())) ?? ''; } +// A Windows path that is only a root: a drive, a share ('\\\\server\\share'), or a drive, +// share or volume in the long form that starts '\\\\?\\'. +const WINDOWS_ROOT = /^(?:[a-z]:|\\\\[?.]\\(?:unc\\[^\\]+\\[^\\]+|[^\\]+)|\\\\[^\\]+\\[^\\]+)\\?$/i; + +/** The name a local path is uploaded under: its last component, or '' for a root + * ('/', 'C:\\', '\\\\server\\share'), which has no name to give it on the server. A path + * with no '/' is a Windows one, where '\\' separates; elsewhere '\\' is part of a name. */ +export function baseName(path: string): string { + const sep = path.includes('/') ? '/' : '\\'; + if (sep === '\\' && WINDOWS_ROOT.test(path)) return ''; + return path.split(sep).filter(Boolean).at(-1) ?? ''; +} + /** Human-readable byte size for a listing row or a transfer bar. */ export function formatBytes(bytes: number): string { if (bytes < 1024) return `${bytes} B`; @@ -112,6 +173,27 @@ export function formatBytes(bytes: number): string { return `${value.toFixed(value < 10 ? 1 : 0)} ${units[unit]}`; } +const dateFormat = new Intl.DateTimeFormat(undefined, { + year: 'numeric', + month: '2-digit', + day: '2-digit', + hour: '2-digit', + minute: '2-digit' +}); +const dateFormatFull = new Intl.DateTimeFormat(undefined, { + dateStyle: 'full', + timeStyle: 'medium' +}); + +/** A listing row's date from Unix seconds, or an em dash when the time is unknown. + * `full` gives the long form used in the hover title. */ +export function formatDate(secs: number | null | undefined, full = false): string { + if (secs == null || !Number.isFinite(secs)) return '—'; + const date = new Date(secs * 1000); + if (Number.isNaN(date.getTime())) return '—'; + return (full ? dateFormatFull : dateFormat).format(date); +} + /** Widen the pending refresh target: two different sides collapse to `both`. */ export function mergeRefresh( current: PaneSide | 'both' | undefined, @@ -133,20 +215,34 @@ export function applyProgress(session: SftpSession, p: TransferProgressDto): Sft }; } +/** What the transfer strip shows: the running transfer's last tick or, from its dispatch + * until the first tick (the core may spend a while walking a folder), that it is being + * prepared. */ +export function transferState( + session: SftpSession +): (Transfer & { preparing: boolean }) | undefined { + if (session.transfer) return { ...session.transfer, preparing: false }; + const front = session.pending[0]; + if (front?.kind !== 'upload' && front?.kind !== 'download') return undefined; + return { kind: front.kind, name: front.name ?? '', done: 0, total: 0, preparing: true }; +} + /** Fold an `sftp-op-done` in: pop the front pending op (FIFO), record its refresh * target, clear the transfer display if it was a transfer, and surface any error. */ export function applyOpDone(session: SftpSession, ok: boolean, error?: string): SftpSession { if (session.pending.length === 0) return session; const [front, ...rest] = session.pending; const wasTransfer = front.kind === 'upload' || front.kind === 'download'; + // A later op's success must NOT wipe an earlier op's failure in the same batch — that + // silently masks e.g. a non-empty-folder delete beside a deleted sibling. Nor may the + // user's own cancel. The error persists until the next batch clears it (`clearError`, + // called on enqueue). + const keep = ok || (error === CANCELLED && session.error != null); return { ...session, pending: rest, refresh: mergeRefresh(session.refresh, front.refresh), - // A later op's success must NOT wipe an earlier op's failure in the same batch — that - // silently masks e.g. a non-empty-folder delete beside a deleted sibling. The error - // persists until the next batch clears it (`clearError`, called on enqueue). - error: ok ? session.error : (error ?? 'Operation failed'), + error: keep ? session.error : (error ?? 'Operation failed'), transfer: wasTransfer ? undefined : session.transfer }; } diff --git a/crates/omnyssh/src/app/action.rs b/crates/omnyssh/src/app/action.rs index 550eb83b..519d953c 100644 --- a/crates/omnyssh/src/app/action.rs +++ b/crates/omnyssh/src/app/action.rs @@ -144,6 +144,8 @@ pub enum AppAction { FmConfirmRename(String), /// Close the active file-manager popup (Esc). FmClosePopup, + /// Cancel the running transfer, or hide its progress once cancelling (Esc). + FmCancelTransfer, /// Navigate the cursor inside the host-picker popup (j/k). FmHostPickerNav(i8), // +1 = down, -1 = up diff --git a/crates/omnyssh/src/app/actions.rs b/crates/omnyssh/src/app/actions.rs index 2d5d0fd1..72693eac 100644 --- a/crates/omnyssh/src/app/actions.rs +++ b/crates/omnyssh/src/app/actions.rs @@ -616,6 +616,27 @@ impl App { self.view.file_manager.popup = None; } + AppAction::FmCancelTransfer => { + let fm = &mut self.view.file_manager; + if fm.cancelling { + // One slow to stop need not hold the screen: a new batch waits + // for it, and how it ended is still reported. + fm.popup = None; + } else { + // Items not sent yet are dropped; the running one stops at its + // next step and reports in. + if !self.sftp_queue.is_empty() { + self.sftp_queue.clear(); + fm.op_error.get_or_insert_with(|| CANCELLED.to_string()); + } + if let Some(mgr) = &self.sftp_manager { + mgr.cancel(); + } + fm.cancelling = true; + self.view.status_message = Some("Cancelling…".to_string()); + } + } + AppAction::FmOpenHostPicker => { self.view.file_manager.popup = Some(FileManagerPopup::HostPicker { cursor: 0 }); } diff --git a/crates/omnyssh/src/app/file_manager.rs b/crates/omnyssh/src/app/file_manager.rs index d5d1ac80..08fbb4be 100644 --- a/crates/omnyssh/src/app/file_manager.rs +++ b/crates/omnyssh/src/app/file_manager.rs @@ -184,10 +184,19 @@ pub struct FileManagerView { pub preview_path: Option, /// Transfer id of an in-progress transfer (for the progress popup). pub active_transfer: Option, - /// Number of queued transfer operations not yet completed. + /// Queued operations sent and not yet reported in: at most one. pub pending_ops: usize, + /// The first failure among them, reported once they are all done. + pub op_error: Option, + /// How many of them failed. + pub op_failures: usize, + /// Esc asked the running transfer to stop, and it has not reported in yet. + pub cancelling: bool, } +/// The core's error for a transfer that was cancelled. +pub(crate) const CANCELLED: &str = "Transfer cancelled"; + impl App { // ----------------------------------------------------------------------- // File Manager private helper methods @@ -317,6 +326,9 @@ impl App { if let Some(old) = self.sftp_manager.take() { old.disconnect(); } + // What the old session still had to send or report is moot now. + self.sftp_queue.clear(); + self.view.file_manager.pending_ops = 0; self.view.file_manager.connected_host = None; self.view.file_manager.remote = FilePanelView::default(); @@ -462,8 +474,7 @@ impl App { /// Pastes all clipboard contents into the active panel (upload / download). /// - /// All files are queued as individual SFTP commands and processed sequentially - /// by the background task. `pending_ops` tracks how many are still in flight. + /// Each item goes to the background task once the one before it is done. pub(crate) async fn fm_paste(&mut self) { let Some(clipboard) = self.view.file_manager.clipboard.clone() else { self.view.status_message = Some("Nothing in clipboard.".to_string()); @@ -482,6 +493,16 @@ impl App { return; } + if self.sftp_busy() { + return; + } + + // Nothing would be sent, and the progress would wait for it for good. + if self.sftp_manager.is_none() { + self.view.status_message = Some("Not connected to a host.".to_string()); + return; + } + let dst_cwd = match &dst_panel { FmPanel::Local => self.view.file_manager.local.cwd.clone(), FmPanel::Remote => self.view.file_manager.remote.cwd.clone(), @@ -490,7 +511,9 @@ impl App { let count = clipboard.paths.len(); let first_tid = self.next_transfer_id; self.next_transfer_id += count as u64; - self.view.file_manager.pending_ops = count; + self.view.file_manager.op_error = None; + self.view.file_manager.op_failures = 0; + self.view.file_manager.cancelling = false; // Show progress popup for the first file; subsequent files update it // via FileTransferProgress events. @@ -508,40 +531,54 @@ impl App { total: 0, }); - // Queue every file as a separate SFTP command. - for (i, src_path) in clipboard.paths.iter().enumerate() { - let tid = first_tid + i as u64; - let fname = filename_of(src_path); - let dst = format!("{}/{}", dst_cwd.trim_end_matches('/'), fname); - - match (&clipboard.source_panel, &dst_panel) { - (FmPanel::Local, FmPanel::Remote) => { - if let Some(mgr) = &self.sftp_manager { - mgr.send(SftpCommand::Upload { - local: src_path.clone(), - remote: dst, - transfer_id: tid, - }); - } - } - (FmPanel::Remote, FmPanel::Local) => { - if let Some(mgr) = &self.sftp_manager { - mgr.send(SftpCommand::Download { - remote: src_path.clone(), - local: dst, - transfer_id: tid, - }); - } - } - _ => unreachable!("same-panel case handled above"), - } + for (i, src) in clipboard.paths.iter().enumerate() { + let transfer_id = first_tid + i as u64; + let dst = format!("{}/{}", dst_cwd.trim_end_matches('/'), filename_of(src)); + self.sftp_queue.push_back(match clipboard.source_panel { + FmPanel::Local => SftpCommand::Upload { + local: src.clone(), + remote: dst, + transfer_id, + }, + FmPanel::Remote => SftpCommand::Download { + remote: src.clone(), + local: dst, + transfer_id, + }, + }); } + self.send_next_op(); if count > 1 { - self.view.status_message = Some(format!("Queued {count} files for transfer…")); + self.view.status_message = Some(format!("Queued {count} items for transfer…")); } } + /// Sends the next queued operation, once the one before it is done. + pub(crate) fn send_next_op(&mut self) { + if self.view.file_manager.pending_ops > 0 { + return; + } + let Some(cmd) = self.sftp_queue.pop_front() else { + return; + }; + self.view.file_manager.pending_ops = 1; + if let Some(mgr) = &self.sftp_manager { + mgr.send(cmd); + } + } + + /// Whether a batch is still running, saying so. A new one waits for it, or its + /// results would be taken for the new batch's. + fn sftp_busy(&mut self) -> bool { + let busy = self.view.file_manager.pending_ops > 0 || !self.sftp_queue.is_empty(); + if busy { + self.view.status_message = + Some("Wait for the running operation to finish.".to_string()); + } + busy + } + /// Deletes items listed in the `DeleteConfirm` popup. pub(crate) async fn fm_delete(&mut self) { let popup = self.view.file_manager.popup.take(); @@ -552,14 +589,15 @@ impl App { let is_remote = self.view.file_manager.active_panel == FmPanel::Remote; if is_remote { - // Track how many ops are in flight so SftpOpDone can count down. - self.view.file_manager.pending_ops = paths.len(); - // Send delete commands for all paths. - for path in paths { - if let Some(mgr) = &self.sftp_manager { - mgr.send(SftpCommand::Delete(path)); - } + if self.sftp_busy() { + return; } + self.view.file_manager.op_error = None; + self.view.file_manager.op_failures = 0; + self.view.file_manager.cancelling = false; + self.sftp_queue + .extend(paths.into_iter().map(SftpCommand::Delete)); + self.send_next_op(); } else { let tx = self.core_tx.clone(); tokio::spawn(async move { @@ -677,6 +715,7 @@ fn roots_after(roots: &[String], cwd: &str) -> Vec { #[cfg(test)] mod tests { use super::*; + use crossterm::event::{KeyCode, KeyEvent, KeyModifiers}; #[test] fn the_next_drive_wraps_and_ignores_case() { @@ -694,6 +733,7 @@ mod tests { path: path.to_string(), size: 0, is_dir: false, + modified: None, } } @@ -990,4 +1030,203 @@ mod tests { assert!(!p.marked.contains("/.secret")); assert!(p.marked.contains("/visible")); } + + // --- Transfers ---------------------------------------------------------- + + /// An app whose file manager is part way into a paste of three items. + async fn transferring_three() -> App { + let mut app = App::default(); + app.state.write().await.screen = Screen::FileManager; + app.view.file_manager.pending_ops = 1; + app.sftp_queue = ["b", "c"] + .into_iter() + .zip(1..) + .map(|(name, transfer_id)| SftpCommand::Upload { + local: format!("/l/{name}"), + remote: format!("/srv/{name}"), + transfer_id, + }) + .collect(); + let fm = &mut app.view.file_manager; + fm.popup = Some(FileManagerPopup::TransferProgress { + transfer_id: 0, + filename: "a (+2 more)".to_string(), + done: 0, + total: 0, + }); + app + } + + async fn done(app: &mut App, result: Result<(), &str>) { + let result = result.map_err(String::from); + let event = CoreEvent::SftpOpDone { result }; + app.handle_core_event(event).await.unwrap(); + } + + fn progress_shown(app: &App) -> bool { + matches!( + app.view.file_manager.popup, + Some(FileManagerPopup::TransferProgress { .. }) + ) + } + + #[tokio::test] + async fn a_transfer_reports_its_first_failure_once_the_rest_is_done() { + let mut app = transferring_three().await; + let skipped = "1 item skipped or failed, first '/l/a/fifo': not a regular file or folder"; + done(&mut app, Err(skipped)).await; + assert!(progress_shown(&app)); + assert_eq!( + app.view.status_message.as_deref(), + Some("2 item(s) remaining…") + ); + done(&mut app, Ok(())).await; + done( + &mut app, + Err("open local file for upload: Permission denied"), + ) + .await; + assert!(!progress_shown(&app)); + assert_eq!( + app.view.status_message, + Some(format!("Transfer failed: {skipped} (+1 more)")) + ); + } + + #[tokio::test] + async fn the_next_item_goes_once_the_one_before_is_done() { + let mut app = transferring_three().await; + done(&mut app, Ok(())).await; + assert!(matches!( + app.sftp_queue.front(), + Some(SftpCommand::Upload { local, .. }) if local == "/l/c" + )); + assert_eq!(app.view.file_manager.pending_ops, 1); + done(&mut app, Ok(())).await; + assert!(app.sftp_queue.is_empty()); + assert!(progress_shown(&app)); + done(&mut app, Ok(())).await; + assert!(!progress_shown(&app)); + assert_eq!(app.view.status_message, None); + } + + #[tokio::test] + async fn a_new_batch_waits_for_the_running_one() { + let mut app = transferring_three().await; + let fm = &mut app.view.file_manager; + fm.active_panel = FmPanel::Remote; + fm.clipboard = Some(FmClipboard { + paths: vec!["/l/d".to_string()], + source_panel: FmPanel::Local, + }); + app.fm_paste().await; + let busy = Some("Wait for the running operation to finish."); + assert_eq!(app.view.status_message.as_deref(), busy); + app.view.status_message = None; + app.view.file_manager.popup = Some(FileManagerPopup::DeleteConfirm { + paths: vec!["/srv/x".to_string()], + }); + app.fm_delete().await; + assert_eq!(app.view.status_message.as_deref(), busy); + assert_eq!(app.sftp_queue.len(), 2); + } + + async fn press_esc(app: &mut App) { + let key = KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE); + let action = app.handle_key(key).await.unwrap(); + assert!(matches!(action, Some(AppAction::FmCancelTransfer))); + app.process_action(action).await.unwrap(); + } + + #[tokio::test] + async fn esc_drops_what_is_queued_and_waits_for_the_running_item() { + let mut app = transferring_three().await; + press_esc(&mut app).await; + assert!(app.sftp_queue.is_empty()); + assert!(progress_shown(&app)); + assert_eq!(app.view.status_message.as_deref(), Some("Cancelling…")); + done(&mut app, Err(CANCELLED)).await; + assert!(!progress_shown(&app)); + assert_eq!(app.view.status_message.as_deref(), Some(CANCELLED)); + } + + #[tokio::test] + async fn items_left_unsent_count_as_cancelled() { + let mut app = transferring_three().await; + press_esc(&mut app).await; + // The running item finished before the cancel reached it. + done(&mut app, Ok(())).await; + assert_eq!(app.view.status_message.as_deref(), Some(CANCELLED)); + } + + #[tokio::test] + async fn a_cancel_too_late_for_the_last_item_says_so() { + let mut app = transferring_three().await; + done(&mut app, Ok(())).await; + done(&mut app, Ok(())).await; + press_esc(&mut app).await; + done(&mut app, Ok(())).await; + assert!(!progress_shown(&app)); + assert_eq!( + app.view.status_message.as_deref(), + Some("The transfer finished before it could be cancelled.") + ); + } + + #[tokio::test] + async fn a_real_failure_outranks_the_cancel() { + let failure = "open local file for upload: Permission denied (os error 13)"; + // Before the cancel. + let mut app = transferring_three().await; + done(&mut app, Err(failure)).await; + press_esc(&mut app).await; + done(&mut app, Err(CANCELLED)).await; + assert_eq!( + app.view.status_message, + Some(format!("Transfer failed: {failure}")) + ); + // While stopping. + let mut app = transferring_three().await; + press_esc(&mut app).await; + done(&mut app, Err(failure)).await; + assert_eq!( + app.view.status_message, + Some(format!("Transfer failed: {failure}")) + ); + } + + #[tokio::test] + async fn a_second_esc_hides_a_transfer_slow_to_stop() { + let mut app = transferring_three().await; + press_esc(&mut app).await; + press_esc(&mut app).await; + assert!(!progress_shown(&app)); + // The panels are free again, but a new batch waits. + app.view.file_manager.popup = Some(FileManagerPopup::MkDir(FormField::default())); + done(&mut app, Err(CANCELLED)).await; + assert!(matches!( + app.view.file_manager.popup, + Some(FileManagerPopup::MkDir(_)) + )); + assert_eq!(app.view.status_message.as_deref(), Some(CANCELLED)); + assert!(!app.view.file_manager.cancelling); + } + + #[tokio::test] + async fn a_paste_needs_a_connected_host() { + let mut app = App::default(); + let fm = &mut app.view.file_manager; + fm.active_panel = FmPanel::Remote; + fm.clipboard = Some(FmClipboard { + paths: vec!["/l/a".to_string()], + source_panel: FmPanel::Local, + }); + app.fm_paste().await; + assert_eq!( + app.view.status_message.as_deref(), + Some("Not connected to a host.") + ); + assert!(!progress_shown(&app)); + assert_eq!(app.view.file_manager.pending_ops, 0); + } } diff --git a/crates/omnyssh/src/app/mod.rs b/crates/omnyssh/src/app/mod.rs index 7e83b458..2ed6db1f 100644 --- a/crates/omnyssh/src/app/mod.rs +++ b/crates/omnyssh/src/app/mod.rs @@ -7,7 +7,7 @@ //! plumbing. Every public item of the submodules is re-exported here so the //! rest of the crate keeps using the flat `crate::app::Type` paths. -use std::collections::HashMap; +use std::collections::{HashMap, VecDeque}; use std::io::Stdout; use std::sync::Arc; use std::time::Duration; @@ -216,6 +216,9 @@ pub struct PasswordPrompt { // App // --------------------------------------------------------------------------- +/// How long quitting waits for a running transfer to stop and clean up after itself. +const QUIT_GRACE: Duration = Duration::from_secs(3); + /// Root application struct — owns the terminal, state, and event channel. pub struct App { /// Shared state readable by background tokio tasks. @@ -234,6 +237,9 @@ pub struct App { core_rx: Option>, /// Persistent SFTP session manager for the File Manager. sftp_manager: Option, + /// SFTP operations of a batch waiting for the one before them to finish. Sent + /// in one burst, those past the task's 64-command queue were dropped unseen. + sftp_queue: VecDeque, /// Monotone counter for assigning unique [`TransferId`] values. next_transfer_id: TransferId, /// Background metrics polling manager. Stored in `App` (not in @@ -279,6 +285,7 @@ impl App { core_tx, core_rx: Some(core_rx), sftp_manager: None, + sftp_queue: VecDeque::new(), next_transfer_id: 0, poll_manager: None, pty_manager: None, @@ -376,6 +383,7 @@ impl App { mgr.shutdown(); } // Gracefully shut down the SFTP session. + self.let_transfer_stop().await; if let Some(sftp) = self.sftp_manager.take() { sftp.disconnect(); } @@ -404,6 +412,28 @@ impl App { result } + /// Cancels a running transfer and waits, [`QUIT_GRACE`] at most, for it to report + /// in: once the app exits, the runtime would drop it mid-step and leave the file it + /// was part way through. + async fn let_transfer_stop(&mut self) { + let Some(sftp) = &self.sftp_manager else { + return; + }; + if self.view.file_manager.pending_ops == 0 { + return; + } + sftp.cancel(); + let events = &mut self.event_rx; + let _ = tokio::time::timeout(QUIT_GRACE, async { + while let Some(event) = events.recv().await { + if matches!(event, AppEvent::Core(CoreEvent::SftpOpDone { .. })) { + break; + } + } + }) + .await; + } + /// Inner loop — separated from `run` so terminal restore always happens. async fn main_loop( &mut self, @@ -1011,6 +1041,8 @@ impl App { CoreEvent::SftpDisconnected { reason } => { self.sftp_manager = None; + // Nothing queued for the lost session may reach the next one. + self.sftp_queue.clear(); self.view.file_manager.connected_host = None; self.view.file_manager.sftp_connecting = false; self.view.file_manager.remote = FilePanelView::default(); @@ -1038,31 +1070,43 @@ impl App { } CoreEvent::SftpOpDone { result } => { - self.view.file_manager.pending_ops = - self.view.file_manager.pending_ops.saturating_sub(1); - let remaining = self.view.file_manager.pending_ops; - - match result { - Ok(()) => { - if remaining == 0 { - // All queued operations finished — close popup and refresh. - self.view.file_manager.popup = None; - self.view.file_manager.active_transfer = None; - self.view.status_message = None; - self.refresh_active_panels().await; - } else { - self.view.status_message = - Some(format!("{remaining} file(s) remaining…")); - } + let fm = &mut self.view.file_manager; + fm.pending_ops = fm.pending_ops.saturating_sub(1); + // The queued operations run on past a failure: the first one is kept + // for the end, where no later result can wipe it, and the rest counted. + // A cancel only stands where nothing really failed. + if let Err(e) = result { + if e != CANCELLED { + fm.op_failures += 1; + } + if fm.op_error.as_deref().is_none_or(|kept| kept == CANCELLED) { + fm.op_error = Some(e); } - Err(e) => { - // Abort remaining: clear popup, show error, refresh. - self.view.file_manager.popup = None; - self.view.file_manager.active_transfer = None; - self.view.file_manager.pending_ops = 0; - self.view.status_message = Some(format!("Transfer failed: {e}")); - self.refresh_active_panels().await; + } + self.send_next_op(); + let fm = &mut self.view.file_manager; + if fm.pending_ops > 0 { + let remaining = fm.pending_ops + self.sftp_queue.len(); + self.view.status_message = Some(format!("{remaining} item(s) remaining…")); + } else { + // All queued operations finished — close the progress and refresh. + // A transfer hidden while stopping leaves other popups alone. + if matches!(fm.popup, Some(FileManagerPopup::TransferProgress { .. })) { + fm.popup = None; } + fm.active_transfer = None; + let cancelling = std::mem::take(&mut fm.cancelling); + let more = std::mem::take(&mut fm.op_failures).saturating_sub(1); + self.view.status_message = match fm.op_error.take() { + Some(e) if e == CANCELLED => Some(e), + Some(e) if more > 0 => Some(format!("Transfer failed: {e} (+{more} more)")), + Some(e) => Some(format!("Transfer failed: {e}")), + None if cancelling => { + Some("The transfer finished before it could be cancelled.".to_string()) + } + None => None, + }; + self.refresh_active_panels().await; } } diff --git a/crates/omnyssh/src/ui/file_manager.rs b/crates/omnyssh/src/ui/file_manager.rs index 4d1a070d..73cbc872 100644 --- a/crates/omnyssh/src/ui/file_manager.rs +++ b/crates/omnyssh/src/ui/file_manager.rs @@ -271,7 +271,7 @@ fn handle_popup_input(key: KeyEvent, kind: PopupKind, view: &mut ViewState) -> O PopupKind::Rename => handle_text_input_popup(key, view, TextPopupKind::Rename), PopupKind::TransferProgress => { if key.code == KeyCode::Esc { - Some(AppAction::FmClosePopup) + Some(AppAction::FmCancelTransfer) } else { None } @@ -393,8 +393,19 @@ fn render_panel( } panel.scroll.set(scroll); - // Build list items. - let items: Vec = panel + // Modification times, once the panel is wide enough to keep names readable. + // They get a strip of their own, so no name, however wide, moves them. + let (names_area, dates_area) = if inner.width >= MTIME_MIN_PANEL_WIDTH { + let [names, dates] = + Layout::horizontal([Constraint::Fill(1), Constraint::Length(1 + MTIME_WIDTH)]) + .areas(inner); + (names, Some(dates)) + } else { + (inner, None) + }; + + // Build list items: each entry's name row and date row. + let (items, dates): (Vec, Vec>) = panel .entries .iter() .enumerate() @@ -432,7 +443,7 @@ fn render_panel( // icon(1) + space(1) + name + space-before-size(1) + size. // The nerd-font glyph occupies a single cell even though its // codepoint is wide. - let name_width = inner + let name_width = names_area .width .saturating_sub(2 + 3 + 1 + 1 + 1 + size_str.len() as u16) as usize; @@ -476,17 +487,33 @@ fn render_panel( )); } + let date_item = dates_area.map(|_| { + let date = if entry.name == ".." { + String::new() + } else { + format_mtime(entry.modified) + }; + ListItem::new(Span::styled( + format!(" {date}"), + Style::default().fg(theme.text_muted), + )) + }); + let item = ListItem::new(Line::from(spans)); if is_cursor { - item.style(Style::default().bg(theme.selected_bg)) + let selected = Style::default().bg(theme.selected_bg); + (item.style(selected), date_item.map(|d| d.style(selected))) } else { - item + (item, date_item) } }) - .collect(); + .unzip(); let list = List::new(items); - frame.render_widget(list, inner); + frame.render_widget(list, names_area); + if let Some(dates_area) = dates_area { + frame.render_widget(List::new(dates.into_iter().flatten()), dates_area); + } // Scrollbar if entries overflow. if panel.entries.len() > visible_rows { @@ -538,7 +565,7 @@ fn render_preview_zone( .. }) = &fm.popup { - render_transfer_progress(frame, area, filename, *done, *total, theme); + render_transfer_progress(frame, area, filename, *done, *total, fm.cancelling, theme); return; } @@ -606,6 +633,7 @@ fn render_transfer_progress( filename: &str, done: u64, total: u64, + cancelling: bool, theme: &crate::ui::theme::Theme, ) { let block = Block::default() @@ -642,9 +670,10 @@ fn render_transfer_progress( rows[0], ); - // Progress gauge. + // Progress gauge. A file that grows while it is copied goes past its planned + // size, and the gauge takes nothing above 100. let percent = if total > 0 { - ((done as f64 / total as f64) * 100.0) as u16 + (((done as f64 / total as f64) * 100.0) as u16).min(100) } else { 0 }; @@ -666,6 +695,26 @@ fn render_transfer_progress( .label(label); frame.render_widget(gauge, rows[1]); + + // A first Esc cancels; a second one hides this while the transfer stops. + let (state, action) = if cancelling { + (" Cancelling…", ":hide") + } else { + ("", ":cancel") + }; + frame.render_widget( + Paragraph::new(Line::from(vec![ + Span::styled(state, Style::default().fg(theme.text_muted)), + Span::styled( + " Esc", + Style::default() + .fg(Color::Yellow) + .add_modifier(Modifier::BOLD), + ), + Span::styled(action, Style::default().fg(theme.text_muted)), + ])), + rows[2], + ); } // --------------------------------------------------------------------------- @@ -974,6 +1023,23 @@ fn render_fm_delete_confirm( // Utility // --------------------------------------------------------------------------- +/// Cells taken by a formatted modification time ("2026-09-30 14:05"). +const MTIME_WIDTH: u16 = 16; + +/// Narrowest panel interior that still shows the modification-time column. +const MTIME_MIN_PANEL_WIDTH: u16 = 48; + +/// Local-time "YYYY-MM-DD HH:MM" for a Unix timestamp, or blank when unknown. +fn format_mtime(secs: Option) -> String { + secs.and_then(|s| chrono::DateTime::from_timestamp(s, 0)) + .map(|t| { + t.with_timezone(&chrono::Local) + .format("%Y-%m-%d %H:%M") + .to_string() + }) + .unwrap_or_default() +} + /// Human-readable file size string (e.g. "45.3K", "1.2M", "3.0G"). fn format_size(bytes: u64) -> String { const KB: u64 = 1024; @@ -1063,12 +1129,83 @@ mod tests { use super::*; use omnyssh_core::ssh::sftp::FileEntry; + #[test] + fn format_mtime_is_fixed_width_or_blank() { + assert_eq!( + format_mtime(Some(1_700_000_000)).chars().count(), + MTIME_WIDTH as usize + ); + assert_eq!(format_mtime(None), ""); + assert_eq!(format_mtime(Some(i64::MAX)), ""); + } + + #[test] + fn a_file_that_grew_while_copied_fills_the_gauge() { + let backend = ratatui::backend::TestBackend::new(60, 5); + let mut terminal = ratatui::Terminal::new(backend).expect("terminal"); + terminal + .draw(|frame| { + let theme = crate::ui::theme::Theme::default(); + render_transfer_progress( + frame, + frame.area(), + "access.log", + 1500, + 1000, + false, + &theme, + ); + }) + .expect("draw"); + let row: String = (0..60) + .map(|x| terminal.backend().buffer()[(x, 2)].symbol()) + .collect(); + assert!(row.contains("(100%)"), "{row:?}"); + } + + #[test] + fn dates_line_up_on_folder_rows_and_after_wide_names() { + let entry = |name: &str, is_dir: bool| FileEntry { + name: name.to_string(), + path: format!("/{name}"), + size: 14, + is_dir, + modified: Some(1_700_000_000), + }; + let panel = FilePanelView { + entries: vec![ + entry("src", true), + entry("notes.txt", false), + // Two glyphs two cells wide each. + entry(&format!("{}.txt", "\u{65e5}\u{672c}".repeat(4)), false), + entry(&"\u{65e5}\u{672c}".repeat(30), false), + ], + ..FilePanelView::default() + }; + let backend = ratatui::backend::TestBackend::new(60, 6); + let mut terminal = ratatui::Terminal::new(backend).expect("terminal"); + terminal + .draw(|frame| { + let theme = crate::ui::theme::Theme::default(); + render_panel(frame, frame.area(), &panel, "LOCAL", true, &theme); + }) + .expect("draw"); + let buffer = terminal.backend().buffer(); + let date = format!(" {}", format_mtime(Some(1_700_000_000))); + // Column 58 is the last one inside the right border. + for y in 1..=4 { + let row: String = (0..=58).map(|x| buffer[(x, y)].symbol()).collect(); + assert!(row.ends_with(&date), "{row:?}"); + } + } + fn file(name: &str, size: u64) -> FileEntry { FileEntry { name: name.to_string(), path: format!("/{name}"), size, is_dir: false, + modified: None, } } diff --git a/crates/omnyssh/src/ui/mod.rs b/crates/omnyssh/src/ui/mod.rs index a652516f..0d9710cc 100644 --- a/crates/omnyssh/src/ui/mod.rs +++ b/crates/omnyssh/src/ui/mod.rs @@ -175,6 +175,7 @@ mod tests { path: format!("/tmp/{name}"), size: 14, is_dir: false, + modified: None, }) .collect(); // The preview repeats the name in its title. diff --git a/crates/omnyssh/src/ui/status_bar.rs b/crates/omnyssh/src/ui/status_bar.rs index 8fc53ab3..e8b40c5d 100644 --- a/crates/omnyssh/src/ui/status_bar.rs +++ b/crates/omnyssh/src/ui/status_bar.rs @@ -245,14 +245,19 @@ pub fn render(frame: &mut Frame, area: Rect, state: &AppState, view: &ViewState) .. } => { let pct = if *total > 0 { - ((*done as f64 / *total as f64) * 100.0) as u64 + (((*done as f64 / *total as f64) * 100.0) as u64).min(100) } else { 0 }; - Line::from(vec![Span::styled( - format!(" Transferring: {} {}% ", filename, pct), - hint_style, - )]) + Line::from(vec![ + Span::styled( + format!(" Transferring: {} {}% ", filename, pct), + hint_style, + ), + sep!(), + key!("Esc"), + hint!("cancel"), + ]) } }; frame.render_widget(