Key Design Decisions

Decisions made during development that are not obvious from reading the code.

extract_scheme() must check npos before comparing to 2

std::string::find() returns npos (SIZE_MAX) when the character is absent. The original guard if (pos < 2) is false for npos on 64-bit systems, causing the function to return the entire input string — every colon-free path was classified as a transport URL. Fixed to if (pos == npos || pos < 2). Caught by the standalone test suite.

pws_cache_remove() must be called BEFORE fclose()

The FILE is used as a map key. If fclose() is called first, the OS may reuse that address for a new FILE before pws_cache_remove() runs, causing a wrong-entry removal. More importantly, using the pointer after fclose() is undefined behaviour. Fixed order: remove → close → store.

pws_os::LoadLibrary is Windows-only

src/os/lib.h declares LoadLibrary / FreeLibrary / GetSymbol but only windows/lib.cpp exists — there is no unix/lib.cpp. Transport loader uses dlopen / dlsym / dlclose directly.

wxFilePickerCtrl cannot accept URL input

DbSelectionPanel::DoValidation() called wxFileName(path).FileExists() which is a direct OS call that bypasses our pws_os::FileExists intercept. Additionally wxFilePickerCtrl does not allow free-text URL entry. Solution: replace with wxComboBox (free text + recent-location dropdown) + "Browse..." button that opens wxFileDialog for local files.

Plugin filename is the primary lookup key; identity string is verification

Searching by filename first (pwsafe-<scheme>.so) is O(1) and avoids scanning all .so files. The identity string (PWS_TRANSPORT_INFO:...) is a secondary check: it catches renamed or misplaced files before dlopen(). First match wins; the assumption is at most one plugin per scheme.

DEVELOPMENT macro gates cwd plugin search

In production builds, plugins are only loaded from the directory containing the pwsafe binary. In development builds (-DDEVELOPMENT), the current working directory is also searched. This lets developers run tests without installing. The macro is defined in TransportTest.cpp and used in the standalone test build command.

file: plugin returns ENOTSUP for lock()

The file: plugin is local-only and has no meaningful locking. Returning ENOTSUP from lock() is the agreed signal that means "proceed without a lock". The transport dispatcher in LockFile() treats ENOTSUP as success (returns true) but could emit a one-time warning for network transports.

strip_file_scheme() must handle single-slash file:/ URLs

RFC 8089 §2 permits file:/path (one slash) as a valid local-file URI. The original strip_file_scheme() only matched file:// (two slashes), so file:/home/john/x.psafe3 was returned unchanged, causing fs::copy_file to fail with ENOENT. Fix: an else if branch for file:/ that strips the five-character file: prefix, leaving /path. All four forms are now handled.

Open URL history stored in wxConfig, not in PWSprefs MRU

The PWSprefs MRU list is user-configured, finite, and shared with Recent Databases. Mixing URLs into it would pollute the file picker. Instead, URL history is stored separately in wxConfig under /URLHistory/url0…url9 (newest first, max 10 entries). On a successful open via URL the entry is also added to recentDatabases() so it appears in the File → Recent Databases submenu.

wxArrayString::Remove() asserts if element is absent

wxArrayString::Remove(value) calls wxASSERT internally and aborts if the value is not in the array. Pattern: always use Index() first, then RemoveAt(idx) by index only if idx != wxNOT_FOUND. The bug appeared in SaveUrlHistory() where Remove(url) was used to deduplicate before inserting — correct intent, wrong API call on first use when the URL was not yet in history.

BackupCurFile must be skipped for transport URLs

PWScore::BackupCurFile creates a PWSFileSig from the raw filename (stat call) and calls pws_os::RenameFile to rename the database to a .ibak path. For a transport URL, the stat and rename both fail — there is no local file at that path (the real file is in the XDG cache). Fix: in the UI save path (MenuFileHandlers.cpp), call pws_is_transport_url() before attempting backup and skip BackupCurFile entirely for URL-backed databases.

IsLockedFile uses in-process registry, not a server query

IsLockedFile answers "do WE hold the lock on this URL?" — not "is this URL locked by anyone?". The right data source is the in-process s_active_locks registry (updated by LockFile / UnlockFile), not a WebDAV HEAD request.

The old approach (FileExists(url + ".plk")) made a WebDAV HEAD request for a non-existent .plk file → 404 → IsLockedFile always returned false → SafeUnlockFile never called UnlockFile → server lock never released → next open got HTTP 423.

The registry approach is also async-signal-safe (no network I/O in signal handlers) and handles race-free transitions between locked and unlocked state.

Lock daemon for LOCK/UNLOCK (not inline libcurl)

libcurl is not async-signal-safe. pwsafe's UnlockFile is called from signal handlers, atexit handlers, and C++ destructors. Calling libcurl from any of these contexts risks crashes, deadlocks, or silent failures.

Solution: fork a child process on first lock acquisition. The child inherits the plugin's loaded .so and runs a command loop. The parent communicates via write()/read() on a socketpair — both are async-signal-safe per POSIX. The parent never calls libcurl directly after fork.

Consequence: all plugin state modifications (s_lock_tokens) happen only in the child. See 'Store routed through daemon' below.

Plugin ignores the 'token' argument to unlock()

The PWSTransport ABI passes token as a parameter to unlock(). For the WebDAV plugin, the token is stored internally in s_lock_tokens (URL → opaque token string). The token argument in the plugin's unlock() implementation is always ignored; the plugin looks up the token from its own map.

Why: the lock daemon design means that the parent never holds the token — the child does, in s_lock_tokens. The parent sends CMD_UNLOCK with just the URL; the child looks up the token itself. This avoids changing the file.h public API (which would require updating all callers).

ENOTSUP → proceed without lock (not an error)

If a WebDAV server doesn't support DAV:2 (Class 2), lock() returns ENOTSUP. The transport layer (LockFile in file.cpp) treats ENOTSUP as 'server doesn't support locking — warn once and continue'. The user is warned but the database can still be opened and saved.

This matches the design goal: native URL access should work even on basic WebDAV servers (Class 1 only). Locking is best-effort, not a hard requirement.

Store routed through daemon child, not parent

After fork(), the parent and child have independent copies of all in-process state. s_lock_tokens (in the WebDAV plugin) is populated when the child calls t->lock(). The parent's copy of this map remains permanently empty.

If the parent called t->store() directly (as it originally did), it checked its own empty map, omitted the If: (<token>) header, and got HTTP 423 Locked. The in-memory changes were silently lost.

Fix: FClose tests pws_has_lock(url). If true, it calls pws_lockd_store(cache, url) — which sends CMD_STORE to the child. The child's t->store() sees the correct s_lock_tokens entry and the PUT succeeds.

.plk files retained for local-file locking

.plk sidecar files are used by the existing local-file lock mechanism (non-transport paths). We do not remove or replace this mechanism — it serves a different purpose (cooperative lock between two pwsafe instances on the same machine opening the same local file).

For transport URLs, the lock is held server-side (WebDAV LOCK token) and tracked via the in-process registry. The .plk mechanism is never involved for transport URLs.

Lock daemon IPC uses binary frames, not text (execve vs system analogy)

The original protocol used newline-delimited text: "LOCK url\n". This is analogous to system() — any delimiter in user input (\n in a URL) injects extra commands into the parser.

The replacement is length-prefixed binary frames: [uint8_t opcode][uint32_t url_len LE][url_len bytes]. This is analogous to execve(argv[]) — each element is a separate, self-contained object. A URL can contain any byte value, including \n, space, or NUL, without affecting framing.

See also: PROGRAMMING_RULES/security-interfaces

version2
updated2026-02-27