No code change needed. 10.2.6.1 replaces the Pluralize.NET dependency
with an internal pluralizer inside Dinah.Core; the Pluralize and
PluralizeWithCount extensions this repo uses behave the same, which the
existing tests that assert on their output confirm.
Co-authored-by: rmcrackan <rmcrackan@gmail.com>
SaferEnumerateFiles returned a lazy sequence, so an I/O error was raised where
the sequence was walked rather than where it was created - past the try/catch
callers had wrapped around it. IgnoreInaccessible did not help either: it only
forgives permissions, not a volume that has stopped answering.
Walk the enumerator defensively instead, keeping what was read and reporting
the reason, and let a caller ask whether a directory can be read at all.
See issue #1984.
Co-authored-by: rmcrackan <rmcrackan@gmail.com>
All three Windows CI legs failed on master while the other six passed, and not on
an assertion: every test in FileLiberator.Tests' PDF path suite failed in
TestInitialize with an AggregateException wrapping FileNotFoundException, naming a
path none of those tests had anything to do with.
The watcher had raised Created for a folder an earlier test's cleanup then
deleted. AddPath asked whether the path existed, was told yes, asked what it was,
and got an exception - Exists and GetAttributes can disagree over a long \\?\
path, and the answer to the first can stop being true before the second is asked
anyway. That exception ended the background scanner, so nothing further reached
the cache, and it was stored on the task, so the next Stop() rethrew it as an
AggregateException at whoever had called Refresh(). In the app that caller is the
Books directory refresh after every download.
Three changes, smallest first: the attribute read is guarded and returns 'nothing
to add' where the existence check used to say it, which also removes the race
rather than narrowing it; the scanner survives an event it cannot apply; and
Stop() waits on a scanner that has already failed without handing the failure to
a caller that is about to replace it.
Co-authored-by: rmcrackan <rmcrackan@gmail.com>
Seven references across six projects, all now on published versions.
What the app gains is from Dinah.Core: OsSecretStore.Create bounds how
long it waits for the backend, so a Linux or macOS start with a keyring
that never answers falls through to the portable master key path instead
of hanging there. IdentityTokenStorageWiring.ResolveSecretStore is the
caller, and it runs at startup whenever no key file or env var is set.
AudibleApi 11.0.3.1 carries no code change for us - it is the release
where its nuspec finally declares the Dinah.Core floor its own code needs.
Dinah.Core.WindowsDesktop and Dinah.EntityFrameworkCore move to 10.2.4.1
as well, keeping every Dinah package on one version.
Co-authored-by: rmcrackan <rmcrackan@gmail.com>
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
