A Windows CI leg failed with every test passing. FileLiberator.Tests
exited 0xE0434352:
Unhandled exception. System.InvalidOperationException: The collection has
been marked as complete with regards to additions.
at BlockingCollection`1.Add(T item)
at FileManager.BackgroundFileSystem.FileSystemWatcher_Changed(...)
at FileSystemWatcher.ReadDirectoryChangesCallback(...)
Stop() disposes the watcher and then completes the collection, on the
assumption that disposing stops events. It does not stop the ones the OS
has already buffered, and on Windows those arrive on a native completion
callback, where an exception is not a failed call - it is a dead process.
So a Libation run that reinitialises or shuts down its file cache while
the Books directory is busy can take the app with it, which is the same
race the tests hit.
Adding to a completed collection is now caught and the event dropped.
That is the right answer rather than a swallow: whoever called Stop() is
either reinitialising, which rebuilds the cache from disk, or disposing.
Stop() also detaches its handlers before disposing and clears the fields,
which narrows the window and makes a second Stop() harmless - the
collection field is cleared only after CompleteAdding, since the
background scanner is waiting on that.
First tests for the class, since it had none: dispose under a flood of
events, dispose twice, and find a file created before and after
construction. They cannot prove this fix - the crash does not reproduce on
Linux even with the original code, because inotify does not deliver
post-dispose events the way Windows does. Windows CI is the only place
that can, so the guard is aimed there.
Co-authored-by: rmcrackan <rmcrackan@gmail.com>
Windows CI caught the atomic replace failing with UnauthorizedAccessException:
renaming over a file is denied while another handle holds it open, however
generously that handle shares the file. In production the CLI, a second GUI
instance or a virus scanner can each hold Settings.json for a moment, so retry the
replace a few times before letting the caller see the failure. The previous
File.WriteAllText threw on the same holds, so this is strictly more forgiving.
ExternalReaderNeverSeesAPartiallyWrittenFile keeps a handle open almost
continuously, which no retry budget can outlast on Windows, so restrict it to unix
where it actually tests write atomicity. Write_SurvivesATemporarilyUnwritableDirectory
covers the retry instead by revoking write permission on the containing directory.
Co-authored-by: rmcrackan <rmcrackan@gmail.com>
Dinah.Core 10.2.2.2 gives SecretString a Redacted property, so a
destructured secret carries its length without the logger being told
anything. That was the only thing AsScalar was buying - safety never
depended on it - so the registration and its explanation go away, and the
knowledge lives in the type instead of in this file.
MaskedLogEntryPolicy stays: an ILogMasked has no equivalent property, and
without the policy one logged as {@Account} is still written out property
by property.
Co-authored-by: rmcrackan <rmcrackan@gmail.com>
File.WriteAllText truncates the destination before writing, so an interrupted
write leaves a half-written or empty Settings.json, and the in-process lock added
in the previous commit cannot help a reader in another process - the GUI and the
CLI share this file.
Route every write through Dinah.Core.IO.AtomicFileWriter, which writes a sibling
temp file, flushes to disk and renames it over the destination. Validate the temp
file parses as json before the swap, the same way JsonFilePersister<T> already
saves AccountsSettings.json, so a bad payload leaves the existing file untouched.
Co-authored-by: rmcrackan <rmcrackan@gmail.com>
Configuration.Instance is a process-wide singleton read and written from the UI
thread, BackgroundWorker callbacks and download workers at the same time, but
PersistentDictionary guarded only its file writes. Its two Dictionary caches were
touched without synchronization, so concurrent inserts eventually corrupted them
and threw "Operations that change non-concurrent collections must have exclusive
access" (issue #1959, reported from MainVM.UpdateCountsBw_Completed reading
AutoDownloadEpisodes while a second GetCounts pass ran).
Reads on the file were unguarded too: Exists/GetJObject could observe a partially
written Settings.json, and readFile responds to empty contents by rewriting the
file, so a reader racing a writer could scramble the settings on disk.
Take one lock across each operation's cache and file access, and keep logging
outside it by having writeFile report whether it rewrote the file.
Co-authored-by: rmcrackan <rmcrackan@gmail.com>
AudibleApi 11 holds token, key, and cookie values in a SecretString
rather than a string, so nothing public exposes plaintext for a reflective
logger to find. Picking it up is a breaking upgrade: the seven package
references move, and the nine places that read a secret now call Reveal().
Two of those needed thought rather than a mechanical edit. Mkb79Auth
exports to and imports from audible-cli's JSON format, which is plaintext
by definition, so the cookie projections reveal explicitly in both
directions and the file format is unchanged. And the account's own
DecryptKey stays a plain string here: converting it is separate work.
This is the dependency bump only. The log leak it enables fixing - an
AuthenticationRequiredException carrying a live Account, whose address and
activation bytes Serilog.Exceptions writes into a shared log - is still
open, and none of the account-side masking has landed yet.
Co-authored-by: rmcrackan <rmcrackan@gmail.com>
- fixed documentation
- regexp-checks running with timeout and culture-invariant matching
- changed check-building in ConditionalTagCollection to use NonNull parameters. So no warnings occure.
- add tests for <!is ...> and escaped chars
Introduce <is-> Tag. Like <has-> but with additional check on content.
Retrieve objects instead of string for conditions
Pass undefined formats as null instead of empty strings
