don't do atomic overrides with failed files

We deploy atomic renames for some files, but these renames also happen
if something about the file failed which isn't really the point of the
exercise…

Closes: 828908

diff --git a/apt-pkg/contrib/fileutl.cc b/apt-pkg/contrib/fileutl.cc
index 94d1432cfa908bc14261ea5f08b48f45bfcccd47..388849144551be2af436195f6b7e1f4613f3e44c 100644 (file)
--- a/apt-pkg/contrib/fileutl.cc
+++ b/apt-pkg/contrib/fileutl.cc
@@ -2623,7 +2623,7 @@ bool FileFd::Close()
    }
 
    if ((Flags & Replace) == Replace) {
-      if (rename(TemporaryFileName.c_str(), FileName.c_str()) != 0)
+      if (Failed() == false && rename(TemporaryFileName.c_str(), FileName.c_str()) != 0)
         Res &= _error->Errno("rename",_("Problem renaming the file %s to %s"), TemporaryFileName.c_str(), FileName.c_str());
 
       FileName = TemporaryFileName; // for the unlink() below.

diff --git a/test/libapt/fileutl_test.cc b/test/libapt/fileutl_test.cc
index 8cd5132ce23bb962199826728a6a5a4aa6d795ff..0a299b51af045f1c577b3bfe14eed7759f5ef5c6 100644 (file)
--- a/test/libapt/fileutl_test.cc
+++ b/test/libapt/fileutl_test.cc
@@ -327,6 +327,7 @@ TEST(FileUtlTest, flAbsPath)
 
 static void TestDevNullFileFd(unsigned int const filemode)
 {
+   SCOPED_TRACE(filemode);
    FileFd f("/dev/null", filemode);
    EXPECT_FALSE(f.Failed());
    EXPECT_TRUE(f.IsOpen());
@@ -354,3 +355,37 @@ TEST(FileUtlTest, WorkingWithDevNull)
    TestDevNullFileFd(FileFd::WriteTemp);
    TestDevNullFileFd(FileFd::WriteAtomic);
 }
+constexpr char const * const TESTSTRING = "This is a test";
+static void TestFailingAtomicKeepsFile(char const * const label, std::string const &filename)
+{
+   SCOPED_TRACE(label);
+   EXPECT_TRUE(FileExists(filename));
+   FileFd fd;
+   EXPECT_TRUE(fd.Open(filename, FileFd::ReadOnly));
+   char buffer[50];
+   EXPECT_NE(nullptr, fd.ReadLine(buffer, sizeof(buffer)));
+   EXPECT_STREQ(TESTSTRING, buffer);
+}
+TEST(FileUtlTest, FailingAtomic)
+{
+   FileFd fd;
+   std::string filename;
+   createTemporaryFile("failingatomic", fd, &filename, TESTSTRING);
+   TestFailingAtomicKeepsFile("init", filename);
+
+   FileFd f;
+   EXPECT_TRUE(f.Open(filename, FileFd::ReadWrite | FileFd::Atomic));
+   f.EraseOnFailure();
+   EXPECT_FALSE(f.Failed());
+   EXPECT_TRUE(f.IsOpen());
+   TestFailingAtomicKeepsFile("before-fail", filename);
+   EXPECT_TRUE(f.Write("Bad file write", 10));
+   f.OpFail();
+   EXPECT_TRUE(f.Failed());
+   TestFailingAtomicKeepsFile("after-fail", filename);
+   EXPECT_TRUE(f.Close());
+   TestFailingAtomicKeepsFile("closed", filename);
+
+   if (filename.empty() == false)
+      unlink(filename.c_str());
+}