diff --git a/apps/pdfcraft/src/main.rs b/apps/pdfcraft/src/main.rs index 8fae5b584..9bbf2f5ac 100644 --- a/apps/pdfcraft/src/main.rs +++ b/apps/pdfcraft/src/main.rs @@ -193,23 +193,51 @@ fn main() -> eframe::Result { } /// Write the control endpoint so that only the current user can read the token. +/// +/// The JSON goes to a new file next to `path`, created fresh (owner-only on Unix), which then +/// replaces `path`. Opening `path` itself would follow a link planted there and truncate whatever +/// it points at; a rename replaces the link and leaves its target alone. A reader polling for the +/// file also never sees it half-written. fn write_control_file(path: &str, port: u16, token: &str) -> std::io::Result<()> { + use std::io::{ErrorKind, Write}; let json = serde_json::json!({ "port": port, "token": token, "pid": std::process::id() }).to_string(); - let mut opts = std::fs::OpenOptions::new(); - opts.write(true).create(true).truncate(true); - #[cfg(unix)] - { - use std::os::unix::fs::OpenOptionsExt; - opts.mode(0o600); + let path = std::path::Path::new(path); + let name = path.file_name().ok_or_else(|| std::io::Error::new(ErrorKind::InvalidInput, "not a file name"))?; + // Unpredictable suffixes, so the name can't be planted in advance. `RandomState` is keyed from + // the operating system's random source. + let random = std::hash::RandomState::new(); + for i in 0..16u32 { + let mut staged = std::ffi::OsString::from("."); + staged.push(name); + staged.push(format!(".{:016x}.tmp", std::hash::BuildHasher::hash_one(&random, i))); + let staged = path.with_file_name(staged); + let mut opts = std::fs::OpenOptions::new(); + opts.write(true).create_new(true); + #[cfg(unix)] + { + use std::os::unix::fs::OpenOptionsExt; + opts.mode(0o600); + } + let file = match opts.open(&staged) { + Ok(f) => f, + Err(e) if e.kind() == ErrorKind::AlreadyExists => continue, + // Windows refuses `create_new` on a folder with "access denied"; the name is taken all + // the same. A folder we can't write to, with nothing at the name, still fails here. + Err(e) if e.kind() == ErrorKind::PermissionDenied && staged.symlink_metadata().is_ok() => continue, + Err(e) => return Err(e), + }; + // The block closes the file before it is renamed. + let written = { + let mut file = file; + file.write_all(json.as_bytes()) + }; + let done = written.and_then(|()| std::fs::rename(&staged, path)); + if done.is_err() { + let _ = std::fs::remove_file(&staged); + } + return done; } - use std::io::Write; - let mut f = opts.open(path)?; - #[cfg(unix)] - { - use std::os::unix::fs::PermissionsExt; - f.set_permissions(std::fs::Permissions::from_mode(0o600))?; - } - f.write_all(json.as_bytes()) + Err(std::io::Error::new(ErrorKind::AlreadyExists, "no free name for the control file's temporary copy")) } /// How wgpu finds a GPU. Each choice yields to its wgpu environment variable. @@ -377,4 +405,76 @@ mod tests { assert!(super::linux_display_gpus(std::path::Path::new("/nonexistent/drm")).is_empty()); Ok(()) } + + /// A fresh folder per call: tests run in parallel. + fn scratch() -> std::path::PathBuf { + static N: std::sync::atomic::AtomicU32 = std::sync::atomic::AtomicU32::new(0); + let n = N.fetch_add(1, std::sync::atomic::Ordering::Relaxed); + let d = std::env::temp_dir().join(format!("pdfcraft-control-file-{}-{n}", std::process::id())); + let _ = std::fs::remove_dir_all(&d); + std::fs::create_dir_all(&d).unwrap(); + d + } + + /// The control file's port and token, and every name in its folder. + fn written(dir: &std::path::Path) -> (serde_json::Value, Vec) { + let json = serde_json::from_str(&std::fs::read_to_string(dir.join("ctl.json")).unwrap()).unwrap(); + let mut names: Vec = std::fs::read_dir(dir).unwrap().map(|e| e.unwrap().file_name().to_string_lossy().into_owned()).collect(); + names.sort(); + (json, names) + } + + #[test] + fn the_control_file_replaces_a_hard_link_instead_of_writing_through_it() { + // The file used to be opened with create + truncate, so a link planted at the path had its + // target truncated and overwritten with the token. + let dir = scratch(); + std::fs::write(dir.join("victim.txt"), "keep me").unwrap(); + std::fs::hard_link(dir.join("victim.txt"), dir.join("ctl.json")).unwrap(); + super::write_control_file(&dir.join("ctl.json").to_string_lossy(), 4242, "t0ken").unwrap(); + assert_eq!(std::fs::read_to_string(dir.join("victim.txt")).unwrap(), "keep me"); + let (json, names) = written(&dir); + assert_eq!((json["port"].as_u64(), json["token"].as_str()), (Some(4242), Some("t0ken"))); + assert_eq!(names, ["ctl.json", "victim.txt"], "no temporary file left behind"); + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn the_control_file_replaces_a_symlink_instead_of_writing_through_it() { + let dir = scratch(); + let (link, target) = (dir.join("ctl.json"), dir.join("victim.txt")); + std::fs::write(&target, "keep me").unwrap(); + #[cfg(unix)] + let made = std::os::unix::fs::symlink(&target, &link); + #[cfg(windows)] + let made = std::os::windows::fs::symlink_file(&target, &link); + if let Err(e) = made { + // Windows needs Developer Mode (or admin) for symlinks. + eprintln!("skipped: can't create a symlink here: {e}"); + let _ = std::fs::remove_dir_all(&dir); + return; + } + super::write_control_file(&link.to_string_lossy(), 4242, "t0ken").unwrap(); + assert_eq!(std::fs::read_to_string(&target).unwrap(), "keep me"); + assert!(!std::fs::symlink_metadata(&link).unwrap().file_type().is_symlink()); + assert_eq!(written(&dir).0["token"].as_str(), Some("t0ken")); + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn the_control_file_replaces_the_one_a_previous_run_left() { + let dir = scratch(); + std::fs::write(dir.join("ctl.json"), r#"{"port":1,"token":"old"}"#).unwrap(); + super::write_control_file(&dir.join("ctl.json").to_string_lossy(), 4242, "t0ken").unwrap(); + let (json, names) = written(&dir); + assert_eq!(json["token"].as_str(), Some("t0ken")); + assert_eq!(names, ["ctl.json"]); + #[cfg(unix)] + { + use std::os::unix::fs::PermissionsExt; + let mode = std::fs::metadata(dir.join("ctl.json")).unwrap().permissions().mode(); + assert_eq!(mode & 0o077, 0, "only the owner can read the token: {mode:o}"); + } + let _ = std::fs::remove_dir_all(&dir); + } } diff --git a/contributors/people.toml b/contributors/people.toml index d84dff38e..2fa78330e 100644 --- a/contributors/people.toml +++ b/contributors/people.toml @@ -22,6 +22,11 @@ real_name = "Brandon Thomas" display_name = "Brandon Thomas" sync_github_name = false +[people.jcl-bit] +real_name = "James Locke" +display_name = "James Locke" +sync_github_name = false + [people.switch87] real_name = "Gert Pellin" display_name = "Gert Pellin" diff --git a/crates/print/README.md b/crates/print/README.md index 62ac388a8..2ba1515e4 100644 --- a/crates/print/README.md +++ b/crates/print/README.md @@ -33,7 +33,8 @@ annotations); sheets place them with a clip. The result is a fresh, unencrypted, garbage-collected file (callers check the print permission). `spool` talks to CUPS (`lpstat -p -d`, `lp` with copies, collation, duplex and monochrome -options). Other platforms report that printing to a printer isn't available yet; the +options). The job is piped to `lp` on stdin and never written to a temp file, where another +local user could read or swap it. Other platforms report that printing to a printer isn't available yet; the print-ready PDF can always be saved. Not yet: Windows and web spoolers, print as image, poster labels, PostScript output, colour diff --git a/crates/print/src/spool.rs b/crates/print/src/spool.rs index c9c0fcbf3..176b5a2db 100644 --- a/crates/print/src/spool.rs +++ b/crates/print/src/spool.rs @@ -1,6 +1,6 @@ //! The system print spooler. On macOS and Linux this is CUPS: printers come from `lpstat`, jobs -//! go to `lp` with the job options (copies, collation, duplex, colour). Other platforms report -//! that printing isn't available yet; the print-ready PDF can still be saved. +//! are piped to `lp` with the job options (copies, collation, duplex, colour). Other platforms +//! report that printing isn't available yet; the print-ready PDF can still be saved. use crate::PrintError; @@ -45,8 +45,8 @@ pub fn parse_lpstat(out: &str) -> Vec { .collect() } -/// The `lp` arguments for a job printing `file`. -pub fn lp_args(job: &Job, file: &str) -> Vec { +/// The `lp` arguments for a job. There is no file argument: `lp` reads the job from stdin. +pub fn lp_args(job: &Job) -> Vec { let mut a = Vec::new(); if let Some(p) = &job.printer { a.extend(["-d".to_string(), p.clone()]); @@ -65,8 +65,6 @@ pub fn lp_args(job: &Job, file: &str) -> Vec { } // The sheets are already laid out at their final size. opt("fit-to-page=false"); - a.push("--".into()); - a.push(file.to_string()); a } @@ -99,19 +97,7 @@ pub fn printers() -> Vec { pub fn submit(pdf: &[u8], job: &Job) -> Result { #[cfg(all(unix, not(target_arch = "wasm32")))] { - let dir = std::env::temp_dir().join(format!("pdfcraft-print-{}", std::process::id())); - std::fs::create_dir_all(&dir).map_err(|e| PrintError::Spool(e.to_string()))?; - let file = dir.join(format!("job-{}.pdf", std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).map_or(0, |d| d.as_nanos()))); - std::fs::write(&file, pdf).map_err(|e| PrintError::Spool(e.to_string()))?; - let out = std::process::Command::new("lp").args(lp_args(job, &file.to_string_lossy())).output(); - let _ = std::fs::remove_file(&file); - let out = out.map_err(|e| PrintError::Spool(format!("the print spooler is not available: {e}")))?; - if out.status.success() { - Ok(String::from_utf8_lossy(&out.stdout).trim().to_string()) - } else { - let err = String::from_utf8_lossy(&out.stderr).trim().to_string(); - Err(PrintError::Spool(if err.is_empty() { "the print job was refused".into() } else { err })) - } + submit_via(std::process::Command::new("lp"), pdf, job) } #[cfg(not(all(unix, not(target_arch = "wasm32"))))] { @@ -119,3 +105,64 @@ pub fn submit(pdf: &[u8], job: &Job) -> Result { Err(PrintError::Spool("printing to a printer isn't available on this platform yet; save the print-ready PDF instead".into())) } } + +/// [`submit`] with the spooler command given, so tests can stand in for `lp`. +/// +/// The job goes to `lp` on stdin, never through a file: a predictable job folder in a shared +/// temp directory lets another local user read printed documents or swap the file before `lp` +/// reads it. +#[cfg(any(test, all(unix, not(target_arch = "wasm32"))))] +pub(crate) fn submit_via(mut lp: std::process::Command, pdf: &[u8], job: &Job) -> Result { + use std::io::Write; + use std::process::Stdio; + let mut child = lp + .args(lp_args(job)) + .stdin(Stdio::piped()) + .stdout(Stdio::piped()) + .stderr(Stdio::piped()) + .spawn() + .map_err(|e| PrintError::Spool(format!("the print spooler is not available: {e}")))?; + let stdin = child.stdin.take(); + // Feed the job on its own thread while collecting the output, so neither side can fill a + // pipe and wait on the other. Dropping `stdin` at the end closes it: the end of the job. + let fed_and_out = std::thread::scope(|s| { + let feeder = std::thread::Builder::new().name("print job".into()).spawn_scoped(s, move || match stdin { + Some(mut stdin) => stdin.write_all(pdf), + None => Err(std::io::Error::other("no pipe to the spooler")), + }); + match feeder { + Ok(feeder) => { + let out = child.wait_with_output(); + Ok((feeder.join(), out)) + } + Err(e) => { + // No thread to feed it: stop `lp` rather than leave it with an empty job. + let _ = child.kill(); + let _ = child.wait(); + Err(PrintError::Spool(format!("the print job could not be sent to the spooler: {e}"))) + } + } + }); + let (fed, out) = fed_and_out?; + let out = out.map_err(|e| PrintError::Spool(format!("the print spooler is not available: {e}")))?; + if !out.status.success() { + // `lp` may refuse before reading the job (an unknown printer); its message says why, + // and the broken pipe that leaves behind does not. + let err = String::from_utf8_lossy(&out.stderr).trim().to_string(); + return Err(PrintError::Spool(if err.is_empty() { "the print job was refused".into() } else { err })); + } + match fed { + Ok(Ok(())) => Ok(job_message(&out.stdout)), + Ok(Err(e)) => Err(PrintError::Spool(format!("the print job could not be sent to the spooler: {e}"))), + Err(_) => Err(PrintError::Spool("the print job could not be sent to the spooler".into())), + } +} + +/// `lp`'s reply ("request id is Office-12"), without the "(0 file(s))" CUPS adds when the job came +/// on stdin: it reads as if nothing was sent. +#[cfg(any(test, all(unix, not(target_arch = "wasm32"))))] +pub(crate) fn job_message(stdout: &[u8]) -> String { + let s = String::from_utf8_lossy(stdout); + let s = s.trim(); + s.strip_suffix("(0 file(s))").map_or(s, str::trim_end).to_string() +} diff --git a/crates/print/src/tests.rs b/crates/print/src/tests.rs index 30ca598bd..101e3d4c2 100644 --- a/crates/print/src/tests.rs +++ b/crates/print/src/tests.rs @@ -193,10 +193,10 @@ fn spooler_arguments_and_printer_list() { let job = Job { printer: Some("Office_Laser".into()), copies: 3, collate: false, duplex: Duplex::LongEdge, grayscale: true, title: "memo.pdf".into() }; assert_eq!( - lp_args(&job, "/tmp/x.pdf").join(" "), - "-d Office_Laser -n 3 -t memo.pdf -o collate=false -o sides=two-sided-long-edge -o print-color-mode=monochrome -o fit-to-page=false -- /tmp/x.pdf" + lp_args(&job).join(" "), + "-d Office_Laser -n 3 -t memo.pdf -o collate=false -o sides=two-sided-long-edge -o print-color-mode=monochrome -o fit-to-page=false" ); - assert_eq!(lp_args(&Job::default(), "f.pdf")[0], "-n", "no -d: the default printer"); + assert_eq!(lp_args(&Job::default())[0], "-n", "no -d: the default printer"); } #[test] @@ -275,3 +275,87 @@ fn multiple_rejects_hostile_grids_without_panicking() { assert!(matches!(layout(&[size], &settings(vec![0], cut_stack(2, 2))), Err(PrintError::Invalid(_)))); } } + +/// Set on the child process when a test runs this test binary as a stand-in for `lp`. +const STAND_IN_LP: &str = "PDFCRAFT_STAND_IN_LP"; + +/// Not a test of its own: [`stand_in_lp`] re-runs this binary with only this test selected, and it +/// then plays `lp`. It records its arguments and stdin in the folder named by [`STAND_IN_LP`], or, +/// when the folder is `refuse`, exits without reading stdin, the way `lp` refuses an unknown +/// printer. +#[test] +fn stand_in_for_lp() { + let Some(record) = std::env::var_os(STAND_IN_LP) else { return }; + if record == "refuse" { + eprintln!("lp: The printer or class does not exist."); + std::process::exit(1); + } + let record = std::path::PathBuf::from(record); + let args: Vec = std::env::args().skip_while(|a| a != "--").skip(1).collect(); + let mut stdin = Vec::new(); + std::io::Read::read_to_end(&mut std::io::stdin(), &mut stdin).unwrap(); + std::fs::write(record.join("args"), args.join(" ")).unwrap(); + std::fs::write(record.join("stdin"), stdin).unwrap(); +} + +/// A spooler command that runs [`stand_in_for_lp`] with `record` (see there). +fn stand_in_lp(record: &std::ffi::OsStr) -> std::process::Command { + let mut c = std::process::Command::new(std::env::current_exe().unwrap()); + c.args(["--exact", "tests::stand_in_for_lp", "--nocapture", "--"]).env(STAND_IN_LP, record); + c +} + +#[test] +fn print_jobs_reach_lp_on_stdin_never_through_a_shared_temp_folder() { + // The job used to be written to `/pdfcraft-print-/job-