diff --git a/apps/wordcraft-cli/src/main.rs b/apps/wordcraft-cli/src/main.rs index 752f9f68b..c8e46c93c 100644 --- a/apps/wordcraft-cli/src/main.rs +++ b/apps/wordcraft-cli/src/main.rs @@ -387,6 +387,8 @@ fn valid_connect_addr(addr: &str) -> bool { !host.is_empty() && port.parse::().is_ok() } +/// Run the subcommand named by `args` (the command line after the program name). Files named +/// there are opened by their real names, which may not be valid Unicode. fn run>(args: &[S]) -> Result<(), Failure> { let script_path = script_operand(args); let (args, file) = text_args(args)?; @@ -657,14 +659,14 @@ fn run>(args: &[S]) -> Result<(), Failure> { let out = wordcraft_zotero::client::run_command(&opts, command, &mut |call| { let r = bridge.handle(&mut s, &mut host, call); if trace { - eprintln!("← {} {}", call.method, clip(&Value::Array(call.args.clone()).to_string())); - eprintln!("→ {}", clip(&String::from_utf8_lossy(&wordcraft_zotero::wire::encode_reply(&r)))); + eprintln!("← {} {}", escape_peer(&call.method), escape_peer(&clip(&Value::Array(call.args.clone()).to_string()))); + eprintln!("→ {}", escape_peer(&clip(&String::from_utf8_lossy(&wordcraft_zotero::wire::encode_reply(&r))))); } r }) .map_err(|e| e.to_string())?; for a in &host.alerts { - eprintln!("Zotero: {a}"); + eprintln!("Zotero: {}", escape_peer(a)); } eprintln!("{} calls, {}", out.calls, if out.completed { "completed" } else { "ended without completing (cancelled?)" }); if let Some(o) = arg_value(&rest, "--save") { @@ -687,6 +689,27 @@ fn clip(s: &str) -> String { } } +/// Text from another program (Zotero, or whatever answered on its port) for one diagnostic +/// line: backslashes, control characters (newline, escape, C1…), line separators and bidi +/// overrides are shown as escapes (`\\`, `\n`, `\u{1b}`), so it can't end its line, drive the +/// terminal or pass for other output. +fn escape_peer(s: &str) -> String { + let mut out = String::with_capacity(s.len()); + for c in s.chars() { + match c { + '\\' => out.push_str("\\\\"), + '\n' => out.push_str("\\n"), + '\r' => out.push_str("\\r"), + '\t' => out.push_str("\\t"), + c if c.is_control() || matches!(c, '\u{061c}' | '\u{200e}' | '\u{200f}' | '\u{2028}'..='\u{202e}' | '\u{2066}'..='\u{2069}') => { + out.extend(c.escape_unicode()) + } + c => out.push(c), + } + } + out +} + fn parity_markdown(p: &Value) -> String { let mut s = String::from( "# WordCraft feature parity\n\nGenerated by `cargo xtask parity` (`wordcraft-cli parity --markdown`): the word-processor feature catalog (`crates/engine/src/catalog.rs`) compared with the live command registry.\n\n", @@ -733,6 +756,15 @@ mod tests { values.iter().map(|value| (*value).into()).collect() } + #[test] + fn peer_text_stays_on_one_visible_line() { + let alert = "Done.\nwrote /etc/passwd\r\u{1b}[2J\u{9b}31m\u{202e}txt.exe\\n\tok é"; + let shown = escape_peer(alert); + assert_eq!(shown, "Done.\\nwrote /etc/passwd\\r\\u{1b}[2J\\u{9b}31m\\u{202e}txt.exe\\\\n\\tok é"); + assert!(!shown.chars().any(|c| c.is_control())); + assert_eq!(escape_peer("Citation (Doe, 2020) — “ok”"), "Citation (Doe, 2020) — “ok”"); + } + #[test] fn validators_accept_only_the_documented_options() { assert!(validate_options("info", &args(&["file.docx"]), PASSWORD_OPTIONS).is_ok()); diff --git a/crates/zotero/src/bridge.rs b/crates/zotero/src/bridge.rs index b8dfaedac..50ea5fcd5 100644 --- a/crates/zotero/src/bridge.rs +++ b/crates/zotero/src/bridge.rs @@ -5,6 +5,8 @@ //! Field ids handed to Zotero stay valid for the transaction: they are kept in a list parallel //! to the document-order list of Zotero fields, updated as fields are inserted and removed. +use std::collections::HashMap; + use serde_json::{Value, json}; use wordcraft_doc::edit::Fragment; use wordcraft_doc::para::OBJ; @@ -192,6 +194,7 @@ impl Bridge { } } + /// Answer one call by its method name; [`Bridge::handle`] wraps it so a panic becomes an error. fn dispatch(&mut self, s: &mut Session, host: &mut dyn Host, c: &Call) -> R { match c.method.as_str() { "Application_getActiveDocument" => { @@ -274,21 +277,8 @@ impl Bridge { // citation moves between the text, footnotes and endnotes (switching styles). "Document_convert" => { let Value::Array(ids) = arg(c, 1)? else { return Err("field ids must be an array".into()) }; - let ids: Vec = ids.iter().take(100_000).cloned().collect(); - let notes = c.args.get(3).cloned().unwrap_or(Value::Null); - for (i, idv) in ids.iter().enumerate() { - let id = match idv { - Value::Number(n) => n.as_u64(), - Value::String(x) => x.trim().parse().ok(), - _ => None, - } - .ok_or("bad field id")?; - let note = match ¬es { - Value::Array(a) => a.get(i).and_then(Value::as_i64).unwrap_or(0), - v => v.as_i64().unwrap_or(0), - }; - self.convert_field(s, id, note)?; - } + let notes = c.args.get(3).unwrap_or(&Value::Null); + self.convert_fields(s, ids, notes)?; Ok(Value::Null) } "Field_convert" => { @@ -484,6 +474,49 @@ impl Bridge { Ok(()) } + /// `Document_convert`: each id once (listed again, its last note type wins), every id + /// checked against one indexed list of the fields before anything moves. Only a citation + /// that actually moves costs a fresh list. + fn convert_fields(&mut self, s: &mut Session, ids: &[Value], notes: &Value) -> Result<(), String> { + let mut todo: Vec<(u64, i64)> = Vec::new(); + let mut slot: HashMap = HashMap::new(); + for (i, idv) in ids.iter().take(fields::MAX_FIELDS).enumerate() { + let id = match idv { + Value::Number(n) => n.as_u64(), + Value::String(x) => x.trim().parse().ok(), + _ => None, + } + .ok_or("bad field id")?; + let note = match notes { + Value::Array(a) => a.get(i).and_then(Value::as_i64).unwrap_or(0), + v => v.as_i64().unwrap_or(0), + }; + match slot.get(&id).and_then(|k| todo.get_mut(*k)) { + Some(t) => t.1 = note, + None => { + slot.insert(id, todo.len()); + todo.push((id, note)); + } + } + } + let index = |ids: &[u64]| -> HashMap { ids.iter().enumerate().map(|(k, id)| (*id, k)).collect() }; + let mut list = self.fields(s); + let mut at = index(&self.ids); + if let Some((id, _)) = todo.iter().find(|(id, _)| !at.contains_key(id)) { + return Err(format!("no field with id {id}")); + } + for (id, note) in todo { + let f = at.get(&id).and_then(|k| list.get(*k)).ok_or_else(|| format!("no field with id {id}"))?; + if !moves(s, f, note) { + continue; + } + self.convert_field(s, id, note)?; + list = self.fields(s); + at = index(&self.ids); + } + Ok(()) + } + fn convert_placeholders(&mut self, s: &mut Session, ph: &[Value], note: i64) -> R { let mut ids = Vec::new(); for v in ph { @@ -519,6 +552,20 @@ impl Bridge { } } +/// Whether moving `f` to the text (`note` 0), a footnote (1) or an endnote (2) changes anything, +/// as [`Bridge::convert_field`] decides it. +fn moves(s: &Session, f: &ZField, note: i64) -> bool { + let current = match f.start.story { + StoryRef::Body => 0, + StoryRef::Part(pid) => match s.doc.parts.get(&pid).map(|p| p.kind) { + Some(PartKind::Footnote) => 1, + Some(PartKind::Endnote) => 2, + _ => return false, + }, + }; + current != if matches!(note, 1 | 2) { note } else { 0 } +} + /// Delete the selection; returns where it collapsed. fn delete_selection(s: &mut Session) -> Result { let (a, b) = s.sel.ordered(); diff --git a/crates/zotero/src/client.rs b/crates/zotero/src/client.rs index bb6fc98b9..f6e31c83a 100644 --- a/crates/zotero/src/client.rs +++ b/crates/zotero/src/client.rs @@ -88,7 +88,11 @@ pub struct Outcome { /// Run `command`, answering every call with `answer`. pub fn run_command(opts: &Options, command: Command, answer: &mut dyn FnMut(&Call) -> Result) -> Result { + crate::peer::check_addr(&opts.addr)?; let mut stream = TcpStream::connect_timeout(&opts.addr, opts.connect_timeout).map_err(|e| ZoteroError::NotRunning(e.to_string()))?; + // Before Zotero sees anything: the command, and then the document, go only to this computer + // and, on Linux only, not to a listener the socket table shows belongs to another user. + crate::peer::check(&stream)?; let _ = stream.set_nodelay(true); stream.set_read_timeout(Some(opts.idle_timeout)).map_err(|e| ZoteroError::Io(e.to_string()))?; wire::write_frame(&mut stream, 0, &wire::command_payload(command.wire_name()))?; diff --git a/crates/zotero/src/fields.rs b/crates/zotero/src/fields.rs index c3776f9a5..21f7d2cc0 100644 --- a/crates/zotero/src/fields.rs +++ b/crates/zotero/src/fields.rs @@ -63,7 +63,14 @@ fn story_fields(doc: &Document, story: StoryRef, note_index: u32) -> Vec .collect() } +/// Most fields [`list`] hands Zotero (no real document comes near it). +pub const MAX_FIELDS: usize = 100_000; +/// Most bytes of field codes [`list`] hands Zotero, all fields together. +pub const MAX_CODE_BYTES: usize = 64 * 1024 * 1024; + /// Every Zotero field in document order: the body's, with each note's fields at its reference. +/// A note the body references more than once (a crafted file) is listed at its first reference +/// only. Past [`MAX_FIELDS`] fields or [`MAX_CODE_BYTES`] of codes the rest are left out. pub fn list(doc: &Document) -> Vec { enum Ev { Field(ZField), @@ -71,10 +78,12 @@ pub fn list(doc: &Document) -> Vec { } let mut evs: Vec<(Pos, Ev)> = story_fields(doc, StoryRef::Body, 0).into_iter().map(|f| (f.start.clone(), Ev::Field(f))).collect(); let (mut foot, mut end) = (0u32, 0u32); + let mut seen = std::collections::HashSet::new(); for path in doc.para_paths(StoryRef::Body) { let Some(p) = doc.para(StoryRef::Body, &path) else { continue }; - for off in p.object_offsets() { - if let Some(InlineObject::NoteRef { kind, id, custom }) = p.object_at(off) { + // The k-th object marker holds the k-th object. + for (off, obj) in p.object_offsets().into_iter().zip(&p.objects) { + if let InlineObject::NoteRef { kind, id, custom } = obj { let n = match kind { NoteKind::Footnote => &mut foot, NoteKind::Endnote => &mut end, @@ -82,16 +91,31 @@ pub fn list(doc: &Document) -> Vec { if custom.is_empty() { *n = n.saturating_add(1); } - evs.push((Pos::new(StoryRef::Body, path.clone(), off), Ev::Note(*id, (*n).max(1)))); + if seen.insert(*id) { + evs.push((Pos::new(StoryRef::Body, path.clone(), off), Ev::Note(*id, (*n).max(1)))); + } } } } evs.sort_by(|a, b| a.0.cmp(&b.0)); let mut out = Vec::new(); + let mut bytes = 0usize; + let mut take = |f: ZField, out: &mut Vec| { + bytes = bytes.saturating_add(f.code.len()); + let fits = out.len() < MAX_FIELDS && bytes <= MAX_CODE_BYTES; + if fits { + out.push(f); + } + fits + }; for (_, ev) in evs { - match ev { - Ev::Field(f) => out.push(f), - Ev::Note(id, n) => out.extend(story_fields(doc, StoryRef::Part(id), n)), + let fits = match ev { + Ev::Field(f) => take(f, &mut out), + Ev::Note(id, n) => story_fields(doc, StoryRef::Part(id), n).into_iter().all(|f| take(f, &mut out)), + }; + if !fits { + log::warn!("Zotero: the document has more citation fields than WordCraft hands Zotero; the rest are left out"); + break; } } out diff --git a/crates/zotero/src/lib.rs b/crates/zotero/src/lib.rs index 40eba55eb..cf4c93443 100644 --- a/crates/zotero/src/lib.rs +++ b/crates/zotero/src/lib.rs @@ -18,6 +18,7 @@ mod bridge; pub mod client; pub mod fields; +mod peer; mod rich; pub mod wire; @@ -37,7 +38,13 @@ pub enum ZoteroError { /// Zotero sent something that isn't the protocol. #[error("unexpected data from Zotero: {0}")] Protocol(String), + /// What answered on Zotero's port may not be Zotero (another computer, on any system; on + /// Linux only, another user's program); it gets no access to the document. + #[error("WordCraft won't let this program edit the document: {0}")] + Refused(String), } #[cfg(test)] mod tests; +#[cfg(test)] +mod tests_limits; diff --git a/crates/zotero/src/peer.rs b/crates/zotero/src/peer.rs new file mode 100644 index 000000000..07df98e79 --- /dev/null +++ b/crates/zotero/src/peer.rs @@ -0,0 +1,234 @@ +//! Who answered on Zotero's port. Zotero's protocol has no handshake that proves the other end +//! is Zotero, and whatever answers gets to read and rewrite the open document. Two checks: +//! +//! - **Every system:** WordCraft only connects to this computer (a loopback address). +//! - **Linux only:** the program listening there must not run as another user, from the +//! kernel's socket table (`/proc/net/tcp`): another account on a shared machine that took the +//! port while Zotero wasn't running is refused. A listener the table doesn't show (WSL2 +//! mirrored networking with Zotero on Windows, a local proxy, a sandbox) leaves only the +//! loopback rule. +//! +//! On Windows, macOS and the BSDs there is **no same-user check**: the loopback rule is the +//! only one, so any local account's listener on the port is taken for Zotero. Those systems have +//! no such table to read without OS calls this crate doesn't make (see `docs/zotero.md`, #3831). + +use std::net::{IpAddr, Ipv4Addr, Ipv6Addr, SocketAddr, TcpStream}; + +use crate::ZoteroError; + +/// Whether `addr` is on this computer; anything else is refused before connecting. +pub(crate) fn check_addr(addr: &SocketAddr) -> Result<(), ZoteroError> { + if addr.ip().to_canonical().is_loopback() { + Ok(()) + } else { + Err(ZoteroError::Refused(format!("{addr} is not on this computer; WordCraft only talks to Zotero on 127.0.0.1"))) + } +} + +/// Refuse the program at the other end of `stream` when it may not be ours (see the module +/// documentation). +pub(crate) fn check(stream: &TcpStream) -> Result<(), ZoteroError> { + let io = |e: std::io::Error| ZoteroError::Io(e.to_string()); + let (local, peer) = (stream.local_addr().map_err(io)?, stream.peer_addr().map_err(io)?); + check_addr(&peer)?; + owner_check(local, peer) +} + +/// Refuse a listener at `peer` that the kernel's socket tables say belongs to another user. +#[cfg(any(target_os = "linux", target_os = "android"))] +fn owner_check(local: SocketAddr, peer: SocketAddr) -> Result<(), ZoteroError> { + use std::io::Read; + /// Longest socket table read (a busy server's table is a few MB). + const MAX_TABLE: u64 = 64 * 1024 * 1024; + let mut table = String::new(); + for path in ["/proc/net/tcp", "/proc/net/tcp6"] { + if let Ok(f) = std::fs::File::open(path) { + let mut part = String::new(); + if f.take(MAX_TABLE).read_to_string(&mut part).is_ok() { + table.push_str(&part); + } + } + } + // Our uid when our own socket's row is missing: `/proc/self` belongs to this process's user. + let me = std::fs::metadata("/proc/self").ok().map(|m| std::os::unix::fs::MetadataExt::uid(&m)); + same_owner(&table, local, peer, me) +} + +/// Windows, macOS, the BSDs: no same-user check (no socket table to read without OS calls this +/// crate doesn't make, #3831); the loopback rule alone decides. +#[cfg(not(any(target_os = "linux", target_os = "android")))] +fn owner_check(_local: SocketAddr, _peer: SocketAddr) -> Result<(), ZoteroError> { + Ok(()) +} + +/// From a `/proc/net/tcp`-format socket table: refuse when the socket at `peer` connected to +/// `local` (the listener's side) belongs to another user than ours. Our uid is our own socket's +/// row, else `me` (this process's). When the listener isn't listed (or the table couldn't be +/// read, or our uid is unknown) its owner is unknown and the loopback rule decides. +#[cfg_attr(not(any(target_os = "linux", target_os = "android")), allow(dead_code))] +pub(crate) fn same_owner(table: &str, local: SocketAddr, peer: SocketAddr, me: Option) -> Result<(), ZoteroError> { + let key = |a: SocketAddr| (a.ip().to_canonical(), a.port()); + let (local, peer) = (key(local), key(peer)); + let (mut ours, mut theirs) = (None, None); + for line in table.lines() { + let f: Vec<&str> = line.split_whitespace().take(8).collect(); + let (Some(l), Some(r), Some(uid)) = (f.get(1).and_then(|a| parse_addr(a)), f.get(2).and_then(|a| parse_addr(a)), f.get(7)) else { + continue; + }; + let Ok(uid) = uid.parse::() else { continue }; + let (l, r) = (key(l), key(r)); + if l == local && r == peer { + ours = Some(uid); + } else if l == peer && r == local { + theirs = Some(uid); + } + } + match (ours.or(me), theirs) { + (Some(a), Some(b)) if a == b => Ok(()), + (Some(_), Some(b)) => { + Err(ZoteroError::Refused(format!("the program on port {} runs as another user (uid {b}), so it isn't your Zotero", peer.1))) + } + _ => { + log::debug!( + "Zotero: couldn't tell which user runs the program on port {} (not in the socket table); trusting the loopback address alone", + peer.1 + ); + Ok(()) + } + } +} + +/// `0100007F:5A5C` (IPv4) or 32 hex digits and a port (IPv6): each 32-bit word as the kernel +/// prints it, in this machine's byte order. +#[cfg_attr(not(any(target_os = "linux", target_os = "android")), allow(dead_code))] +fn parse_addr(s: &str) -> Option { + let (ip, port) = s.split_once(':')?; + let port = u16::from_str_radix(port, 16).ok()?; + let word = |k: usize| ip.get(k * 8..k * 8 + 8).and_then(|w| u32::from_str_radix(w, 16).ok()).map(u32::to_ne_bytes); + let ip = match ip.len() { + 8 => IpAddr::V4(Ipv4Addr::from(word(0)?)), + 32 => { + let mut b = [0u8; 16]; + for (k, w) in b.as_chunks_mut::<4>().0.iter_mut().enumerate() { + *w = word(k)?; + } + IpAddr::V6(Ipv6Addr::from(b)) + } + _ => return None, + }; + Some(SocketAddr::new(ip, port)) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// `a` the way the kernel's socket table prints it. + fn hex(a: SocketAddr) -> String { + let ip = match a.ip() { + IpAddr::V4(v4) => format!("{:08X}", u32::from_ne_bytes(v4.octets())), + IpAddr::V6(v6) => v6.octets().as_chunks::<4>().0.iter().map(|w| format!("{:08X}", u32::from_ne_bytes(*w))).collect(), + }; + format!("{ip}:{:04X}", a.port()) + } + + /// A socket table row for the connection from `l` to `r`, owned by `uid`. + fn row(l: SocketAddr, r: SocketAddr, uid: u32) -> String { + format!(" 0: {} {} 01 00000000:00000000 00:00000000 00000000 {uid:>4} 0 12345 1 0000000000000000 20 4 30 10 -1\n", hex(l), hex(r)) + } + + const HEADER: &str = " sl local_address rem_address st tx_queue rx_queue tr tm->when retrnsmt uid timeout inode\n"; + + /// Our side and Zotero's listening side of a loopback connection. + fn addrs() -> (SocketAddr, SocketAddr) { + ("127.0.0.1:41234".parse().unwrap(), "127.0.0.1:23116".parse().unwrap()) + } + + #[test] + fn addresses_parse_as_the_kernel_prints_them() { + for a in ["127.0.0.1:23116", "[::1]:23116", "[::ffff:127.0.0.1]:80"] { + let a: SocketAddr = a.parse().unwrap(); + assert_eq!(parse_addr(&hex(a)), Some(a)); + } + for bad in ["", "0100007F", "0100007F:", "XYZ00000:0050", "0100007F:12345", "0100:0050"] { + assert_eq!(parse_addr(bad), None, "{bad}"); + } + } + + #[test] + fn a_listener_of_another_user_is_refused() { + let (local, peer) = addrs(); + let same = format!("{HEADER}{}{}", row(local, peer, 1000), row(peer, local, 1000)); + assert_eq!(same_owner(&same, local, peer, None), Ok(())); + let other = format!("{HEADER}{}{}", row(local, peer, 1000), row(peer, local, 1001)); + assert!(matches!(same_owner(&other, local, peer, None), Err(ZoteroError::Refused(e)) if e.contains("another user"))); + // Junk lines are skipped. + let junk = format!("{HEADER}garbage\n 1: zz:zz yy:yy 01 x x x x\n{}{}", row(local, peer, 7), row(peer, local, 7)); + assert_eq!(same_owner(&junk, local, peer, None), Ok(())); + } + + /// WSL2 mirrored networking (Zotero on Windows), a local proxy or a sandbox: the listener + /// isn't in our table, so the loopback rule decides, as on other systems. + #[test] + fn an_unlisted_listener_falls_back_to_the_loopback_rule() { + let (local, peer) = addrs(); + let missing = format!("{HEADER}{}", row(local, peer, 1000)); + assert_eq!(same_owner(&missing, local, peer, None), Ok(())); + // The table couldn't be read. + assert_eq!(same_owner("", local, peer, None), Ok(())); + assert_eq!(same_owner("", local, peer, Some(1000)), Ok(())); + } + + /// Our own socket's row missing doesn't hide another user's listener: our uid comes from + /// the process itself. + #[test] + fn a_listed_listener_is_checked_against_our_process_uid() { + let (local, peer) = addrs(); + let only_theirs = format!("{HEADER}{}", row(peer, local, 1001)); + assert!(matches!(same_owner(&only_theirs, local, peer, Some(1000)), Err(ZoteroError::Refused(e)) if e.contains("another user"))); + assert_eq!(same_owner(&only_theirs, local, peer, Some(1001)), Ok(())); + // Neither our row nor our process uid: unknown, so the loopback rule decides. + assert_eq!(same_owner(&only_theirs, local, peer, None), Ok(())); + // Our socket's row is ours by definition and wins over the process uid. + let both = format!("{HEADER}{}{}", row(local, peer, 1001), row(peer, local, 1001)); + assert_eq!(same_owner(&both, local, peer, Some(1000)), Ok(())); + } + + #[test] + fn a_dual_stack_listener_is_matched_through_its_mapped_address() { + let (local, peer) = addrs(); + let mapped = |a: SocketAddr| match a.ip() { + IpAddr::V4(v4) => SocketAddr::new(IpAddr::V6(v4.to_ipv6_mapped()), a.port()), + IpAddr::V6(_) => a, + }; + let table = format!("{HEADER}{}{}", row(local, peer, 1000), row(mapped(peer), mapped(local), 1001)); + assert!(matches!(same_owner(&table, local, peer, None), Err(ZoteroError::Refused(e)) if e.contains("another user"))); + } + + #[test] + fn only_this_computer_is_contacted() { + for ok in ["127.0.0.1:23116", "127.9.9.9:1", "[::1]:23116", "[::ffff:127.0.0.1]:23116"] { + assert_eq!(check_addr(&ok.parse().unwrap()), Ok(()), "{ok}"); + } + for bad in ["192.0.2.1:23116", "0.0.0.0:23116", "[2001:db8::1]:23116", "[::]:23116"] { + assert!(matches!(check_addr(&bad.parse().unwrap()), Err(ZoteroError::Refused(_))), "{bad}"); + } + } + + #[test] + fn another_computer_is_refused_before_connecting() { + let opts = crate::client::Options { addr: "192.0.2.1:23116".parse().unwrap(), ..Default::default() }; + let r = crate::client::run_command(&opts, crate::Command::Refresh, &mut |_| Ok(serde_json::Value::Null)); + assert!(matches!(r, Err(ZoteroError::Refused(_))), "{r:?}"); + } + + /// A listener this process opened runs as us: the real socket table agrees. + #[cfg(any(target_os = "linux", target_os = "android"))] + #[test] + fn our_own_listener_passes_the_owner_check() { + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let stream = TcpStream::connect(listener.local_addr().unwrap()).unwrap(); + let _accepted = listener.accept().unwrap(); + assert_eq!(check(&stream), Ok(())); + } +} diff --git a/crates/zotero/src/rich.rs b/crates/zotero/src/rich.rs index 2fabb62bb..7ff3377f1 100644 --- a/crates/zotero/src/rich.rs +++ b/crates/zotero/src/rich.rs @@ -4,10 +4,12 @@ use wordcraft_doc::edit::Fragment; use wordcraft_doc::para::OBJ; -use wordcraft_doc::{Block, CharProps, Document, Paragraph}; +use wordcraft_doc::{Block, CharProps, Document, Paragraph, Run}; /// Most paragraphs taken from one text (a bibliography of thousands of entries is fine). const MAX_PARAS: usize = 100_000; +/// Most formatting runs taken from one text; past it, text keeps the formatting before it. +pub(crate) const MAX_RUNS: usize = 1_000_000; fn style_props(base: &CharProps, src: &CharProps) -> CharProps { let mut p = base.clone(); @@ -38,18 +40,35 @@ fn style_props(base: &CharProps, src: &CharProps) -> CharProps { p } +/// The body paragraphs of `d` (Zotero's RTF or HTML, read), at most [`MAX_PARAS`], formatted on +/// top of `base` with the run styling they carry (past [`MAX_RUNS`] runs, text keeps the +/// formatting before it). Objects are dropped. fn from_doc(d: &Document, base: &CharProps) -> Vec { let mut out = Vec::new(); + let mut runs = 0usize; for b in d.body.iter().take(MAX_PARAS) { let Some(src) = b.as_para() else { continue }; let mut p = Paragraph::new(); p.mark = base.clone(); + // Appended run by run, merging only with the run before; normalized once at the end. for (range, props) in src.run_ranges() { let Some(t) = src.text.get(range) else { continue }; let t: String = t.chars().filter(|c| *c != OBJ).collect(); - let at = p.len(); - let _ = p.insert_text(at, &t, &style_props(base, props)); + if t.is_empty() { + continue; + } + let props = style_props(base, props); + p.text.push_str(&t); + match p.runs.last_mut() { + Some(last) if last.props == props || runs >= MAX_RUNS => last.len += t.len(), + _ => { + p.runs.push(Run { len: t.len(), props }); + runs += 1; + } + } } + p.normalize(); + p.touch(); out.push(p); } out diff --git a/crates/zotero/src/tests_limits.rs b/crates/zotero/src/tests_limits.rs new file mode 100644 index 000000000..ce6b12c67 --- /dev/null +++ b/crates/zotero/src/tests_limits.rs @@ -0,0 +1,122 @@ +//! Hostile documents and calls stay cheap: work grows with the document, not with how often a +//! crafted file or call repeats something (#3692). + +use std::time::{Duration, Instant}; + +use serde_json::{Value, json}; +use wordcraft_doc::para::{NoteKind, OBJ}; +use wordcraft_doc::{CharProps, Document, InlineObject, Paragraph, PartKind, Run, StoryRef, para_block}; +use wordcraft_engine::Session; + +use crate::wire::Call; +use crate::{Bridge, Headless, fields}; + +const CITE: &str = r#"ADDIN ZOTERO_ITEM CSL_CITATION {"citationID":"k1","citationItems":[{"id":7}]}"#; + +/// A paragraph of `objects`, one marker each, in one plain run. +fn objects_para(objects: Vec) -> Paragraph { + let mut p = Paragraph::new(); + p.text = std::iter::repeat_n(OBJ, objects.len()).collect(); + p.runs = vec![Run { len: p.text.len(), props: CharProps::default() }]; + p.objects = objects; + p +} + +/// `n` empty Zotero citation fields in a row. +fn citations(n: usize) -> Vec { + (0..n).flat_map(|_| [InlineObject::FieldStart { instr: CITE.into(), locked: false, dirty: false }, InlineObject::FieldEnd]).collect() +} + +// Finding 12: one endnote referenced many times is listed once, not once per reference. +#[test] +fn a_note_referenced_many_times_is_listed_once() { + let mut doc = Document::from_text(""); + let note = doc.add_part(PartKind::Endnote, vec![para_block(objects_para(citations(200)))]); + let refs = (0..5_000).map(|_| InlineObject::NoteRef { kind: NoteKind::Endnote, id: note, custom: String::new() }).collect(); + doc.body = vec![para_block(objects_para(refs))]; + // No clock: listing the note once per reference gave a million fields, which the count catches. + let list = fields::list(&doc); + assert_eq!(list.len(), 200, "each field once, at the note's first reference"); + assert!(list.iter().all(|f| f.start.story == StoryRef::Part(note) && f.note_index == 1)); +} + +#[test] +fn listing_stops_at_the_field_budget() { + let mut doc = Document::from_text(""); + // Many short paragraphs: the per-paragraph object scan is another finding's (#3614). + doc.body = (0..fields::MAX_FIELDS / 100 + 1).map(|_| para_block(objects_para(citations(100)))).collect(); + assert_eq!(fields::list(&doc).len(), fields::MAX_FIELDS); + // Codes count too: a few huge codes fill the byte budget before the field budget. + let big = format!("{CITE}{}", "x".repeat(fields::MAX_CODE_BYTES / 4 - 1000)); + let huge = (0..5).flat_map(|_| [InlineObject::FieldStart { instr: big.clone(), locked: false, dirty: false }, InlineObject::FieldEnd]).collect(); + doc.body = vec![para_block(objects_para(huge))]; + assert_eq!(fields::list(&doc).len(), 4); +} + +// Finding 16: rich text with alternating formatting is built in one pass, not run by run with +// every earlier run merged again. +#[test] +fn rich_text_with_many_runs_is_linear() { + let html: String = "ab".repeat(50_000); + let t = Instant::now(); + let frag = crate::rich::fragment(&html, true, &CharProps::default()); + let elapsed = t.elapsed(); + let Some(wordcraft_doc::Block::Para(p)) = frag.blocks.first() else { panic!("no paragraph") }; + assert_eq!(p.text, "ab".repeat(50_000)); + assert_eq!(p.runs.len(), 100_000); + assert_eq!(p.runs.iter().map(|r| r.len).sum::(), p.text.len()); + // Under half a second on a loaded machine; merging again after every run ran past 9 minutes. + assert!(elapsed < Duration::from_secs(10), "{elapsed:?}"); + // Neighbours with the same formatting are one run. + let frag = crate::rich::fragment("abc", true, &CharProps::default()); + let Some(wordcraft_doc::Block::Para(p)) = frag.blocks.first() else { panic!("no paragraph") }; + assert_eq!((p.text.as_str(), p.runs.len()), ("abc", 2)); + assert_eq!(p.props_of_char(1).italic, Some(true)); +} + +/// Call `method` on a bridge over `s`. +fn call(b: &mut Bridge, s: &mut Session, method: &str, args: Value) -> Result { + let Value::Array(args) = args else { return Err("args must be an array".into()) }; + b.handle(s, &mut Headless::default(), &Call::new(method, args)) +} + +// Finding 32: Document_convert checks each distinct id once against one list of the fields. +#[test] +fn convert_with_one_id_repeated_is_one_check() { + let mut doc = Document::from_text(""); + // Checking each id against a fresh list of the fields took 81 s with 300 of them; with 1,000 + // that is minutes, against milliseconds for one indexed list. + doc.body = (0..1000).map(|_| para_block(objects_para(citations(1)))).collect(); + let mut s = Session::new(doc); + let mut b = Bridge::new(); + call(&mut b, &mut s, "Application_getActiveDocument", json!([3])).unwrap(); + let got = call(&mut b, &mut s, "Document_getFields", json!([1, "Field"])).unwrap(); + let first = got[0][0].clone(); + let ids = vec![first.clone(); 100_000]; + let before = format!("{:?}", s.doc.body); + let t = Instant::now(); + call(&mut b, &mut s, "Document_convert", json!([1, ids, 0, 0])).unwrap(); + assert!(t.elapsed() < Duration::from_secs(10), "{:?}", t.elapsed()); + assert_eq!(format!("{:?}", s.doc.body), before, "already in the text: nothing to do"); + // An unknown id is refused before anything moves. + let r = call(&mut b, &mut s, "Document_convert", json!([1, [first.clone(), 999_999_999u64], 0, [1, 1]])); + assert!(r.is_err()); + assert!(s.doc.parts.is_empty(), "nothing converted"); +} + +#[test] +fn convert_with_a_repeated_id_takes_its_last_note_type() { + let mut doc = Document::from_text(""); + doc.body = (0..3).map(|_| para_block(objects_para(citations(1)))).collect(); + let mut s = Session::new(doc); + let mut b = Bridge::new(); + call(&mut b, &mut s, "Application_getActiveDocument", json!([3])).unwrap(); + let got = call(&mut b, &mut s, "Document_getFields", json!([1, "Field"])).unwrap(); + let (a, c) = (got[0][0].clone(), got[0][2].clone()); + call(&mut b, &mut s, "Document_convert", json!([1, [a.clone(), c, a], 0, [2, 1, 1]])).unwrap(); + let kinds: Vec = s.doc.parts.values().map(|p| p.kind).collect(); + assert_eq!(kinds, vec![PartKind::Footnote, PartKind::Footnote]); + // Ids stay valid: the fields are where Zotero now expects them. + let got = call(&mut b, &mut s, "Document_getFields", json!([1, "Field"])).unwrap(); + assert_eq!(got[2], json!([1, 0, 2])); +} diff --git a/docs/zotero.md b/docs/zotero.md index f05b62c6e..cf5813494 100644 --- a/docs/zotero.md +++ b/docs/zotero.md @@ -61,6 +61,32 @@ Word's "Field" in the file.) - Zotero's document transfer (Export/Import Document) isn't needed and isn't supported. - The web version can't reach Zotero (browsers can't open the socket). +## Who can drive the document + +Zotero's protocol has no handshake that proves the program on port 23116 is Zotero, and that +program can read the citations and edit the open document. WordCraft limits who that can be. +In short: **every system** gets the loopback rule; **only Linux** also checks that the listener +isn't another user's. On Windows and macOS (and the BSDs) the loopback rule is the only check. + +- Every system: it only connects to this computer (a loopback address such as `127.0.0.1`); + any other address is refused before connecting. +- Linux only: it checks, in the kernel's socket table (`/proc/net/tcp`, `/proc/net/tcp6`), that the + program listening on the port runs as the same user as WordCraft, before sending anything. If + another account on a shared machine took the port while Zotero wasn't running, WordCraft + refuses with an error and the document isn't touched. When the table doesn't show the + listener (WSL2 mirrored networking with Zotero running on Windows, a local proxy, a sandbox) + or can't be read, its owner is unknown and only the loopback rule applies, as below. +- Windows, macOS and the BSDs: **no same-user check.** They have no such table to read + without OS calls WordCraft doesn't make (it uses no `unsafe` code), so only the loopback rule + applies there (and on Linux when the listener isn't in the table). Remaining risk: on a + computer shared with other accounts, a program another user leaves on port 23116 while + Zotero is closed would be taken for Zotero. An owner check for these systems is tracked in + #3831. A program running as you is trusted on every system; it could change your documents + anyway. +- `wordcraft-cli zotero` prints Zotero's messages (and `--trace` lines) with control + characters, line breaks and backslashes shown as escapes (`\n`, `\u{1b}`, `\\`), so a peer + can't forge other output or drive the terminal. + ## How it works `crates/zotero` implements Zotero's published word-processor wire protocol (port 23116):