From 7c83ea0900d0d78c83b53444b45f7f238e62829b Mon Sep 17 00:00:00 2001 From: Andreas Stefl Date: Tue, 6 Oct 2026 22:05:40 +0200 Subject: [PATCH] fix: preserve files during filesystem copies --- CHANGELOG.md | 3 ++ src/odr/internal/common/filesystem.cpp | 8 ++-- test/src/internal/common/filesystem_test.cpp | 41 ++++++++++++++++++++ 3 files changed, 47 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 518855d3a..23238f181 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,9 @@ The release run heads these entries with the version and opens a fresh ## Unreleased +- Filesystem copies preserve their source when copying onto itself and leave + existing destinations intact when reading or writing fails. + - PDF Identity-H/V fonts retain two-byte character codes even when their text-extraction map declares a conflicting codespace, preserving glyphs and spacing. diff --git a/src/odr/internal/common/filesystem.cpp b/src/odr/internal/common/filesystem.cpp index f4beb11f1..6be1e2360 100644 --- a/src/odr/internal/common/filesystem.cpp +++ b/src/odr/internal/common/filesystem.cpp @@ -141,11 +141,9 @@ bool SystemFilesystem::copy(const AbsPath &from, const AbsPath &to) { std::shared_ptr SystemFilesystem::copy(const abstract::File &from, const AbsPath &to) { - // `create_file` and `open` translate `to` themselves - const auto istream = from.stream(); - const auto ostream = create_file(to); - - util::stream::pipe(*istream, *ostream); + util::file::write_atomic( + to_system_path_(to).string(), + [&](std::ostream &out) { util::stream::pipe(*from.stream(), out); }); return open(to); } diff --git a/test/src/internal/common/filesystem_test.cpp b/test/src/internal/common/filesystem_test.cpp index bd9259d95..7575b4070 100644 --- a/test/src/internal/common/filesystem_test.cpp +++ b/test/src/internal/common/filesystem_test.cpp @@ -4,6 +4,7 @@ #include #include #include +#include #include #include @@ -11,6 +12,7 @@ #include #include #include +#include #include #include #include @@ -45,6 +47,45 @@ filesystem_of(const std::vector &files, } // namespace +TEST(Filesystem, copying_a_file_over_itself_preserves_its_bytes) { + const MemoryFile source(std::string(8193, 'x')); + const auto file = TemporaryDiskFileFactory::system_default().copy(source); + SystemFilesystem disk(file.disk_path()->parent()); + const AbsPath target("/" + file.name()); + + const auto copied = disk.copy(file, target); + + ASSERT_NE(copied, nullptr); + EXPECT_EQ(MemoryFile(*copied).content(), source.content()); +} + +TEST(Filesystem, a_failed_copy_preserves_the_destination) { + class UnreadableFile final : public abstract::File { + public: + odr::FileLocation location() const noexcept override { + return odr::FileLocation::memory; + } + std::size_t size() const override { return 11; } + std::string name() const override { return {}; } + std::optional disk_path() const override { return std::nullopt; } + std::optional memory_data() const override { + return std::nullopt; + } + std::unique_ptr stream() const override { + auto result = std::make_unique("replacement"); + result->setstate(std::ios::badbit); + return result; + } + } source; + const auto file = + TemporaryDiskFileFactory::system_default().copy(MemoryFile("original")); + SystemFilesystem disk(file.disk_path()->parent()); + const AbsPath target("/" + file.name()); + + EXPECT_THROW(disk.copy(source, target), std::ios_base::failure); + EXPECT_EQ(MemoryFile(file).content(), "original"); +} + TEST(Filesystem, copied_walkers_advance_independently) { struct Directory final { std::filesystem::path path = std::filesystem::temp_directory_path() /