diff --git a/Cargo.lock b/Cargo.lock index b701c13..6bd3feb 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -993,6 +993,7 @@ dependencies = [ "serde_json", "tokio", "tokio-tungstenite 0.30.0", + "unicode-width", "ureq", ] diff --git a/cli/Cargo.toml b/cli/Cargo.toml index 7a5ac89..30deef9 100644 --- a/cli/Cargo.toml +++ b/cli/Cargo.toml @@ -52,6 +52,10 @@ pkarr = { version = "8.0.0", default-features = false, features = ["dht"] } # against it. See docs/plans/interactive-terminal-ui-plan-2026-09-10.md. ratatui = "0.30.2" crossterm = "0.29.0" +# Display width of a name a peer chose, so eliding it counts a CJK character as +# the two columns it takes. Pure Rust, and already in the tree through ratatui, +# so this adds no crate to the build. +unicode-width = "0.2" [dev-dependencies] api = { path = ".." } diff --git a/cli/src/client.rs b/cli/src/client.rs index 199dc3f..20ab4ac 100644 --- a/cli/src/client.rs +++ b/cli/src/client.rs @@ -98,7 +98,8 @@ pub fn create_session(origin: &str, ciphertext_size: u64) -> Result Err(ClientError::Transport(format!( "could not reach {origin}: {error}" diff --git a/cli/src/display.rs b/cli/src/display.rs new file mode 100644 index 0000000..6e065b5 --- /dev/null +++ b/cli/src/display.rs @@ -0,0 +1,262 @@ +//! Putting text somebody else chose on this terminal. +//! +//! A filename is chosen by the sender, an error message by the peer or the +//! relay, and none of them is trusted. Printed as-is, a name can carry escape +//! sequences that move the cursor, clear the line, retitle the window, or plant +//! a hyperlink. It can also carry bidirectional controls that make `exe.pdf` +//! read as `fdp.exe`. That matters most exactly where Drop asks a person to +//! decide something about the name they are looking at, and it matters already +//! for the "Receiving" line, which is printed before any decision exists. +//! +//! Everything here replaces rather than removes. A `\u{FFFD}` where a control +//! character was tells the reader the name was odd; silently deleting it would +//! show them a cleaner name than the one that exists. +//! +//! Legitimate names in any script pass through unchanged: only control and +//! formatting characters are touched, never letters, marks or emoji. + +use unicode_width::{UnicodeWidthChar, UnicodeWidthStr}; + +/// What stands in for anything that should not reach a terminal. +const REPLACEMENT: char = '\u{FFFD}'; + +/// Columns a name may take before its middle is elided. +/// +/// Wide enough for ordinary names, narrow enough that a hostile one cannot push +/// a question onto a line of its own and off the screen. +pub const NAME_COLUMNS: usize = 80; + +/// Columns kept from the end of an elided name, so the extension — the part +/// that says what kind of file this is — always survives. +const TAIL_COLUMNS: usize = 24; + +/// Columns a message from a peer or a relay may take. +const MESSAGE_COLUMNS: usize = 200; + +/// Characters that are not controls but still change how a terminal lays text +/// out: bidirectional overrides, embeddings, isolates and marks; line and +/// paragraph separators; and invisible characters that let two different names +/// look identical. +/// +/// Zero-width joiner and non-joiner (U+200D, U+200C) are deliberately absent. +/// Emoji sequences and several scripts need them, and they cannot reorder or +/// hide text. +fn is_layout_control(character: char) -> bool { + matches!( + character, + '\u{061C}' // arabic letter mark + | '\u{200B}' // zero width space + | '\u{200E}' | '\u{200F}' // left-to-right and right-to-left marks + | '\u{2028}' | '\u{2029}' // line and paragraph separators + | '\u{202A}'..='\u{202E}' // embeddings and overrides + | '\u{2060}' // word joiner + | '\u{2066}'..='\u{2069}' // isolates + | '\u{FEFF}' // byte order mark + ) +} + +/// Makes untrusted text safe to print, without shortening it. +/// +/// Control characters (C0, DEL, C1, which includes the bytes that start every +/// escape sequence) and layout controls become `\u{FFFD}`. Runs of whitespace +/// collapse to one space, so padding cannot push the end of a name out of +/// sight. +pub fn for_terminal(text: &str) -> String { + let mut out = String::with_capacity(text.len()); + let mut in_whitespace = false; + + for character in text.chars() { + if character.is_control() || is_layout_control(character) { + out.push(REPLACEMENT); + in_whitespace = false; + } else if character.is_whitespace() { + if !in_whitespace { + out.push(' '); + } + in_whitespace = true; + } else { + out.push(character); + in_whitespace = false; + } + } + + out +} + +/// A name a peer chose, ready to print on one line. +/// +/// Sanitised by [`for_terminal`], then elided in the middle if it is wider than +/// [`NAME_COLUMNS`]. The middle rather than the end, because the end holds the +/// extension and the extension is what says whether this is a document or a +/// program. +pub fn name(text: &str) -> String { + elide_middle(&for_terminal(text), NAME_COLUMNS) +} + +/// A message a peer or a relay sent, ready to print after `error: `. +/// +/// Neither is trusted, so this is sanitised too, and capped so a hostile one +/// cannot fill the screen. +pub fn peer_message(text: &str) -> String { + elide_middle(&for_terminal(text), MESSAGE_COLUMNS) +} + +/// Shortens `text` to at most `columns` display columns by replacing its middle +/// with an ellipsis, measuring wide characters as two columns. +fn elide_middle(text: &str, columns: usize) -> String { + if text.width() <= columns { + return text.to_string(); + } + + let tail_budget = TAIL_COLUMNS.min(columns / 2); + // One column for the ellipsis itself. + let head_budget = columns - tail_budget - 1; + + let mut head = String::new(); + let mut used = 0; + for character in text.chars() { + let width = character.width().unwrap_or(0); + if used + width > head_budget { + break; + } + head.push(character); + used += width; + } + + let mut tail: Vec = Vec::new(); + let mut used = 0; + for character in text.chars().rev() { + let width = character.width().unwrap_or(0); + if used + width > tail_budget { + break; + } + tail.push(character); + used += width; + } + tail.reverse(); + + format!("{head}\u{2026}{}", tail.into_iter().collect::()) +} + +#[cfg(test)] +mod tests { + use super::{NAME_COLUMNS, for_terminal, name, peer_message}; + use unicode_width::UnicodeWidthStr; + + #[test] + fn a_csi_sequence_cannot_reach_the_terminal() { + // Clear the line, move up, and print something else. + let hostile = "report.pdf\x1b[2K\x1b[1Ainvoice.pdf"; + let shown = for_terminal(hostile); + + assert!(!shown.contains('\x1b'), "escape survived: {shown:?}"); + assert!( + shown.contains('\u{FFFD}'), + "the tampering should be visible" + ); + assert!(shown.contains("report.pdf") && shown.contains("invoice.pdf")); + } + + #[test] + fn an_osc_hyperlink_and_a_window_title_are_neutralised() { + let hyperlink = "\x1b]8;;https://example.invalid\x07click.txt\x1b]8;;\x07"; + let title = "\x1b]0;owned\x07name.txt"; + + for hostile in [hyperlink, title] { + let shown = for_terminal(hostile); + assert!( + !shown.contains('\x1b') && !shown.contains('\x07'), + "{shown:?}" + ); + } + } + + #[test] + fn c1_controls_are_replaced_too() { + // U+009B is a single-character CSI on terminals that honour C1. + let shown = for_terminal("a\u{009B}2Kb"); + assert_eq!(shown, "a\u{FFFD}2Kb"); + } + + #[test] + fn newlines_and_tabs_cannot_break_a_line() { + let shown = for_terminal("first\nsecond\rthird\tfourth"); + assert!(!shown.contains(['\n', '\r', '\t']), "{shown:?}"); + } + + #[test] + fn every_bidirectional_control_is_replaced() { + let controls = [ + '\u{061C}', '\u{200E}', '\u{200F}', '\u{202A}', '\u{202B}', '\u{202C}', '\u{202D}', + '\u{202E}', '\u{2066}', '\u{2067}', '\u{2068}', '\u{2069}', + ]; + + for control in controls { + let shown = for_terminal(&format!("invoice{control}fdp.exe")); + assert!( + !shown.contains(control), + "U+{:04X} survived: {shown:?}", + control as u32 + ); + } + } + + #[test] + fn the_classic_right_to_left_override_shows_its_real_extension() { + // Rendered naively this reads "invoice_exe.pdf". + let shown = name("invoice_\u{202E}fdp.exe"); + assert!(shown.ends_with("fdp.exe"), "{shown:?}"); + } + + #[test] + fn whitespace_padding_collapses() { + let shown = for_terminal("report.pdf .exe"); + assert_eq!(shown, "report.pdf .exe"); + } + + #[test] + fn a_long_name_keeps_its_extension() { + let long = format!("{}.exe", "a".repeat(300)); + let shown = name(&long); + + assert!(shown.width() <= NAME_COLUMNS, "{} columns", shown.width()); + assert!(shown.ends_with(".exe"), "{shown:?}"); + assert!(shown.contains('\u{2026}')); + } + + #[test] + fn wide_characters_count_as_two_columns() { + let long = format!("{}.txt", "東".repeat(100)); + let shown = name(&long); + + assert!(shown.width() <= NAME_COLUMNS, "{} columns", shown.width()); + assert!(shown.ends_with(".txt")); + } + + #[test] + fn honest_names_in_any_script_are_untouched() { + for honest in [ + "Łódź 東京 🎉.txt", + "résumé-final (2).pdf", + "مرحبا.docx", + "👩‍👩‍👧 family.jpg", // zero-width joiners inside the emoji + "10:30 standup.md", + ] { + assert_eq!(name(honest), honest); + } + } + + #[test] + fn a_short_name_is_not_elided() { + assert_eq!(name("report.pdf"), "report.pdf"); + } + + #[test] + fn a_peer_message_is_sanitised_and_capped() { + let hostile = format!("\x1b[2J{}", "x".repeat(10_000)); + let shown = peer_message(&hostile); + + assert!(!shown.contains('\x1b')); + assert!(shown.width() <= 200); + } +} diff --git a/cli/src/lib.rs b/cli/src/lib.rs index ffbddd2..536fa9e 100644 --- a/cli/src/lib.rs +++ b/cli/src/lib.rs @@ -9,6 +9,7 @@ pub mod direct; // the browser client can compile it to WebAssembly. Keeping the name `crypto` // means every call site below reads the same as before the split. pub use drop_crypto as crypto; +pub mod display; pub mod payload; pub mod progress; pub mod recv; diff --git a/cli/src/recv.rs b/cli/src/recv.rs index 7933db5..91309ab 100644 --- a/cli/src/recv.rs +++ b/cli/src/recv.rs @@ -10,7 +10,7 @@ use std::{ use serde_json::json; use crate::{ - client, crypto, direct, + client, crypto, direct, display, payload::{GZIP_MIME, TAR_GZIP_MIME, TAR_MIME}, progress::Progress, transport::{Frame, Transport, relay}, @@ -179,6 +179,14 @@ pub async fn run(code: &str, options: ReceiveOptions) -> Result<(), Box String { + display::peer_message(payload["message"].as_str().unwrap_or(otherwise)) +} + fn missing_record() -> Box { "nobody has published a peer-to-peer address for this code".into() } @@ -279,9 +287,11 @@ pub(crate) async fn receive_transfer( let mut opener = crypto::Opener::new(&keys, size); + // The sender chose this name. It reaches the terminal only through + // `display`, because an escape sequence in it would otherwise run here. eprintln!( "Receiving {} ({})", - filename, + display::name(&filename), crate::progress::format_bytes(size) ); @@ -364,11 +374,7 @@ pub(crate) async fn receive_transfer( return Ok(()); } Some("error") => { - return Err(payload["message"] - .as_str() - .unwrap_or("the relay reported an error") - .to_string() - .into()); + return Err(peer_error(&payload, "the relay reported an error").into()); } _ => {} }, @@ -418,11 +424,7 @@ async fn exchange_keys( } } Some("error") => { - return Err(payload["message"] - .as_str() - .unwrap_or("the relay reported an error") - .to_string() - .into()); + return Err(peer_error(&payload, "the relay reported an error").into()); } _ => {} } @@ -470,11 +472,7 @@ async fn wait_for_meta( } } Some("error") => { - return Err(payload["message"] - .as_str() - .unwrap_or("the relay reported an error") - .to_string() - .into()); + return Err(peer_error(&payload, "the relay reported an error").into()); } _ => {} } @@ -525,10 +523,13 @@ fn open_target( if path != requested { eprintln!( "{} already exists; saving as {} instead", - safe_name, - path.file_name() - .unwrap_or(path.as_os_str()) - .to_string_lossy() + display::name(&safe_name), + display::name( + &path + .file_name() + .unwrap_or(path.as_os_str()) + .to_string_lossy() + ) ); } @@ -654,19 +655,21 @@ fn report(target: &Target, received: u64) { Target::File { path, .. } => { eprintln!( "Saved {} ({}).", - path.display(), + display::for_terminal(&path.display().to_string()), crate::progress::format_bytes(received) ); } Target::Archive { root, extractor } => { for warning in extractor.warnings() { - eprintln!("warning: {warning}"); + // Warnings quote entry names from the archive, which the + // sender chose. + eprintln!("warning: {}", display::for_terminal(warning)); } eprintln!( "Extracted {} files into {}.", extractor.files_written(), - root.display() + display::for_terminal(&root.display().to_string()) ); } } diff --git a/cli/src/send.rs b/cli/src/send.rs index 3f75906..439e40e 100644 --- a/cli/src/send.rs +++ b/cli/src/send.rs @@ -140,14 +140,16 @@ pub async fn run( let payload = Payload::prepare(path, options.compress)?; for warning in &payload.warnings { - eprintln!("warning: {warning}"); + // Names from the tree being sent. They are local, but a hostile name + // can already be sitting on a shared disk. + eprintln!("warning: {}", crate::display::for_terminal(warning)); } if payload.size == 0 { return Err("there is nothing to send: the payload is empty".into()); } - eprintln!("Sending {}", payload.summary); + eprintln!("Sending {}", crate::display::for_terminal(&payload.summary)); eprintln!("Size {}", crate::progress::format_bytes(payload.size)); // The relay bounds and accounts for what actually crosses it, which is @@ -742,11 +744,16 @@ async fn await_completion( Err("the transfer connection closed before the receiver confirmed the file".into()) } +/// The message in an `error` frame, made safe to print. +/// +/// The relay wrote it, or on the direct path the receiver did. Neither is +/// trusted to put bytes on this terminal. fn relay_error(payload: &Value) -> String { - payload["message"] - .as_str() - .unwrap_or("the relay reported an error") - .to_string() + crate::display::peer_message( + payload["message"] + .as_str() + .unwrap_or("the relay reported an error"), + ) } #[cfg(test)] diff --git a/cli/src/transport/relay.rs b/cli/src/transport/relay.rs index ddc2d7a..995acf5 100644 --- a/cli/src/transport/relay.rs +++ b/cli/src/transport/relay.rs @@ -80,12 +80,12 @@ fn while_waiting(payload: &Value) -> WhileWaiting { Some("status") if payload["status"].as_str() == Some("receiver_connected") => { WhileWaiting::PeerArrived } - Some("error") => WhileWaiting::Refused( + // The relay is untrusted, and this reaches the terminal verbatim. + Some("error") => WhileWaiting::Refused(crate::display::peer_message( payload["message"] .as_str() - .unwrap_or("the relay reported an error") - .to_string(), - ), + .unwrap_or("the relay reported an error"), + )), _ => WhileWaiting::KeepWaiting, } } diff --git a/cli/src/ui/app.rs b/cli/src/ui/app.rs index cb7343a..0318011 100644 --- a/cli/src/ui/app.rs +++ b/cli/src/ui/app.rs @@ -347,7 +347,10 @@ fn browse(screen: &mut Screen, start: PathBuf, picking: Picking) -> Outcome