From f5be6d2662144f33cb40bd4bebd0063188b331df Mon Sep 17 00:00:00 2001 From: Bob Date: Wed, 16 Sep 2026 08:42:36 +0000 Subject: [PATCH 1/5] fix(aw-sync): open peer databases read-only; skip version mismatch Pull used Datastore::new on files this device does not own, which flipped journal_mode to WAL (creating -wal/-shm sidecars), ran migrations, and aborted the whole pass on user_version skew. Open peers with file:?mode=ro&immutable=1, never migrate, and skip + warn when user_version != NEWEST. Own staging still uses the read-write constructor. ActivityWatch/aw-server-rust#693 v0.14.0 slice (the full tolerant reader for old indexes is not in this PR). Git-Session-Id: 3a97ef00-28f8-50e5-99de-b20a862e44e0 --- aw-datastore/src/datastore.rs | 2 +- aw-datastore/src/lib.rs | 5 ++ aw-datastore/src/worker.rs | 92 +++++++++++++++++++++++++++------ aw-datastore/tests/datastore.rs | 78 +++++++++++++++++++++++++++- aw-sync/src/sync.rs | 48 ++++++++++++----- 5 files changed, 195 insertions(+), 30 deletions(-) diff --git a/aw-datastore/src/datastore.rs b/aw-datastore/src/datastore.rs index bbe12a64..aa414036 100644 --- a/aw-datastore/src/datastore.rs +++ b/aw-datastore/src/datastore.rs @@ -32,7 +32,7 @@ fn _get_db_version(conn: &Connection) -> i32 { * 5: Replaced single-column events indexes with a composite index * 6: Added an endtime-first index for recent interval reads */ -static NEWEST_DB_VERSION: i32 = 6; +pub const NEWEST_DB_VERSION: i32 = 6; fn _create_tables(conn: &Connection, version: i32) -> bool { let mut first_init = false; diff --git a/aw-datastore/src/lib.rs b/aw-datastore/src/lib.rs index a93bf896..1dd60961 100644 --- a/aw-datastore/src/lib.rs +++ b/aw-datastore/src/lib.rs @@ -24,12 +24,16 @@ mod privacy_filter; mod worker; pub use self::datastore::DatastoreInstance; +pub use self::datastore::NEWEST_DB_VERSION; pub use self::worker::Datastore; #[derive(Clone)] pub enum DatastoreMethod { Memory(), File(String), + /// Existing file, opened `mode=ro&immutable=1`. Never migrates, never + /// creates `-wal`/`-shm`. Used by aw-sync when reading a peer's db. + FileReadOnly(String), /// Encrypted SQLite file using SQLCipher. Only available with the /// `encryption` or `encryption-vendored` feature flags. #[cfg(any(feature = "encryption", feature = "encryption-vendored"))] @@ -41,6 +45,7 @@ impl fmt::Debug for DatastoreMethod { match self { DatastoreMethod::Memory() => write!(f, "Memory()"), DatastoreMethod::File(p) => write!(f, "File({p:?})"), + DatastoreMethod::FileReadOnly(p) => write!(f, "FileReadOnly({p:?})"), #[cfg(any(feature = "encryption", feature = "encryption-vendored"))] DatastoreMethod::FileEncrypted(p, _) => write!(f, "FileEncrypted({p:?}, )"), } diff --git a/aw-datastore/src/worker.rs b/aw-datastore/src/worker.rs index 0a3a4b1a..7d8882b2 100644 --- a/aw-datastore/src/worker.rs +++ b/aw-datastore/src/worker.rs @@ -12,6 +12,7 @@ use chrono::Utc; use rusqlite::Connection; use rusqlite::DropBehavior; +use rusqlite::OpenFlags; use rusqlite::Transaction; use rusqlite::TransactionBehavior; @@ -26,6 +27,41 @@ use crate::DatastoreMethod; type RequestSender = mpsc_requests::RequestSender>; type RequestReceiver = mpsc_requests::RequestReceiver>; +/// SQLite URI for a side-effect-free open: no `-wal`/`-shm`, no locks that +/// fight a file syncer. `?`/`#`/`%` in the path are encoded so they cannot +/// be parsed as the query string. +fn sqlite_readonly_uri(path: &str) -> String { + let mut encoded = path + .replace('%', "%25") + .replace('?', "%3F") + .replace('#', "%23"); + if cfg!(windows) { + encoded = encoded.replace('\\', "/"); + if encoded.len() >= 2 && encoded.as_bytes().get(1) == Some(&b':') { + return format!("file:///{encoded}?mode=ro&immutable=1"); + } + } + format!("file:{encoded}?mode=ro&immutable=1") +} + +fn open_readonly_connection(path: &str) -> rusqlite::Result { + Connection::open_with_flags( + sqlite_readonly_uri(path), + OpenFlags::SQLITE_OPEN_READ_ONLY | OpenFlags::SQLITE_OPEN_URI, + ) +} + +/// Read `user_version` without mutating the file. +fn probe_user_version(path: &str) -> Result { + let conn = open_readonly_connection(path).map_err(|e| { + DatastoreError::InternalError(format!("read-only open failed for {path}: {e}")) + })?; + conn.pragma_query_value(None, "user_version", |row| row.get(0)) + .map_err(|e| { + DatastoreError::InternalError(format!("user_version read failed for {path}: {e}")) + }) +} + #[derive(Clone)] pub struct Datastore { requester: RequestSender, @@ -148,6 +184,8 @@ impl DatastoreWorker { } fn work_loop(&mut self, method: DatastoreMethod) { + let read_only = matches!(&method, DatastoreMethod::FileReadOnly(_)); + // Open SQLite connection let mut conn = match &method { DatastoreMethod::Memory() => { @@ -156,6 +194,9 @@ impl DatastoreWorker { DatastoreMethod::File(path) => { Connection::open(path).expect("Failed to create datastore") } + DatastoreMethod::FileReadOnly(path) => { + open_readonly_connection(path).expect("Failed to open datastore read-only") + } #[cfg(any(feature = "encryption", feature = "encryption-vendored"))] DatastoreMethod::FileEncrypted(path, key) => { let conn = Connection::open(path).expect("Failed to create encrypted datastore"); @@ -176,24 +217,22 @@ impl DatastoreWorker { conn.busy_timeout(std::time::Duration::from_secs(5)) .expect("Failed to set busy timeout"); - // WAL turns each commit into a single sequential WAL append+fsync where - // delete mode paid two fsyncs plus journal-file churn, and lets future - // reader connections proceed while a commit is in flight. - // synchronous=FULL is set explicitly (rather than relying on the - // default) so a commit remains durable on disk the moment it returns; - // with NORMAL the WAL is only synced at checkpoints, which would - // silently widen the loss window on power failure. - // In-memory databases ignore the request (journal_mode stays "memory"). - let journal_mode: String = conn - .pragma_update_and_check(None, "journal_mode", "WAL", |row| row.get(0)) - .expect("Failed to query journal_mode"); - if !matches!(&method, DatastoreMethod::Memory()) && journal_mode != "wal" { - warn!("Failed to enable WAL (journal_mode={journal_mode}), continuing without it"); + // WAL / synchronous=FULL are writes. Skip them on a peer file: a pull + // must not create `-wal`/`-shm` in a directory this device does not + // own (ActivityWatch/aw-server-rust#693). In-memory databases ignore + // the request (journal_mode stays "memory"). + if !read_only { + let journal_mode: String = conn + .pragma_update_and_check(None, "journal_mode", "WAL", |row| row.get(0)) + .expect("Failed to query journal_mode"); + if !matches!(&method, DatastoreMethod::Memory()) && journal_mode != "wal" { + warn!("Failed to enable WAL (journal_mode={journal_mode}), continuing without it"); + } + conn.pragma_update(None, "synchronous", "FULL") + .expect("Failed to set synchronous=FULL"); } - conn.pragma_update(None, "synchronous", "FULL") - .expect("Failed to set synchronous=FULL"); - let mut ds = DatastoreInstance::new(&conn, true).unwrap(); + let mut ds = DatastoreInstance::new(&conn, !read_only).unwrap(); // Load persisted privacy filters before serving inserts. The engine // starts empty; without this, rules saved in a previous process sit @@ -508,6 +547,27 @@ impl Datastore { Datastore::_new_internal(method, legacy_import) } + /// Open an existing database without writing to it. + /// + /// Uses `file:…?mode=ro&immutable=1`, never runs migrations, never sets + /// `journal_mode`/`synchronous`. Returns `OldDbVersion` when + /// `user_version != NEWEST_DB_VERSION` so a caller can skip that peer + /// (ActivityWatch/aw-server-rust#693). + pub fn open_read_only(dbpath: String) -> Result { + let version = probe_user_version(&dbpath)?; + if version != crate::NEWEST_DB_VERSION { + return Err(DatastoreError::OldDbVersion(format!( + "Tried to open a database with an incompatible database version! \ + Database has version {version} while the supported version is {}", + crate::NEWEST_DB_VERSION + ))); + } + Ok(Datastore::_new_internal( + DatastoreMethod::FileReadOnly(dbpath), + false, + )) + } + pub fn new_in_memory(legacy_import: bool) -> Self { let method = DatastoreMethod::Memory(); Datastore::_new_internal(method, legacy_import) diff --git a/aw-datastore/tests/datastore.rs b/aw-datastore/tests/datastore.rs index f4d836cb..85e6fc4b 100644 --- a/aw-datastore/tests/datastore.rs +++ b/aw-datastore/tests/datastore.rs @@ -14,7 +14,7 @@ mod datastore_tests { use chrono::Utc; use serde_json::json; - use aw_datastore::Datastore; + use aw_datastore::{Datastore, DatastoreError}; use aw_models::Bucket; use aw_models::BucketMetadata; @@ -1124,4 +1124,80 @@ mod datastore_tests { let _ = std::fs::remove_file(db_path.with_extension("db-wal")); let _ = std::fs::remove_file(db_path.with_extension("db-shm")); } + + /// ActivityWatch/aw-server-rust#693: a pull must not create `-wal`/`-shm` + /// beside a file this device does not own. + #[test] + fn test_read_only_open_does_not_create_wal_sidecars() { + let test_dir = tempfile::tempdir().unwrap(); + let db_path = test_dir.path().join("peer-readonly.db"); + let db_path_str = db_path.to_str().unwrap().to_string(); + + { + let ds = Datastore::new(db_path_str.clone(), false); + create_test_bucket(&ds); + ds.insert_events("testid", &[test_event(Utc::now(), Duration::seconds(1))]) + .unwrap(); + ds.force_commit().unwrap(); + ds.close(); + } + + // Checkpoint so we can delete WAL sidecars without losing the row. + { + let conn = rusqlite::Connection::open(&db_path).unwrap(); + let _ = conn.execute_batch("PRAGMA wal_checkpoint(TRUNCATE);"); + } + + let wal = db_path.with_extension("db-wal"); + let shm = db_path.with_extension("db-shm"); + let _ = std::fs::remove_file(&wal); + let _ = std::fs::remove_file(&shm); + + { + let ds = Datastore::open_read_only(db_path_str).unwrap(); + let buckets = ds.get_buckets().unwrap(); + assert!( + buckets.contains_key("testid"), + "read-only open must still see existing buckets, got {buckets:?}" + ); + let events = ds.get_events("testid", None, None, None).unwrap(); + assert_eq!(events.len(), 1); + ds.close(); + } + + assert!( + !wal.exists(), + "read-only open must not create {}", + wal.display() + ); + assert!( + !shm.exists(), + "read-only open must not create {}", + shm.display() + ); + } + + #[test] + fn test_read_only_open_skips_old_user_version() { + let test_dir = tempfile::tempdir().unwrap(); + let db_path = test_dir.path().join("peer-v4.db"); + { + let conn = rusqlite::Connection::open(&db_path).unwrap(); + conn.pragma_update(None, "user_version", 4).unwrap(); + } + + let wal = db_path.with_extension("db-wal"); + let shm = db_path.with_extension("db-shm"); + let _ = std::fs::remove_file(&wal); + let _ = std::fs::remove_file(&shm); + + match Datastore::open_read_only(db_path.to_str().unwrap().to_string()) { + Err(DatastoreError::OldDbVersion(msg)) => { + assert!(msg.contains("version 4"), "got {msg}"); + } + other => panic!("expected OldDbVersion, got {other:?}"), + } + assert!(!wal.exists(), "version probe must not create a WAL sidecar"); + assert!(!shm.exists(), "version probe must not create a SHM sidecar"); + } } diff --git a/aw-sync/src/sync.rs b/aw-sync/src/sync.rs index 31b7e9d0..4e2af511 100644 --- a/aw-sync/src/sync.rs +++ b/aw-sync/src/sync.rs @@ -101,11 +101,13 @@ pub fn sync_run( } } - // TODO: Check for compatible remote db version before opening + // Peer files are opened read-only: never migrate, never flip WAL + // (ActivityWatch/aw-server-rust#693). Version mismatch is skipped, not fatal. let mut ds_remotes = Vec::new(); for path in &remote_dbfiles { - match create_datastore(path) { - Ok(ds) => ds_remotes.push(ds), + match open_peer_datastore(path) { + Ok(Some(ds)) => ds_remotes.push(ds), + Ok(None) => {} Err(e) => { warn!("Failed to open remote db {}: {e}", path.display()); return Err(e.into()); @@ -166,12 +168,12 @@ pub fn list_buckets(client: &AwClient) -> Result<(), Box> { let remote_dbfiles = crate::util::find_remotes_nonlocal(sync_directory, device_id, None)?; info!("Found remotes: {:?}", remote_dbfiles); - // TODO: Check for compatible remote db version before opening - let ds_remotes: Vec = remote_dbfiles - .iter() - .map(|p| p.as_path()) - .map(create_datastore) - .collect::, _>>()?; + let mut ds_remotes = Vec::new(); + for path in &remote_dbfiles { + if let Some(ds) = open_peer_datastore(path)? { + ds_remotes.push(ds); + } + } log_buckets(client)?; log_buckets(&ds_localremote)?; @@ -208,12 +210,34 @@ fn setup_local_remote(path: &Path, device_id: &str) -> Result Result { - let pathstr = path - .to_str() - .ok_or_else(|| format!("Sync database path is not valid UTF-8: {}", path.display()))?; + let pathstr = utf8_db_path(path)?; Ok(Datastore::new(pathstr.to_string(), false)) } +/// Open a *peer* database for pull: read-only, no migration, no WAL sidecars. +/// +/// Returns `Ok(None)` when `user_version` does not match this binary so the +/// caller can skip that peer and keep walking (ActivityWatch/aw-server-rust#693). +fn open_peer_datastore(path: &Path) -> Result, String> { + let pathstr = utf8_db_path(path)?; + match Datastore::open_read_only(pathstr.to_string()) { + Ok(ds) => Ok(Some(ds)), + Err(DatastoreError::OldDbVersion(msg)) => { + warn!("Skipping peer db {}: {msg}", path.display()); + Ok(None) + } + Err(e) => Err(format!( + "Failed to open remote db {}: {e:?}", + path.display() + )), + } +} + +fn utf8_db_path(path: &Path) -> Result<&str, String> { + path.to_str() + .ok_or_else(|| format!("Sync database path is not valid UTF-8: {}", path.display())) +} + /// Returns the sync-destination bucket for a given bucket, creates it if it doesn't exist. /// /// Returns an error rather than panicking on a datastore failure or on bucket From 72f69a8aedb94c0854e1030cfb59a0dadcf311c1 Mon Sep 17 00:00:00 2001 From: Bob Date: Wed, 16 Sep 2026 08:53:06 +0000 Subject: [PATCH 2/5] fix(aw-datastore): deferred txns and document immutable=1 on read-only opens Peer workers used BEGIN IMMEDIATE, which is a write lock. Deferred is the correct read-only behavior. Doc-comment what immutable=1 gives up so it is not dropped later to "see the WAL". Git-Session-Id: ca842b73-e7ac-505f-b5bd-1136e4137c8d --- aw-datastore/src/worker.rs | 42 +++++++++++++++++++++++++++++--------- 1 file changed, 32 insertions(+), 10 deletions(-) diff --git a/aw-datastore/src/worker.rs b/aw-datastore/src/worker.rs index 7d8882b2..98221591 100644 --- a/aw-datastore/src/worker.rs +++ b/aw-datastore/src/worker.rs @@ -30,6 +30,12 @@ type RequestReceiver = mpsc_requests::RequestReceiver String { let mut encoded = path .replace('%', "%25") @@ -261,19 +267,28 @@ impl DatastoreWorker { } } + // BEGIN IMMEDIATE takes a reserved (write) lock. On a read-only + // connection that is either SQLITE_READONLY (the worker retried + // forever) or a no-op depending on SQLite version — Deferred is the + // correct read-only behavior either way. + let tx_behavior = if read_only { + TransactionBehavior::Deferred + } else { + TransactionBehavior::Immediate + }; + // Start handling and respond to requests loop { let last_commit_time: DateTime = Utc::now(); - let mut tx: Transaction = - match conn.transaction_with_behavior(TransactionBehavior::Immediate) { - Ok(tx) => tx, - Err(err) => { - error!("Unable to start transaction! {:?}", err); - // Wait 1s before retrying - std::thread::sleep(std::time::Duration::from_millis(1000)); - continue; - } - }; + let mut tx: Transaction = match conn.transaction_with_behavior(tx_behavior) { + Ok(tx) => tx, + Err(err) => { + error!("Unable to start transaction! {:?}", err); + // Wait 1s before retrying + std::thread::sleep(std::time::Duration::from_millis(1000)); + continue; + } + }; // Snapshot BEFORE the request loop. SetKeyValue/DeleteKeyValue // reload the engine from the still-open transaction so a later // insert in the same batch is filtered. If commit fails we restore @@ -553,6 +568,13 @@ impl Datastore { /// `journal_mode`/`synchronous`. Returns `OldDbVersion` when /// `user_version != NEWEST_DB_VERSION` so a caller can skip that peer /// (ActivityWatch/aw-server-rust#693). + /// + /// `immutable=1` means SQLite will not look at a peer's `-wal`/`-shm`. + /// Committed-but-not-yet-checkpointed frames in that WAL are therefore + /// invisible. aw-sync checkpoints on clean close, so the steady-state + /// file is self-contained; this only bites mid-push. A stale-but- + /// consistent snapshot is strictly better than a torn one, and dropping + /// `immutable` would reintroduce `-shm` files in a foreign directory. pub fn open_read_only(dbpath: String) -> Result { let version = probe_user_version(&dbpath)?; if version != crate::NEWEST_DB_VERSION { From d260b1683f5397e24b112168bf84cae00f7f058b Mon Sep 17 00:00:00 2001 From: Bob Date: Wed, 16 Sep 2026 09:00:23 +0000 Subject: [PATCH 3/5] fix(aw-datastore): emit SQLite UNC URIs for peer read-only opens \\server\share\peer.db was becoming file://server/share/... which parses `server` as the URI authority. Detect Windows path shapes from the path (not cfg!(windows)) and use file:////server/share/... Document the in-place-rewrite caveat on immutable=1. ActivityWatch/aw-server-rust#700 Git-Session-Id: fc3a102a-6378-50f2-828f-8b5e6f9b5df9 --- aw-datastore/src/worker.rs | 53 ++++++++++++++++++++++++++++++++++++-- 1 file changed, 51 insertions(+), 2 deletions(-) diff --git a/aw-datastore/src/worker.rs b/aw-datastore/src/worker.rs index 98221591..727332b3 100644 --- a/aw-datastore/src/worker.rs +++ b/aw-datastore/src/worker.rs @@ -35,17 +35,37 @@ type RequestReceiver = mpsc_requests::RequestReceiver String { let mut encoded = path .replace('%', "%25") .replace('?', "%3F") .replace('#', "%23"); - if cfg!(windows) { + let looks_windows = encoded.contains('\\') + || encoded.starts_with("//") + || (encoded.len() >= 2 && encoded.as_bytes().get(1) == Some(&b':')); + if looks_windows { encoded = encoded.replace('\\', "/"); if encoded.len() >= 2 && encoded.as_bytes().get(1) == Some(&b':') { return format!("file:///{encoded}?mode=ro&immutable=1"); } + if encoded.starts_with("//") { + // SQLite UNC form is file:////server/share/file.db (four slashes). + return format!("file://{encoded}?mode=ro&immutable=1"); + } } format!("file:{encoded}?mode=ro&immutable=1") } @@ -848,3 +868,32 @@ impl Datastore { } } } + +#[cfg(test)] +mod sqlite_readonly_uri_tests { + use super::sqlite_readonly_uri; + + #[test] + fn posix_absolute() { + assert_eq!( + sqlite_readonly_uri("/var/lib/activitywatch/peer.db"), + "file:/var/lib/activitywatch/peer.db?mode=ro&immutable=1" + ); + } + + #[test] + fn windows_drive_letter() { + assert_eq!( + sqlite_readonly_uri(r"C:\Users\bob\peer.db"), + "file:///C:/Users/bob/peer.db?mode=ro&immutable=1" + ); + } + + #[test] + fn windows_unc() { + assert_eq!( + sqlite_readonly_uri(r"\\server\share\peer.db"), + "file:////server/share/peer.db?mode=ro&immutable=1" + ); + } +} From 955c5161fbb86e09beaf4b8d6f70e55e99fd1083 Mon Sep 17 00:00:00 2001 From: Bob Date: Wed, 16 Sep 2026 09:17:40 +0000 Subject: [PATCH 4/5] fix(aw-datastore): only treat drive-letter/UNC paths as Windows for readonly URI A bare backslash is a valid POSIX filename character. sqlite_readonly_uri() was rewriting it to a forward slash whenever any backslash was present, corrupting a POSIX path that happens to contain one and potentially opening the wrong file or failing to open at all. Only classify a path as Windows when it has a drive letter or a UNC prefix. Git-Session-Id: e82d --- aw-datastore/src/worker.rs | 38 +++++++++++++++++++++++++------------- 1 file changed, 25 insertions(+), 13 deletions(-) diff --git a/aw-datastore/src/worker.rs b/aw-datastore/src/worker.rs index 727332b3..c1e85686 100644 --- a/aw-datastore/src/worker.rs +++ b/aw-datastore/src/worker.rs @@ -50,22 +50,24 @@ type RequestReceiver = mpsc_requests::RequestReceiver String { - let mut encoded = path + let encoded = path .replace('%', "%25") .replace('?', "%3F") .replace('#', "%23"); - let looks_windows = encoded.contains('\\') - || encoded.starts_with("//") - || (encoded.len() >= 2 && encoded.as_bytes().get(1) == Some(&b':')); - if looks_windows { - encoded = encoded.replace('\\', "/"); - if encoded.len() >= 2 && encoded.as_bytes().get(1) == Some(&b':') { - return format!("file:///{encoded}?mode=ro&immutable=1"); - } - if encoded.starts_with("//") { - // SQLite UNC form is file:////server/share/file.db (four slashes). - return format!("file://{encoded}?mode=ro&immutable=1"); - } + // A bare backslash is not evidence of a Windows path — it's a valid POSIX + // filename character, and rewriting it to `/` would corrupt the path. Only + // an unambiguous Windows shape (drive letter, or a UNC prefix using either + // slash direction) triggers backslash normalization. + let has_drive_letter = encoded.len() >= 2 && encoded.as_bytes().get(1) == Some(&b':'); + let is_unc = encoded.starts_with("\\\\") || encoded.starts_with("//"); + if has_drive_letter { + let encoded = encoded.replace('\\', "/"); + return format!("file:///{encoded}?mode=ro&immutable=1"); + } + if is_unc { + // SQLite UNC form is file:////server/share/file.db (four slashes). + let encoded = encoded.replace('\\', "/"); + return format!("file://{encoded}?mode=ro&immutable=1"); } format!("file:{encoded}?mode=ro&immutable=1") } @@ -896,4 +898,14 @@ mod sqlite_readonly_uri_tests { "file:////server/share/peer.db?mode=ro&immutable=1" ); } + + #[test] + fn posix_path_with_literal_backslash_is_not_rewritten() { + // A backslash is a valid POSIX filename character. It must not be + // mistaken for a Windows path separator and rewritten to `/`. + assert_eq!( + sqlite_readonly_uri(r"/home/erik/we\ird.db"), + r"file:/home/erik/we\ird.db?mode=ro&immutable=1" + ); + } } From 3c1eeb10d394e675dcea482526e2aafc2ad42a96 Mon Sep 17 00:00:00 2001 From: Bob Date: Wed, 16 Sep 2026 09:58:55 +0000 Subject: [PATCH 5/5] fix(aw-datastore): gate Windows readonly URI rewrite on cfg!(windows) A backslash is a legal POSIX filename character. Path-shape detection still ran the rewrite/UNC branches on Linux, so a peer path containing `\` could target a different file or abort the pass. Wrap drive-letter and UNC normalisation in cfg!(windows); POSIX paths pass through. ActivityWatch/aw-server-rust#700 Git-Session-Id: d65e2e09-0f68-5f08-b9c2-226c2d762c15 --- aw-datastore/src/worker.rs | 55 +++++++++++++++++++------------------- 1 file changed, 28 insertions(+), 27 deletions(-) diff --git a/aw-datastore/src/worker.rs b/aw-datastore/src/worker.rs index c1e85686..d0c0959f 100644 --- a/aw-datastore/src/worker.rs +++ b/aw-datastore/src/worker.rs @@ -45,29 +45,26 @@ type RequestReceiver = mpsc_requests::RequestReceiver String { let encoded = path .replace('%', "%25") .replace('?', "%3F") .replace('#', "%23"); - // A bare backslash is not evidence of a Windows path — it's a valid POSIX - // filename character, and rewriting it to `/` would corrupt the path. Only - // an unambiguous Windows shape (drive letter, or a UNC prefix using either - // slash direction) triggers backslash normalization. - let has_drive_letter = encoded.len() >= 2 && encoded.as_bytes().get(1) == Some(&b':'); - let is_unc = encoded.starts_with("\\\\") || encoded.starts_with("//"); - if has_drive_letter { + if cfg!(windows) { let encoded = encoded.replace('\\', "/"); - return format!("file:///{encoded}?mode=ro&immutable=1"); - } - if is_unc { - // SQLite UNC form is file:////server/share/file.db (four slashes). - let encoded = encoded.replace('\\', "/"); - return format!("file://{encoded}?mode=ro&immutable=1"); + let has_drive_letter = encoded.len() >= 2 && encoded.as_bytes().get(1) == Some(&b':'); + let is_unc = encoded.starts_with("//"); + if has_drive_letter { + return format!("file:///{encoded}?mode=ro&immutable=1"); + } + if is_unc { + // SQLite UNC form is file:////server/share/file.db (four slashes). + return format!("file://{encoded}?mode=ro&immutable=1"); + } } format!("file:{encoded}?mode=ro&immutable=1") } @@ -884,6 +881,19 @@ mod sqlite_readonly_uri_tests { } #[test] + #[cfg(not(windows))] + fn posix_path_with_literal_backslash_is_not_rewritten() { + // A backslash is a valid POSIX filename character. Windows + // normalisation is cfg!(windows)-gated so this path is passed + // through untouched on Linux/macOS. + assert_eq!( + sqlite_readonly_uri(r"/home/erik/we\ird.db"), + r"file:/home/erik/we\ird.db?mode=ro&immutable=1" + ); + } + + #[test] + #[cfg(windows)] fn windows_drive_letter() { assert_eq!( sqlite_readonly_uri(r"C:\Users\bob\peer.db"), @@ -892,20 +902,11 @@ mod sqlite_readonly_uri_tests { } #[test] + #[cfg(windows)] fn windows_unc() { assert_eq!( sqlite_readonly_uri(r"\\server\share\peer.db"), "file:////server/share/peer.db?mode=ro&immutable=1" ); } - - #[test] - fn posix_path_with_literal_backslash_is_not_rewritten() { - // A backslash is a valid POSIX filename character. It must not be - // mistaken for a Windows path separator and rewritten to `/`. - assert_eq!( - sqlite_readonly_uri(r"/home/erik/we\ird.db"), - r"file:/home/erik/we\ird.db?mode=ro&immutable=1" - ); - } }