Repository navigation
fix: preserve source resources during document path saves - #1148
Merged
Merged
Conversation
andiwand
force-pushed
the
review/129-pdf-calculator-types
branch
from
October 6, 2026 14:53
759f921 to
17166d2
Compare
The owner-only mode applied to every file the temporary factory made, so write_atomic gave a document saved to a new path mode 0600 where the old ofstream gave 0666 minus the umask. Only temporary copies, which can hold decrypted content, are private to their owner now. A new save gets the permissions any new file in its directory gets. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MxyTMutqSUJRGfxA8CyzMc
andiwand
force-pushed
the
review/130-document-path-save
branch
from
October 6, 2026 15:07
fd99b7e to
16476bf
Compare
preserve_source sat on abstract::File and every filesystem, and the document walked all of its entries to reach the one ZIP reader that can act. The ODF and OOXML files now keep the zip::util::Archive they open and hand it to their documents. abstract::Document::release_source does nothing by default, and common::Document forwards it to the archive's ReadSource, which moves onto a private copy if it reads the destination. The reader compares the files with the error_code overload, so a source deleted after open is no error. TemporaryDiskFile::persist renames the written file over the destination, keeps the destination's permissions, reports a failed rename as FileWriteError and stops owning the file, so DiskFile::path stays protected. write_atomic refuses a destination its owner may not write, as truncating it did before. A new test truncates the source in place after the release, because the pooled read streams keep replaced bytes readable on POSIX. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MxyTMutqSUJRGfxA8CyzMc
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
🤖 Generated with Claude Code
Saving a ZIP-backed document over its source truncated unread resources. Path saves now write and close a temporary file beside the destination, then rename it over the destination (
util::file::write_atomic,TemporaryDiskFile::persist). A failed write leaves the destination unchanged.Lazily read packages need one more step.
ReadSourceopens the source again by path for each new read stream, and on Windows a rename over a file with open handles fails. So before the replace,Document::savecallsabstract::Document::release_source(default: nothing). The ODF and OOXML files keep thezip::util::Archivethey open and pass it to their documents.common::Documentforwards the call to the archive'sReadSource, which moves onto a private temporary copy if it reads the destination (std::filesystem::equivalent, so hard links and symlinks count).The shared path follows existing symlinks, refuses dangling ones, keeps the permissions of an existing destination and refuses one that its owner may not write, as truncation did. A new destination gets the default permissions of its directory; only temporary copies are private to the owner, because they can hold decrypted content. Stream saves keep their behaviour.
Limitations: only the saving document moves to a copy. A second document open on the same file is not protected, and a hard link to the destination keeps the old bytes after the rename.
Validation: 2397 tests pass.
a_released_source_reads_from_its_own_copytruncates the source in place after the release and fails without the hook; the pooled read streams hide the problem from a plain save-over test on POSIX. The full library and CLI build passes clang-tidy 18 in an Ubuntu 24.04 container that matches the tidy job. No Windows runtime validation.