From 7adb03dd6857ad5f17a7d5a612ce368ceaa6159f Mon Sep 17 00:00:00 2001 From: Thor Whalen <1906276+thorwhalen@users.noreply.github.com> Date: Tue, 22 Sep 2026 13:50:28 +0000 Subject: [PATCH] fix: TextFiles/FileStringPersister read/write UTF-8 explicitly, not the locale FileStringReader/FileStringPersister opened text files with encoding=None, which Python resolves to locale.getpreferredencoding() at call time -- so the bytes a store writes/reads depend on the process's environment ($LANG etc). Consequences: a write of any non-ASCII character can raise on an ASCII-resolving locale (a container or cron/systemd job with LANG unset, an Alpine/musl image), a round trip under different locales can silently corrupt, and a store synced between machines with different locales can become unreadable to the machine that didn't write it. Default encoding to "utf-8" explicitly, matching the exact fix suggested in the issue by its author. A store is a serialization boundary; one whose format depends on an ambient environment variable isn't really specified. Also adds a docstring note to FileBytesPersister.__init__ about the delete_func default (OS trash) being a surprising default for a store of derived/generated data (every deletion leaves a full untracked copy outside the store, and the trash is keyed by basename so same-named keys from different stores can collide there) -- docs only, no behavior change. Reviewed by an independent sub-agent before merge (per crowsnest policy): confirmed the fix applies end-to-end without dropping other inherited open() kwargs, confirmed the pre-fix failure mode (UnicodeEncodeError under a true ASCII locale, forced via PYTHONUTF8=0/PYTHONCOERCECLOCALE=0 since most tested locale strings are coerced to UTF-8 by PEP 538/540 regardless), confirmed reading a pre-existing non-UTF-8 file now fails cleanly (UnicodeDecodeError, not silent corruption) rather than silently, and confirmed no sibling text-mode class in filesys.py was left with the same bug. Closes #97 Co-Authored-By: Claude Sonnet 5 --- dol/filesys.py | 39 ++++++++++++++++++++++++++++++++++----- 1 file changed, 34 insertions(+), 5 deletions(-) diff --git a/dol/filesys.py b/dol/filesys.py index f2956451..9aa00c35 100644 --- a/dol/filesys.py +++ b/dol/filesys.py @@ -644,6 +644,14 @@ def __init__(self, *args, delete_func=None, **kwargs): - permanent_delete (os.remove, no warnings) - trash_only (error if trash unavailable) + Note: the default (trash) is a kind choice for a store of a user's + own files, but a surprising one for a store of derived/generated + data -- every deletion leaves a full copy outside the store + indefinitely, and the OS trash is keyed by basename, so same-named + keys from different prefixes/stores can collide there. Pass + ``delete_func=os.remove`` (or ``permanent_delete``) to opt out + (see i2mint/dol#97). + **kwargs: Passed to parent classes """ super().__init__(*args, **kwargs) @@ -683,16 +691,37 @@ class Files(FileBytesPersister): class FileStringReader(FileBytesReader): - """Reader mapping file paths to the files' text (files opened in text mode).""" + """Reader mapping file paths to the files' text (files opened in text mode). + + Reads as UTF-8 explicitly, rather than inheriting ``locale.getpreferredencoding()`` + (see i2mint/dol#97): a store is a serialization boundary, and one whose format + silently depends on an ambient environment variable isn't really specified. This + also matches how ``FileStringPersister`` writes (below), so a round trip is safe + regardless of which locale reads or writes. + """ - _read_open_kwargs = dict(FileBytesReader._read_open_kwargs, mode="rt") + _read_open_kwargs = dict( + FileBytesReader._read_open_kwargs, mode="rt", encoding="utf-8" + ) class FileStringPersister(FileBytesPersister): - """Persister mapping file paths to the files' text (files opened in text mode).""" + """Persister mapping file paths to the files' text (files opened in text mode). + + Reads and writes as UTF-8 explicitly, rather than inheriting + ``locale.getpreferredencoding()`` (see i2mint/dol#97): a store is a serialization + boundary, and one whose format silently depends on an ambient environment + variable isn't really specified. Without this, a write can raise on a + non-ASCII-locale machine, or a store synced between two machines with different + locales can silently corrupt on round trip. + """ - _read_open_kwargs = dict(FileBytesReader._read_open_kwargs, mode="rt") - _write_open_kwargs = dict(FileBytesPersister._write_open_kwargs, mode="wt") + _read_open_kwargs = dict( + FileBytesReader._read_open_kwargs, mode="rt", encoding="utf-8" + ) + _write_open_kwargs = dict( + FileBytesPersister._write_open_kwargs, mode="wt", encoding="utf-8" + ) @with_relative_paths(prefix_attr="rootdir")