diff --git a/linux/ARCHITECTURE.md b/linux/ARCHITECTURE.md index b3c518a3be..f2f4724214 100644 --- a/linux/ARCHITECTURE.md +++ b/linux/ARCHITECTURE.md @@ -202,7 +202,7 @@ The cancel token is checked between stages, and a stage that finishes after a ca PostgreSQL, SQL Server and ClickHouse open every stream through `open()`. sqlx dials its own sockets, so the MySQL driver passes it the route instead: host and port, or the forward's socket path. Nothing relays bytes between the two. The socket options above apply only to streams `open()` makes, so a direct MySQL connection has no keepalive and finds a dead peer through the probe and its deadlines. -SSH runs the system OpenSSH client ([ADR 0013](docs/decisions/0013-openssh-client-transport.md)): one ControlMaster (`ssh -M -N -T`) per connection, with the user's `~/.ssh/config` applied, and one `ssh -O forward` Unix socket per server it reaches. The sockets live under `$XDG_RUNTIME_DIR/app//ssh/` in directories only the user can open, with mode 0600. Every prompt (host key, password, passphrase, keyboard-interactive) reaches the app through the `tablepro-askpass` helper, and the 20 s handshake timeout pauses while a prompt is open. Under Flatpak, `ssh` comes from the GNOME runtime, and a `ProxyCommand` or `Match exec` that runs a host program, the host's `/etc/ssh` and FIDO keys kept in files do not work. +SSH runs the system OpenSSH client ([ADR 0013](docs/decisions/0013-openssh-client-transport.md)): one ControlMaster (`ssh -M -N -T`) per connection, with the user's `~/.ssh/config` applied, and one `ssh -O forward` Unix socket per server it reaches. The sockets live under `$XDG_RUNTIME_DIR/app//ssh/` in directories only the user can open, with mode 0600. Only Flatpak creates `app/`, so a host install makes the missing levels with mode 0700, each opened without following a symlink and checked to be the user's; with `XDG_RUNTIME_DIR` unset, GLib puts the same layout under the cache directory. Every prompt (host key, password, passphrase, keyboard-interactive) reaches the app through the `tablepro-askpass` helper, and the 20 s handshake timeout pauses while a prompt is open. Under Flatpak, `ssh` comes from the GNOME runtime, and a `ProxyCommand` or `Match exec` that runs a host program, the host's `/etc/ssh` and FIDO keys kept in files do not work. ### Trust diff --git a/linux/Cargo.lock b/linux/Cargo.lock index 5a1af6ffb2..46d73fb052 100644 --- a/linux/Cargo.lock +++ b/linux/Cargo.lock @@ -4610,6 +4610,7 @@ version = "0.0.0" dependencies = [ "async-trait", "futures", + "rustix", "secrecy", "tablepro-core", "tablepro-test-support", diff --git a/linux/crates/app/Cargo.toml b/linux/crates/app/Cargo.toml index 7f67634c2d..01fe197258 100644 --- a/linux/crates/app/Cargo.toml +++ b/linux/crates/app/Cargo.toml @@ -28,6 +28,13 @@ name = "logging" path = "tests/logging.rs" harness = false +# A panic hook is process-wide and the crash ends the process, so each +# case runs in a child of the binary. +[[test]] +name = "crash_report" +path = "tests/crash_report.rs" +harness = false + # These start the application, which GTK allows only on the process's # main thread, so they run without the test harness. [[test]] diff --git a/linux/crates/app/src/application/crash_notice.rs b/linux/crates/app/src/application/crash_notice.rs new file mode 100644 index 0000000000..29ff4b724c --- /dev/null +++ b/linux/crates/app/src/application/crash_notice.rs @@ -0,0 +1,62 @@ +use std::path::PathBuf; + +use gtk4::glib; +use gtk4::prelude::*; +use gtk4::subclass::prelude::*; + +use super::TableProApplication; +use crate::ui::SessionWindow; + +impl TableProApplication { + /// Looks off the GTK thread for a report the last run left when it + /// crashed, and has a window say so. + pub(super) fn look_for_crash_report(&self) { + let Some(services) = self.services() else { + return; + }; + let dir = services.storage.paths().crashes_dir(); + let looking = services + .tasks + .spawn_blocking_task(move || crate::crash_report::take_unannounced(&dir)); + let app = self.downgrade(); + glib::spawn_future_local(async move { + let report = match looking.await { + Ok(Ok(Some(report))) => report, + Ok(Ok(None)) => return, + Ok(Err(error)) => { + tracing::warn!(%error, "could not look for a crash report"); + return; + } + Err(failure) => { + tracing::warn!(%failure, "the crash report lookup failed"); + return; + } + }; + if let Some(app) = app.upgrade() { + app.announce_crash(report); + } + }); + } + + /// The window in front says it, or the first one to open when the + /// windows of a restored session are still on their way. + fn announce_crash(&self, report: PathBuf) { + let window = self.active_window().and_downcast::().or_else(|| { + self.windows() + .into_iter() + .find_map(|window| window.downcast::().ok()) + }); + match window { + Some(window) => window.announce_crash(report), + None => { + self.imp().crash_report.replace(Some(report)); + } + } + } + + pub(super) fn hand_over_crash_report(&self, window: &SessionWindow) { + if let Some(report) = self.imp().crash_report.take() { + window.announce_crash(report); + } + } +} diff --git a/linux/crates/app/src/application/imp.rs b/linux/crates/app/src/application/imp.rs index 03efb25f1b..bdcb53208b 100644 --- a/linux/crates/app/src/application/imp.rs +++ b/linux/crates/app/src/application/imp.rs @@ -33,6 +33,8 @@ pub struct TableProApplication { /// Files handed over while the record is read, opened after the /// windows it restores. pub(super) pending_files: RefCell>, + /// A crash report found before any window opened. + pub(super) crash_report: RefCell>, } #[derive(Debug, Default, Clone, Copy, PartialEq, Eq)] @@ -82,6 +84,7 @@ impl ApplicationImpl for TableProApplication { install_application_accelerators(&*app); if let Some(services) = self.services.get() { services.start(); + app.look_for_crash_report(); self.session_signals .replace(Some(SessionSignals::start(&services.sessions))); } diff --git a/linux/crates/app/src/application/mod.rs b/linux/crates/app/src/application/mod.rs index 79cd75c8d5..70e9f21ddc 100644 --- a/linux/crates/app/src/application/mod.rs +++ b/linux/crates/app/src/application/mod.rs @@ -8,6 +8,7 @@ mod accelerators; mod app_actions; mod app_services; mod app_window_host; +mod crash_notice; mod debug_info; mod imp; mod legal_section; @@ -101,6 +102,7 @@ impl TableProApplication { let host: Rc = Rc::new(AppWindowHost::new(self)); self.imp().windows.insert(window.id(), window.clone()); window.start(SessionWindowInit { services, host, launch }); + self.hand_over_crash_report(&window); window.present(); Some(window.upcast()) } diff --git a/linux/crates/app/src/crash_report.rs b/linux/crates/app/src/crash_report.rs new file mode 100644 index 0000000000..733a930480 --- /dev/null +++ b/linux/crates/app/src/crash_report.rs @@ -0,0 +1,186 @@ +//! What a crash leaves behind for the user to send, and how the next +//! launch finds it. + +use std::backtrace::Backtrace; +use std::panic::PanicHookInfo; +use std::path::{Path, PathBuf}; +use std::sync::OnceLock; + +use tablepro_storage::StorageError; + +use crate::config; + +/// GTK calls the app back through C, which a panic cannot unwind, so a +/// panic on the GTK thread ends the process. A panic on a runtime thread +/// is caught there and shown as a failed task, so only these are +/// crashes. +const GTK_THREAD: &str = "main"; + +const REPORT_PREFIX: &str = "crash-"; +const REPORT_SUFFIX: &str = ".txt"; +/// Holds the name of the newest report the user was told about. +const ANNOUNCED: &str = "announced"; +/// A launch that crashes every time leaves one report each, so the +/// oldest go once there are more than this. +const KEPT_REPORTS: usize = 20; + +/// Keep the default hook, which writes the panic to stderr and so to +/// the journal, and also write it with a backtrace to a file of its own +/// in `dir`. A GTK callback that panics panics a second time when it +/// cannot unwind, and that goes in the same file. +pub fn install(dir: PathBuf) { + let report = OnceLock::new(); + let default_hook = std::panic::take_hook(); + std::panic::set_hook(Box::new(move |info| { + default_hook(info); + if std::thread::current().name() != Some(GTK_THREAD) { + return; + } + let path = report.get_or_init(|| dir.join(report_name(chrono::Utc::now(), std::process::id()))); + let text = report_text(info, &Backtrace::force_capture(), chrono::Utc::now()); + if let Err(error) = tablepro_storage::fs::append_private_blocking(path, text.as_bytes()) { + tracing::error!(%error, "could not save the crash report"); + } + })); +} + +/// Names sort by time, so the newest report is the last name. +fn report_name(now: chrono::DateTime, pid: u32) -> String { + format!("{REPORT_PREFIX}{}-{pid}{REPORT_SUFFIX}", now.format("%Y%m%dT%H%M%SZ")) +} + +fn report_text(info: &PanicHookInfo<'_>, backtrace: &Backtrace, now: chrono::DateTime) -> String { + let message = info.payload_as_str().unwrap_or("(no message)"); + let location = info + .location() + .map_or_else(|| "an unknown place".to_owned(), ToString::to_string); + let build = if cfg!(debug_assertions) { "debug" } else { "release" }; + format!( + "TablePro {version}\n\ + Application ID: {app_id}\n\ + Profile: {profile}\n\ + Build: {build}\n\ + Time: {time}\n\ + \n\ + Panicked at {location}:\n\ + {message}\n\ + \n\ + Backtrace:\n\ + {backtrace}\n\n", + version = config::VERSION, + app_id = config::APP_ID, + profile = config::PROFILE, + time = now.to_rfc3339_opts(chrono::SecondsFormat::Secs, true), + ) +} + +/// The newest report the user has not been told about, marked as told. +/// Runs at startup, before this run could have written one, and drops +/// the oldest reports past `KEPT_REPORTS`. +pub fn take_unannounced(dir: &Path) -> Result, StorageError> { + let mut reports = match std::fs::read_dir(dir) { + Ok(entries) => entries + .filter_map(Result::ok) + .filter_map(|entry| entry.file_name().into_string().ok()) + .filter(|name| name.starts_with(REPORT_PREFIX) && name.ends_with(REPORT_SUFFIX)) + .collect::>(), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(None), + Err(source) => { + return Err(StorageError::Io { + path: dir.to_owned(), + source, + }); + } + }; + reports.sort_unstable(); + let surplus = reports.len().saturating_sub(KEPT_REPORTS); + for name in reports.drain(..surplus) { + if let Err(error) = std::fs::remove_file(dir.join(&name)) { + tracing::warn!(%error, "could not remove an old crash report"); + } + } + let Some(newest) = reports.pop() else { + return Ok(None); + }; + let marker = dir.join(ANNOUNCED); + let announced = std::fs::read_to_string(&marker).unwrap_or_default(); + if announced.trim() >= newest.as_str() { + return Ok(None); + } + tablepro_storage::fs::write_private_blocking(&marker, newest.as_bytes())?; + Ok(Some(dir.join(newest))) +} + +#[cfg(test)] +mod tests { + use chrono::TimeZone; + + use super::*; + + fn report(dir: &Path, name: &str) { + tablepro_storage::fs::append_private_blocking(&dir.join(name), b"a report").expect("a report"); + } + + #[test] + fn report_names_sort_by_time() { + let earlier = chrono::Utc + .with_ymd_and_hms(2026, 9, 30, 23, 59, 59) + .single() + .expect("a time"); + let later = chrono::Utc + .with_ymd_and_hms(2026, 10, 1, 0, 0, 0) + .single() + .expect("a time"); + + assert_eq!(report_name(later, 42), "crash-20261001T000000Z-42.txt"); + assert!(report_name(earlier, 99_999) < report_name(later, 1)); + } + + #[test] + fn the_newest_report_is_announced_once() { + let dir = tempfile::tempdir().expect("a directory"); + report(dir.path(), "crash-20261001T000000Z-1.txt"); + report(dir.path(), "crash-20261002T000000Z-2.txt"); + + let first = take_unannounced(dir.path()).expect("a listing"); + + assert_eq!(first, Some(dir.path().join("crash-20261002T000000Z-2.txt"))); + assert_eq!(take_unannounced(dir.path()).expect("a listing"), None); + + report(dir.path(), "crash-20261003T000000Z-3.txt"); + assert_eq!( + take_unannounced(dir.path()).expect("a listing"), + Some(dir.path().join("crash-20261003T000000Z-3.txt")) + ); + } + + #[test] + fn no_directory_is_no_report() { + let dir = tempfile::tempdir().expect("a directory"); + + assert_eq!(take_unannounced(&dir.path().join("crashes")).expect("a listing"), None); + } + + #[test] + fn only_the_newest_reports_are_kept() { + let dir = tempfile::tempdir().expect("a directory"); + for day in 1..=KEPT_REPORTS + 3 { + report(dir.path(), &format!("crash-202610{day:02}T000000Z-1.txt")); + } + + take_unannounced(dir.path()).expect("a listing"); + + let left = std::fs::read_dir(dir.path()) + .expect("a listing") + .filter_map(Result::ok) + .filter(|entry| entry.file_name().to_string_lossy().starts_with(REPORT_PREFIX)) + .count(); + assert_eq!(left, KEPT_REPORTS); + assert!(!dir.path().join("crash-20261001T000000Z-1.txt").exists()); + assert!( + dir.path() + .join(format!("crash-202610{:02}T000000Z-1.txt", KEPT_REPORTS + 3)) + .exists() + ); + } +} diff --git a/linux/crates/app/src/lib.rs b/linux/crates/app/src/lib.rs index d229488b68..bd13811331 100644 --- a/linux/crates/app/src/lib.rs +++ b/linux/crates/app/src/lib.rs @@ -8,6 +8,7 @@ use tablepro_storage::{AppSettings, StoragePaths}; pub mod application; pub mod config; +pub mod crash_report; pub mod i18n; pub mod logging; mod persistence; @@ -45,10 +46,12 @@ pub fn run() -> Result { tracing::warn!(%error, "translations unavailable; falling back to the source strings"); } + let paths = StoragePaths::from_glib(config::storage_dir_name(), config::APP_ID); + crash_report::install(paths.crashes_dir()); + let runtime = runtime::AppRuntime::build().map_err(StartupError::Runtime)?; gio::resources_register_include!("tablepro.gresource").map_err(StartupError::Resources)?; let settings = Rc::new(AppSettings::open(config::APP_ID)?); - let paths = StoragePaths::from_glib(config::storage_dir_name(), config::APP_ID); let drivers = build_registry(); tracing::info!(drivers = drivers.len(), "starting tablepro"); diff --git a/linux/crates/app/src/ui/editor/mod.rs b/linux/crates/app/src/ui/editor/mod.rs index f427c7721a..4eca35f97c 100644 --- a/linux/crates/app/src/ui/editor/mod.rs +++ b/linux/crates/app/src/ui/editor/mod.rs @@ -46,7 +46,7 @@ use self::imp::RunningScript; use self::result_widgets::PageActions; use self::run_log::{EntryKind, RunEvent, RunLog}; use self::run_scope::ScopedText; -use self::script_runner::{ScriptRun, plan_batches, run_in_tab}; +use self::script_runner::{RUN_ROW_LIMIT, ScriptRun, plan_batches, run_in_tab}; use super::clipboard::{CopyRequest, InClauseRows}; use super::export::{ExportOffer, ExportOrigin, ExportQuery, ExportSource}; use super::grid::{GridMsg, GridSink, present_row_json}; @@ -383,6 +383,7 @@ impl SqlEditor { policy, statement_timeout: deps.settings.query_timeout(), row_limit: RowLimit::EDITOR_DEFAULT, + run_row_limit: RUN_ROW_LIMIT, }; let metadata = session.metadata().clone(); // Read again at the end from the run's own session, never the diff --git a/linux/crates/app/src/ui/editor/run_log.rs b/linux/crates/app/src/ui/editor/run_log.rs index 861aeeb35d..1cceb46635 100644 --- a/linux/crates/app/src/ui/editor/run_log.rs +++ b/linux/crates/app/src/ui/editor/run_log.rs @@ -94,6 +94,25 @@ pub enum RunEvent { Finished, } +/// The rows a batch may still keep under its run's limit. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct RowRoom { + /// The run's limit, which a result it cuts short names. + pub run_limit: u64, + pub left: u64, + /// The statement was sent with `left` as its row limit, below its + /// own, so a result the driver cut short was cut by the run's limit. + pub limits_statement: bool, +} + +impl RowRoom { + pub const UNLIMITED: Self = Self { + run_limit: u64::MAX, + left: u64::MAX, + limits_statement: false, + }; +} + /// Turns the events of one run into results list lines, on the runtime, /// so only finished lines cross to the GTK thread. #[derive(Debug)] @@ -103,6 +122,7 @@ pub struct BatchCollector { entries: Vec, /// Where each result set of this run sits in `entries`. sets: Vec, + room: RowRoom, } impl BatchCollector { @@ -111,9 +131,15 @@ impl BatchCollector { statement, entries: Vec::new(), sets: Vec::new(), + room: RowRoom::UNLIMITED, } } + /// Rows past `room` are dropped, and the result says why. + pub fn within(self, room: RowRoom) -> Self { + Self { room, ..self } + } + pub fn record(&mut self, event: ResultEvent) { match event { ResultEvent::ResultSetStarted { columns } => { @@ -128,36 +154,46 @@ impl BatchCollector { result.columns = columns; } } - ResultEvent::Rows(rows) => match self.last_set() { - Some(result) => { - result.rows.extend(rows.into_iter().map(Into::into)); + ResultEvent::Rows(rows) => { + if self.sets.is_empty() { + // Rows with no columns would draw a grid that cannot + // say what any cell is. + tracing::warn!(rows = rows.len(), "rows arrived before their result set"); + return; + } + let kept = usize::try_from(self.room.left).unwrap_or(usize::MAX).min(rows.len()); + self.room.left -= kept as u64; + let cut_by = (kept < rows.len()).then_some(self.room.run_limit); + if let Some(result) = self.last_set() { + result.rows.extend(rows.into_iter().take(kept).map(Into::into)); result.summary.retained = result.rows.len() as u64; + result.summary.run_limit = result.summary.run_limit.or(cut_by); } - // Rows with no columns would draw a grid that cannot say - // what any cell is. - None => tracing::warn!(rows = rows.len(), "rows arrived before their result set"), - }, + } // Until the set ends and says how far the read went, all // that is known is that more rows were left behind. - ResultEvent::RowLimitReached { retained } => { + ResultEvent::RowLimitReached { .. } => { + let cut_by = self.room.limits_statement.then_some(self.room.run_limit); if let Some(result) = self.last_set() { result.summary = ResultSummary { - retained, + retained: result.rows.len() as u64, total_rows: None, truncation: Truncation::ServerStopped, + run_limit: result.summary.run_limit.or(cut_by), }; } } ResultEvent::ResultSetFinished { - retained, - total_rows, - truncation, + total_rows, truncation, .. } => { + let cut_by = (self.room.limits_statement && truncation == Truncation::ReadPastLimit) + .then_some(self.room.run_limit); if let Some(result) = self.last_set() { result.summary = ResultSummary { - retained, + retained: result.rows.len() as u64, total_rows, truncation, + run_limit: result.summary.run_limit.or(cut_by), }; } } @@ -514,6 +550,7 @@ mod tests { retained: 2, total_rows: Some(5), truncation: Truncation::ServerStopped, + run_limit: None, } ); assert_eq!(result.statement.as_str(), "SELECT a FROM t"); @@ -521,6 +558,37 @@ mod tests { assert!(matches!(entries[2], EntryKind::Command { rows_affected: Some(4) })); } + #[test] + fn a_set_the_driver_cut_at_the_runs_room_names_the_run_limit() { + let room = RowRoom { + run_limit: 100, + left: 1, + limits_statement: true, + }; + let mut collector = BatchCollector::new(UserSql::new("SELECT a FROM t")).within(room); + for event in [ + ResultEvent::ResultSetStarted { + columns: columns(&["a"]), + }, + ResultEvent::Rows(vec![vec![Value::Int(1)]]), + ResultEvent::RowLimitReached { retained: 1 }, + ResultEvent::ResultSetFinished { + retained: 1, + total_rows: None, + truncation: Truncation::ReadPastLimit, + }, + ] { + collector.record(event); + } + let (entries, _, _) = collector.finish(summary(RunEnd::Completed), EditorSessionState::Ready, 0); + + let EntryKind::ResultSet(result) = &entries[0] else { + panic!("expected rows: {entries:?}"); + }; + assert_eq!(result.row_count(), 1); + assert_eq!(result.summary.run_limit, Some(100)); + } + #[test] fn a_set_cut_by_the_limit_and_then_stopped_stays_truncated() { let (entries, _) = collect( @@ -552,11 +620,13 @@ mod tests { retained: 100, total_rows: None, truncation: Truncation::ServerStopped, + run_limit: None, }; let counted = ResultSummary { retained: 100, total_rows: Some(250), truncation: Truncation::ReadPastLimit, + run_limit: None, }; let history = |entries: Vec| { let mut log = RunLog::new(vec![UserSql::new("SELECT a FROM t")], Vec::new()); diff --git a/linux/crates/app/src/ui/editor/script_runner.rs b/linux/crates/app/src/ui/editor/script_runner.rs index dc486e4e02..4a57b1b79b 100644 --- a/linux/crates/app/src/ui/editor/script_runner.rs +++ b/linux/crates/app/src/ui/editor/script_runner.rs @@ -4,9 +4,8 @@ use std::sync::{Arc, Mutex, MutexGuard, PoisonError}; use std::time::{Duration, Instant}; use tablepro_core::sql_syntax::SqlGrammar; -use tablepro_core::sql_syntax::script::{ - BatchErrorPolicy, LexicalSettings, ScriptDiagnostic, ScriptPlan, text_offsets, -}; +use tablepro_core::sql_syntax::script::text_offsets::CharCounter; +use tablepro_core::sql_syntax::script::{BatchErrorPolicy, LexicalSettings, ScriptDiagnostic, ScriptPlan}; use tablepro_core::{ EditorSession, EditorSessionState, ResultEvent, ResultSink, RowLimit, RunOptions, SinkFlow, UserSql, }; @@ -15,7 +14,7 @@ use tokio::sync::mpsc; use tokio_util::sync::CancellationToken; use super::editor_session_slot::{AcquireError, EditorSessionSlot, close_quietly}; -use super::run_log::{BatchCollector, BatchReport, EntryKind, RunEvent, Verdict, continues}; +use super::run_log::{BatchCollector, BatchReport, EntryKind, RowRoom, RunEvent, Verdict, continues}; use super::run_scope::ScopedText; /// One unit the server is sent: a statement on most engines, a whole @@ -29,6 +28,11 @@ pub struct RunBatch { pub error_base: usize, } +/// Rows one run keeps across all its results. `row_limit` bounds each +/// result, and a script of many SELECTs would otherwise hold all of them +/// until the next run. +pub const RUN_ROW_LIMIT: u64 = 100_000; + /// What one press of Run sends, and how. #[derive(Debug, Clone)] pub struct ScriptRun { @@ -36,6 +40,21 @@ pub struct ScriptRun { pub policy: BatchErrorPolicy, pub statement_timeout: Option, pub row_limit: RowLimit, + pub run_row_limit: u64, +} + +/// What a statement may keep once the run has kept `kept` rows, and the +/// row limit it goes out with. With no room left it still runs, for +/// what it changes, and reads one row, which is dropped. +fn room_after(run: &ScriptRun, kept: u64) -> (RowRoom, RowLimit) { + let left = run.run_row_limit.saturating_sub(kept); + let own = run.row_limit.get(); + let room = RowRoom { + run_limit: run.run_row_limit, + left, + limits_statement: left < own, + }; + (room, RowLimit::new(left.clamp(1, own)).unwrap_or(run.row_limit)) } /// The batches of the scoped text and what the plan warns about. @@ -50,6 +69,7 @@ pub fn plan_batches( base_line: usize, ) -> (Vec, BatchErrorPolicy, Vec) { let plan = ScriptPlan::build(&scoped.text, grammar, LexicalSettings::default_for(grammar)); + let mut chars = CharCounter::new(&scoped.text); let batches = plan .batches() .iter() @@ -58,7 +78,7 @@ pub fn plan_batches( (!text.is_empty()).then(|| RunBatch { text: text.to_owned(), repeat: batch.repeat.get(), - error_base: scoped.base_char + text_offsets::byte_to_char(&scoped.text, batch.range.start), + error_base: scoped.base_char + chars.char_at(batch.range.start), }) }) .collect(); @@ -94,16 +114,18 @@ pub async fn run_batches( cancel: &CancellationToken, events: &mpsc::Sender, ) { + let mut kept: u64 = 0; 'batches: for (index, batch) in run.batches.iter().enumerate() { for _ in 0..batch.repeat.max(1) { if cancel.is_cancelled() { break 'batches; } let sql = UserSql::new(batch.text.as_str()); - let collector = Arc::new(Mutex::new(BatchCollector::new(sql.clone()))); + let (room, row_limit) = room_after(run, kept); + let collector = Arc::new(Mutex::new(BatchCollector::new(sql.clone()).within(room))); let options = RunOptions::new(cancel.child_token()) .with_deadline(run.statement_timeout) - .with_row_limit(run.row_limit); + .with_row_limit(row_limit); let sink = Box::new(CollectingSink { collector: collector.clone(), }); @@ -113,6 +135,13 @@ pub async fn run_batches( let collected = std::mem::replace(&mut *lock(&collector), BatchCollector::new(sql)); let state = editor.state(); let (entries, verdict, transaction) = collected.finish(outcome, state, batch.error_base); + kept += entries + .iter() + .map(|entry| match entry { + EntryKind::ResultSet(result) => result.row_count() as u64, + _ => 0, + }) + .sum::(); let report = BatchReport { batch: index, entries, @@ -226,6 +255,7 @@ mod tests { policy, statement_timeout: None, row_limit: RowLimit::EDITOR_DEFAULT, + run_row_limit: RUN_ROW_LIMIT, }, statements, warnings, @@ -250,6 +280,16 @@ mod tests { cancel: &CancellationToken, ) -> RunLog { let (run, statements, warnings) = script(text, grammar); + play(editor, &run, statements, warnings, cancel).await + } + + async fn play( + editor: &Arc, + run: &ScriptRun, + statements: Vec, + warnings: Vec, + cancel: &CancellationToken, + ) -> RunLog { let (events, mut received) = mpsc::channel(8); let session: Arc = editor.clone(); let collect = async move { @@ -263,7 +303,7 @@ mod tests { } log }; - let (_, log) = tokio::join!(run_batches(&session, &run, cancel, &events), collect); + let (_, log) = tokio::join!(run_batches(&session, run, cancel, &events), collect); log } @@ -444,6 +484,60 @@ mod tests { assert!(log.lost()); } + fn sixty_rows() -> FakeRun { + let columns: Arc<[tablepro_core::column::ResultColumn]> = vec![tablepro_core::column::ResultColumn::new( + "n", + tablepro_core::column::ColumnType::untyped_text("int4"), + )] + .into(); + FakeRun::completed(vec![ + ResultEvent::ResultSetStarted { columns }, + ResultEvent::Rows((0..60).map(|n| vec![tablepro_core::Value::Int(n)]).collect()), + ResultEvent::ResultSetFinished { + retained: 60, + total_rows: None, + truncation: tablepro_core::Truncation::Complete, + }, + ]) + } + + #[tokio::test] + async fn a_run_keeps_rows_up_to_its_limit_across_all_results() { + let mut runs = Replies::default(); + runs.always(sixty_rows()); + let editor = Arc::new(FakeEditorSession::new(runs)); + let (mut run, statements, warnings) = script("SELECT 1; SELECT 2; SELECT 3", SqlGrammar::PostgreSql); + run.run_row_limit = 100; + + let log = play(&editor, &run, statements, warnings, &CancellationToken::new()).await; + + assert_eq!(texts(&editor).len(), 3, "a statement past the limit was not sent"); + let kept: Vec<(usize, Option)> = log + .entries() + .iter() + .filter_map(|entry| match &entry.kind { + EntryKind::ResultSet(result) => Some((result.row_count(), result.summary.run_limit)), + _ => None, + }) + .collect(); + assert_eq!(kept, [(60, None), (40, Some(100)), (0, Some(100))]); + } + + #[test] + fn a_statement_reads_only_the_rows_its_run_has_room_for() { + let (mut run, _, _) = script("SELECT 1", SqlGrammar::PostgreSql); + run.run_row_limit = 100_000; + let limit = |kept| { + let (room, limit) = room_after(&run, kept); + (room.left, room.limits_statement, limit.get()) + }; + + assert_eq!(limit(0), (100_000, false, 10_000)); + assert_eq!(limit(95_000), (5_000, true, 5_000)); + assert_eq!(limit(100_000), (0, true, 1)); + assert_eq!(limit(250_000), (0, true, 1)); + } + #[test] fn batches_know_where_they_start_in_the_buffer() { let scoped = ScopedText { @@ -457,6 +551,26 @@ mod tests { assert_eq!(batches[1].error_base, 50 + 14); } + #[test] + fn a_long_script_finds_every_start_in_one_pass() { + // Counting each start from the top of the text is about 10^11 + // steps here, which no run finishes; one pass is about 10^6. + const STATEMENTS: usize = 100_000; + let statement = "SELECT 'é';\n"; + let scoped = ScopedText { + text: statement.repeat(STATEMENTS), + base_char: 7, + }; + + let (batches, _, _) = plan_batches(&scoped, SqlGrammar::PostgreSql, 0); + + assert_eq!(batches.len(), STATEMENTS); + let width = statement.chars().count(); + for (index, batch) in batches.iter().enumerate() { + assert_eq!(batch.error_base, 7 + index * width, "batch {index}"); + } + } + #[test] fn a_timed_out_run_is_a_stop() { assert!(!continues(Verdict::Stopped, BatchErrorPolicy::ContinueNextBatch)); diff --git a/linux/crates/app/src/ui/grid/cell_edit.rs b/linux/crates/app/src/ui/grid/cell_edit.rs index d2d8e5a706..6fc3b0ddeb 100644 --- a/linux/crates/app/src/ui/grid/cell_edit.rs +++ b/linux/crates/app/src/ui/grid/cell_edit.rs @@ -3,7 +3,7 @@ use gtk4::prelude::*; use tablepro_core::edit::{CellInput, EditParseError, EditText, edit_text}; use super::calendar_editor::CalendarEditor; -use super::cell_editor_kind::CellEditorKind; +use super::cell_editor_kind::{CellEditorKind, EDIT_MAX_BYTES}; use super::edit_commit::empty_placeholder; use super::inline_editor::{InlineEditor, InlineOptions}; use super::multiline_editor::MultilineEditor; @@ -44,6 +44,12 @@ pub(crate) fn begin_edit(cell: &GridCell) { } return; } + EditText::Editable(editable) if editable.len() > EDIT_MAX_BYTES => { + if let Some(sink) = cell.sink() { + sink(GridMsg::ShowToast(too_large_to_edit())); + } + return; + } EditText::Editable(editable) => editable.clone(), EditText::ReadOnly(reason) => { if let Some(sink) = cell.sink() { @@ -81,6 +87,11 @@ pub(crate) fn begin_edit(cell: &GridCell) { } } +fn too_large_to_edit() -> String { + let size = gtk::glib::format_size(EDIT_MAX_BYTES as u64); + crate::i18n::gettext_f("Values over {size} can't be edited here", &[("size", size.as_str())]) +} + pub(super) fn install_edit_triggers(cell: &GridCell) { // Editing takes a double-click, since a single click in a grid // selects the row. Capture phase, because the column view's row @@ -104,3 +115,35 @@ pub(super) fn install_edit_triggers(cell: &GridCell) { }); cell.add_controller(gesture); } + +#[cfg(test)] +mod tests { + use tablepro_core::Value; + + use super::*; + use crate::ui::grid::test_grid::{Grid, named}; + + #[gtk4::test] + fn a_value_too_large_to_lay_out_does_not_open_and_says_why() { + let large = "x".repeat(EDIT_MAX_BYTES + 1); + let grid = Grid::open( + vec![named("id", "integer", true), named("body", "text", false)], + vec![vec![Value::Int(1), Value::Text(large)]], + ); + let cell = grid.cell_of(1, 1); + + begin_edit(&cell); + + assert!(!cell.is_editing(), "an editor opened on the whole value"); + let toasts: Vec = grid + .take_messages() + .into_iter() + .filter_map(|message| match message { + GridMsg::ShowToast(text) => Some(text), + _ => None, + }) + .collect(); + // GLib puts a no-break space between the number and the unit. + assert_eq!(toasts, ["Values over 1.0\u{a0}MB can't be edited here"]); + } +} diff --git a/linux/crates/app/src/ui/grid/cell_editor_kind.rs b/linux/crates/app/src/ui/grid/cell_editor_kind.rs index ff88848b00..4a64716827 100644 --- a/linux/crates/app/src/ui/grid/cell_editor_kind.rs +++ b/linux/crates/app/src/ui/grid/cell_editor_kind.rs @@ -10,6 +10,11 @@ use super::CellLock; /// one-line entry lays all of it out at once and shows almost none. pub const INLINE_EDIT_MAX_BYTES: usize = 4096; +/// Past this many bytes a value does not open for editing at all: the +/// multi-line editor lays out every line of it on the GTK thread, and a +/// value of megabytes stalls the window. +pub const EDIT_MAX_BYTES: usize = 1_000_000; + /// The editor a cell opens with. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum CellEditorKind { diff --git a/linux/crates/app/src/ui/grid/row_json_dialog.rs b/linux/crates/app/src/ui/grid/row_json_dialog.rs index 901f27bece..ca85a19a49 100644 --- a/linux/crates/app/src/ui/grid/row_json_dialog.rs +++ b/linux/crates/app/src/ui/grid/row_json_dialog.rs @@ -16,6 +16,10 @@ use crate::ui::format::{format_thousands, plural_count}; /// Longer text is cut in the dialog: the view lays out every character /// it holds, and a value of megabytes would stall it. const SHOWN_CHARS: usize = 10_000; +/// Binary shows as hex, two characters a byte. +const SHOWN_BYTES: usize = SHOWN_CHARS / 2; +/// Each item of an array is a line of its own. +const SHOWN_ITEMS: usize = 1_000; /// One row to show as JSON, under the grid's visible columns. #[derive(Debug)] @@ -35,19 +39,7 @@ fn render(request: RowJsonRequest) -> RowJson { let values = request.row.to_values(); let full = tablepro_core::export::row_to_json(&request.columns, &values); let mut shortened = false; - let shown_values: Vec = values - .into_iter() - .map(|value| match value { - Value::Text(text) => match shorten(&text) { - Some(cut) => { - shortened = true; - Value::Text(cut) - } - None => Value::Text(text), - }, - other => other, - }) - .collect(); + let shown_values: Vec = values.into_iter().map(|value| shown(value, &mut shortened)).collect(); let shown = match shortened { true => tablepro_core::export::row_to_json(&request.columns, &shown_values), false => full.clone(), @@ -55,23 +47,71 @@ fn render(request: RowJsonRequest) -> RowJson { RowJson { full, shown, shortened } } +/// `value` as the dialog shows it, with anything long cut short and +/// saying how much is left out. A document cut short is no longer JSON, +/// so it shows as text. +fn shown(value: Value, shortened: &mut bool) -> Value { + let cut = match &value { + Value::Text(text) => shorten(text), + Value::Json(json) => shorten(json.as_str()), + Value::Other(other) => shorten(&other.text), + Value::Bits(bits) => shorten(&bits.to_string()), + Value::Decimal(decimal) => shorten(&decimal.to_string()), + Value::Bytes(bytes) => shorten_bytes(bytes), + _ => None, + }; + if let Some(cut) = cut { + *shortened = true; + return Value::Text(cut); + } + let Value::Array(items) = value else { + return value; + }; + let more = items.len().saturating_sub(SHOWN_ITEMS) as u64; + let mut kept: Vec = items + .into_iter() + .take(SHOWN_ITEMS) + .map(|item| shown(item, shortened)) + .collect(); + if more > 0 { + *shortened = true; + kept.push(Value::Text(crate::i18n::ngettext_f( + "… ({count} more item)", + "… ({count} more items)", + plural_count(more), + &[("count", &format_thousands(more))], + ))); + } + Value::Array(kept) +} + /// `text` cut at a character boundary after `SHOWN_CHARS` characters, /// saying how much is left out, or `None` when it fits. fn shorten(text: &str) -> Option { let (end, _) = text.char_indices().nth(SHOWN_CHARS)?; - let more = (text.len() - end) as u64; + Some(format!("{}{}", &text[..end], more_bytes((text.len() - end) as u64))) +} + +/// The hex the grid writes for binary, cut after `SHOWN_BYTES` bytes. +fn shorten_bytes(bytes: &[u8]) -> Option { + let more = bytes.len().checked_sub(SHOWN_BYTES).filter(|more| *more > 0)?; + let head = bytes.get(..SHOWN_BYTES)?; Some(format!( - "{}{}", - &text[..end], - crate::i18n::ngettext_f( - "… ({count} more byte)", - "… ({count} more bytes)", - plural_count(more), - &[("count", &format_thousands(more))], - ) + "0x{}{}", + tablepro_core::hex::encode_lower(head), + more_bytes(more as u64) )) } +fn more_bytes(more: u64) -> String { + crate::i18n::ngettext_f( + "… ({count} more byte)", + "… ({count} more bytes)", + plural_count(more), + &[("count", &format_thousands(more))], + ) +} + /// Render the row off the GTK thread, then show it over `parent`. pub(crate) async fn present_row_json( parent: gtk::Widget, @@ -231,6 +271,56 @@ mod tests { runtime.shutdown(); } + #[test] + fn every_kind_of_long_value_is_cut_for_display_and_kept_for_copy() { + const LONG: usize = 50_000; + let column = |name: &str, kind: &str| ResultColumn::new(name, ColumnType::untyped_text(kind)); + let document = format!("[{}]", vec!["1"; LONG].join(",")); + let shape = "x".repeat(LONG); + let request = RowJsonRequest { + columns: vec![ + column("doc", "jsonb"), + column("blob", "bytea"), + column("shape", "geometry"), + column("tags", "text[]"), + ] + .into(), + row: RowSnapshot::new( + vec![ + Value::Json(tablepro_core::value::JsonText::parse(document.clone()).expect("JSON")), + Value::Bytes(vec![0xab; LONG]), + Value::Other(Box::new(tablepro_core::value::OtherValue { + type_name: "geometry".to_owned(), + text: shape.clone(), + })), + Value::Array(vec![Value::Text("t".to_owned()); LONG]), + ] + .into(), + Vec::new(), + ), + }; + + let rendered = render(request); + + assert!(rendered.shortened); + let longest = rendered.shown.lines().map(str::len).max().unwrap_or_default(); + assert!(longest < SHOWN_CHARS + 100, "a line of {longest} bytes is laid out"); + assert!(rendered.shown.len() < 100_000, "{} bytes shown", rendered.shown.len()); + assert!( + rendered.shown.contains("… (45,000 more bytes)"), + "the binary does not say what is left" + ); + assert!( + rendered.shown.contains("… (49,000 more items)"), + "the array does not say what is left" + ); + let full: serde_json::Value = serde_json::from_str(&rendered.full).expect("the copy is JSON"); + assert_eq!(full["doc"].as_array().map(Vec::len), Some(LONG)); + assert_eq!(full["blob"].as_str().map(str::len), Some(2 + 2 * LONG)); + assert_eq!(full["shape"], serde_json::json!(shape)); + assert_eq!(full["tags"].as_array().map(Vec::len), Some(LONG)); + } + #[gtk4::test] fn a_closed_dialog_lets_go_of_the_row() { adw::init().expect("libadwaita initialises under the test backend"); diff --git a/linux/crates/app/src/ui/result_pane/result_summary.rs b/linux/crates/app/src/ui/result_pane/result_summary.rs index baa8bc8474..af01ec7ab8 100644 --- a/linux/crates/app/src/ui/result_pane/result_summary.rs +++ b/linux/crates/app/src/ui/result_pane/result_summary.rs @@ -12,6 +12,9 @@ pub struct ResultSummary { /// What the server says the whole set held, when it says. pub total_rows: Option, pub truncation: Truncation, + /// The run's limit on the rows of all its results, when that limit + /// cut this one short. + pub run_limit: Option, } impl ResultSummary { @@ -21,11 +24,12 @@ impl ResultSummary { retained, total_rows: None, truncation: Truncation::Complete, + run_limit: None, } } pub(crate) fn is_truncated(&self) -> bool { - self.truncation != Truncation::Complete + self.truncation != Truncation::Complete || self.run_limit.is_some() } /// What the banner over a cut-short result says. `None` for a @@ -36,6 +40,21 @@ impl ResultSummary { fn limit_title_with(&self, format: impl Fn(u64) -> String) -> Option { let shown = format(self.retained); + if let Some(limit) = self.run_limit { + let limit = format(limit); + return Some(match self.retained { + 0 => gettext_f( + "No rows kept; a run keeps at most {limit} rows across its results", + &[("limit", &limit)], + ), + retained => ngettext_f( + "Showing the first {shown} row; a run keeps at most {limit} rows across its results", + "Showing the first {shown} rows; a run keeps at most {limit} rows across its results", + plural_count(retained), + &[("shown", &shown), ("limit", &limit)], + ), + }); + } match (self.truncation, self.total_rows) { (Truncation::Complete, _) => None, (Truncation::ReadPastLimit, Some(total)) => Some(ngettext_f( @@ -97,6 +116,7 @@ mod tests { retained, total_rows, truncation, + run_limit: None, } } @@ -125,6 +145,25 @@ mod tests { ); } + #[test] + fn limit_title_names_the_run_limit_when_it_cut_the_result() { + let cut = |retained| ResultSummary { + run_limit: Some(100_000), + ..summary(retained, None, Truncation::Complete) + }; + + assert_eq!( + cut(2_500).limit_title_with(english).as_deref(), + Some("Showing the first 2,500 rows; a run keeps at most 100,000 rows across its results") + ); + assert_eq!( + cut(0).limit_title_with(english).as_deref(), + Some("No rows kept; a run keeps at most 100,000 rows across its results") + ); + assert_eq!(cut(0).tab_label_with(4, english), "Result 5 (0+)"); + assert_eq!(cut(0).history_row_count(), None); + } + #[test] fn limit_title_none_when_complete() { assert_eq!( diff --git a/linux/crates/app/src/ui/session_window/connect_failed_page.rs b/linux/crates/app/src/ui/session_window/connect_failed_page.rs index a3653a9d66..58fd91e327 100644 --- a/linux/crates/app/src/ui/session_window/connect_failed_page.rs +++ b/linux/crates/app/src/ui/session_window/connect_failed_page.rs @@ -83,8 +83,11 @@ impl ConnectFailedPage { imp.edit_button.set_visible(view.can_edit); if let Some(kind) = reenter { imp.reenter_button.set_label(&super::secret_retry::reenter_label(kind)); + // The target goes first: an action that takes one, named on a + // button without it, warns that it cannot be activated. imp.reenter_button .set_action_target_value(Some(&kind.attribute().to_variant())); + imp.reenter_button.set_action_name(Some("win.retry-with-new-secret")); imp.reenter_button.set_visible(true); imp.retry_button.remove_css_class("suggested-action"); } @@ -159,3 +162,34 @@ impl ConnectFailedPage { self.imp().announced.borrow().clone() } } + +#[cfg(test)] +mod tests { + use super::*; + + fn page(reenter: Option) -> ConnectFailedPage { + adw::init().expect("libadwaita initialises under the test backend"); + crate::register_test_resources(); + let view = ConnectFailureView { + message: "refused".to_owned(), + can_edit: true, + }; + ConnectFailedPage::new(&view, "Edit Connection…", reenter, None) + } + + #[gtk4::test] + fn the_reenter_action_is_named_only_with_its_target() { + let hidden = page(None).imp().reenter_button.get(); + assert!(!hidden.is_visible()); + assert_eq!( + hidden.action_name(), + None, + "an action that takes a target was named on a button without one" + ); + + let kind = SecretKind::DatabasePassword; + let shown = page(Some(kind)).imp().reenter_button.get(); + assert_eq!(shown.action_name().as_deref(), Some("win.retry-with-new-secret")); + assert_eq!(shown.action_target_value(), Some(kind.attribute().to_variant())); + } +} diff --git a/linux/crates/app/src/ui/session_window/crash_notice.rs b/linux/crates/app/src/ui/session_window/crash_notice.rs new file mode 100644 index 0000000000..b35ec65431 --- /dev/null +++ b/linux/crates/app/src/ui/session_window/crash_notice.rs @@ -0,0 +1,159 @@ +use std::path::{Path, PathBuf}; + +use gtk4::prelude::*; +use gtk4::subclass::prelude::*; +use gtk4::{gio, glib}; +use libadwaita as adw; + +use super::SessionWindow; +use crate::i18n::gettext; +use crate::ui::file_reveal::reveal_failure_toast; + +/// The report the last run left when it crashed, said until the user or +/// the toast's timeout dismisses it. +pub(crate) struct CrashNotice { + report: PathBuf, + shown: Option<(adw::Toast, glib::SignalHandlerId)>, +} + +impl SessionWindow { + pub(crate) fn announce_crash(&self, report: PathBuf) { + self.withdraw_crash_toast(); + self.imp() + .crash_notice + .replace(Some(CrashNotice { report, shown: None })); + self.show_crash_notice(); + } + + /// A toast lives in the overlay of the page it was added to, so a + /// restored session, which goes from the picker through connecting + /// to its workspace, would take an unread one away. Each page shown + /// gets it again until it is dismissed. + pub(super) fn show_crash_notice(&self) { + self.withdraw_crash_toast(); + let Some(report) = self + .imp() + .crash_notice + .borrow() + .as_ref() + .map(|notice| notice.report.clone()) + else { + return; + }; + let toast = adw::Toast::builder() + .title(gettext("TablePro quit unexpectedly last time")) + .button_label(gettext("Show Report")) + .use_markup(false) + .build(); + let window = self.downgrade(); + toast.connect_button_clicked(move |_| { + if let Some(window) = window.upgrade() { + window.reveal_crash_report(&report); + } + }); + let window = self.downgrade(); + let dismissed = toast.connect_dismissed(move |_| { + if let Some(window) = window.upgrade() { + window.imp().crash_notice.take(); + } + }); + if let Some(notice) = self.imp().crash_notice.borrow_mut().as_mut() { + notice.shown = Some((toast.clone(), dismissed)); + } + self.add_toast(toast); + } + + /// Takes the toast off the page it is on without counting that as + /// the user dismissing it. An overlay that goes away dismisses its + /// toasts, so this runs before the old page can. + fn withdraw_crash_toast(&self) { + let shown = self + .imp() + .crash_notice + .borrow_mut() + .as_mut() + .and_then(|notice| notice.shown.take()); + if let Some((toast, dismissed)) = shown { + toast.disconnect(dismissed); + toast.dismiss(); + } + } + + fn reveal_crash_report(&self, report: &Path) { + let Some(state) = self.imp().state.get() else { + return; + }; + let revealing = state + .services + .file_revealer + .reveal(gio::File::for_path(report), Some(self.clone().upcast())); + let window = self.downgrade(); + glib::spawn_future_local(async move { + let revealed = revealing.await; + if let (Some(toast), Some(window)) = (reveal_failure_toast(&revealed), window.upgrade()) { + window.add_toast(toast); + } + }); + } +} + +#[cfg(test)] +mod tests { + use std::rc::Rc; + + use super::super::SessionWindowInput; + use super::super::tests::{Script, TestWindow, WAIT}; + use super::*; + use crate::test_support::{descendants, drain_main_context, wait_until}; + use crate::ui::file_reveal::RecordingRevealer; + + const TITLE: &str = "TablePro quit unexpectedly last time"; + + fn said(window: &SessionWindow) -> usize { + window + .imp() + .toasts + .borrow() + .iter() + .filter(|toast| toast.title().as_deref() == Some(TITLE)) + .count() + } + + fn show_report(window: &SessionWindow) -> Option { + descendants(window) + .into_iter() + .filter_map(|widget| widget.downcast::().ok()) + .find(|button| button.label().as_deref() == Some("Show Report") && button.is_mapped()) + } + + #[gtk4::test] + fn a_crash_report_is_said_on_each_page_until_the_user_opens_it() { + let revealer = Rc::new(RecordingRevealer::succeeding()); + let test = TestWindow::with_script( + "CrashNotice", + Script { + revealer: Some(revealer.clone()), + ..Script::default() + }, + ); + let report = test.dir().join("crash-20261017T000000Z-1.txt"); + + test.window.announce_crash(report.clone()); + assert_eq!(said(&test.window), 1); + + test.window + .send(SessionWindowInput::OpenSaved(Box::new(test.saved.clone()))); + test.wait_for_page("workspace"); + assert!(said(&test.window) > 1, "the toast stayed on the picker"); + wait_until(WAIT, || show_report(&test.window).is_some()).expect("the toast on the workspace"); + + show_report(&test.window).expect("the button").emit_clicked(); + drain_main_context(); + + assert_eq!(revealer.revealed(), [report]); + assert!(test.window.imp().crash_notice.borrow().is_none()); + let before = said(&test.window); + test.window.show_crash_notice(); + assert_eq!(said(&test.window), before, "a dismissed notice came back"); + } +} diff --git a/linux/crates/app/src/ui/session_window/imp.rs b/linux/crates/app/src/ui/session_window/imp.rs index 32cb4e34b2..391937f241 100644 --- a/linux/crates/app/src/ui/session_window/imp.rs +++ b/linux/crates/app/src/ui/session_window/imp.rs @@ -8,6 +8,7 @@ use libadwaita as adw; use libadwaita::subclass::prelude::*; use tablepro_session::{ConnectAttempts, SessionKey}; +use super::crash_notice::CrashNotice; use super::locate_file::LocateChooser; use super::picker_page::PickerPage; use super::session_page::SessionPage; @@ -56,6 +57,8 @@ pub struct SessionWindow { /// looked for. pub(super) dialog_opening: Cell, pub(super) shut_down: Cell, + /// The crash the last run ended in, until its toast is dismissed. + pub(super) crash_notice: RefCell>, /// Every toast shown, which the overlay does not list. #[cfg(test)] pub(super) toasts: RefCell>, diff --git a/linux/crates/app/src/ui/session_window/mod.rs b/linux/crates/app/src/ui/session_window/mod.rs index 30ae2db2c2..12f9f04570 100644 --- a/linux/crates/app/src/ui/session_window/mod.rs +++ b/linux/crates/app/src/ui/session_window/mod.rs @@ -9,6 +9,7 @@ mod connect_failure_view; mod connect_flow; mod connecting_page; mod connections; +mod crash_notice; mod imp; mod locate_file; mod notification_request; @@ -446,6 +447,7 @@ impl SessionWindow { } let left = imp.page.replace(page); imp.page_toasts.replace(toasts); + self.show_crash_notice(); // A Cancel or Retry left as the default would take Enter on a // page that no longer shows it. self.set_default_widget(None::<>k::Widget>); diff --git a/linux/crates/app/src/ui/session_window/tests.rs b/linux/crates/app/src/ui/session_window/tests.rs index 01fe6c315f..7aa6f62721 100644 --- a/linux/crates/app/src/ui/session_window/tests.rs +++ b/linux/crates/app/src/ui/session_window/tests.rs @@ -53,6 +53,8 @@ pub(crate) struct Script { pub unreadable_list: bool, /// The size the window opens at, as the settings remember it. pub window_size: Option<(i32, i32)>, + /// Stands in for the file manager. + pub revealer: Option>, } /// A TablePro over fakes with one window open on the picker and `saved` @@ -97,6 +99,7 @@ impl TestWindow { tunnel, unreadable_list, window_size, + revealer, } = script; if let Some(size) = window_size { remember_window_size(&settings, size); @@ -133,7 +136,10 @@ impl TestWindow { storage, tasks: runtime.tasks(), }; - let services = scripted_services(parts, hold.clone(), credentials, ssh); + let mut services = scripted_services(parts, hold.clone(), credentials, ssh); + if let Some(revealer) = revealer { + services.file_revealer = revealer; + } let (app, window) = open_picker_window(name, services); let services = app.services().expect("the application's services"); let test = Self { diff --git a/linux/crates/app/tests/crash_report.rs b/linux/crates/app/tests/crash_report.rs new file mode 100644 index 0000000000..f2731e54a0 --- /dev/null +++ b/linux/crates/app/tests/crash_report.rs @@ -0,0 +1,144 @@ +//! A panic in a GTK callback cannot unwind through C, so it ends the +//! process. The hook has to leave a report behind that the next launch +//! finds, and the default output on stderr has to stay. +//! +//! A panic hook is process-wide and the crash ends the process, so each +//! case runs in a child of its own. + +#![expect( + clippy::print_stderr, + reason = "a harness-free test binary reports its own failures on stderr" +)] + +use std::os::unix::fs::PermissionsExt; +use std::path::{Path, PathBuf}; +use std::process::{Command, ExitCode, Output, Stdio}; + +const CHILD_DIR: &str = "TABLEPRO_CRASH_REPORT_DIR"; +const CHILD_OFF_THREAD: &str = "TABLEPRO_CRASH_REPORT_OFF_THREAD"; +const PROBE: &str = "the crash report probe"; + +fn main() -> ExitCode { + match std::env::var_os(CHILD_DIR) { + Some(dir) => child(PathBuf::from(dir)), + None => parent(), + } +} + +#[expect(clippy::panic, reason = "the panic is what the case is about")] +fn child(dir: PathBuf) -> ExitCode { + tablepro_app::crash_report::install(dir); + if std::env::var_os(CHILD_OFF_THREAD).is_some() { + // A runtime thread's panic is caught where it happens and ends + // a task, not the app. + let worker = std::thread::Builder::new() + .name("tokio-runtime-worker".to_owned()) + .spawn(|| panic!("{PROBE}")); + if let Ok(worker) = worker { + let _ = worker.join(); + } + return ExitCode::SUCCESS; + } + glib::idle_add_local_once(|| panic!("{PROBE}")); + glib::MainLoop::new(None, false).run(); + ExitCode::SUCCESS +} + +fn parent() -> ExitCode { + let Ok(root) = tempfile::tempdir() else { + eprintln!("no temporary directory"); + return ExitCode::FAILURE; + }; + let crashes = root.path().join("crashes"); + match crash_leaves_one_private_report(root.path(), &crashes) { + Ok(()) => ExitCode::SUCCESS, + Err(message) => { + eprintln!("{message}"); + ExitCode::FAILURE + } + } +} + +fn crash_leaves_one_private_report(root: &Path, crashes: &Path) -> Result<(), String> { + let caught = run_child(root, crashes, true)?; + if !caught.status.success() || crashes.exists() { + return Err(format!( + "a panic off the GTK thread was taken for a crash ({}):\n{}", + caught.status, + String::from_utf8_lossy(&caught.stderr) + )); + } + + let crashed = run_child(root, crashes, false)?; + let stderr = String::from_utf8_lossy(&crashed.stderr); + if crashed.status.success() { + return Err(format!("a panic in a GTK callback did not end the process:\n{stderr}")); + } + if !stderr.contains(PROBE) { + return Err(format!( + "the default hook no longer writes the panic to stderr:\n{stderr}" + )); + } + + let reports = reports_in(crashes)?; + let [report] = reports.as_slice() else { + return Err(format!("expected one report, found {reports:?}")); + }; + let mode = |path: &Path| std::fs::metadata(path).map(|metadata| metadata.permissions().mode() & 0o777); + if mode(crashes).ok() != Some(0o700) || mode(report).ok() != Some(0o600) { + return Err(format!( + "the report is not private: directory {:?}, file {:?}", + mode(crashes).ok(), + mode(report).ok() + )); + } + let text = std::fs::read_to_string(report).map_err(|error| format!("could not read the report: {error}"))?; + for needle in [ + format!("TablePro {}", tablepro_app::config::VERSION), + format!("Profile: {}", tablepro_app::config::PROFILE), + PROBE.to_owned(), + "Backtrace:\n".to_owned(), + ] { + if !text.contains(&needle) { + return Err(format!("the report is missing {needle:?}:\n{text}")); + } + } + + let announced = tablepro_app::crash_report::take_unannounced(crashes).map_err(|error| error.to_string())?; + if announced.as_ref() != Some(report) { + return Err(format!("the next launch found {announced:?}, not {}", report.display())); + } + let again = tablepro_app::crash_report::take_unannounced(crashes).map_err(|error| error.to_string())?; + if again.is_some() { + return Err("the same crash was announced twice".to_owned()); + } + Ok(()) +} + +fn run_child(root: &Path, crashes: &Path, off_thread: bool) -> Result { + let mut command = Command::new(std::env::args_os().next().unwrap_or_default()); + command + .env(CHILD_DIR, crashes) + .env_remove("RUST_BACKTRACE") + // An abort can leave a core file in the working directory. + .current_dir(root) + .stdin(Stdio::null()); + if off_thread { + command.env(CHILD_OFF_THREAD, "1"); + } + command + .output() + .map_err(|error| format!("could not run the child: {error}")) +} + +fn reports_in(dir: &Path) -> Result, String> { + let entries = std::fs::read_dir(dir).map_err(|error| format!("no crash directory: {error}"))?; + Ok(entries + .filter_map(Result::ok) + .map(|entry| entry.path()) + .filter(|path| { + path.file_name() + .is_some_and(|name| name.to_string_lossy().starts_with("crash-")) + }) + .collect()) +} diff --git a/linux/crates/core/src/sql_syntax/script/text_offsets.rs b/linux/crates/core/src/sql_syntax/script/text_offsets.rs index acd4991528..21fab65215 100644 --- a/linux/crates/core/src/sql_syntax/script/text_offsets.rs +++ b/linux/crates/core/src/sql_syntax/script/text_offsets.rs @@ -4,14 +4,58 @@ pub fn char_to_byte(text: &str, char_offset: usize) -> usize { .map_or(text.len(), |(byte, _)| byte) } +/// How many chars start before `byte`. pub fn byte_to_char(text: &str, byte: usize) -> usize { - text.char_indices().take_while(|(index, _)| *index < byte).count() + CharCounter::new(text).char_at(byte) +} + +/// `byte_to_char` for offsets that only grow, such as a script's batch +/// starts. Each call counts only the text since the previous one, so a +/// whole script is one pass rather than one pass per batch. +#[derive(Debug)] +pub struct CharCounter<'a> { + text: &'a str, + byte: usize, + chars: usize, +} + +impl<'a> CharCounter<'a> { + pub fn new(text: &'a str) -> Self { + Self { + text, + byte: 0, + chars: 0, + } + } + + /// An offset behind the previous one counts from the start again. + pub fn char_at(&mut self, byte: usize) -> usize { + let byte = byte.min(self.text.len()); + if byte < self.byte { + self.byte = 0; + self.chars = 0; + } + self.chars += self.text.as_bytes().get(self.byte..byte).map_or(0, char_starts); + self.byte = byte; + self.chars + } +} + +/// Every byte that is not a UTF-8 continuation byte starts a char. +fn char_starts(bytes: &[u8]) -> usize { + bytes.iter().filter(|byte| **byte & 0xC0 != 0x80).count() } #[cfg(test)] mod tests { + use proptest::prelude::*; + use super::*; + fn counted_from_the_start(text: &str, byte: usize) -> usize { + text.char_indices().take_while(|(index, _)| *index < byte).count() + } + #[test] fn converts_ascii_offsets_one_to_one() { assert_eq!(char_to_byte("select", 3), 3); @@ -23,4 +67,25 @@ mod tests { assert_eq!(char_to_byte("ab", 10), 2); assert_eq!(byte_to_char("ab", 10), 2); } + + #[test] + fn an_offset_inside_a_char_counts_that_char() { + // 'é' is bytes 1 and 2. + assert_eq!(byte_to_char("aéb", 2), counted_from_the_start("aéb", 2)); + assert_eq!(byte_to_char("aéb", 2), 2); + } + + proptest! { + #[test] + fn the_counter_agrees_with_counting_from_the_start( + text in "[ -~\n\té😀]{0,80}", + mut offsets in proptest::collection::vec(0usize..400, 0..12), + ) { + offsets.sort_unstable(); + let mut counter = CharCounter::new(&text); + for offset in offsets.iter().copied().chain(offsets.iter().rev().copied()) { + prop_assert_eq!(counter.char_at(offset), counted_from_the_start(&text, offset)); + } + } + } } diff --git a/linux/crates/session/tests/session_postgres.rs b/linux/crates/session/tests/session_postgres.rs index a794a2f7b8..a12b64996b 100644 --- a/linux/crates/session/tests/session_postgres.rs +++ b/linux/crates/session/tests/session_postgres.rs @@ -93,8 +93,8 @@ fn isolated_ssh() -> IsolatedSsh { 0o755, ) .expect("the ssh wrapper"); - let runtime = dir.path().join("runtime"); - std::fs::create_dir(&runtime).expect("the runtime directory"); + // `app/` is left for the runtime to create, as on a host install. + let runtime = dir.path().join("runtime").join("app").join("app.tablepro.TablePro"); let services = SshServices { runtime: Arc::new(SshRuntimeCell::new(runtime)), ssh_program: wrapper, diff --git a/linux/crates/ssh/Cargo.toml b/linux/crates/ssh/Cargo.toml index 5f3cdeda7a..e7491d1ac2 100644 --- a/linux/crates/ssh/Cargo.toml +++ b/linux/crates/ssh/Cargo.toml @@ -17,6 +17,8 @@ path = "src/bin/tablepro-askpass.rs" tablepro-core = { path = "../core" } async-trait.workspace = true futures.workspace = true +# mkdirat and openat with O_NOFOLLOW, and the effective uid, none of which std has. +rustix = { version = "1.1", default-features = false, features = ["std", "fs", "process"] } secrecy.workspace = true thiserror.workspace = true tokio = { workspace = true, features = ["io-util", "process", "net", "time", "sync", "fs"] } diff --git a/linux/crates/ssh/src/runtime.rs b/linux/crates/ssh/src/runtime.rs index ecacc293b8..88f90936dd 100644 --- a/linux/crates/ssh/src/runtime.rs +++ b/linux/crates/ssh/src/runtime.rs @@ -4,6 +4,8 @@ use std::os::unix::fs::{DirBuilderExt, MetadataExt, OpenOptionsExt}; use std::path::{Path, PathBuf}; use std::sync::Arc; +use rustix::fs::{CWD, Mode, OFlags, fstat, mkdirat, openat}; +use rustix::io::Errno; use tablepro_core::SshFailure; use uuid::Uuid; @@ -17,6 +19,7 @@ pub struct SshRuntime { impl SshRuntime { pub fn acquire(runtime_dir: &Path) -> Result, SshFailure> { + create_missing_levels(runtime_dir)?; let root = runtime_dir.join("ssh"); let root_metadata = private_dir(&root)?; let (instance_id, instance_dir) = create_unique_dir(&root, 8)?; @@ -65,6 +68,67 @@ impl SshRuntime { } } +/// Only Flatpak creates `$XDG_RUNTIME_DIR/app/`, and with the +/// variable unset GLib names a directory under the cache instead, so a +/// host install makes the missing levels itself. Each one is made and +/// then opened through its parent's descriptor without following a +/// symlink, and must belong to this user, so a link or a directory +/// planted in the path cannot move the control sockets. +fn create_missing_levels(runtime_dir: &Path) -> Result<(), SshFailure> { + let missing: Vec<&Path> = runtime_dir + .ancestors() + .filter(|dir| !dir.as_os_str().is_empty()) + .take_while(|dir| is_missing(dir)) + .collect(); + let Some(top) = missing.last() else { + return Ok(()); + }; + let base = top + .parent() + .filter(|parent| !parent.as_os_str().is_empty()) + .unwrap_or(Path::new(".")); + let mut parent = openat( + CWD, + base, + OFlags::RDONLY | OFlags::DIRECTORY | OFlags::CLOEXEC, + Mode::empty(), + ) + .map_err(|errno| unsafe_dir(base, &errno.into()))?; + let owner = rustix::process::geteuid().as_raw(); + for dir in missing.into_iter().rev() { + let Some(name) = dir.file_name() else { + return Err(SshFailure::RuntimeDirUnsafe { + path: dir.to_owned(), + detail: "the path does not end in a directory name".to_owned(), + }); + }; + match mkdirat(&parent, name, Mode::RWXU) { + Ok(()) | Err(Errno::EXIST) => {} + Err(errno) => return Err(unsafe_dir(dir, &errno.into())), + } + let child = openat( + &parent, + name, + OFlags::RDONLY | OFlags::DIRECTORY | OFlags::NOFOLLOW | OFlags::CLOEXEC, + Mode::empty(), + ) + .map_err(|errno| unsafe_dir(dir, &errno.into()))?; + let stat = fstat(&child).map_err(|errno| unsafe_dir(dir, &errno.into()))?; + if stat.st_uid != owner { + return Err(SshFailure::RuntimeDirUnsafe { + path: dir.to_owned(), + detail: "the directory belongs to another user".to_owned(), + }); + } + parent = child; + } + Ok(()) +} + +fn is_missing(path: &Path) -> bool { + matches!(std::fs::symlink_metadata(path), Err(error) if error.kind() == io::ErrorKind::NotFound) +} + fn private_dir(path: &Path) -> Result { match DirBuilder::new().mode(0o700).create(path) { Ok(()) => {} @@ -150,6 +214,37 @@ mod tests { assert_ne!(first.instance_dir(), second.instance_dir()); } + #[test] + fn acquire_creates_the_app_levels_a_host_install_lacks() { + let temp = tempfile::tempdir().unwrap(); + let xdg_runtime = temp.path().join("runtime"); + std::fs::create_dir(&xdg_runtime).unwrap(); + std::fs::set_permissions(&xdg_runtime, std::fs::Permissions::from_mode(0o700)).unwrap(); + let runtime_dir = xdg_runtime.join("app").join("app.tablepro.TablePro"); + + let runtime = SshRuntime::acquire(&runtime_dir).unwrap(); + + assert_eq!(runtime.root(), runtime_dir.join("ssh")); + assert_eq!(mode(&xdg_runtime.join("app")), 0o700); + assert_eq!(mode(&runtime_dir), 0o700); + assert!(runtime.create_master_dir().is_ok()); + } + + #[test] + fn acquire_creates_every_level_of_the_cache_fallback() { + // With XDG_RUNTIME_DIR unset GLib answers the cache directory, + // which a new account may not have yet either. + let home = tempfile::tempdir().unwrap(); + let runtime_dir = home.path().join(".cache").join("app").join("app.tablepro.TablePro"); + + let runtime = SshRuntime::acquire(&runtime_dir).unwrap(); + + for level in [home.path().join(".cache"), home.path().join(".cache/app"), runtime_dir] { + assert_eq!(mode(&level), 0o700, "{}", level.display()); + } + assert_eq!(mode(runtime.root()), 0o700); + } + #[test] fn group_writable_root_is_unsafe() { let temp = tempfile::tempdir().unwrap(); diff --git a/linux/crates/ssh/tests/openssh_session.rs b/linux/crates/ssh/tests/openssh_session.rs index 7eb888cdd7..5a648010c3 100644 --- a/linux/crates/ssh/tests/openssh_session.rs +++ b/linux/crates/ssh/tests/openssh_session.rs @@ -65,8 +65,11 @@ async fn start() -> TestResult { 0o755, )?; - let runtime_dir = temp.path().join("runtime"); - std::fs::create_dir(&runtime_dir)?; + // The layout a .deb or .rpm install gets: `XDG_RUNTIME_DIR` exists, + // the `app/` levels under it do not. + let xdg_runtime = temp.path().join("runtime"); + std::fs::create_dir(&xdg_runtime)?; + let runtime_dir = xdg_runtime.join("app").join("app.tablepro.TablePro"); let services = SshServices { runtime: Arc::new(SshRuntimeCell::new(runtime_dir)), ssh_program: wrapper, diff --git a/linux/crates/storage/src/fs.rs b/linux/crates/storage/src/fs.rs index 869cfde71f..b9edaa5cc4 100644 --- a/linux/crates/storage/src/fs.rs +++ b/linux/crates/storage/src/fs.rs @@ -1,5 +1,5 @@ use std::fs::{DirBuilder, OpenOptions, Permissions}; -use std::io::ErrorKind; +use std::io::{ErrorKind, Write}; use std::os::unix::fs::{DirBuilderExt, OpenOptionsExt, PermissionsExt}; use std::path::Path; @@ -89,6 +89,28 @@ pub fn create_private_file_if_missing(path: &Path) -> Result<(), StorageError> { } } +/// Add bytes to the end of a file, creating it 0600 under a 0700 +/// directory when it is absent. Unlike `write_private_blocking` it never +/// replaces what is there, so a second write in the same moment adds to +/// the first. +pub fn append_private_blocking(path: &Path, bytes: &[u8]) -> Result<(), StorageError> { + if let Some(parent) = path.parent() { + ensure_private_dir(parent)?; + } + let io_error = |source| StorageError::Io { + path: path.to_owned(), + source, + }; + let mut file = OpenOptions::new() + .append(true) + .create(true) + .mode(PRIVATE_FILE) + .open(path) + .map_err(io_error)?; + file.write_all(bytes).map_err(io_error)?; + file.sync_all().map_err(io_error) +} + /// Rename a file out of the way, returning the name it now has. /// /// The caller shows that name, so the user can find the file again. The @@ -353,6 +375,19 @@ mod tests { ); } + #[test] + fn append_private_creates_0600_under_0700_and_adds_to_the_end() { + let root = tempfile::tempdir().expect("tempdir"); + let file = root.path().join("crashes").join("crash.txt"); + + append_private_blocking(&file, b"first\n").expect("append"); + append_private_blocking(&file, b"second\n").expect("append"); + + assert_eq!(mode_of(&file), 0o600); + assert_eq!(mode_of(file.parent().expect("parent")), 0o700); + assert_eq!(std::fs::read(&file).expect("read"), b"first\nsecond\n"); + } + #[test] fn create_private_file_if_missing_keeps_existing_contents() { let root = tempfile::tempdir().expect("tempdir"); diff --git a/linux/crates/storage/src/paths.rs b/linux/crates/storage/src/paths.rs index 09ab19737f..31b11ec7c0 100644 --- a/linux/crates/storage/src/paths.rs +++ b/linux/crates/storage/src/paths.rs @@ -61,6 +61,10 @@ impl StoragePaths { pub fn drafts_dir(&self) -> PathBuf { self.data.join("drafts") } + + pub fn crashes_dir(&self) -> PathBuf { + self.state.join("crashes") + } } #[cfg(test)] diff --git a/linux/crates/storage/tests/runtime_fallback.rs b/linux/crates/storage/tests/runtime_fallback.rs new file mode 100644 index 0000000000..f215e2ad83 --- /dev/null +++ b/linux/crates/storage/tests/runtime_fallback.rs @@ -0,0 +1,25 @@ +//! With `XDG_RUNTIME_DIR` unset, as under some display managers and in a +//! plain `ssh -X` login, GLib answers the cache directory, so the SSH +//! control sockets go under `$XDG_CACHE_HOME/app/`, which nothing +//! else creates. +//! +//! GLib reads the variables once per process, which is why this is a test +//! binary of its own with one test, and why they are set before GLib is +//! asked anything. + +use tablepro_storage::StoragePaths; + +#[test] +fn an_unset_runtime_dir_falls_back_to_the_cache_dir() { + let cache = tempfile::tempdir().expect("a directory"); + // SAFETY: the harness runs this binary's one test alone, and nothing + // in it reads the environment yet. + unsafe { + std::env::remove_var("XDG_RUNTIME_DIR"); + std::env::set_var("XDG_CACHE_HOME", cache.path()); + } + + let paths = StoragePaths::from_glib("tablepro", "app.tablepro.TablePro"); + + assert_eq!(paths.runtime, cache.path().join("app").join("app.tablepro.TablePro")); +} diff --git a/linux/data/resources/ui/connect-failed-page.ui b/linux/data/resources/ui/connect-failed-page.ui index 1dded9256b..289ab02b16 100644 --- a/linux/data/resources/ui/connect-failed-page.ui +++ b/linux/data/resources/ui/connect-failed-page.ui @@ -22,11 +22,12 @@ + would only send the refused secret again. Its + action takes the secret's kind, so the code + names it once it has set that target. --> false Enter Password… - win.retry-with-new-secret